Commit Graph

649 Commits

Author SHA1 Message Date
yogesh sirsat e4b74c2099 streams: Fix misleading comment in "test_subs". 2022-09-08 16:56:07 -07:00
Lauryn Menard 885c3d6593 subscriptions: Send update events for `is_muted` property.
In Zulip 2.1.0, the `is_muted` stream subscription property was
added and replaced the `in_home_view` property. But the server has
still only been sending subscription update events with the
`in_home_view` property.

Updates `do_change_subscription_property` to send a subscription
update event for both `is_muted` and `in_home_view`, so that
clients can fully migrate away from using `in_home_view` allowing
us to eventually remove it completely.
2022-08-16 16:50:51 -07:00
Sahil Batra 3e6463804e streams: Allow changing history access without is_private parameter.
We now allow changing access to history of the stream by only passing
"history_public_to_subscribers" parameter. Previously, "is_private"
parameter was also required to change history_public_to_subscribers
otherwise the request was silently ignored.

We also raise error when only history_public_to_subscribers parameter
is passed with value False without "is_private: True" for a public
or web-public stream since we do not allow public streams with
protected history.
2022-08-09 17:05:38 -07:00
Sahil Batra 6ccfebac56 streams: Raise error when making public stream with private history.
We raise error when we try to change a public stream (except for
zephyr mirror realms) to be public with protected history, as we do
not support such streams yet.

Previously, in such case we changed nothing and a notification was
sent to the "stream events" topic with message being "stream is
changed from public to public" and was weird.

Note that this commit only handles the case when both is_private and
history_public_to_subscribers parameters are passed to API and commit
not covers the case when only "history_public_to_subscribers" with
value False is passed to API, since we currently ignore requests
which has only history_public_to_subscribers parameter with not None
and not is_private and is_web_public.
We would do this in further commits when we add support for accepting
only history_public_to_subscribers parameter.
2022-08-09 17:05:38 -07:00
Anders Kaseorg 443b974b3e python: Apply changes from pyupgrade.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-07-20 22:33:28 -07:00
Sahil Batra 093a74bd54 models: Remove "role" field from Subscription.
This commit removes "role" field from subscription
objects since we are not moving forward with stream
administrator concept and instead working on new
permssions model as per #19525.
2022-07-13 17:07:44 -07:00
Sahil Batra d31fc27912 streams: Remove is_stream_admin property and its uses.
This commit removes the is_stream_admin property of Subscription
model and also updates check_stream_access_for_delete_or_update
to not return true when is_stream_admin is True.

We also removes the relevant tests.

This change is done as we would not be moving forward with the
stream administrator concept as we have decided to modify the
permissions model as per #19525.
2022-07-13 17:07:44 -07:00
Zixuan James Li 426f8ce385 tests: Replace `HttpResponse` with `TestHttpResponse.`
Since `HttpResponse` is an inaccurate representation of the
monkey-patched response object returned by the Django test client, we
replace it with `_MonkeyPatchedWSGIResponse` as `TestHttpResponse`.

This replaces `HttpResponse` in zerver/tests, analytics/tests, coporate/tests,
zerver/lib/test_classes.py, and zerver/lib/test_helpers.py with
`TestHttpResponse`. Several files in zerver/tests are excluded
from this substitution.

This commit is auto-generated by a script, with manual adjustments on certain
files squashed into it.

This is a part of the django-stubs refactorings.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-06-08 11:25:03 -07:00
Sahil Batra dfacd62d93 streams: Update error message returned for administrative access.
We have now decided to not continue with the stream administrator
concept as we are changing the permissions model to be based on
user groups as per #19525. So, this commit updates the error message
to "Must be an organization administrator".
2022-06-07 09:36:24 -07:00
Zixuan James Li a142fbff85 tests: Refactor away result.json() calls with helpers.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-06-06 23:06:00 -07:00
Sahil Batra 708204290e streams: Capitalize "ID" in invalid stream errors in API.
This commit changes the error message from "Invalid stream id"
to "Invalid stream ID" for cases where invalid stream IDs are
passed to API endpoints to make it consistent with other similar
error messages.
2022-05-27 17:06:03 -07:00
Zixuan James Li 44ecd66eae types: Better types for API fields.
Signed-off-by: Zixuan James Li <359101898@qq.com>
2022-05-27 14:43:00 -07:00
Sahil Batra ce34b585a5 streams: Add endpoint "GET /streams/{stream_id}" to get stream by id.
Fixes #22082.
2022-05-23 15:14:04 -07:00
Sahil Batra 95a2b580a0 streams: Save one DB query in bulk_remove_susbcriptions.
We remove one call to get_occupied_streams to get occupied
streams before unsubscribing because we already know which
streams can become vacant, i.e. the one from which users are
being unsubscribed, and we can directly use the list of streams
from which users are being unsubscribed and get vacant streams
by checking which of these streams are not in get_occupied_streams
called after unsubscribing users.
2022-05-17 15:09:15 -07:00
Alex Vandiver 803982e872 message_flags: Short-circuit if no messages changed.
Omit sending an event, and updating the database, if there are no
matching messages.
2022-05-12 21:57:55 -07:00
Sahil Batra 0bfe973b65 streams: Mark messages as read only for unsubscribed streams.
Previously, we were marking messages of all the streams passed
to bulk_remove_subscriptions even if user was not subscribed
to some of them and those streams would ideally not have
any unread messages. This code was added in 766511e519.

