Not proxying these requests through camo is a security concern.
Furthermore, on the desktop client, any embed image which is hosted on
a server with an expired or otherwise invalid certificate will trigger
a blocking modal window with no clear source and a confusing error
message; see zulip/zulip-desktop#1119.
Rewrite all `message_embed_image` URLs through camo, if it is enabled.
We add discussion id and url in the comments and highlighted title to
the body of disscussion message to make it more meaningful and accessible.
Fixes#19938.
We aim to use Zulip topics thoughtfully in displaying messages from
discussions, as well as linking to the discussion in every message so
that it's easy to view them.
Fixes#19938.
Supporting URL percent-encoded bytes is possible using `%%20`, but this
is not necessarily very understandable to end-users, even those that
understand percent encoding.
Allow `%20` in linkifier URL format strings, and transform them into
`%%20` in the pattern just before they are applied in markdown
translation. Care must be taken here, such that already-escaped `%`s
are not escaped an extra time.
We do this before rendering, and not before storage, as
a simplification; the JS-side linkifier at present only understands
`%(foo)s` and thus needs no changes, and to avoid an un-escaping pass
before showing in the admin UI.
User-supplied custom realm filter has had some sort of regex-based
validation of the format URL since their introduction in
d7e1e4a2c0 -- and this has always been
in addition to the URLValidator. The URLValidator is the one which
does the security-relevant work of validating that the schema is
reasonable, and that the overall shape of the URL is well-formed. The
regex has served primarily to arbitrary limit the characters that can
appear in the URL, in the mistaken name of safety.
Adjust the regex, such that its only purpose is to verify that the
usages of `%` characters in the URL are reasonable, and leave the URL
validation to the URLValidator, which can do a far better job. This
includes broadening the support to include `%%` as an escape
character; this is likely such a niche case as to be unnecessary, but
costs little.
Fixes#16013.
og:image is supposed to be an absolute URL, but some sites incorrectly
provide a relative URL. In this case, it makes more sense to
interpret it relative to the full page URL after redirects, rather
than relative to just the domain part of the page URL before
redirects.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We don't yet have a do_reactivate_stream function, but we reserve a
number since:
1. It'll likely be added in the future.
2. For now, we can restore archived stream with some manual intervention
in the Django shell, and for that we'll want to create an appropriate
RealmAuditLog entry.
Removes the `/day` and `/night` options from the typeahead menu while
still allowing the commands to be used. Typing `/day` and `/night`
will now suggest `/light` and `/dark`, respectively. Also changes the
`Dark mode` and `Light mode` popups that appear after using the
corresponding command.
Fixes#18318.
Created a schema for the ignored_parameters_unsupported that is
returned by the /settings and /realm/user_settings_defaults endpoints
and removed the duplicated text in the api documentation.
Also cleaned up some small errors in the /realm/user_settings_default
definition and sidebar link /api/update-realm-user-settings-defaults.
Fixes#19674
Since 3853285241, PushNotificationsWorker uses the aioapns library
to send Apple push notifications. This introduces an asyncio event
loop into this worker process, which, if unlucky, can respond poorly
when a SIGALRM is introduced to it:
```
[asyncio] Task exception was never retrieved
future: <Task finished coro=<send_apple_push_notification.<locals>.attempt_send() done, defined at /path/to/zerver/lib/push_notifications.py:166> exception=WorkerTimeoutException(30, 1)>
Traceback (most recent call last):
File "/path/to/zerver/lib/push_notifications.py", line 169, in attempt_send
result = await apns_context.apns.send_notification(request)
File "/path/to/zulip-py3-venv/lib/python3.6/site-packages/aioapns/client.py", line 57, in send_notification
response = await self.pool.send_notification(request)
File "/path/to/zulip-py3-venv/lib/python3.6/site-packages/aioapns/connection.py", line 407, in send_notification
response = await connection.send_notification(request)
File "/path/to/zulip-py3-venv/lib/python3.6/site-packages/aioapns/connection.py", line 189, in send_notification
data = json.dumps(request.message, ensure_ascii=False).encode()
File "/usr/lib/python3.6/json/__init__.py", line 238, in dumps
**kw).encode(obj)
File "/usr/lib/python3.6/json/encoder.py", line 199, in encode
chunks = self.iterencode(o, _one_shot=True)
File "/usr/lib/python3.6/json/encoder.py", line 257, in iterencode
return _iterencode(o, 0)
File "/path/to/zerver/worker/queue_processors.py", line 353, in timer_expired
raise WorkerTimeoutException(limit, len(events))
zerver.worker.queue_processors.WorkerTimeoutException: Timed out after 30 seconds processing 1 events
```
...which subsequently leads to the worker failing to make any progress
on the queue.
Remove the timeout on the worker. This may result in failing to make
forward progress if Apple/Google take overly long handling requests,
but is likely preferable to failing to make forward progress if _one_
request takes too long and gets unlucky with when the signal comes
through.
This makes logging more consistent between FCM and APNs codepaths, and
makes clear which user-ids are for local users, and which are opaque
integers namespaced from some remote zulip server.
Being able to determine how many distinct users are getting push
notifications per remote host is useful, as is the distribution of
device counts. This parallels the log line in
handle_push_notification for push notifications from local realms,
handled via the event queue.
We should use more selective query for UserGroupMembership
objects in tests for checking adding and removing members.
This is done by checking the membership counts for the
particular user group only.
This will help in keeping the tests more understandable
after we add members to the role-based system groups,
since that would create a lot of membership objects.
We make the UserGroup queries in user group creation and
deletion tests more selective by fitering the user groups
which belong to the realm and not the one included in
lear realm, etc.
This will help us to keep the tests more understandable
when the counts of UserGroup increases due to addition of
system groups. There is no need to consider system groups
of other realms in these tests.
It is confusing to have the plan type constants not be namespaced
by the thing they represent. We already have a namespacing
convention in place for constants, so we should use it for
Realm.plan_type as well.
* Remove unnecessary json_validator for full_name parameter.
* Update frontend to pass the right parameter.
* Update documentation and note the change.
Fixes#18409.
`rendered_content` in historical messages may be empty; examining the
history of them may thus require diff'ing two empty strings, which
itself produces an empty string.
Use `lxml.html.fragment_fromstring` to be able to successfully parse
these, rather than 500.
Part of #19559.
As detailed in the comments, the default behavior is undesirable for us
because we can't really predict all possibilities of exceptions that may
be raised - and thus putting str(e) in the http response is potentially
insecure as it may leak some unexpected sensitive information that was
in the exception.
As a hypothetical example - KeyError resulting from some buggy
some_dict[secret_string] call would leak information. Though of course
we aim to never write code like that.
We pass allow_realm_admin as True to access_stream_by_id for
`GET users/{user_id}/subscriptions{stream_id}` endpoint
because we want to allow non-subscribed admins to get
subscription status in private streams.
Fixes#19077.
This commit adds related_name parameter to UserGroup.direct_members
such that we can use direct_groups instead of the default
usergroupmembership_set for getting all the groups of which the
user is direct member.
This commit also sets related_name of UserGroupMembership.user_group
and UserGroupMembership.user_profile to "+" which means that we will
not be having backward relations for these. This change is correct
since we would need to use the recursive queries to get all the
groups of a user and all the members of a group after we add the
subgroups concept in next commit. This leads to us using direct_members
field of UserGroup instead of usergroupmembership_set in mention code,
but this will soon be replaced with the recursive query function to
include subgroup's members as well.
Extracted this commit from #19866.
Authored-by : Anders Kaseorg <anders@zulip.com>
This commit makes the query in get_user_group_direct_members
efficient by directly fetching user-profile ids instead of
first fetching user profile object and then id.
The previous commit apparently didn't have its migration attached properly.
Since this is just a reference to a ManyToManyField, this migration
doesn't actually modify the database, but it is needed for CI to pass.
This commit renames members field of UserGroup to direct_members
for better readability because in the new permissions model, a
user group can be a sub-group of another group and thus technically
members of sub-group will also be members of that group.
This is a prep commit for new permissions model.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This is a prep commit for new permissions model in
which a user group would be able to have a subgroup.
This commit renames get_memberships_of_users to
get_direct_memberships_of_users to specify that
the function is used only to fetch the direct
memberships and not memberships of subgroups of
the direct group.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This is a prep commit for new permissions model in which a user
group would be able to have a subgroup.
This commit renames get_user_groups to get_direct_user_groups
to specify that the function is used only to fetch the direct
groups that user is part of and not subgroups of the direct
group.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This is a prep commit for new permissions model in which a user group would
be able to have a subgroup.
This commit renames get_user_group_members to get_user_group_direct_members
to specify that the function is used only to fetch direct members of group
and excludes the subgroup's members.
Extracted this commit from #19866.
Co-authored-by: Anders Kaseorg <anders@zulip.com>
This will be useful to let users enable/disable
sharing read receipts once we add that feature.
Note: Added "I've" to IGNORED_PHRASES in
tools/lib/capitalization.py to avoid capitalization
errors for the label text of this setting.
Note: These are not functional in enabling/disabling sending of
typing notifications with this commit.
Refactored the privacy settings update to keep the code less
duplicated along with making the addition of new settings easier.
From 430c5cb, in `fetch_initial_state_data`,
we only include legacy settings in the top level of
`state` and the newer ones are stored in `state['user_settings']`.
That should've had a corresponding change in apply_event().
Also, fixed a test related to this logic.
From 430c5cb, we do not send events for settings added after
introduction of `user_settings` event.
This test wasn't considering that and started failing on
adding new user_settings.
This commit removes _test_user_settings_for_adding_streams
and its callers for testing public and private streams
because it uses excessive mocking and we also test the same
thing in _test_user_settings_for_creating_streams without
mocking, so this test doesn't add anything.
For users who are not logged in and for those who don't have
'prefers_web_public_view' set in session, we redirect them
to the default login page where they can choose to login
as spectator or authenticated user.
This commit adds can_create_web_public_streams helper
in models.py which will be used to validate whether
user is allowed to create a web-public stream or not.
This commit also adds the checks for Realm.POLICY_OWNERS_ONLY
in check_has_permission_policies.
This commit adds create_web_public_stream_policy
field to Realm table which controls the roles that
can create web-public streams and by default its
value is set to POLICY_OWNERS_ONLY.
This commit enforces invite_only argument to be named
in _test_user_settings_for_creating_streams. This will
help in improving readability especially when we will
add is_web_public argument in further commits.
This commit adds tests for POLICY_EVERYONE and POLICY_NOBODY
in check_has_permission_policies test. The original code
used these values but these were not covered in test.
We send three events when changing delivery email of a user - one
for updating the delivery_email field of user, one for avatar url
change, and one for changing email field if email_address_visibility
is set to 'EMAIL_ADDRESS_VISIBILITY_EVERYONE'.
There is already a test for delivery_email and avatar_url event with
the visibility setting set to 'EMAIL_ADDRESS_VISIBILITY_ADMINS_ONLY',
but no test for verifying the email update event sent when email
address is public, so this commit adds a test for checking the schema
of event for updating email field.
When email_address_visibility is changed and either the old value
or the updated value is EMAIL_ADDRESS_VISIBILITY_EVERYONE then
email field of all users is updated and we also send the corresponding
event to clients. But apply_event code did not update the data on
receiving the event, so this commit fixes the code to correctly
handle the event in apply_event.
(We also use this event when just changing a user's email address).
This commit also adds the tests and openapi schema for the event.
We use the lists defined in models.py like Realm.COMMON_POLICY_TYPES,
Realm.COMMON_MESSAGE_POLICY_TYPES, etc. in do_set_realm_property_test
instead of using defining list there (eg - [4, 3, 2, 1]). We do the
same thing in do_set_realm_property_test in test_realm.py.
We skip email_address_visibility values in this commit because it
requires some change in openapi schema as well.
Since the calls to the translation function `_()` are made outside
of the `send_message_moved_breadcrumbs` function, these strings are
translated outside of the `with override_language` block, leading to
translated strings even when we don't intend them to be translated.
We now use gettext_lazy with appropriate testing to avoid this.
Zulip attempts to validate that the regular expressions that admins
enter for linkifiers are well-formatted, and only contain a specific
subset of regex grammar. The process of checking these
properties (via a regex!) can cause denial-of-service via
backtracking.
Furthermore, this validation itself does not prevent the creation of
linkifiers which themselves cause denial-of-service when they are
executed. As the validator accepts literally anything inside of a
`(?P<word>...)` block, any quadratic backtracking expression can be
hidden therein.
Switch user-provided linkifier patterns to be matched in the Markdown
processor by the `re2` library, which is guaranteed constant-time.
This somewhat limits the possible features of the regular
expression (notably, look-head and -behind, and back-references);
however, these features had never been advertised as working in the
context of linkifiers.
A migration removes any existing linkifiers which would not function
under re2, after printing them for posterity during the upgrade; they
are unlikely to be common, and are impossible to fix automatically.
The denial-of-service in the linkifier validator was discovered by
@erik-krogh and @yoff, as GHSL-2021-118.
This removes a false-positive ReDoS, since the input is always
checked-in code. It also incidentally refactors to make the regexes
be more explicit about the values they expect, and removes unnecessary
capturing groups.
It removes an optional parenthesized status code for fixtures,
unnecessary since 981e4f8946, as well as
optional key-value language options, unnecessary since
a2be9a0e2d.
Thank you to @erik-krogh and @yoff for bringing this to our attention.
This fixes the issue where 'None' would appear in the rendered
html in case of a missing tab display_name. Now,
'test-help-documentation' will fail in case of any tab display_name
being missing.
In case of a tab_section with no tabs, currently a single tab with
the name 'null_tab' gets added. Added the display name 'None' for
'null_tab', to keep in line with the existing behaviour.
Fixes#19822
This makes our onboarding guide for education organizations much
simpler, since new organizations will start with these settings
correctly configured.
Fixes#19682
Users wanted a feature where they could specify
which users can create public streams and which users can
create private streams.
This splits stream creation code into two parts,
public and private stream creation.
Fixes#17009.
This commit replaces 'allow_message_deleting' boolean setting
with an integer setting 'delete_own_message_policy'. We have a
separate dropdown now for deciding which user-roles can delete
messages sent by themselves and the time-limit setting droddown
is different.
This new setting has two options - everyone and admins only. Other
options including moderators will be added further.
We also remove the "Never" option from the original time-limit
dropdown, as admins are always allowed to delete message. This
never option resembled the case of only admins being allowed to
delete but this state is now resembled by setting the dropdown
to "admins only" and we also disable the time-limit dropdown in
this case as admins are allowed to delete irrespective of limit.
Note, this setting is only for deleting messages sent by the
deleting user themselves, and only admins are allowed to delete
messages sent by others as before.
We make zero invalid value for message_content_delete_limit_seconds and
for handling the case of "Allow to delete message any time", the API-level
value of message_content_delete_limit_seconds is "anytime" and "None"
as the DB-level value. We also use these values for message retention
setting, so it helps maintain consistency.
This commit does not remove the 'enable_login_emails' field from
RealmUserDefault table but it is just not used and cannot be
changed from UI or API similar to 'enable_marketing_emails' setting.
This is a somewhat subtle function, that deserves a few comments
explaining subtle details of its logic, and there's no good reason to
have multiple copies of that logic that are slightly inconsistent.
Because the main changes here are just checking for invariant
failures, the behavioral change here should be limited to ensuring
deactivated streams are not considered available even if they were
tagged as web public streams before deactivation.
This fixes a problem where we could not import zerver.lib.streams from
zerver.lib.message, which would otherwise be reasonable, because the
former implicitly imported many modules due to this issue.
Requests to the root subdomain weren't getting request_notes.realm set
even if a realm exists on the root subdomain - which is actually a
common scenario, because simply having one organization, on the root
subdomain, is the simplest and common way for self-hosted deployments.
This reverts commit cd93d0967f.
This check_or is redundant with check_union; it gives a misleading
error message for the non-matching case; and it has no type safety.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
In maybe_send_resolve_topic_notifications, since the calls to the
translation function `_()` are made outside of the `override_language`
block, the strings are not translated correctly.
This commit refactors the function to make sure that the translation
happens in the right block of code.
Fixes#19730.
Apparently, our slack compatible outgoing webhook format didn't
exactly match Slack, especially in the types used for values. Fix
this by using a much more consistent format, where we preserve their
pattern of prefixing IDs with letters.
This fixes a bug where Zulip's team_id could be the empty string,
which tripped up using GitLab's slash commands with Zulip.
Fixes#19588.
This commit removes the existing default_twenty_four_hour_time field in
Realm table which was used to set the twenty_four_hour_time setting of
new user on joining and instead we now use the twenty_four_hour_time
field of RealmUserDefault table for the same.
With some tweaks by tabbott to clarify the documentation.
These values are currently either a string already or a List[int]. We
should do the conversion in
do_update_user_custom_profile_data_if_changed properly: if the value is
already a string, it can be used directly - if it's not, orjson.dumps is
a more future-proof way of converting than str(). Using orjson.dumps
here also allows us to change the converter of the USER type
CustomProfileField to orjson.loads, which is nicer to have than
ast.literal_eval.
While orjson.dumps() and str() give the same output when
given the special case of List[int],
ast.literal_eval was previously used due to orjson.loads not being
a good inverse function to str in general. That gets straightened out
now.
Tim Abbott
Mateusz Mandera
Alex Vandiver
Shlok Patel
RISHABH SIDANA
Anders Kaseorg
Jonny Tran
Lauryn Menard
Sahil Batra
rht
Eeshan Garg
Gaurav Pandey
Vishnu KS
Alya Abbott
Dinesh
Aman Agrawal
Pradyumna Sinha
Ganesh Pawar
sahil839
seiwailai
Kevin Scott