A bug in the implementation of the topic moving API resulted in
organization administrators being able to move messages to streams they
shouldn't be allowed to - private streams they weren't subscribed to and
streams in other organization hosted by the same Zulip installation.
In our current model realm admins can't send messages to private streams
they're not subscribed to - and being able move messages to a
stream effectively allows to send messages to that stream and thus the
two need to be consistent.
A bug in the implementation of the all_public_streams API feature
resulted in guest users being able to receive message traffic to public
streams that should have been only accessible to members of the
organization.
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.
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.
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.
We had a mix of the two names, and "video call provider" both feels
more professional and more clear about precisely what it does.
We don't change the API fields, since it doesn't seem worth an API
migration.
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.
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.
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.
In zulip.yaml, `pattern` and `url_format_string` are extracted
as a shared parameters.
This is done as a preparatory commit for `Add settings UI to edit
linkifiers`.
Related issue: #10830.
In "test_curl_examples.py" we find the functions that registered but
never called. To improve readablity, now we have the full
implementation in curl_param_value_generators, rather than inspecting
its fields from another module.
Currently, there are separate tests for testing change of one role
to other, precisely 8, with most of them having similar structure
of code. This commit adds a helper function check_user_role_change
which contains all the code for testing and the tests for different
role just use this helper function to avoid duplication of code.
This refactor is helpful considering we would want to add tests
for moderators also, which would contain multiple tests for
testing changing different user roles to moderator and vice versa.
Tweaked by timabbott to make the code more readable by checking for
every user role flag instead of just checking the certain flags and
using conditionals.
Co-authored-by: Tim Abbott
This commit removes can_access_all_realm_members function as
it is not used anywhere in code other than tests.
This function was originally added in 4483e33102 and was
only used in digest.py other than the tests, but its use
in diget.py was removed in 735b6cb761 and the function
itself was not removed from models.py.
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.
Plurals are handled natively by the ICU MessageFormat syntax, so I
think we don’t have to do anything here.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
* 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.
Changed the name of the test-user cordelia from `Cordelia Lear` to
`Cordelia, Lear's daughter`.
This change will enable us to test users with escape characters in
their names.
I also updated the Node, Puppeteer, Backend tests and Fixtures to
support this change.
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.
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.
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.
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.
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`.
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.
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.
* Don't require strings to be unnecessarily JSON-encoded.
* Use check_capped_string rather than custom code for length checks.
* Update frontend to pass the right parameters.
With a much simplified populate_data_for_request design suggested by
Anders; we only support a handful of data types, all of which are
correctly encoded automatically by jQuery.
Fixes part of #18035.
Previously, when unmuting a user, we used to make
two database fetches - one to verify that the user
is has been muted before, and one while actually
unmuting the user.
This reduces that to one, by passing around the
`MutedUser` object fetched in the first round.
Since the new function returns `Optional[MutedUser]`,
we need to use a hack for events tests, because
mypy does not yet use the type inferred from
`assert foo is not None` in nested functions like lambdas.
See python/mypy@8780d45507.
Instead of using internal functions for data setup,
we use the API so that these tests are more
end-to-end.
This commit also removes a now unnecessary
`if date_muted is None` check.
This cleans up some code added in 3bfcaa3968.
Also fixes some indentation to be more readable:
- `mock.patch` is in a single line.
- Dictionaries are one field per line.
This was used by the old native Zulip Android app
(zulip/zulip-android). That app has been undeveloped for enough years
that we believe it no longer functions; as a result, there's no reason
to keep a prototype API endpoint for it (that we believe never worked).
This endpoint was needed by the ancient pre-electron desktop app
written in QT; we removed support for that in practice a long time
ago, and even the custom error messages for it in
5a22e73cc6.
So we can delete this endpoint as well.
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.
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.
We keep the error message same for all cases when a user is not
allowed to invite others for all values of invite_to_realm_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.
This makes it much more clear that this feature does JSON encoding,
which previously was only indicated in the documentation.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This fixes the API documentation tests following recent changes, but
isn't the right solution -- we probably want to change the API itself
to not require this strange JSON-encoding-of-a-string.
But this is necessary to have CI pass.
The comments explain in some detail, but basically we were displaying
the types for booleans incorrectly, and the types for strings in a
somewhat confusing fashion. Fix this with comments explaining the logic.
Using JSON dumping also results in our showing strings inside
quotation marks in our examples, which seems net helpful.
Thanks to ArunSankarKs for finding where we needed to change the
codebase.
Fixes#18021.
The moderator role was not included in the tests for create_stream_policy
and invite_to_stream_policy. The tests are do_set_realm_property_test
in test_events.py and do_test_realm_update_api in test_realm.py.
This should have been added for create_stream_policy in 5b32dcd and
in 5b32dcd for invite_to_stream_policy, but was missed by mistake.
This commit adds backend code for passing can_invite_others_to_realm
field to clients using the fetch_initial_state_data in the page_params
object.
Though this field is not used by webapp as of now, but will be used
to fix a bug of incorreclty showing the invite users option in
settings overlay in the next commit.
We add moderators and full members option to invite_to_realm_policy
by using COMMON_POLICY_TYPES and use can_invite_others_to_realm helper
added in previous commit. This commit only does the backend work,
frontend work will be done in separate commit.
This commit adds can_invite_others_to_realm helper which will be used in
further in next commit when invite_to_realm_policy will be modified to
support all values of COMMON_POLICY_TYPES.
It is important for this commit's correctness that
INVITE_TO_REALM_POLICY_TYPES was initialized to use the same values.
This commit replaces invite_by_admins_policy, which was a bool field,
with a new enum field invite_by_realm_policy.
Though the final goal is to add moderators and full members option
using COMMON_POLICY_TYPES, but this will be done in a separate
commit to make this easy for review.
This commit implements a subtle optimization (described in more detail
in the comment) that can save a few hundred milliseconds in when the
sender sees that their message has sent when sending to very large
streams.
Fixes#17898.
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.
We send the whole data set as a part of the event rather than
doing an add/remove operation for couple of reasons:
* This would make the client logic simpler.
* The playground data is small enough for us to not worry
about performance.
Tweaked both `fetch_initial_state_data` and `apply_events` to
handle the new playground event.
Tests added to validate the event matches the expected schema.
Documented realm_playgrounds sections inside /events and
/register to support our openapi validation system in test_events.
Tweaked other tests like test_event_system.py and test_home.py
to account for the new event being generated.
Lastly, documented the changes to the API endpoints in
api/changelog.md and bumped API_FEATURE_LEVEL.
Tweaked by tabbott to add an `id` field in RealmPlayground objects
sent to clients, which is essential to sending the API request to
remove one.
Similar to the previous commit, we have added a `do_*` function
which does the deletion from the DB. The next commit handles sending
the events when both adding and deleting a playground entry.
Added the openAPI format data to zulip.yaml for DELETE
/realm/playgrounds/{playground_id}. Also added python and curl
examples to remove-playground.md.
Tests added.
This endpoint will allow clients to create a playground entry
containing the name, pygments language and url_prefix for the
playground of their choice.
Introduced the `do_*` function in-charge of creating the entry in
the model. Handling the process of sending events which will be
done in a follow up commit.
Added the openAPI format data to zulip.yaml for POST
/realm/playgrounds. Also added python and curl examples for using
the endpoint in its markdown documented (add-playground.md).
Tests added.
Tweaked exports.py to add the config object there so that our export
tool can include the table when exporting. Also includes all the
changes required to import the new table from the exported data.
Helper function `get_realm_playgrounds` added to fetch all
playgrounds in a realm.
Tests amended.
Realm administrators already get creation and deletion events for all
streams, including private streams. So these should be reflected in
the initial state data.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Adds backend code for the mute users feature.
This is just infrastructure work (database
interactions, helpers, tests, events, API docs
etc) and does not involve any behavioral/semantic
aspects of muted users.
Adds POST and DELETE endpoints, to keep the
URL scheme mostly consistent in terms of `users/me`.
TODOs:
1. Add tests for exporting `zulip_muteduser` database table.
2. Add dedicated methods to python-zulip-api to be used
in place of the current `client.call_endpoint` implementation.
It does not seem like an official version supporting Webpack 4 (to say
nothing of 5) will be released any time soon, and we can reimplement
it in very little code.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
`call_on_each_event` now supports additional params other than
`event_type` and `narrow`; Ex: `all_public_streams` to fetch events
of all public streams.
Also add a bit of explanation of how this parameter works.
Fixeszulip/python-zulip-api#647
This is a prep change to eventually completely
replace the term "filter" with "linkifier" in
the codebase.
This only renames files. Code changes will be
done in further commits.
This renames the test file for muting to have
the term `topic` in it, along with an ambiguously
named helper.
This is a prep change for implementing the mute
users feature.
We use GIPHY web SDK to create popover containing GIFs in a
grid format. Simply clicking on the GIFs will insert the GIF in the compose
box.
We add GIPHY logo to compose box action icons which opens the GIPHY
picker popover containing GIFs with "Powered by GIPHY"
attribution.
Previously, if a user subscribed to a stream with
history_public_to_subscribers, and then was looking at old messages in
the stream, they would not get live-updates for that stream, because
of the structure in how notify_reaction_update only looked at
UserMessage rows (we had a previous workaround involving the
`historical` field in `UserMessage` which had already made it work if
the user themselves added the reaction).
We fix this by including all subscribers with history access in the
set of recipients for update events.
Fixes a bug that was confused with #16942.
Amazon SES has a limit on the size of address fields, and rejects
emails with too-long "From" combinations of name and address. This
limit is set to 320 bytes and comes from an RFC limitation on the
size of addresses. This RFC standard states that an email address
should not be composed of a local part (before the '@') longer than
64 bytes and a domain part (after the '@') longer than 255 bytes.
It is possible that Amazon SES misinterprets this limitation as it
checks the length of the combination of the name and the email
address of the sender.
To ensure that this problem is not encountered in the send_email
module of Zulip the length of this combination is now checked
against this limit and the from_name field is removed to only
keep the from_address field when it is necessary in order to
stay below 320 bytes.
If the from_address field alone is longer than 320 bytes the
sending process will raise an SMTPDataError exception.
Tests for this new check are added to the backend test suite in
order to test if build_email correctly outputs an email with filled
from_name and from_address fields when the total length is lower
than 320 bytes and that it correctly throws the from_name field
away when necessary.
Fixes: #17558.
We're renaming "stream deletion" language to "stream archiving"
and these pages were moved in the process, so we should keep redirects
for them for a while.
This reverts commit 9c6d8d9d81 (#16916).
This feature has known bugs, and also wants some design changes to
make it customizable like linkifiers, so we’re retargeting this to
post-4.x.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The Session middleware only adds `Vary: cookie` if it sees an access
to the from inside of it. Because we are effectively, from the Django
session middleware's point of view, returning the static content of
`request.saved_response` and never accessing the session, it does not
set `Vary: cookie` on longpoll requests.
Explicitly mark Tornado requests as varying by cookie.
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.
Instead of validating `op` value later, this commit does that
in `REQ`.
Also helps avoiding duplication of this validation when
stream typing notifications feature is added.
These flags were put in place in the first commit that introduced
Tornado (9afd63692f) with unclear
utility.
Remove them, since they have never been documented, and do not have a
clear need.
The `X-Forwarded-For` header is a list of proxies' IP addresses; each
proxy appends the remote address of the host it received its request
from to the list, as it passes the request down. A naïve parsing, as
SetRemoteAddrFromForwardedFor did, would thus interpret the first
address in the list as the client's IP.
However, clients can pass in arbitrary `X-Forwarded-For` headers,
which would allow them to spoof their IP address. `nginx`'s behavior
is to treat the addresses as untrusted unless they match an allowlist
of known proxies. By setting `real_ip_recursive on`, it also allows
this behavior to be applied repeatedly, moving from right to left down
the `X-Forwarded-For` list, stopping at the right-most that is
untrusted.
Rather than re-implement this logic in Django, pass the first
untrusted value that `nginx` computer down into Django via `X-Real-Ip`
header. This allows consistent IP addresses in logs between `nginx`
and Django.
Proxied calls into Tornado (which don't use UWSGI) already passed this
header, as Tornado logging respects it.
The `widget_content` key is expected to contain a string which parses
as JSON; in the event that it does not, log the error and notify the
bot owner, instead of failing silently.
Fixes#16850.
In `validate_account_and_subdomain` we check
if user's realm is not deactivated. In case
of failure of this check, we raise our standard
JsonableError. While this works well in most
cases but it creates difficulties in handling
of users with deactivated realms for non-browser
clients.
So we register a new REALM_DEACTIVATED error
code so that clients can distinguish if error
is because of deactivated account. Following
these changes `validate_account_and_subdomain`
raises RealmDeactivatedError if user's realm
is deactivated.
This error is also documented in
`/api/rest-error-handling`.
Testing: I have mostly relied on automated
backend tests to test this.
Fixes#17763.
In validate_account_and_subdomain we check if
user's account is not deactivated. In case of
failure of this check we raise our standard
JsonableError. While this works well in most
cases but it creates difficulties in handling
of deactivated accounts for non-browser clients.
So we register a new USER_DEACTIVATED error
code so that clients can distinguish if error
is because of deactivated account. Following
these changes `validate_account_and_subdomain`
raises UserDeactivatedError if user's account
is deactivated.
This error is also documented in
`/api/rest-error-handling`.
Testing: I have mostly relied on automated
backend tests to test this.
Partially addresses issue #17763.
We add a TUTORIAL_ENABLED setting for self-hosters who want to
disable the tutorial entirely on their system. For this, the
default value (True) is placed in default_settings.py, which
can be overwritten by adding an entry in /etc/zulip/settings.py.
Updated database query to filter out deactivated streams from the
return of the get_topic_mutes method. Added optional
include_deactivated parameter to the method to make the behavior
default but overrideable. Added test case in test_muting for these
changes. Fixes blueslip warnings thrown by muting.js set_muted_topics
when passed deactivated streams via page_params.
With the previous two commits deployed, we're ready to use the
denormalization to optimize the query.
With dev environment db prepared using
./manage.py populate_db --extra-users=2000 --extra-streams=400
this takes the execution time of the query in
bulk_get_subscriber_user_ids from 1.5-1.6s to 0.4-0.5s on my machine.
The new comment explains the issue in some detail, but basically if we
deactivate the bots first, then an error partway through is corrected
by a retry; if we deactivate the user first, then we may leak
undeactivated bots if a failure occurs.
This adds the is_user_active with the appropriate code for setting the
value correctly in the future. In the following commit a migration to
backfill the value for existing Subscriptions will be added.
To ensure correct user_profile.is_active handling also in tests, we
replace all direct .is_active mutation with calls to appropriate
functions.
These procedures should be done atomically overall, with the exception
of the code that sends events to avoid block if there's a delay
communicating with Tornado.
We add the savepoint=False on underlying function that already
executes inside an atomic context - to avoid the overhead of creating
savepoints where they aren't needed.
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.
We extract a helper which checks whether to allow the sender to send the
message to a stream according to the stream_post_policy. The purpose
of extracting it out is to avoid additional code for checking the access
for bot owners in case of bot sending the messages and instead calling
the handler two times - one time for sender and one time for bot owner if
sender is a bot.
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.
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.
This commit adds an assert statement in the last block of
has_permission which checks whether the policy_value is
POLICY_FULL_MEMBERS_ONLY. This assert statement is added
for readability.
The current API documentation uses call_endpoint to upload a file;
since we've added a custom helper in python-zulip-api, we should
document that cleaner approach.
The session object provides a common place to set headers on all
requests, no matter which implementation.
Because the `headers` attribute of Session is not a true static
attribute, but rather exposed via overriding `__getstate__`, `mock`'s
autospec cannot know about it, and thus throws an error; in tests that
mock the Session, we thus must explicitly set the `session.headers`.
The existing organization, of returning an opaque blob from
`build_bot_request`, which was later consumed by
`send_data_to_server`, is not particularly sensible; the steps become
oddly split between the OutgoingWebhookWorker, `do_rest_call`, and the
`OutgoingWebhookServiceInterface`.
Make the `OutgoingWebhookServiceInterface` in charge of building,
making, and returning the request in one method; another method
handles extracting content from a successful response. `do_rest_call`
is responsible for calling both halves of this, and doing common error
handling.
The comments in stream-policy tests in test_message_send.py specifies
the restriction of creating streams based on stream_post_policy. But
this restriction was removed in 9aaa61963 and we now allow everyone to
create all type of streams. So this commit fixes the stream creation
parts in comments.
The STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS option of stream_post_policy
was explained as "Only new members can post" in the api docs. It should
instead be "Only full members can post" and this commit fixes it.
The /events and /register endpoints were excluded from schema validations,
because they were earlier not completely documented. However, they can
now be added for proper checking. Removed them from excluded endpoints list
and fixed the documentation for /register and /events after the checking.
Fixes#17796.
Backend logic for handling user mention was cluttered
because it was handled at two stages first in
get_possible_mentions_info while fetching mention data
based on the messsage and then later in UserMentionPattern
which handles processing of text for mention.
Ideally UserMentionPattern should depend on
get_possible_mentions_info only for data but there was a
shared logic between these two that made it hard to debug
any possible bugs.
Updates in this commit make both of these functions
coherent in terms of logic and also add appropiate
comments to improve readability of these functions.
There was also a hidden bug that if a user A is
mentioned in with @**name|id** then @**invalid|id**
again mentioned A because of the way we handled mentions
earlier. It is solved as a result of this refactor and
appropiate test has been added for this.
This has been tested manually as well as by adding new
test to address missing case.
I noticed this because the test_events.py tests had the extremely
weird pattern of calling the actual change function, and then testing
the `notify` function's state changes (which should always be noops),
rather than actually testing the state change function.
Fixing the test made it clear that the actual logic in events.py
simply did not handle deleting custom_profile_field_value elements
from user objects when a custom_profile_field object was deleted.
So we fix that bit of logic as well.
It appears this bug was unique -- at least we don't have any other
notify_* functions being used directly in test_events.py, and the
handful of state_change_expected=False entries are all events for data
not present in page_params.
* `op` (operation) field, added in f6fb88549f, was never intended for
`custom_profile_fields` event. This commit removes the `op` as it doesn't
have any use in the code.
* As a part of cleanup, this also eliminates the schema check warnings
for `custom_profile_fields` event, mentioned in #17568.
In 1a12e112d9, this page was converted
to use portico styling, but we intentionally left this page not using
the portico_content class since we didn't want the header/footer.
We still don't want the header/footer clutter, so instead, we achieve
that same goal using the isolated_page flag.
When running some tests multiple times in the same call,
were failing because of the data duplication.
This commit resolves that issue by resetting the test
environment (i.e: Re-cloning test database and clearing
cache) after each run.
Fixes#17607.
Since the list of streams returned by a query which is not sorted
can vary, the tests which use it become flaky.
NormalActionsTest.test_default_stream_groups_events became
flaky due to this and hopefully sorting the streams should
fix it.
I have the updated the documentation page for the hello world
integration to include numbers to bring it up to standard and make it
more readable.
Fixes part of #17633.
This commit migrates some of the backend tests in test_import_export
to use assertLogs(), instead of mock.patch() as planned in #15331.
Logs for tests in this file are suppressed and are not asserted as
that made changes to import/export codebase more fragile. As we
already have checks for the actual functionalities, it made less
sense to assert those logs.
Currently, there was no markdown page for deactivate-own-user API
endpoint. Created deactivate-own-user.md for the API page and
created a new owner client in test-api to reactivate the client
deactivated during testing.
Also changed endpoint name from deactivate-my-account to
deactivate-own-user, for better consistency with other endpoints.
Fixes#16163.
This is no longer used in any important place,
get_user_profile_by_email is meant to be used only in manage.py shell
now and thus there's no point in this function being cached.
Emails are not unique, so we can only sensibly cache using keys formed
with both email and realm.
This requires adding a new cache key function for caching by delivery
email - user_profile_delivery_email_cache_key.
Extend our markdown system to support mentioning of users
by id also. Following these changes, it would be possible
to mention users with @**|user_id** and silently mention
using @_**|user_id**.
Main intention for extending the mention syntax is to make
it convenient for bots to mention a users using their ids. It
is to be noted that previous syntax are also supported.
Documentation tweaked by tabbott for better readability.
The changes were tested manually in development server, and also
by adding some new backend and frontend tests.
Fixes: #17487.
We add support to shorten links and test their shortening in
well-organized, clean manner that makes it trivial to extend the
GitHub approach for GitLab and perhaps other services.
We only shorten basic types of GitHub links (issue, PR, commit) that
fit a set of simple common patterns; the default behaviour of Autolink
is kept for everything else.
Logic added in frontend and backend Markdown Processor is identical.
This makes easy to extend the logic for other services like GitLab.
Fixes#11895.
The schema for unread_msgs was missing additionalProperties: false
that was causing tests to pass even with undocumented parameters
which were validated as an additional property. Set
additionalProperties to false and added documentation for missing
count variable.
Fixes#17728.
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.
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.
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.
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.
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.
This add the schema checker, openapi schema, and also a test for
realm/deactivated event.
With several block comments by tabbott explaining the logic behind our
behavior here.
Part of #17568.
We discovered recently that some ops for events were just not
implemented in events.py (specifically, realm/deactivated).
Since our goal is for events.py to be complete, we add this bit of
hardening to ensure that it stays that way.
Modifies `StreamPattern` and `StreamTopicPattern` to inherit
from InlineProcessor instead of Pattern. This change is done
because Pattern stopped checking for matching patterns as soon
as it found a match which was not a valid stream. Due to this
all the subsequent mention failed, even if they were valid.
This bug was only present in backend renderring due to
markdown.inlinepatterns.Pattern.
Due to above changes verbose_compile is no longer used for
precompiling STREAM_LINK_REGEX, STREAM_TOPIC_LINK_REGEX as
adds ^(.*?) and (.*?)$ which cause extra overhead of matching
pattern which is not required. With new InlineProcessor these
extra patterns at beggining and end are not required.
So, StreamPattern and StreamTopicPattern now define their own
__init__ method for precompiling the regex.
Fixes#17535.
These changes were tested locally in dev server and by adding
some new markdown tests to test these.
Modifies `UserGroupMentionPattern` to inherit from InlineProcessor
instead of Pattern. This change is done because Pattern
stopped checking for matching patterns as soon as it found
a match which was not a valid user group. Due to this all
the subsequent user group mention failed, even if they were
valid. This bug was only present in backend renderring due to
markdown.inlinepatterns.Pattern.
This was reported as issue #17535.
These changes were tested locally in dev server and by adding
some new markdown tests to test these.
Modifies `UserMentionPattern` to inherit from InlineProcessor
instead of Pattern. This change is done because Pattern
stopped checking for matching patterns as soon as it found
a match which was not a valid user. Due to this all the
subsequent user mention failed. This bug was only present in
backend renderring due to markdown.inlinepatterns.Pattern.
This was reported as issue #17535.
These changes were tested locally in dev server and by adding
some new markdown tests to test these.
This removes the `add` from op list of stream event, as we do not
actually generate the stream/add event in the API, and when a stream
is created we identify it using the `create` operation.
(This was likely just a mistake introduced as a result of the fact
that `create` does not fit the normal naming scheme; probably
long-term we should actually migrate this to "add", but more important
for now is to document what's accurate).
Part of #17568.
This is preparatory work for investigating reports of missing unread
messages.
It's a little surprising that not test failed after adding the code
without API documentation.
Co-Author-By: Tushar Upadhyay (tushar912).
This is a prep commit which modifies the
`send_message_moved_breadcrumbs` function to take
message strings as input.
This is done to reuse the function in other places
like the /digress command.
Structurally, exception, failure_message, and status_code are mutually
exclusive in how this function is called, and it's best for the
function's flow to represent that.
The message from the bot which triggered the 407 error message notifies
the bot owner about the exceptions as well in the error message. This
commit handles it more gracefully and shows a generic message.
The messages from the bot which were triggered by the outgoing_webhooks
didn't have the bot name in them. This commit adds the bot name to it
and makes the corresponding changes in the tests.
This adds an option for restricting a ldap user
to only be allowed to login into certain realms.
This is done by configuring an attribute mapping of "org_membership"
to an ldap attribute that will contain the list of subdomains the ldap
user is allowed to access. This is analogous to how it's done in SAML.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
On replying to an email notifcation from a stream where the user
does not come under the stream_post_policy will subsequently result
in a failure. In such a case, the user does not receive feedback
regarding the failure.
Notify the user via notification bot if their email
message failed to send.
Fixes#16642.
If the client has an old version of the code which is not present on
the server, don't throw a 500; instead, default to the same `unable to
look up in source map` message is used when the line numbers don't
line up.
I have updated the documentation for the Zabbix integration to give the
correct instructions for the latest version of Zabbix (5.2). The old
instructions are now obsolete.
I have also updated the message that is PMd to a user if the webhook
doesn't receive a complete payload to also align with the new
instructions.
Using get_user_profile_by_email is invalid, as it omits the realm, and
also fetches via .delivery_email - our convention is that .email is
supposed to be used for user-facing purposes like this.
self.example_user("hamlet") uses get_user_by_delivery_email, so it
doesn't actually cache anything. This should use a cached function, like
the test below: test_do_change_realm_subdomain_clears_user_realm_cache.
This is part of our general process of replacing emails, which are not
static with time, with user_ids when referring to users in the API.
We still keep the `email` reference option, since it can be useful for
linking third-party applications to Zulip on an intranet that might
have a user's corporate email handy and not want to do the extra round
trip to lookup the user.
The name of the parameter, user_id_or_email, was chosen to to make it
clear that the default/preferred option is user_id.
Fixes#14304.
TextField is used to allow users to set long stream + topic narrow
names in the urls.
We currently restrict users to only set "all_messages" and
"recent_topics" as narrows.
This commit achieves 3 things:
* Removes recent topics as the default view which loads when
hash is empty.
* Loads default_view when hash is empty.
* Loads default_view on pressing escape key when it is unhandled by
other present UI elements.
NOTE: After this commit loading zulip with an empty hash will
automatically set hash to default_view. Ideally, we'd just display
the default view without a hash, but that involves extra complexity.
One exception is when user is trying to load an overlay directly,
i.e. zulip is loaded with an overlay hash. In this case,
we render recent topics is background irrespective of default_view.
We consider this last detail to be a bug not important enough to block
adding this setting.
The query string parameter authentication method is now deprecated for
newly created Slack applications since the 24th of February[1]. This
causes Slack imports to fail, claiming that the token has none of the
required scopes.
Two methods can be used to solve this problem: either include the
authentication token in the header of an HTTP GET request, or include
it in the body of an HTTP POST request. The former is preferred, as
the code was already written to use HTTP GET requests.
Change the way the parameters are passed to the "requests.get" method
calls, to pass the token via the `Authorization` header.
[1] https://api.slack.com/changelog/2020-11-no-more-tokens-in-querystrings-for-newly-created-appsFixes: #17408.
Minimized code duplication by integrating POSTRequestMock into
HostRequestMock and then updating the required files with
HostRequestMock.
Fixes part of #1211.
A deprecated import shouldn’t be used even in a migration, since the
migration will need to remain runnable in the future. We never needed
a migration for this switch anyway; we just needed to edit the old
migration, since no actual state changes are involved.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
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.
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.
This was inaccurate after testing the implementation, and there's an
argument that we shouldn't move it as it will simplify migrating to a
world where (some) private message threads can have topics.
These are all referring to email_gateway_bot, when they're supposed to
refer to the notification and welcome bots, respectively. The values are
the same though, so the tests were passing anyway.
Its likely that we would implement new hotspots that aren't
a part of the tutorial hotspots, in the future. For instance,
a hotspot to advertise new features. Hence, grouping them into
categories like INTRO_HOTSPOTS would be a good start. We also
have an aggregate of all types of hotspots we may add in the
future, under ALL_HOTSPOTS.
Fixes#17238.
In process_new_human user, the queries were wrong, revoking all invites
sent to the email address, even in other realms than the one where the
new account just got created.
The description of request parameter of update-subscription-settings was
wrongly pasted in yaml and wasn't completely removed from the md file.
Made appropriate fixes in yaml and md file.
test_signup: This test was wrong, because the inviter UserProfile was
from a different realm. Such a PreregistrationUser shouldn't be
considered valid.
test_tutorial: The direct call to internal_send_private_message was
using sender's realm as the realm argument which is not valid. It
doesn't lead to any error because the codepath seems to mostly not care
about the realm arg if the sender is a cross-realm bot. From my reading
of the code I think that wrong realm arg here would break user mentions,
because it makes its way to check_message() and then to
build_message_send_dict - but overall the message gets sent without
errors. Either way, this was a bug in the test and should be fixed.
Currently, the ID and Type fields didn't have a description,
and weren't being displayed. Added a schema component to add
descriptions, and display on the api page. Fixes part of #15967.
This commmit includes ROLE_MODERATOR in realm_user_count_by_role.
We also update test_change_role in test_audit_log.py to include
changes for moderator role as well.
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.
Have not included "ROLE_MODERATOR" in UserProfile.ROLE_TYPES
in this commit because did not want to update the openapi
docs at this stage as it will be a user-facing change and
not updating the openapi docs with moderator role included in
UserProfile.ROLE_TYPES gives error in ./tools/check-schemas.
The `message_id` was made an `str` object because
the request expected `Dict[str, str]`. The request is now
casted to `Dict[str, Any]` to fix the issue and removed
typecast of `message_id` to str.
python-zulip-api reference:
https://github.com/zulip/python-zulip-api/pull/653
Currently there are only tests for verifying the error case and there
are no tests to check the case where messages are sent successfully
in 'STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS' stream.
This commit adds tests for checking that full members and bots owned
by them can send message successfully in streams with post policy as
'STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS'.
We currently not allow new bots to send message in stream with post
policy as 'STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS', but we should
allow them to send messages if their owner is a full member.
This will make it consistent with behavior in stream with post
policy as 'STREAM_POST_POLICY_ADMINS_ONLY' where we allow non admin
bots with owner as admin to send messages.
According to tests we should not allow bot without owners to
post in streams with STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS.
But the code does not handle this and the related test passes
and raises error for case of bots without owner because the bot
is itself a new member.
This commit fixes this by adding a condition to check if there
is no bot owner and then raise error if there is no owner.
This is a minor refactor which renames the
notify_topic_moved_streams function to
send_message_moved_breadcrumbs.
This is done because this function will be also used
for other things in the future, when moving streams
or when using the /digress command, for example.
Added assertion to check that if a deprecated flag is in a field's
schema, then it should have deprecated mentioned in description
as well, and moved these checks to a separate function.
Fixes part of #15967.
Add new rest api endpoint GET users/{email} for looking up a user by
email, which is useful especially for corporate API applications that
might already have a user's email address.
Fixes#14302.
It looks like this ritual was born when a type comment wasn’t working
because it was mistyped without the colon.
Signed-off-by: Anders Kaseorg <anders@zulip.com>'
A few internal fields used for tracking which types of notifications
have already been sent for a given message, like `hander_id` and the
`push_notified` bundle of fields were being incorrectly included in
message events delivered to clients clients.
One could argue these fields might be useful hints to clients, but
because notifications can be triggered later on via
`missedmessage_hook`, they have no useful purpose in the API.
This commit move these extended event field on a `internal_data`
object within the event object, and delete this field in `contents()`
for call points that would serve data to clients.
Tweaked by tabbott to provide a cleaner interface.
We're not bumping API_FEATURE_LEVEL because these fields have always
been documented as being present only due to a bug, so no clients
should be expecting or relying on them.
Fixes: #15947.
Tim Abbott
Mateusz Mandera
Aman Agrawal
m-e-l-u-h-a-n
akshatdalton
Anders Kaseorg
100RABHpy
sahil839
Abhijeet Prasad Bodas
Arun Sankar
Zeeshan Equbal
shanukun
Sumanth V Rao
Dinesh
Signior-X
PIG208
LoopThrough-i-j
Cyril Pletinckx
Siddharth Asthana
Alex Vandiver
WookieMonkeys
Riken Shah
Sundar Guntnur
lukem1
Vishnu KS
vagrant
Suyash Vardhan Mathur
tushar912
Adam Birds
Steve Howell
Strifel
Gaurav Pandey
Rex Ferrer
Tushar Upadhyay
neiljp (Neil Pilgrim)