This commit changes the code to only mark messages of actually
unsubscribed streams as read.
2022-05-12 18:53:45 -07:00
Anders Kaseorg 6331a314d4 Correctly hyphenate “non-”.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-27 22:10:31 -07:00
Anders Kaseorg 729019acdd actions: Move part into zerver.lib.test_classes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:37 -07:00
Anders Kaseorg e01faebd7e actions: Split out zerver.actions.create_realm.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:37 -07:00
Anders Kaseorg 59f6b090c7 actions: Split out zerver.actions.realm_settings.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:37 -07:00
Anders Kaseorg 5fcbc412cf actions: Split out zerver.actions.streams.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:35 -07:00
Anders Kaseorg d7981dad62 actions: Split out zerver.actions.users.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:34 -07:00
Anders Kaseorg 9dd7e34ab3 actions: Move part into zerver.lib.subscription_info.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:33 -07:00
Anders Kaseorg a29f1b39da actions: Move part into zerver.lib.streams.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:32 -07:00
Anders Kaseorg 1ac7496855 actions: Split out zerver.actions.default_streams.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-04-14 17:14:31 -07:00
Lauryn Menard 260012a87e tests: Remove ignored parameters from stream groups tests.
`update_default_stream_group_info` was being passed `op` and
`group_name` in various tests, which are not implemented as
parameters for that endpoint / code path. So this removes those
from the existing tests. This is not a documented API endpoint,
so perhaps these were just overlooked when these tests were
written / last refactored.
2022-04-08 11:39:06 -07:00
Lauryn Menard 0008a76703 tests: Remove ignored `stream_name` parameter from test.
Removes unnecessary `stream_name` parameter from
`test_stream_permission_changes_updates_updates_attachments`.
2022-03-30 11:30:31 -07:00
jai2201 20e6315705 streams: Fix empty description in stream change notifications.
Send 'No description' text when either the old or new stream
description is empty, to avoid odd looking notifications.
2022-03-29 01:39:55 -07:00
Tim Abbott c789097184 streams: Fix empty description handling in new stream notifications.
"No description." is conveniniently an existing translated string, in
addition to being the right content for this setting.
2022-03-29 01:39:55 -07:00
Tim Abbott d149af936d models: Rework Attachment.is_*_public to be a cache.
Previously, Attachment.is_realm_public and its cousin,
Attachment.is_web_public, were properties that began as False and
transitioned to True only when a message containing a link to the
attachment was sent to the appropriate class of stream, or such a link
was added as part of editing a message.

This pattern meant that neither field was updated in situations where
the access permissions for a message changed:

* Moving the message to a different stream.
* Changing the permissions for a stream containing links to the message.

This correctness issue has limited security impact, because uploaded
files are secured both by a random URL and by these access checks.

To fix this, we reformulate these fields as a cache, with code paths
that change the permissions affecting an attachment responsible for
setting these values to the `None` (uncached) state. We prefer setting
this `None` state over computing the correct permissions, because the
correct post-edit permissions are a function of all messages
containing the attachment, and we don't want to be responsible for
fetching all of those messages in the edit code paths.
2022-03-24 10:49:59 -07:00
Steve Howell f9ddd80122 subscriptions: Improve types for subscription info.
The newly extracted module here has significant changes
to all the functions, but they achieve the same thing
as before.
2022-03-14 18:01:39 -07:00
Steve Howell 8f99894302 streams: Extract stream_color library.
This is a pure code move.
2022-03-14 18:01:36 -07:00
Steve Howell 200a89cef0 streams: Extract stream_traffic library.
This is a pure code move.
2022-03-14 18:01:36 -07:00
Tim Abbott d693a6717c i18n: Remove quote syntax from stream description notification.
Translators found it confusing, since it's not at all obvious that the
word "quote" should not be translated.

I'm not altogether happy with the code formatting for this.

While we're changing this, also standardize on the "```` quote" style
of quote blocks to ensure code/quote blocks in stream descriptions are
unlikely to conflict with this syntax.
2022-03-09 15:22:57 -08:00
Steve Howell df47e4312b unread: Refactor aggregated message info.
We want TypedDicts that have actual teeth.

In order to make type checks meaningful, we want
to avoid Any, object, or crazy Union types when
we aggregate each type of message, so we replaced
a generic function with three concrete functions.
2022-03-07 15:34:56 -08:00
NerdyLucifer 4b9770e270 stream_settings: Show stream privacy & description in stream events.
Provide stream privacy and description in stream notification events
when stream is created.
In function "send_messages_for_new_subscribers" for when stream is
created, put policy name and description of the stream.

