Commit Graph

6021 Commits

Author SHA1 Message Date
Mateusz Mandera b7b1ec0aeb outgoing_webhook: Improve invalid json handling when parsing response.
It's better to just raise JsonableError here, as that makes this error
processed in the central place for this kind of thing in do_rest_call:
---------
except JsonableError as e:
    response_message = e.msg
    logging.info("Outhook trigger failed:", stack_info=True)
    fail_with_message(event, response_message)
    response_message = f"The outgoing webhook server attempted to send a message in Zulip, but that request resulted in the following error:\n> {e}"
    notify_bot_owner(event, failure_message=response_message)
    return None
----------

which does all the things that are supposed to happen -
fail_with_message, appropriate logging and notifying the bot owner.
2021-04-26 09:32:35 -07:00
Mateusz Mandera b998138d3a outgoing_webhook: Handle valid, but unexpected json in response.
Responses such as "null" or "true" are valid json, but json.loads
returns different objects than dicts that the codepath expects.

Fixes #18223.
2021-04-26 09:32:35 -07:00
Anders Kaseorg 6060d0d364 docs: Add missing space to compound verbs “log in”, “set up”, etc.
Noun: backup, checkout, cleanup, login, logout, setup, shutdown, signup,
timeout.

Verb: back up, check out, clean up, log in, log out, set up, shut
down, sign up, time out.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-26 09:31:08 -07:00
Anders Kaseorg e3f2ffa681 docs: Capitalize “Markdown” consistently.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-26 09:31:08 -07:00
Anders Kaseorg 178736c8eb docs: Fix spelling errors caught by codespell.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-26 09:31:08 -07:00
Aman Agrawal ebe822341d message_edit: Don't add content edit to all messages.
Remove content edit keys if present in edit_history_event
when passing to update_messages_for_topic_edit.

Since content edit is only applied to the edited_message,
this shouldn't be part of the rest of the messages for which
topic was edited. This was a bug identified by
editing topic and content of a message at the same time
when more than 1 message is affected.
2021-04-24 13:51:49 -07:00
Aman Agrawal 79d748ba7d message_edit: Use update_edit_history to update message history. 2021-04-23 15:12:09 -07:00
Aman Agrawal de50f4ae25 message_edit: Extract update_edit_history. 2021-04-23 15:12:09 -07:00
Aman Agrawal 736fdcda49 update_messages_for_topic_edit: Remame `message` variable. 2021-04-23 15:12:09 -07:00
Mateusz Mandera 1a8ad796f8 models: Replace __id syntax with _id where possible.
model__id syntax implies needing a JOIN on the model table to fetch the
id. That's usually redundant, because the first table in the query
simply has a 'model_id' column, so the id can be fetched directly.
Django is actually smart enough to not do those redundant joins, but we
should still avoid this misguided syntax.

The exceptions are ManytoMany fields and queries doing a backward
relationship lookup. If "streams" is a many-to-many relationship, then
streams_id is invalid - streams__id syntax is needed. If "y" is a
foreign fields from X to Y:
class X:
  y = models.ForeignKey(Y)

