This commit adds code to check whether a user is allowed to use
wildcard mention in a large stream or not while editing a message
based on the realm settings.
Previously this was only checked while sending message, thus user
was easily able to use wildcard mention by first sending a normal
message and then using a wildcard mention by editing it.
(cherry picked from commit b68ebf5a22)
Race conditions in stream unsubscription may lead to multiple
back-to-back SUBSCRIPTION_DEACTIVATED RealmAuditLog entries for the
same stream. The current logic constructs duplicate UserMessage
entries for such, which then later fail to insert.
Keep a set of message-ids that have been prep'd to be inserted, so
that we don't duplicate them if there is a duplicated
SUBSCRIPTION_DEACTIVATED row. This also renames the `message` local
variable, which otherwise overrode the `message` argument of a
different type.
(cherry picked from commit 6b6dcf6ce1)
This adds `soupsieve` as an explicit dependency, but intentionally
does not adjust the provision version, as it was already an indirect
dependency.
(cherry picked from commit 6a40c17ccf)
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.
(cherry picked from commit 52f74bbd9b)
Only clear_scheduled_emails previously took a lock on the users before
removing them; make deliver_scheduled_emails do so as well, by using
prefetch_related to ensure that the table appears in the SELECT. This
is not necessary for correctness, since all accesses of
ScheduledEmailUser first access the ScheduledEmail and lock it; it is
merely for consistency.
Since SELECT ... FOR UPDATE takes an UPDATE lock on all tables
mentioned in the SELECT, merely doing the prefetch is sufficient to
lock both tables; no `on=(...)` is needed to `select_for_update`.
This also does not address the pre-existing potential deadlock from
these two use cases, where both try to lock the same ScheduledEmail
rows in opposite orders.
(cherry picked from commit 4c518c2bba)
No codepath except tests passes in more than one user_profile -- and
doing so is what makes the deduplication necessary.
Simplify the API by making it only take one user_profile id.
(cherry picked from commit ebaafb32f3)
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.
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 is much faster than calling generate_presigned_url each time.
```
In [3]: t = time.time()
...: for i in range(250):
...: x = u.get_public_upload_url("foo")
...: print(time.time()-t)
0.0010945796966552734
```
Fixes#18915
This was very slow, causing performance issues. After investigating,
generate_presigned_url is the cheap part of this, but the
session.client() call is expensive - so that's what we should cache.
Before the change:
```
In [4]: t = time.time()
...: for i in range(250):
...: x = u.get_public_upload_url("foo")
...: print(time.time()-t)
6.408717393875122
```
After:
```
In [4]: t = time.time()
...: for i in range(250):
...: x = u.get_public_upload_url("foo")
...: print(time.time()-t)
0.48990607261657715
```
This is not good enough to avoid doing something ugly like replacing
generate_presigned_url with some manual URL manipulation, but it's a
helpful structure that we may find useful with further refactoring.
This causes avatars and emoji which are hosted by Zulip in S3 (or
compatible) servers to no longer go through camo. Routing these
requests through camo does not add any privacy benefit (as the request
logs there go to the Zulip admins regardless), and may break emoji
imported from Slack before 1bf385e35f,
which have `application/octet-stream` as their stored Content-Type.
Our current logic only allows S3 block storage providers whose
upload URL matches with the format used by AWS. This also allows
other styles such as the "virtual host" format used by Oracle cloud.
Fixes#17762.
modified_user=sub_info.user and modified_stream=sub_info.stream, added
by commit 6d1f9de7d3 (#16553), were
always coming from the last entry in the loop above, not from the
enclosing list comprehension.
Found by the Pylint rule undefined-loop-variable.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
?dl=1 causes Dropbox to send Content-Type: application/binary, which
can’t be interpreted by Camo. Use ?raw=1 instead.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
These logs were pretty spammy, and there have long been much better
ways to communicate to system administrators that the incoming email
gateway is great, including, most importantly, in the section of the
emails themselves that explains how replying works.
Previously only admins were allowed to move messages between streams
and admins are allowed to post in any stream irresepctive of stream
post policy, so there was no need to check for stream post policy.
But as we now allow other members to also move messages, we need
to check whether the user who is moving the message is allowed
to post to the target stream (i.e. stream to which the messages
are being moved) and thus we allow moving messages only if the
user is allowed to post in target stream.
b7b1ec0aeb made our checks of the response
format stronger, to enforce that the json translates to a valid dict.
However, old client code (zulip_botserver) was using "" as equivalent to
response_not_required - so we need to keep backward-compatibility to not
break things built on it.
Currently, moving messages between streams is an action limited to
organization administrators. A big part of the motivation for that
restriction was to prevent users from moving messages from a private
stream without shared history as a way to access messages they should
not have access to.
Organization administrators can already just make the stream have
shared history if they want to access its messages, but allowing
non-administrators to move messages between would have
introduced a security bug without this change.
This completes the effort to make it possible to use
bulk_access_message in contexts where there are more than a handful of
messages without creating performance issues.
This new function optimizes how we fetch subscriptions
for streams. Basically, it excludes most long-term-idle
users from the query.
With 8k users, of which all but 400 are long term idle,
this speeds up get_recipient_info from about 150ms
to 50ms.
Overall this change appears to save a factor of 2-3 in the backend
processing time for sending or editing a message in large, public
streams in chat.zulip.org (at 18K users today).
If the caller has already fetched the Stream or subscription details
for the user, those can be passed to has_message_access to avoid extra
database queries.
When the format of the response received from the outgoing webhook
server is invalid (unparsable json, or just wrong format that doesn't
translate into a dictionary etc.), a message with the error is sent to
the bot owner. We should include the actual payload to make reasonable
debugging possible.
In notify_bot_owner we have to move the `if response_content` block to
append the payload to the message whenever it was specified as an
argument to the function. It shouldn't be nested inside
`elif status_code` as before.
This makes it parallel with deliver_scheduled_messages, and clarifies
that it is not used for simply sending outgoing emails (e.g. the
`email_senders` queue).
This also renames the supervisor job to match.
The screenshot generating mechanism doesn't work for newrelic and
causes error because its configuration file doesn't exist. This
commit fixes the configuration and re-generate the screenshots.
Also link to it from the API documentation page,
other help pages, and the confirmation dialog for
muting a user.
With substantial edits by tabbott and alya.
A message containing wildcard mention when quoted (which
is turned into a silent mention) or message with silent
wildcard mention notifies the users by sending desktop,
sound, and missed message email notifications. This
is clearly a bug which is fixed by this commit.
Fixes: #18354.
