We already override PUSH_NOTIFICATION_BOUNCER_URL in
test_extra_settings.py, so making this change should have as its only
impact making it a bit easier to test the push notifications bouncer
manually in a development environment.
I submitted a related PR to the mobile app documentation for testing
the push notifications software against a production server motivated
by this.
See https://chat.zulip.org/#narrow/stream/6-frontend/topic/typing.20notifications.20efficiency/near/1664991.
As detailed in that discussion,
`TYPING_STARTED_EXPIRY_PERIOD_MILLISECONDS` and
`TYPING_STARTED_WAIT_PERIOD_MILLISECONDS` are coupled constants, and
the impact of them being large is mainly that if a user closes their
computer or loses network in the middle of typing something (not
exactly a common occasion), then the client will suggest they kept on
typing longer than they in fact did.
There's a substantial decrease in resources consumed by this feature
associated with raising `TYPING_STARTED_WAIT_PERIOD_MILLISECONDS`, so
that at least seems worth doing.
Meanwhile, because TYPING_STOPPED_WAIT_PERIOD_MILLISECONDS measures
how long we should wait before deciding to stop suggesting a user is
still typing if they were previously typing a message but paused doing
so without closing the compose box (example causes being stepping away
from the computer, tabbing to go look something up, or just thinking
for a bit).
On the one hand, even the original 5 seconds is a fairly long time to
pause to think without touching the keyboard; on the other hand,
sitting with text you've written in the compose box is likely still a
quite high intent-to-send-soon state. Increasing this to 12 seconds
seems like a reasonable balance between being too trigger-happy here
here and avoiding someone who left their computer appearing like they
are still typing for a long time afterwards.
This commit adds a new endpoint 'users/me/onboarding_steps'
deprecating the older 'users/me/hotspots' to mark hotspot as read.
We also renamed the view `mark_hotspot_as_read` to
`mark_onboarding_step_as_read`.
Reason: Our plan is to make this endpoint flexible to support
other types of UI elements not just restricted to hotspots.
Currently, the sender names for outgoing emails sent by Zulip
are hardcoded. It should be configurable for self-hosted systems.
This commit makes the 'Zulip' part a variable in the following
email sender names: 'Zulip Account Security', 'Zulip Digest',
and 'Zulip Notifications' by introducing a settings variable
'SERVICE_NAME' with the default value as f"{EXTERNAL_HOST} Zulip".
Fixes: #23857
This makes it possible for a self-hosted realm administrator to
directly access a logged-page on the push notifications bouncer
service, enabling billing, support contacts, and other administrator
for enterprise customers to be managed without manual setup.
The presence and user status update events are only sent to accessible
users, i.e. guests do not receive presence and user status updates for
users they cannot access.
The original behavior of this setting was to disable LDAP
authentication for any realms not configured to use it. This was an
arbitrary choice, and its only value was to potentially help catch
typos for users who are lazy about testing their configuration.
Since it makes it a very inconvenient to potentially host multiple
organizations with different LDAP configurations, remove that
behavior.
This makes it possible to send notifications to more than one app ID
from the same server: for example, the main Zulip mobile app and the
new Flutter-based app, which has a separate app ID for use through its
beta period so that it can be installed alongside the existing app.
This commit adds new API endpoint to get stream email which is
used by the web-app as well to get the email when a user tries
to open the stream email modal.
The stream email is returned only to the users who have access
to it. Specifically for private streams only subscribed users
have access to its email. And for public streams, all non-guest
users and only subscribed guests have access to its email.
All users can access email of web-public streams.
In 0b3f7a5a6 we started automatically setting this in dev if we
found a cert file in an appropriate place. But for running tests,
we don't want such variability. Set it back to None there.
Originally, this was how the notification emails worked, but that was changed
in 797a7ef97b, with this old behavior
available as an option.
The footer and from address of emails that are sent when this
setting is set to True are confusing, especially when more people
are involved in a stream and since we have changed the way we send
emails, it should be removed. It’s also not widely used.
Fixes#26609.
This is an exception that we should be generally catching like the
others, which will give our standard /login/ redirect and proper logging
- as opposed to a 500 if we don't catch.
Addresses directly a bug we occurred in the wild, where a SAMLResponse
was submitted without issuers specified in a valid way, causing this
exception. The added test tests this specific type of scenario.
This endpoint verifies that the services that Zulip needs to function
are running, and Django can talk to them. It is designed to be used
as a readiness probe[^1] for Zulip, either by Kubernetes, or some other
reverse-proxy load-balancer in front of Zulip. Because of this, it
limits access to only localhost and the IP addresses of configured
reverse proxies.
Tests are limited because we cannot stop running services (which would
impact other concurrent tests) and there would be extremely limited
utility to mocking the very specific methods we're calling to raising
the exceptions that we're looking for.
[^1]: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/
Rewrite the test so that we don't have a dedicated URL for testing.
dev_update_subgroups is called directly from the tests without using the
test client.
**Background**
User groups are expected to comply with the DAG constraint for the
many-to-many inter-group membership. The check for this constraint has
to be performed recursively so that we can find all direct and indirect
subgroups of the user group to be added.
This kind of check is vulnerable to phantom reads which is possible at
the default read committed isolation level because we cannot guarantee
that the check is still valid when we are adding the subgroups to the
user group.
**Solution**
To avoid having another transaction concurrently update one of the
to-be-subgroup after the recursive check is done, and before the subgroup
is added, we use SELECT FOR UPDATE to lock the user group rows.
The lock needs to be acquired before a group membership change is about
to occur before any check has been conducted.
Suppose that we are adding subgroup B to supergroup A, the locking protocol
is specified as follows:
1. Acquire a lock for B and all its direct and indirect subgroups.
2. Acquire a lock for A.
For the removal of user groups, we acquire a lock for the user group to
be removed with all its direct and indirect subgroups. This is the special
case A=B, which is still complaint with the protocol.
**Error handling**
We currently rely on Postgres' deadlock detection to abort transactions
and show an error for the users. In the future, we might need some
recovery mechanism or at least better error handling.
**Notes**
An important note is that we need to reuse the recursive CTE query that
finds the direct and indirect subgroups when applying the lock on the
rows. And the lock needs to be acquired the same way for the addition and
removal of direct subgroups.
User membership change (as opposed to user group membership) is not
affected. Read-only queries aren't either. The locks only protect
critical regions where the user group dependency graph might violate
the DAG constraint, where users are not participating.
**Testing**
We implement a transaction test case targeting some typical scenarios
when an internal server error is expected to happen (this means that the
user group view makes the correct decision to abort the transaction when
something goes wrong with locks).
To achieve this, we add a development view intended only for unit tests.
It has a global BARRIER that can be shared across threads, so that we
can synchronize them to consistently reproduce certain potential race
conditions prevented by the database locks.
The transaction test case lanuches pairs of threads initiating possibly
conflicting requests at the same time. The tests are set up such that exactly N
of them are expected to succeed with a certain error message (while we don't
know each one).
**Security notes**
get_recursive_subgroups_for_groups will no longer fetch user groups from
other realms. As a result, trying to add/remove a subgroup from another
realm results in a UserGroup not found error response.
We also implement subgroup-specific checks in has_user_group_access to
keep permission managing in a single place. Do note that the API
currently don't have a way to violate that check because we are only
checking the realm ID now.
Adds typing notification constants to the response given by
`POST /register`. Until now, these were hardcoded by clients
based on the documentation for implementing typing notifications
in the main endpoint description for `api/set-typing-status`.
This change also reflects updating the web-app frontend code
to use the new constants from the register response.
Co-authored-by: Samuel Kabuya <samuel.mwangikabuya@kibo.school>
Co-authored-by: Wilhelmina Asante <wilhelmina.asante@kibo.school>
This adds API support to reorder linkifiers and makes sure that the
returned lists of linkifiers from `GET /events`, `POST /register`, and
`GET /realm/linkifiers` are always sorted with the order that they
should processed when rendering linkifiers.
We set the new `order` field to the ID with the migration. This
preserves the order of the existing linkifiers.
New linkifiers added will always be ordered the last. When reordering,
the `order` field of all linkifiers in the same realm is updated, in
a manner similar to how we implement ordering for
`custom_profile_fields`.
Restore the default django.utils.log.AdminEmailHandler when
ERROR_REPORTING is enabled. Those with more sophisticated needs can
turn it off and use Sentry or a Sentry-compatible system.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Uploads are well-positioned to use S3's "intelligent tiering" storage
class. Add a setting to let uploaded files to declare their desired
storage class at upload time, and document how to move existing files
to the same storage class.
Pass the HttpRequest explicitly through the two webhooks that log to
the webhook loggers.
get_current_request is now unused, so remove it (in the same commit
for test coverage reasons).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Combine nginx and Django middlware to stop putting misleading warnings
about `CSRF_TRUSTED_ORIGINS` when the issue is untrusted proxies.
This attempts to, in the error logs, diagnose and suggest next steps
to fix common proxy misconfigurations.
See also #24599 and zulip/docker-zulip#403.
Having exactly 17 or 18 middlewares, on Python 3.11.0 and above,
causes python to segfault when running tests with coverage; see
https://github.com/python/cpython/issues/106092
Work around this by adding one or two no-op middlewares if we would
hit those unlucky numbers. We only add them in testing, since
coverage is a requirement to trigger it, and there is no reason to
burden production with additional wrapping.
This endpoint was previously marked as `intentionally_undocumented`
but that was mistake.
Removed `intentionally_undocumented` and added proper documentation
with valid `python_example` for this Endpoint.
Fixes: #24084
THUMBNAIL_IMAGES was previously set to true as there were tests on a new
thumbnail functionality. The feature was never stable enough to remain in
the codebase and the setting was left enabled. This setting also doesn't
reflect how the production deployments are and it has been decided that we
should drop setting from test_extra_settings altogether.
Co-authored-by: Joseph Ho <josephho678@gmail.com>
Failing to remove all of the rules which were added causes action at a
distance with other tests. The two methods were also only used by
test code, making their existence in zerver.lib.rate_limiter clearly
misplaced.
This fixes one instance of a mis-balanced add/remove, which caused
tests to start failing if run non-parallel and one more anonymous
request was added within a rate-limit-enabled block.
This in-progress feature was started in 2018 and hasn't
been worked on much since. It's already in a broken state,
which makes it hard to iterate on the existing search bar
since it's hard to know how those changes will affect search
pills.
We do still want to add search pills eventually, and when
we work on that, we can refer to this diff to readd the
changes back.
Part of splitting creating and editing scheduled messages.
Should be merged with final commit in series. Breaks tests.
Splits out editing an existing scheduled message into a new
view function and updated `edit_scheduled_message` function.
This doesn't have any obvious security implications right now, but
nonetheless such information is not meant to stick around in the session
if authentication didn't succeed and not cleaning up would be a bug.
This is a useful improvement in general for making correct
LogoutRequests to Idps and a necessary one to make SP-initiated logout
fully work properly in the desktop application. During desktop auth
flow, the user goes through the browser, where they log in through their
IdP. This gives them a logged in browser session at the IdP. However,
SAML SP-initiated logout is fully conducted within the desktop
application. This means that proper information needs to be given to the
the IdP in the LogoutRequest to let it associate the LogoutRequest with
that logged in session that was established in the browser. SessionIndex
is exactly the tool for that in the SAML spec.
This gives more flexibility on a server with multiple organizations and
SAML IdPs. Such a server can have some organizations handled by IdPs
with SLO set up, and some without it set up. In such a scenario, having
a generic True/False server-wide setting is insufficient and instead
being able to specify the IdPs/orgs for SLO is needed.
Closes#20084
This is the flow that this implements:
1. A logged-in user clicks "Logout".
2. If they didn't auth via SAML, just do normal logout. Otherwise:
3. Form a LogoutRequest and redirect the user to
https://idp.example.com/slo-endpoint?SAMLRequest=<LogoutRequest here>
4. The IdP validates the LogoutRequest, terminates its own user session
and redirects the user to
https://thezuliporg.example.com/complete/saml/?SAMLRequest=<LogoutResponse>
with the appropriate LogoutResponse. In case of failure, the
LogoutResponse is expected to express that.
5. Zulip validates the LogoutResponse and if the response is a success
response, it executes the regular Zulip logout and the full flow is
finished.
We now set tos_version to "-1" for imported users and the ones
created using API or using other methods like LDAP, SCIM and
management commands. This value will help us to allow users to
change email address visibility setting during first login.
The immediate application of this will be for SAML SP-initiated logout,
where information about which IdP was used for authenticating the
session needs to be accessed. Aside of that, this seems like generally
valuable session information to keep that other features may benefit
from in the future.
This will help us remove scheduled message and reminder logic
from `/messages` code path.
Removes `deliver_at`/`defer_until` and `tz_guess` parameters. And
adds the `scheduled_delivery_timestamp` instead. Also updates the
scheduled message dicts to return `scheduled_delivery_timestamp`.
Also, revises some text in `/delete-scheduled-message` endpoint
and in the `ScheduledMessage` schema in the API documentation.
In #23380 we want to change all ocurrences of `uri` to `url`. This
commit changes the ocurrences of `uri` appeared in files related to
email, including templates (`.html`, `.txt`) and backend (`.py`)
codes.
In `email.md`, `base_images_uri` is changed to `images_base_url` -
the words `base` and `images` are swapped and plural form is added
for `image`. This is becasue the former is not found anywhere in
the codebase while the later appears a lot. To reduce confusion,
this doccumentation changed accordingly.
This implements the core of the rewrite described in:
For the backend data model for UserPresence to one that supports much
more efficient queries and is more correct around handling of multiple
clients. The main loss of functionality is that we no longer track
which Client sent presence data (so we will no longer be able to say
using UserPresence "the user was last online on their desktop 15
minutes ago, but was online with their phone 3 minutes ago"). If we
consider that information important for the occasional investigation
query, we have can construct that answer data via UserActivity
already. It's not worth making Presence much more expensive/complex
to support it.
For slim_presence clients, this sends the same data format we sent
before, albeit with less complexity involved in constructing it. Note
that we at present will always send both last_active_time and
last_connected_time; we may revisit that in the future.
This commit doesn't include the finalizing migration, which drops the
UserPresenceOld table.
The way to deploy is to start the backfill migration with the server
down and then start the server *without* the user_presence queue worker,
to let the migration finish without having new data interfering with it.
Once the migration is done, the queue worker can be started, leading to
the presence data catching up to the current state as the queue worker
goes over the queued up events and updating the UserPresence table.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
Moves jwt_fetch_api_key endpoint to v1_api_mobile_patterns so
that tools/test-api detects it as an API endpoint that is pending
documentation.
Fixes#24982.
So far, we've used the BitField .authentication_methods on Realm
for tracking which backends are enabled for an organization. This
however made it a pain to add new backends (requiring altering the
column and a migration - particularly troublesome if someone wanted to
create their own custom auth backend for their server).
Instead this will be tracked through the existence of the appropriate
rows in the RealmAuthenticationMethods table.
This commit adds a new endpoint, 'POST /user_topics' which
is used to update the personal preferences for a topic.
Currently, it is used to update the visibility policy of
a user-topic row.
Some well-intentioned adblockers also block Sentry client-side error
reporting. Provide an endpoint on the Zulip server which forwards to
the Sentry server, so that these requests are not blocked.
While the function which processes the realm registration and
signup remains the same, we use different urls and functions to
call the process so that we can separately track them. This will
help us know the conversion rate of realm registration after
receiving the confirmation link.
Zulip already has integrations for server-side Sentry integration;
however, it has historically used the Zulip-specific `blueslip`
library for monitoring browser-side errors. However, the latter sends
errors to email, as well optionally to an internal `#errors` stream.
While this is sufficient for low volumes of users, and useful in that
it does not rely on outside services, at higher volumes it is very
difficult to do any analysis or filtering of the errors. Client-side
errors are exceptionally noisy, with many false positives due to
browser extensions or similar, so determining real real errors from a
stream of un-grouped emails or messages in a stream is quite
difficult.
Add a client-side Javascript sentry integration. To provide useful
backtraces, this requires extending the pre-deploy hooks to upload the
source-maps to Sentry. Additional keys are added to the non-public
API of `page_params` to control the DSN, realm identifier, and sample
rates.
7ad06473b6 split out `LOCAL_AVATARS_DIR` and `LOCAL_FILES_DIR` as
derived values from `LOCAL_UPLOADS_DIR`. However, this means that all
places which set `LOCAL_UPLOADS_DIR` need to potentially propagate
that change into the derived values if they come *after*
`computed_settings.py` is applied. It did this successfully in
`zerver/lib/test_runner.py` and the `use_s3_backend` decorator, but
did not adjust the late-set `zproject/test_extra_settings.py`.
This causes tests to share a single common set of avatars and
attachments directories. In puppeteer tests, this leads to assertion
failures checking `assert_is_local_storage_path`; in backend tests,
this leads to races when checking the contents of the local storage
directory when run in parallel mode.
Set `LOCAL_AVATARS_DIR` and `LOCAL_FILES_DIR` based on
`LOCAL_UPLOADS_DIR` when it is pulled from the environment during
testing.
This is the behaviour inherited from Django[^1]. While setting the
password to empty (`email_password = `) in
`/etc/zulip/zulip-secrets.conf` also would suffice, it's unclear what
the user would have been putting into `EMAIL_HOST_USER` in that
context.
Because we previously did not warn when `email_password` was not
present in `zulip-secrets.conf`, having the error message clarify the
correct configuration for disabling SMTP auth is important.
Fixes: #23938.
[^1]: https://docs.djangoproject.com/en/4.1/ref/settings/#std-setting-EMAIL_HOST_USER
We're changing the ping interval from 50s to 60s, because that's what
the mobile apps have hardcoded currently, and backwards-compatibility
is more important there than the web app's previously hardcoded 50s.
For PRESENCE_PING_INTERVAL_SECS, the previous value hardcoded in both
clients was 140s, selected as "plenty of network/other latency more
than 2 x ACTIVE_PING_INTERVAL_MS". This is a pretty aggressive value;
even a single request being missed or 500ing can result in a user
appearing offline incorrectly. (There's a lag of up to one full ping
interval between when the other client checks in and when you check
in, and so we'll be at almost 2 ping intervals when you issue your
next request that might get an updated connection time from that
user).
To increase failure tolerance, we want to change the offline
threshhold from 2 x ACTIVE_PING_INTERVAL + 20s to 3 x
ACTIVE_PING_INTERVAL + 20s, aka 140s => 200s, to be more robust to
temporary failures causing us to display other users as offline.
Since the mobile apps currently have 140s and 60s hardcoded, it should
be safe to make this particular change; the mobile apps will just
remain more aggressive than the web app in marking users offline until
it uses the new API parameters.
The end result in that Zulip will be slightly less aggressive at
marking other users as offline if they go off the Internet. We will
likely be able to tune ACTIVE_PING_INTERVAL downwards once #16381 and
its follow-ups are completed, because it'll likely make these requests
much cheaper.
As of the previous commit, the server provides these parameters in
page_params. The defaults match the values hard-coded in the webapp so
far - so get rid of the hard-coded values in favor of taking them from
page_params.
Anders Kaseorg
Tim Abbott
Aman Agrawal
Prakhar Pratyush
Prakhar Pratyush
Mateusz Mandera
Sahil Batra
Alex Vandiver
Greg Price
Esther Anierobi
Tam Le
Simon Michalke
Zixuan James Li
Samuel
Lauryn Menard
Lalit
evykassirer
AcKindle3
Ruchir Harbhajanka