Fixes #21004
2022-03-07 11:53:49 -08:00
Lauryn Menard bd0ddbe20e tests: Remove unused `stream_name` from tests.
Various tests use the `PATCH /stream/{stream_id} endpoint in
`test_subs.py`. Because the stream id is in the URL path, it
does not also need to be passed as a query parameter.

Removes instances of `stream_name` being passed as a query
parameter to tests.
2022-02-24 12:57:18 -08:00
Shantanu 4371705a4c streams: Update formatting of retention change notification.
This changes the notifications to be more readable like our other
similar stream events notifications.

Fixes #20969.
2022-02-18 11:11:38 -08:00
Aman Agrawal 7614f2203a pricing: Replace "Zulip Standard" with "Zulip Cloud Standard".
Case sensitive replace.
2022-02-09 11:00:24 -08:00
Anders Kaseorg b0ce4f1bce docs: Fix many spelling mistakes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-02-07 18:51:06 -08:00
Tim Abbott 1045737be6 test_subs: Use HostRequestMock for request object.
I incorrectly removed this when simplifying
dbddbee5a115b9352862cb13d4c66820865c30b6; while that commit did not
require the hunk re-added here, the later commit
3be622ffa7 added a call that did require it.
2022-02-04 15:36:57 -08:00
Lauryn Menard 3be622ffa7 backend: Add request as parameter to json_success.
Adds request as a parameter to json_success as a refactor towards
making `ignored_parameters_unsupported` functionality available
for all API endpoints.

Also, removes any data parameters that are an empty dict or
a dict with the generic success response values.
2022-02-04 15:16:56 -08:00
Alex Vandiver d1234ef18c string_validation: Prohibit non-printable characters in stream names.
Fixes part of #20128.

Co-authored-by: Shlok Patel <shlokcpatel2001@gmail.com>
2022-02-01 16:04:34 -08:00
Anders Kaseorg 90e202cd38 docs: Consistently hyphenate “web-public”.
In English, compound adjectives should essentially always be
hyphenated.  This makes them easier to parse, especially for users who
might not recognize that the words “web public” go together as a
phrase.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-01-28 17:45:45 -08:00
Mateusz Mandera 120de1db19 do_deactivate_stream: Use transaction.atomic. 2022-01-28 13:03:39 -08:00
Eeshan Garg aa8b3f9729 streams: Add RealmAuditLog entries for permission changes. 2022-01-21 13:59:35 -08:00
Eeshan Garg 0d99809fd3 streams: Add notifications for permission policy changes.
The change to curl_param_value_generators.py warrants a brief
explanation. Stream permission changes now generate a notification
message. Our curl example test for removing a reaction comes after
the two tests for updating the stream permission changes, thus the
hardcoded message ID in that test needs to be incremented by 2 to
account for the two notification messages that now come before it.

This is a part of #20289.
2022-01-21 13:59:34 -08:00
Eeshan Garg f0ee065292 streams: Use bulleted format for description change notifications.
We want the format for our description change notifications to be
consistent with the format of our stream posting policy change
notifications.
2022-01-21 13:59:34 -08:00
Steve Howell dd1c9c45c7 stream colors: Try harder to avoid collisions.
We now use recipient_id % 24 for new stream colors
when users have already used all 24 of our canned
colors.

This fix doesn't address the scenario that somebody
dislikes one of our current canned colors, so if a
user continually changes canned color N to some other
color for new streams, their new streams will continue
to include color N (and the user will still need to
change them).

This fix doesn't address the fact that it can be expensive
during bulk-add situations to query for all the colors
that users have already used up.

See https://chat.zulip.org/#narrow/stream/3-backend/topic/assigning.20stream.20colors
for more discussion.
2022-01-18 13:56:54 -08:00
Alex Vandiver 94dbb540b1 string_validation: Give a more specific message for empty stream names.
Co-authored-by: Shlok Patel <shlokcpatel2001@gmail.com>
2022-01-11 15:17:53 -08:00
Eeshan Garg d6b92092dd streams: Add RealmAuditLog entries for post policy changes. 2022-01-10 18:29:04 -08:00
Eeshan Garg c30458e174 streams: Add notifications for posting policy changes.
An explanatory note on the changes in zulip.yaml and
curl_param_value_generators is warranted here. In our automated
tests for our curl examples, the test for the API endpoint that
changes the posting permissions of a stream comes before our
existing curl test for adding message reactions.

Since there is an extra notification message due to the change in
posting permissions, the message IDs used in tests that come after
need to be incremented by 1.

This is a part of #20289.
2022-01-10 18:29:04 -08:00
Eeshan Garg 625af3cea9 streams: Add extra line break to description change notification.
The extra line break above "Old description:" aids readability.
2022-01-10 11:36:19 -08:00
Eeshan Garg f97093ba32 streams: Add RealmAuditLog entries for description changes. 2022-01-07 16:13:11 -08:00
Eeshan Garg 80f30f187e streams: Add notifications for description changes.
This is a part of #20289.
2022-01-07 16:13:11 -08:00
Steve Howell a9271e7a99 performance: Cache stream lookups in MentionBackend.
This is useful when you subscribe a bunch of folks
to a stream and need to send them all PMs telling
them about the new subscription.
2021-12-30 11:28:15 -08:00
Steve Howell c4bd4496dd peformance: Cache user mentions for multiple PMs.
It's slightly annoying to plumb Optional[MentionBackend]
down the stack, but it's a one-time change.

I tried to make the cache code relatively unobtrusive
for the single-message use case.

We should be able to eliminate redundant stream queries
using similar techniques.

I considered caching at the level of rendering the message
itself, but this involves nearly as much plumbing, and
you have to account for the fact that several users on
your realm may have distinct default languages (French,
Spanish, Russian, etc.), so you would not eliminate as
many query hops. Also, if multiple streams were involved,
users would get slightly different messages based on
their prior subscriptions.
2021-12-30 11:28:15 -08:00
Steve Howell a6201b430f tests: Improve checks for subscribing users.
We now check both the notification messages for
all three of Hamlet's peers.

And we count queries.
2021-12-30 11:23:25 -08:00
Steve Howell fd925e6045 streams: Add id to user mentions for stream notifications. 2021-12-30 11:23:25 -08:00
Steve Howell 01ebb2c85f refactor: Pass realm to bulk_remove_subscriptions.
We made a very similar change to bulk_add_subscriptions
earlier in the year.
2021-12-28 12:15:02 -08:00
Steve Howell 966d88a78a stream colors: Fix stream color assignment.
The bug here probably didn't come up too much in
practice, but if we were adding a user to multiple
streams when they already had used all N available
colors, all the new streams would be assigned the same
color, since the size of used_colors would stay at N,
thwarting our little modulo-len hackery.

It's not a terrible bug, since users can obviously
customize their stream colors as they see fit.

Usually when we are adding a user to multiple streams,
the users are fairly new, and thus don't have many
existing streams, so I have never heard this bug
reported in the field.

Anyway, assigning the colors in bulk seems to make more
sense, and I added some tests.

For the situations where all the colors have already
been used, I didn't put a ton of thought into exactly
which repeated colors we want to choose; instead, I
just ensure they're different modulo 24. It's possible
that we should just have more than 24 canned colors, or
we should just assign the same default color every time
and let users change it themselves (once they've gone
beyond the 24, to be clear). Or maybe we can just do
something smarter here. I don't have enough time for a
deep dive on this issue.
2021-12-28 12:15:02 -08:00
Steve Howell f638fd6f72 performance: Get used stream colors in separate trip.
This commit sets us up for the next commit, which will
save us a very expensive query.

If you are adding 15k users to a stream, and each user
has about 20 existing streams, then we need to retrieve
300k rows from the database to figure out which stream
colors they already have.  We don't need all the extra
fields from Subscription, so now we get just the two
values we need for making a color map.

In the next commit we'll eliminate the other use case
for the big query, and I will explain in greater
depth how splitting out the color-picking code can
be a huge win. It is possible that some product decisions
could make this codepath easier. We could also do some
engineering specific to stream colors, such as caching
which colors users have already used.

This does cost us an extra round trip to the database.
2021-12-28 12:15:02 -08:00
Steve Howell 2902f8b931 tests: Ensure stream senders get a UserMessage row.
We now complain if a test author sends a stream message
that does not result in the sender getting a
UserMessage row for the message.

This is basically 100% equivalent to complaining that
the author failed to subscribe the sender to the stream
as part of the test setup, as far as I can tell, so the
AssertionError instructs the author to subscribe the
sender to the stream.

We exempt bots from this check, although it is
plausible we should only exempt the system bots like
the notification bot.

I considered auto-subscribing the sender to the stream,
but that can be a little more expensive than the
current check, and we generally want test setup to be
explicit.

If there is some legitimate way than a subscribed human
sender can't get a UserMessage, then we probably want
an explicit test for that, or we may want to change the
backend to just write a UserMessage row in that
hypothetical situation.

For most tests, including almost all the ones fixed
here, the author just wants their test setup to
realistically reflect normal operation, and often devs
may not realize that Cordelia is not subscribed to
Denmark or not realize that Hamlet is not subscribed to
Scotland.

Some of us don't remember our Shakespeare from high
school, and our stream subscriptions don't even
necessarily reflect which countries the Bard placed his
characters in.

There may also be some legitimate use case where an
author wants to simulate sending a message to an
unsubscribed stream, but for those edge cases, they can
always set allow_unsubscribed_sender to True.
2021-12-10 09:40:04 -08:00
Eeshan Garg 8ebe05f644 streams: Add RealmAuditLog entry for message retention updates. 2021-12-07 14:53:50 -08:00
Eeshan Garg d2901892e2 streams: Add notifications for message retention policy updates.
This is a part of #20289.
2021-12-07 14:53:50 -08:00
Eeshan Garg 2cdaae681d actions: Rename do_change_plan_type -> do change_realm_plan_type.
We will soon be adding an equivalent function for RemoteZulipServer,
so it makes sense to rename this function to be more descriptive.
2021-12-06 16:18:53 -08:00
Lauryn Menard 7713b371a5 api: Migrate `/update-subscription-settings` response value.
Migrates the `/update-subscription-settings` api endpoint to the
`ignored_parameters_unsupported` model, which is also currently used
by `/update-settings` and `update-realm-user-settings-defaults`.

This change is a step towards preparing for an eventual migration to
have all endpoints return an `ignored_parameters_unsupported` block.

Previously the `/update-subscription-settings` endpoint returned a
copy of the data object sent in the request.

Fixes #15307.
2021-11-26 22:25:53 -08:00
Sahil Batra ad99b4fac9 streams: Allow changing stream to be web-public based on creation setting.
We allow a user to make an existing stream web-public only if user is
allowed to create web-public streams.
2021-11-23 10:48:20 -08:00
Eeshan Garg b325a4f1be realm: Rename plan type constants to be more descriptive.
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.
2021-10-19 12:20:39 -07:00
Sahil Batra e575f83351 test_subs: Remove _test_user_settings_for_adding_streams.
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.
2021-10-07 14:22:10 -07:00
Sahil Batra 2c2c19c4d4 streams: Restrict creating web public streams based on new setting.
This commit restricts creating web public streams based on the
recently added create_web_public_stream_policy setting.
2021-10-05 09:56:00 -07:00
Sahil Batra 3916181770 models: Add can_create_web_public_streams helper.
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.
2021-10-05 09:48:50 -07:00
Sahil Batra be0387b189 test_subs: Enforce invite_only argument to be named.
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.
2021-10-05 09:12:56 -07:00
Ganesh Pawar fa928d5cd1 streams: Split setting for stream creation policy.
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.
2021-10-01 10:26:42 -07:00
Aman Agrawal 5138652810 update_stream_backend: Add ability to make streams web public.
We allow clients to make existing streams web public via the API.

This feature is still disabled via settings in production
environments, because we may have additional policy rules or UI
warnings we wish to add to this sort of conversion.
2021-09-21 12:16:09 -07:00
Aman Agrawal 6a78112940 subscribe: Allow web public stream creation via the API.
User can now create web public stream via the /subscribe API.
So, when a web public stream present in the API request does not
exist, it will be created now by specifying the is_web_public
parameter. The parameter would have been ignored without this
commit.
2021-09-21 11:20:36 -07:00
Tim Abbott 71b8a1794a streams: Use standard error message when requiring owner.
The new error message is more clear about why, "User cannot create
stream with this settings." was bad English, and in any case removing
an unnecessary string is always an improvement for translators.
2021-09-21 11:05:30 -07:00
Anders Kaseorg 0d061f44c1 actions: Remove acting_client parameter from bulk_remove_subscriptions.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-08-19 01:51:37 -07:00
Sahil Batra 5459a92e4a setting: Use "unlimited" instead of "forever" for retention setting.
This commit updates both the stream-level and realm-level message
retention setting to use 'unlimited' instead of 'forever' to set
message retention setting to "retain messages forever".
2021-08-08 15:56:57 -07:00
Steve Howell 45f6c8d27f page load: Remove sender_ids in unread messages for streams. 2021-08-04 11:44:00 -04:00
Anders Kaseorg ad5f0c05b5 python: Remove default "utf8" argument for encode(), decode().
Partially generated by pyupgrade.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-08-02 15:53:52 -07:00
Anders Kaseorg 3665deb93a python: Remove unnecessary intermediate lists.
Generated automatically by pyupgrade.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-08-02 15:53:52 -07:00
Mateusz Mandera c34260426a bots: Pass realm to remaining get_system_bot calls in tests. 2021-07-26 15:33:13 -07:00
Mateusz Mandera 994ee70497 bots: Pass realm to self.notification_bot test helper. 2021-07-26 15:33:13 -07:00
Tim Abbott 95606a7347 api: Return user IDs, not display emails, in subscribers endpoints.
Sometime in the deep past, Zulip the GET /users/me/subscriptions
endpoint started returning subscribers.  We noticed this and made it
optional via the include_subscribers parameter in
1af72a2745, however, we didn't notice
that they were being returned as emails rather than user IDs.

We migrated the core /register code paths to use subscriber IDs years
ago; this change completes that for the endpoints we forgot about.

The documentation allowed this error because we apparently had no
tests for this code path that used the actual API.
2021-07-18 11:32:28 -07:00
Anders Kaseorg fb3ddf50d4 python: Fix mypy no_implicit_reexport errors.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-07-16 14:02:31 -07:00
akshatdalton 3ea1ff7665 refactor: Use `assertGreaterEqual` helper instead of `assertTrue`. 2021-07-13 13:03:38 -07:00
akshatdalton 0b469e9e4e refactor: Use `assertGreater` helper instead of `assertTrue`. 2021-07-13 13:03:38 -07:00
akshatdalton e203112fd4 refactor: Use `assert_length` helper instead of `assertTrue/assertEqual`. 2021-07-13 13:03:38 -07:00
Vishnu KS 4ad592ed4f populate_db: Use do_create_realm for creating zulip realm.
Since do_create_realm also creates general and core team streams,
we rename general to verona right after the realm is created. Mostly
because we dont really want two additional streams and this might
probably make it easy to review things.

There are puppeteer test changes because, we have a new "core team"
stream in tests as well as there is a new default notification stream
"Verona". Because of this tests in message-basics for example have
to be changed since the newly added core team affects the order in
which we navigate through the streams using arrow keys.

The extra await for selector was added in subscriptions test to make
the tests wait. Without the await the tests were passing ocassionally
and failing in some other times.

Fixes #6967
2021-07-06 17:37:43 -07:00
Vishnu KS acffc0ae0a populate_db: Use do_create_realm for creating zephyr realm. 2021-07-06 17:22:00 -07:00
PIG208 8b9011dff8 json_error: Completely remove json_error.
This completes the migration from `return json_error` to
`raise JsonableError`.
2021-07-06 15:34:33 -07:00
Ganesh Pawar 113e27615d test_subs: Extract out test_user_settings_for_adding_streams.
This is a prep commit in preparation of splitting
create_stream_policy into create_private_stream_policy
and create_public_stream_policy.

This extracts it in a way to make it possible to easily test
different stream policies in the upcoming stream policy split.
2021-06-24 14:08:49 -07:00
Ganesh Pawar a1ab79992c test_subs: Extract out test_user_settings_for_creating_streams.
This is a prep commit in preparation of splitting
create_stream_policy into create_private_stream_policy
and create_public_stream_policy.

This extracts it in a way to make it possible to easily test
different stream policies in the upcoming stream policy split.
2021-06-24 14:08:49 -07:00
Ganesh Pawar e74ad23091 test_subs: Remove test_create_stream_policy_setting.
test_create_stream_policy_setting (in class StreamAdminTest) and
test_user_settings_for_creating_streams (in class SubscriptionAPITest)
test essentially the same thing.

So, remove one of them.

Removing test_create_stream_policy_setting makes sense,
since class StreamAdminTest tests things admins can do, whereas
non-admin users can create streams.
2021-06-24 14:08:49 -07:00
Ganesh Pawar aec493a8d3 test_subs: Remove test_invite_to_stream_by_invite_period_threshold.
test_invite_to_stream_by_invite_period_threshold (in class StreamAdminTest)
and test_user_settings_for_subscribing_other_users
(in class SubscriptionAPITest) test essentially the same thing.

So, remove one of them.

Removing test_invite_to_stream_by_invite_period_threshold makes sense,
since class StreamAdminTest tests things admins can do, whereas
non-admin users can invite other users.
2021-06-24 14:08:49 -07:00
Ganesh Pawar d45e7274f9 test_subs: Avoid direct usage of list_to_streams.
This was used to test can_create_stream property of a guest user.
There are better ways to test it, which are already implemented in
test_can_create_streams.
2021-06-24 14:08:48 -07:00
Abhijeet Prasad Bodas 7a4c212dff test_subs: Add comments about num_events.
This is a bit complex, because many events not directly related
to what we are testing are also sent during these operations, hence
these comments.
2021-05-28 09:42:14 -07:00
Abhijeet Prasad Bodas aa2fd1593e tornado_redirected_to_list: Fix event list leaks in tests.
Ideally this should have been part of
481a890ec5.
2021-05-28 09:42:14 -07:00
Abhijeet Prasad Bodas 3039a01b31 tornado_redirected_to_list: Make expected_num_events required argument.
Follow up to 481a890ec5.
This will make this more explicit and readable.
2021-05-28 09:42:14 -07:00
Abhijeet Prasad Bodas 481a890ec5 tests: Assert num_events in tornado_redirected_to_list. 2021-05-27 13:16:22 -07:00
Abhijeet Prasad Bodas 8bcdbc720e tests: Move tornado_redirected_list to ZulipTestCase.
This is will make it easier to systematically use Django's
`capturOnCommitCallbacks` in tests outside of the main
`test_events` file which involve assertions on events.
2021-05-24 17:45:13 -07:00
Abhijeet Prasad Bodas 352634a851 tests: Consistently use assert_length helper.
This helper does some nice things like printing out
the data structure incase of failure.
2021-05-19 11:55:56 -07:00
Gaurav Pandey dada7bb4bb tests: Clean up tests for realm_domain and update_streams endpoint.
Cleaning up test_realm_domains.RealmDomainTest.test_list_realm_domains,
test_subs.StreamAdminTest.test_private_stream_live_updates,
test_subs.StreamAdminTest.test_realm_admin_can_update_unsub_private_stream
and test_subs.StreamAdminTest.test_non_admin_cannot_access_unsub_private_stream.
2021-05-12 12:38:16 -07:00
Tim Abbott 70244896e5 test_subs: Remove now invalid JSON encoding of parameters.
All of these were missed in the previous commit.
2021-05-10 10:42:11 -07:00
Gaurav Pandey 96e035a2f0 api: Fix encoding of strings in streams endpoint.
* Remove unnecessary json_validator for string parameters.
* Update frontend to pass right parameter.

Bump api feature level and highlight the fix for `emojiset`
parameter of `settings/display` endpoint in zulip.yaml file.

Fixes part of #18035.
2021-05-10 10:29:22 -07:00
Anders Kaseorg 544bbd5398 docs: Fix capitalization mistakes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-05-10 09:57:26 -07:00
Gaurav Pandey b2111f5753 api: Fix encoding of strings in default stream groups.
Remove unnecessary json_validator for string parameters.  This change
does not modify JavaScript because we don't have a frontend for these
API endpoints yet.

Fixes part of #18035.
2021-05-07 14:06:37 -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
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 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 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
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 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
shanukun 0b3324ba77 refactor: Make acting_user a mandatory kwarg for do_change_subscription_property. 2021-04-08 17:50:10 -07:00
sahil839 39c9845cb0 streams: Raise same error for all values of invite_to_stream_policy.
We keep the error message same for all cases when a user is not
allowed to subscribe others for all values of invite_to_stream_policy.

We raise error with different message for guest cases because it
is handled by decorators. We aim to change this behavior in future.

Explaining the details in error message isn't much important as
we do not show errors probably in API only, as we do not the show
the options itself in the frontend.
2021-04-07 14:51:00 -07:00
sahil839 1ffcb0f8b9 streams: Raise same error for all values of create_stream_policy.
We keep the error message same for all cases when a user is not
allowed to create streams for all values of create_stream_policy.

We raise error with different message for guest cases because it
is handled by decorators. We aim to change this behavior in future.

Explaining the details in error message isn't much important as
we do not show errors probably in API only, as we do not the show
the options itself in the frontend.
2021-04-07 14:51:00 -07:00
sahil839 9ad6a856a9 tests: Extract a single function to test functions using has_permission.
The tests for can_create_streams and can_subscribe_other_users shares a
lot of code and we deduplicate the code by extracting most of the code
as check_has_permission_policies which will now be called by the two
tests test_can_create_streams and test_can_subscribe_other_users.

This will also help in avoiding the duplication of code when we will
convert more policies to use COMMON_POLICY_TYPES.
2021-04-06 21:23:13 -07:00
LoopThrough-i-j 277fbb3f02 stream_subscription: Add subscribe_ids_with_stream_history_access.
This new function returns the set of `user_ids` with access to the
stream's full history, for use in send_event calls.
2021-04-05 13:23:11 -07:00
Siddharth Asthana c3f37c2a64 support: Pass acting_user to do_change_plan_type.
acting_user is now a mandatory field, so wherever this function is used,
we are passing acting_user as well.
2021-04-03 08:01:46 -07:00
Mateusz Mandera 82b43a8cfe streams: Use "archive stream" phrasing instead of "delete stream".
The previous phrasing was misleading in relation to what the action
actually does.
2021-04-02 22:06:48 -07:00
Mateusz Mandera d5871f1005 streams: Allow admins to fetch private streams via the get streams API.
We send a list of all private streams to realm admins in
fetch_initial_state since 73c30774cb
anyway and this API code just didn't catch up.
2021-04-02 16:27:50 -07:00
shanukun bcc3bb03fe refactor: Make acting_user a mandatory kwarg for bulk_remove_subscriptions. 2021-04-02 14:44:41 -07:00
shanukun 0bf067b681 refactor: Make acting_user a mandatory kwarg for bulk_add_subscriptions. 2021-04-02 14:44:41 -07:00
shanukun 790085832c refactor: Make acting_user a mandatory kwarg for ensure_stream. 2021-04-02 14:44:41 -07:00
shanukun c39ffe8811 refactor: Make acting_user a mandatory kwarg for do_deactivate_stream. 2021-04-02 14:44:41 -07:00
WookieMonkeys 1b6f68bb59 stream: Add entropy to deactivated streams.
Adding an additional `!` to the stream name each time a stream is
deactivated, to a maximum of 21 times, effectively limits number of
times a stream with a given name can be deactivated.  This is unlikely
to come up in common usage, but may be confusing when testing.

Change what we prepend to deactivated stream names to something with
more entropy than just `!`, by instead prepending a substring of hash
of the stream's ID.  `!`s.  Using 128 bits of the hash means that it
will require more than 10^18th renames to have a 1% chance of collision.

Because too-long stream names are also truncated at 60 characters,
having this entropy in the beginning of the name also helps address
potential issues from stream names that differed only in, e.g. the
60th character.

Fixes #17016.
2021-04-01 17:16:35 -07:00
sahil839 54be0dd1a4 streams: Add moderators option in stream_post_policy.
This commit adds a new option of STREAM_POST_POLICY_MODERATORS
in stream_post_policy which will allow only realm admins and
moderators to post in that stream.
2021-03-30 09:06:20 -07:00
sahil839 bc42eab925 settings: Add moderators-only option for invite_to_stream_policy.
The moderators-only option was actually added in the previous
commit for create_stream_policy as we use the same function
'has_permission' for both the policies. But we add the error
handling code and tests for moderators-only option in this
commit.
2021-03-30 09:06:20 -07:00
sahil839 5b32dcd2e7 settings: Add moderators-only option in create_stream_policy.
This commit modifies the has_permission function to include
realm moderator role. Thus this adds a new option of moderators
only for create_stream_policy.
Though this automatically adds this option for invite_to_stream_policy
also, but we will keep other code for showing error and for tests
in a separate commit.
2021-03-30 09:06:20 -07:00
shanukun c95061e9b9 refactor: Make acting_user a mandatory kwarg for do_deactivate_user. 2021-03-29 15:51:45 -07:00
shanukun 3c3d805dd1 refactor: Make acting_user a mandatory kwarg for do_change_user_role 2021-03-29 15:51:45 -07:00
shanukun 459710a897 refactor: Make acting_user a mandatory kwarg for do_set_realm_property. 2021-03-29 15:51:45 -07:00
Mateusz Mandera a094b80fb1 test_subs: Remove redundant cache_delete calls.
These caches should be getting automatically invalidated upon
realm.save().
2021-03-25 00:47:42 -07:00
sahil839 6d419565b7 streams: Show error according to create_stream_policy on failure.
This commit changes the list_to_streams function to raise error
according to create_stream_policy value when a user cannot create
streams instead of same error for all cases.
2021-03-24 12:33:58 -07:00
sahil839 13e412d553 tests: Modify invite_to_stream_policy tests to check all cases.
This commit modifies test_user_settings_for_subscribing_other_users
to check all the possible cases including the cases when a user
can successfully subscribe other users along with the already
tested failure cases. This commit also adds checks for guest users
which was not present before.
2021-03-24 12:33:58 -07:00
sahil839 b45d6ef0d8 tests: Use do_change_user_role and do_set_realm_property.
This commit replaces the code which directly changes user.role,
realm.create_stream_policy and realm.waiting_period_threshold
with do_change_user_role and do_set_realm_property functions
in test_can_create_streams. This makes the code similar to the
other tests.
2021-03-24 12:33:58 -07:00
sahil839 683b6f7d65 tests: Refactor test_can_create_streams and test_can_subscribe_other_users.
We refactor test_can_create_streams and test_can_subscribe_other_users
in test_subs.py. We want to follow a specific order in such tests
which is just set the policy value one by one and then checking
that the role in policy returns true and role just below that returns
false. This approach is explained in detail below.

Following hierarchy of roles is considered for these tests -
1. Realm admin
2. Full members
3. Members
4. Guests.

Then if the policy is set to admins only, we check that the having
role as admin returns true and the role just below that, i.e. full
member returns false. Similarly, if the policy is set to members
only, we check that a member should return true and role below it
which is guest should return false. We basically follow these as
we can assume that if a user with particular role cannot do the
required task, then user with role below in the hierarchy would
be not allowed to do the task too.

This commit refactors the above mentioned two tests to have above
explained workflow.
2021-03-24 12:33:58 -07:00
sahil839 bc72d2dbc4 tests: Remove unnecessary use of do_change_user_role in test_subs.
This commit removes the unnecessary do_change_user_role function
in test_can_subcribe_other_users. This was added in 1aebf3cab
which replaced the multiple functions like do_change_is_admin
and do_change_is_guest with do_change_user_role.

Previously two functions do_change_is_admin and do_change_is_guest
were used because there were two flags is_realm_admin and is_guest
which were used to determine the role of a user. But then we added
a single field role to UserProfile and removed the multiple flags
and thus also replaced the different functions with a single
do_change_user_role. With addition of a new field role, two
different do_change_* functions were not needed as we only have
a role field instead of different flags, but this was missed in
1aebf3cab and this commit fixes it.
2021-03-24 12:33:58 -07:00
Mateusz Mandera d91d3a05b9 tests: Use do_create_realm where possible.
Using do_create_realm should be preferred over manual creation where
possible, as it creates more realistic data.
2021-03-14 08:50:02 -07:00
sahil839 4ca21a6982 users: Give moderators same permissions as that of full members.
This commit updates the stream creation, subscribing others to
stream, wildcard mention settings and stream post policy to allow
realm moderators even if they are new and the respective setting
is set to allow full members only.
2021-03-02 17:19:31 -08:00
sahil839 b4fd15d516 models: Rename is_new_member to is_provisional_member.
This commit renames the is_new_member property in models.py
to is_provisional_member which will return true for any user
who is not a full member. We will add a condition in further
commit such that this returns 'False' for a moderator as we
will initially give all the rights to moderator that a full
member has.
2021-03-02 17:19:31 -08:00
sahil839 6b5cf231a1 users: Add new user 'shiva' as realm moderator.
Note that at this point, it's not possible to create moderator users;
this just will make it easier to write tests for logic involving them
as we develop the feature.
2021-02-23 15:00:49 -08:00
Anders Kaseorg d001676728 streams: Fix compose_views type safety.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-15 17:05:28 -08:00
Anders Kaseorg 6e4c3e41dc python: Normalize quotes with Black.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 13:11:19 -08:00
Anders Kaseorg 11741543da python: Reformat with Black, except quotes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 13:11:19 -08:00
Mateusz Mandera bf9e5e52ce dependencies: Upgrade to Django 3.0.
Adjustments made due to changes in Django 3.0:
(https://docs.djangoproject.com/en/3.0/releases/3.0/)

- test_signup: INTERNAL_RESET_URL_TOKEN was moved to
  PasswordResetConfirmView.reset_url_token
- test_message_fetch:
  "add_never_cache_headers() and never_cache() now add the private
  directive to Cache-Control headers."
- "django.utils.html.escape() now uses html.escape() to escape HTML.
  This converts ' to &#x27; instead of the previous equivalent decimal
  code &#39;." - this requires adjusting the expected decimal code
  in some of the string fixtures in tests.
2021-01-26 10:20:00 -08:00
Steve Howell f2586d2f9b refactor: Introduce SubscriptionInfo dataclass.
We use this as the return type for
gather_subscriptions_helper and
get_web_public_subs, instead of tuples.
2021-01-21 15:04:07 -08:00
Steve Howell d9740045a5 refactor: Eliminate checks in build_stream_dict_for_sub.
We eliminate some redundant checks.

We also consistently provide a `subscribers` field
in our stream data with `[]`, even if our users
can't access subscribers.  We therefore bump
the API version and tweak the docs.  (See further
down for a detailed justification of the change.)

Even though it is sometimes fine to have redundant code
that is defensive in nature, some upcoming changes are gonna
move subscriber-related logic out of build_stream_dict_for_sub
for certain codepaths as part of our effort to streamline
the payload for subscribers within page_params.

So we can't rely on the code that I removed here
inside of build_stream_dict_for_sub.

Anyway, it makes more sense to do these checks explicitly
in the validate function.

The code in build_stream_dict_for_sub was almost effectively
a noop, since the validation function was already preventing
us from getting subscriber info.  The only difference it
made was sometimes converting `[]` to `None`, and then
subsequently omitting the subscribers field.

Neither ZT nor the webapp make any distinction between
`[]` or <missing key> for the `subscribers` data in
`page_params`.

The webapp has had this code for a long time (and now
equivalent code elsewhere in this PR):

    if (!Object.prototype.hasOwnProperty.call(sub, "subscribers")) {
        sub.subscribers = new LazySet([]);
    }

The webapp calculates access based on booleans, anyway:

    sub.can_access_subscribers =
        page_params.is_admin || sub.subscribed ||
        (!page_params.is_guest && !sub.invite_only);

And ZT would choke if `subscribers` were missing, except that
it never gets to the relevant code due to other checks:

    def get_other_subscribers_in_stream(<snip>):
        assert stream_id is not None or stream_name is not None

        if stream_id:
            assert self.is_user_subscribed_to_stream(stream_id)

            return [sub
                    for sub in self.stream_dict[stream_id]['subscribers']
                    if sub != self.user_id]
        else:
            return [sub
                    for _, stream in self.stream_dict.items()
                    for sub in stream['subscribers']
                    if stream['name'] == stream_name
                    if sub != self.user_id]

You could make a semantic argument that we should prefer
<missing key> to `[]` when subscribers aren't even available, but
we have precedent from the way that `bulk_get_subscriber_user_ids`
has traditionally populated its result:

    result: Dict[int, List[int]] =
        {stream["id"]: [] for stream in stream_dicts}

If we changed `stream_dicts` to `target_stream_dicts` we
would faciliate a move toward `None`, but it would just cause
headaches for other server code as well as the frontends
(which, to reiterate, already prefer the empty array
for convenience).
2021-01-21 15:04:07 -08:00
Mateusz Mandera d0dc04a093 models: Rename is_api_super_user to can_forge_sender, 2020-12-21 13:15:39 -08:00