then object x of class X has the field x.y_id, but y of class Y doesn't
have y.x_id. Thus Y queries need to be done like
Y.objects.filter(x__id__in=some_list)
2021-04-22 14:53:00 -07:00
Alex Vandiver 11177a40da soft_deactivate: Log and continue on failure to catch up a user.
There exists a logic bug (see #18236) which causes duplicate
usermessage rows to be inserted.  Currently, this stops catch-up for
all users.

Catch and record the exception for each affected user, so we at least
make catch-up progress on other users.
2021-04-22 14:38:03 -07:00
Mateusz Mandera 8d4ab69a46 docs: Move the /configure-missed-message-emails help page.
configure-message-notification-emails is the correct name now.
2021-04-21 10:10:54 -07:00
Mateusz Mandera 977a2f7fa0 emails: Rename "missed message email" to "message notification email". 2021-04-21 10:10:54 -07:00
Mateusz Mandera 716449030d emails: Rename missed message email sender to "Zulip notifcations".
It was decided that this is more appropriate naming. "Missed message"
gives it a bit of a sound like something went wrong.
2021-04-21 10:10:54 -07:00
Tim Abbott 6346b9d3eb models: Replace user_profile__is_active queries with is_user_active.
This saves a couple database queries by using the recently added
denormalization for Subscription objects.
2021-04-19 18:30:31 -07:00
Tim Abbott a1cfe25f8d streams: Move can_access_stream_user_ids into streams.py.
This belongs either here or in stream_subscription.py, which arguably
should just be merged into streams.py anyway.
2021-04-19 18:30:31 -07:00
Tim Abbott e4932bd952 actions: Fixed deactivated user IDs being included for stream events.
This was a mostly harmless bug, since those users cannot have active
clients, but fixing it will improve performance in any Zulip
organization where the vast majority of users are deactivated.
2021-04-19 18:30:31 -07:00
akshatdalton 6509c4f8f4 linkifiers: Add an API to support the editing of linkifier.
This commit adds an API to `zproject/urls.py` to edit/update
the realm linkifier. Its helper function to update the
database is added in `zerver/lib/actions.py`.

`zulip.yaml` is documented accordingly as well, clearly
stating that this API updates one linkifier at a time.

The tests are added for the API and helper function which
updates the realm linkifier.

Fixes #10830.
2021-04-19 18:01:45 -07:00
akshatdalton b29bd71a9c Refactor: Use `id` instead of `pk` as key.
Use `id` instead of `pk` as key to get RealmFilter
object in `do_remove_linkifier` function in `actions.py`.
2021-04-19 18:01:45 -07:00
Mateusz Mandera ccfcc186ad subs: Fix subscriber_..._history_access to not exclude subbed guests.
Guests are supposed to have stream history access to public streams
they're subscribed to.
2021-04-19 10:10:51 -07:00
Mateusz Mandera 68d1f2d7ef streams: Add realm check in can_access_stream_history.
The caller is supposed validate the stream and user realm match, but
since this is a security-sensitive function, we should have this
defensive code to protect against some validation bugs in the caller
leading to this being called incorrectly and returning True.
2021-04-19 10:10:51 -07:00
Mateusz Mandera f5c4005f8a actions: Fix some lists incorrectly named "subscribers".
These contain subscriptions, not subscribers.
2021-04-19 10:10:51 -07:00
Mateusz Mandera 4e26a9e9d6 subs: Fix codepaths incorrectly fetching subs of deactivated users.
Fixes #17922.

These two places fetch subscriptions for the sake of getting user ids to
send events to. Clearly deactivated users should be excluded from that.
2021-04-19 10:10:51 -07:00
Mateusz Mandera 50bfbb588e subs: Allow filtering by is_user_active in get_active_subscriptions.
get_active_subscriptions_for_stream_id should allow specifying whether
subscriptions of deactivated users should be included in the result.
Active subs of deactivated users are  a subtlety that's easy to miss
when writing relevant code, so we make include_deactivated_users a
mandatory kwarg - this will force callers to definitely give thought to
whether such subs should be included or not.

This commit is just a refactoring, we keep original behavior everywhere
- there are places where subs of deactivates users should probably be
excluded but aren't - we don't fix that here, it'll be addressed in
follow-up commits.
2021-04-19 10:10:51 -07:00
Mateusz Mandera c3a8a15bae delete_messages: Pass a list of user ids in the event in all cases.
The bulk deletion codepath was using dicts instead of user ids in the
event, as opposed to the other codepath which was adjusted to pass just
user ids before. We make the bulk codepath consistent with the other
one. Due to the dict-type events happening in 3.*, we move the goal for
deleting the compat code in process_notification to 5.0.
2021-04-16 09:54:14 -07:00
Anders Kaseorg f59f2ca165 requirements: Re-drop direct dependency on mock.
This was dropped in commit 840cf4b885
(#15091), but commit 1432067959
(#17047) mistakenly reintroduced it.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-15 21:47:33 -07:00
Anders Kaseorg bdb20a8002 integrations: Convert deprecated Django url to path.
django.conf.urls.url is actually a deprecated alias of
django.urls.re_path, but we want path instead of re_path.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-15 18:01:34 -07:00
Anders Kaseorg 2939d29b6d python: Convert deprecated Django smart_text alias to smart_str.
django.utils.encoding.smart_text is a deprecated alias of
django.utils.encoding.smart_str as of Django 3.0, and will be removed
in Django 4.0.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-15 18:01:34 -07:00
Anders Kaseorg dcdb00a5e6 python: Convert deprecated Django is_safe_url.
django.utils.http.is_safe_url is a deprecated alias of
django.utils.http.url_has_allowed_host_and_scheme as of Django 3.0,
and will be removed in Django 4.0.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-15 18:01:34 -07:00
Anders Kaseorg e7ed907cf6 python: Convert deprecated Django ugettext alias to gettext.
django.utils.translation.ugettext is a deprecated alias of
django.utils.translation.gettext as of Django 3.0, and will be removed
in Django 4.0.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-15 18:01:34 -07:00
Adam Birds 545cd961f4 integrations: Add docs for GitHub Actions integration.
I have added a documentation page for the GitHub Actions integration to
`/integrations/doc/github-actions` with a link to the Zulip GitHub
Actions repository.

Tweaked by tabbott to add cross-links with the main GitHub integration.
2021-04-15 16:42:31 -07:00
Tim Abbott 9d852870ee streams: Delete risky helper get_stream_by_id. 2021-04-14 12:37:34 -07:00
Mateusz Mandera 6e11754642 CVE-2021-30478: Prevent API super users from forging messages to other organizations.
A bug in the implementation of the can_forge_sender permission
(previously is_api_super_user) resulted in users with this permission
being able to send messages appearing as if sent by a system bots,
including to other organizations hosted by the same Zulip installation.

- The send message API had a bug allowing an api super user to
  use forging to send messages to other realms' streams, as a
  cross-realm bot. We fix this most directly by eliminating the
  realm_str parameter - it is not necessary for any valid current use
  case. The email gateway doesn't use this API despite the comment in
  that block suggesting otherwise.
- The conditionals inside access_stream_for_send_message are changed up
  to improve security. They were generally not ordered very well,
  allowing the function to successfully return due to very weak
  acceptance conditions - skipping the higher importance checks that
  should lead to raising an error.
- The query count in test_subs is decreased because
  access_stream_for_send_message returns earlier when doing its check
  for a cross-realm bot sender - some subscription checking queries are
  skipped.
- A linkifier test in test_message_dict needs to be changed. It didn't
  make much sense in the first place, because it was creating a message
  by a normal user, to a stream outside of the user's realm. That
  shouldn't even be allowed.
2021-04-14 12:37:34 -07:00
Mateusz Mandera 4235be759d CVE-2021-30477: Prevent outgoing webhook bots from sending arbitrary messages to any stream.
A bug in the implementation of replies to messages sent by outgoing
webhooks to private streams meant that an outgoing webhook bot could be
used to send messages to private streams that the user was not intended
to be able to send messages to.

Completely skipping stream access check in check_message whenever the
sender is an outgoing webhook bot is insecure, as it might allow someone
with access to the bot's API key to send arbitrary messages to all
streams in the organization. The check is only meant to be bypassed in
send_response_message, where the stream message is only being sent
because someone mentioned the bot in that stream (and thus the bot
posting there is the desired outcome). We get much better control over
what's going by passing an explicit argument to check_message when
skipping the access check is desirable.
2021-04-14 12:37:34 -07:00
Aman Agrawal 802c450b3f realm: Add setting to configure GIPHY rating.
Organization admins can use this setting to restrict the maximum
rating of GIFs that will be retrieved from GIPHY. Also, there
is option to disable GIPHY too.
2021-04-14 10:29:39 -07:00
m-e-l-u-h-a-n dd308528c2 docs(integrations): Document zoom video provider in /integrations.
Moves documentation about using zoom as video call provider
to /integrations. This documentation was earlier present
at /help/start-a-call and is moved as asked in issue #17588.
2021-04-14 08:44:00 -07:00
m-e-l-u-h-a-n 4077673da7 docs(integrations): Add Big Blue Button video provider on /integrations.
Moves documentation about using Big Blue Button as video call
provider to /integrations. This documentation was earlier
present at /help/start-a-call and is moved as asked in issue #17588.
2021-04-14 08:44:00 -07:00
m-e-l-u-h-a-n 13e43917db docs(integrations): Document jitsi video provider in /integrations.
Moves documentation about using jitsi as video call provider
to /integrations. This documentation was earlier present
at /help/start-a-call and is moved as asked in issue #17588.
2021-04-14 08:44:00 -07:00
Tim Abbott 9f57961e5f stream_subscription: Remove opaque reference to guest role. 2021-04-13 21:49:57 -07:00
sahil839 685fbffd91 tests: Refactor check_has_permission_policies to check for all user roles.
We refactor check_has_permission_policies to check for all user roles for
each value of policy. This will help in handle a case where a guest is
allowed to do something but moderator isn't.

We need to do user_profile.refresh_from_db() in validation_func because
the realm object from user_profile is used in has_permission and we need
updated realm instance after changing the policy.

This is a follow-up commit to 9a4c58cb.
2021-04-13 17:48:23 -07:00
Abhijeet Prasad Bodas 3947b0c80a linkifiers: Update API to send data using dictionaries.
* This introduces a new event type `realm_linkifiers` and
a new key for the initial data fetch of the same name.
Newer clients will be expected to use these.

* Backwards compatibility is ensured by changing neither
the current event nor the /register key. The data which
these hold is the same as before, but internally, it is
generated by processing the `realm_linkifiers` data.
We send both the old and the new event types to clients
whenever the linkifiers are changed.
Older clients will simply ignore the new event type, and
vice versa.

* The `realm/filters:GET` endpoint (which returns tuples)
is currently used by none of the official Zulip clients.
This commit replaces it with `realm/linkifiers:GET` which
returns data in the new dictionary format.
TODO: Update the `get_realm_filters` method in the API
bindings, to hit this new URL instead of the old one.

* This also updates the webapp frontend to use the newer
events and keys.
2021-04-13 12:16:07 -07:00
Anders Kaseorg b01d43f339 mypy: Fix strict_equality violations.
puppet/zulip/files/nagios_plugins/zulip_postgresql/check_postgresql_replication_lag:98: error: Non-overlapping equality check (left operand type: "List[List[str]]", right operand type: "Literal[0]")  [comparison-overlap]
zerver/tests/test_realm.py:650: error: Non-overlapping container check (element type: "Dict[str, Any]", container item type: "str")  [comparison-overlap]

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-04-13 09:18:18 -07:00
Tim Abbott 2e928a0853 markdown: Remove logic for creating markdown engines for all realms.
This logic likely never ran due to a combination of bugs.

* Running `maybe_update_markdown_engines` unconditionally meant that
  `if md_engine_key in md_engines` was likely always true.
* Introduced in 65838bb: DEFAULT_MARKDOWN_KEY could never be in
  md_engines, so should we have ever reached that code path, we'd have
  tried to rebuild all markdown engines every time.

And it also wasn't clearly helpful -- because we fetch all linkifiers
for a realm on every request anyway, we don't really save database
queries by doing a bulk fetch on startup, and doing so would likely
result in a material regression to Zulip's overall startup time that
we were creating markdown engines for large numbers of realms in bulk
during process startup.
2021-04-13 09:18:18 -07:00
Abhijeet Prasad Bodas 2b9f2cc8ff mute user: Add some comments on message fetch.
These explain why we don't consider user mutes
in message fetching/unread data.
2021-04-13 09:15:49 -07:00
Abhijeet Prasad Bodas 8b098b95bb mute user: Mark as read old messages immediately.
When a user is muted, in the same request,
we mark any existing unreads from that user
as read.

This is done for all types of messages
(PM/huddle/stream) and regardless of whether
the user was mentioned in them.

This will not break the unread count logic
of the web frontend, because that algorithm
decides which messages to mark as read based
only on the pointer location and the whitespace
at the bottom, not on what messages have already
been marked as read.
2021-04-13 09:08:47 -07:00
Abhijeet Prasad Bodas 2f56f8d0ed mute user: Mark as read new messages.
Messages sent by muted users are marked as read
as soon as they are sent (or, more accurately,
while creating the database entries itself), regardless
of type (stream/huddle/PM).

ede73ee4cd, makes it easy to
pass a list to `do_send_messages` containing user-ids for
whom the message should be marked as read.
We add the contents of this list to the set of muter IDs,
and then pass it on to `create_user_messages`.

This benefits from the caching behaviour of `get_muting_users`
and should not cause performance issues long term.

The consequence is that messages sent by muted users will
not contribute to unread counts and notifications.

This commit does not affect the unread messages
(if any) present just before muting, but only handles
subsequent messages. Old unreads will be handled in
further commits.
2021-04-13 09:08:47 -07:00
Abhijeet Prasad Bodas b140c17441 mute user: Cache list of muter IDs.
This commit defines a new function `get_muting_users`
which will return a list of IDs of users who have muted
a given user.
Whenever someone mutes/unmutes  a user, the cache will be
flushed, and subsequently when that user sends a message,
the cache will be populated with the list of people who
have muted them (maybe empty).

This data is a good candidate for caching because-

1. The function will later be called from the message send
codepath, and we try to minimize database queries there.

2. The entries will be pretty tiny.

3. The entries won't churn too much. An average user will
send messages much more frequently than get muted/unmuted,
and the first time penalty of hitting the db and populating
the cache should ideally get amortized by avoiding several
DB lookups on subsequent message sends.

The actual code to call this function will be written in
further commits.
2021-04-13 09:08:47 -07:00
Abhijeet Prasad Bodas 9602aa1467 mute user: Record entries in RealmAuditLog.
This makes it so that RealmAuditLog entries are
created when a user mutes/unmutes someone.

We don't really need to store the time, but we
do so anyways, because the `event_time` field
is currently a non-nullable one in the `RealmAuditLog`
model, and making it nullable would risk allowing
not specifying the time in other more important
code which also creates `RealmAuditLog` entries.

This also fixes an incorrect test of successfully
unmuting with the API. Earlier it did not mock
the time in the `views/muting.py` code to return
`mute_time`.
2021-04-13 09:08:47 -07:00
Zeeshan Equbal 2da4443cc5
api: Add max_message_length field to API data.
Commit 4a3ad0d introduced some extra stream-level parameters
to the `realm` object. This commit extends that to add a
max_message_length paramter too in the same server_level.
2021-04-12 16:03:31 -07:00
Tim Abbott 4a3ad0da06 api: Improve encoding of stream/topic max field lengths.
Previously, you had to request the `stream` event type in order to get
the stream-level parameters; this was a bad design in part because the
`subscription` event type has similar data and is preferred by most
clients.

So we move these to the `realm` object.  We also add the maximum topic
length, as an adjacent parameter.

While changing this, we also fix these to better match the names of
similar API parameters.
2021-04-10 10:07:57 -07:00