This error message didn’t make sense for the check as written, and our
OpenAPI document already provides the expected format for our 200
responses.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
(cherry picked from commit 0cee3bee00)
Real requests would not validate against the previous version. There
seems to be no consistent way to determine whether a string parameter
should be coerced to an integer for validation against an allOf
schema (which works at the level of JSON objects, not strings).
See also https://github.com/python-openapi/openapi-core/issues/698.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
(cherry picked from commit 0514f92bdb)
Adds a re-usable lockfile_nonblocking helper to context_managers.
Relying on naive `os.mkdir` is not enough especially now that the
successful operation of this command is necessary for push notifications
to work for many servers.
We can't use `lockfile` context manager from
`zerver.lib.context_managers`, because we want the custom behavior of
failing if the lock can't be acquired, instead of waiting.
That's because if an instance of this gets stuck, we don't want to start
queueing up more processes waiting forever whenever the cronjob runs
again and fail->exit is preferrable instead.
(cherry picked from commit f61ed58c8f)
Depending on the kind of config error being shown, different "go back"
links may be more appropriate.
We probably hard-coded /login/ for it, because these config errors are
most commonly used for authentication backend config error, where it
makes sense to have /login/ as "go back", because the user most likely
indeed got there from the login page.
However, for remote_billing_bouncer_not_configured, it doesn't make
sense, because the user almost surely is already logged in and got there
by clicking "Plan management" inside the gear menu in the logged in app.
(cherry picked from commit fcc3d88daf)
It's best for these to just be consistent. Therefore:
1. The .../not-configured/ error page endpoint should be restricted to
.has_billing_access users only.
2. For consistency, self_hosting_auth_view_common is tweaked to also do
the .has_billing_access check as the first thing, to avoid revealing
configuration information via its redirect/error-handling behavior.
The revealed configuration information seems super harmless, but it's
simpler to not have to worry about it and just be consistent.
(cherry picked from commit 7e8c645280)
This was added early on and hasn't been removed since, even though we
have tests for the endpoint for a while now.
(cherry picked from commit a7f187bec3)
Just shows a config error page if the bouncer is not enabled. Uses a new
endpoint for this so that it can work nicely for both browser and
desktop app clients.
It's necessary, because the desktop app expects to get a json response
with either an error or billing_access_url to redirect to. Showing a
nice config error page can't be done via the json error mechanism, so
instead we just serve a redirect to the new error page, which the app
will open in the browser in a new window or tab.
(cherry picked from commit f6d526f798)
Having a non-identity `cache_transformer` is no different from running
it on every row of the query_function. Simplify understanding of the
codepath used in caching by merging the pieces of code.
(cherry picked from commit 547c8f895d)
Rather than pass around a list of message objects in-memory, we
instead keep the same constructed QuerySet which includes the later
propagated messages (if any), and use that same query to pick out
affected Attachment objects, rather than limiting to the set of ids.
This is not necessarily a win -- the list of message-ids *may* be very
long, and thus the query may be more concise, easier to send to
PostgreSQL, and faster for PostgreSQL to parse. However, the list of
ids is almost certainly better-indexed.
After processing the move, the QuerySet must be re-defined as a search
of ids (and possibly a very long list of such), since there is no
other way which is guaranteed to correctly single out the moved
messages. At this point, it is mostly equivalent to the list of
Message objects, and certainly takes no less memory.
(cherry picked from commit eaf58438ec)
Rather than use `bulk_update()` to batch-move chunks of messages, use
a single SQL query to move the messages. This is much more efficient
for large topic moves. Since the `edit_history` field is not yet
JSON (see #26496) this requires that PostgreSQL cast the current data
into `jsonb`, append the new data (also cast to `jsonb`), and then
re-cast that as text.
For single-message moves, this _increases_ the SQL query count by one,
since we have to re-query for the updated data from the database after
the bulk update. However, this is overall still a performance
improvement, which improves to 2x or 3x for larger topic moves. Below
is a table of duration in seconds to run `do_update_message` to move a
topic to a new stream, based on messages in the topic, for before and
after this change:
| Topic size | Before | After |
| ---------- | -------- | ------- |
| 1 | 0.1036 | 0.0868 |
| 2 | 0.1108 | 0.0925 |
| 5 | 0.1139 | 0.0959 |
| 10 | 0.1218 | 0.0972 |
| 20 | 0.1310 | 0.1098 |
| 50 | 0.1759 | 0.1366 |
| 100 | 0.2307 | 0.1662 |
| 200 | 0.3880 | 0.2229 |
| 500 | 0.7676 | 0.4052 |
| 1000 | 1.3990 | 0.6848 |
| 2000 | 2.9706 | 1.3370 |
| 5000 | 7.5218 | 3.2882 |
| 10000 | 14.0272 | 5.4434 |
(cherry picked from commit a2657b843c)
This applies access restrictions in SQL, so that individual messages
do not need to be walked one-by-one. It only functions for stream
messages.
Use of this method significantly speeds up checks if we moved "all
visible messages" in a topic, since we no longer need to walk every
remaining message in the old topic to determine that at least one was
visible to the user. Similarly, it significantly speeds up merging
into existing topics, since it no longer must walk every message in
the new topic to determine if the user could see at least one.
Finally, it unlocks the ability to bulk-update only messages the user
has access to, in a single query (see subsequent commit).
(cherry picked from commit 7dcc7540f9)
LoggingCountStats with a daily duration and that are directly stored
on the RealmCount table (not via aggregation in process_count_stat),
can be in a state, after the hourly cron job to update analytics
counts, where the logged value will be live-updated later, because
the end time for the stat is still in the future.
As these logging counts are designed to be used on the self-hosted
installation for either debugging or rate limiting, sending these
partial/incomplete counts to the bouncer has low value.
(cherry picked from commit 40daf10a74)
The problem was that earlier this was just an uncaught JsonableError,
leading to a full traceback getting spammed to the admins.
The prior commit introduced a clear .code for this error on the bouncer
side, meaning the self-hosted server can now detect that and handle it
nicely, by just logging.error about it and also take the opportunity to
adjust the realm.push_notifications_... flags.
(cherry picked from commit e8018a7285)
This commit updates the API to check the permission to subscribe other
users while creating multi-use invites. The API will raise error if
the user passes the "stream_ids" parameter (even when it contains only
default streams) and the calling user does not have permission to
subscribe others to streams.
We did not add this before as we only allowed admins to create
multiuse invites, but now we have added a setting which can be used
to allow users with other roles as well to create multiuse invites.
It is possible to have multiple users with the same email address --
for instance, when two users are guests in shared channels via two
different other Slack instances.
Combine those Slack user-ids into one Zulip user, by their user-id;
otherwise, we run into problems during import due to duplicate keys.
(cherry picked from commit 937e41bac8)
Anders Kaseorg
Mateusz Mandera
Alex Vandiver
Tim Abbott
Lauryn Menard
Sahil Batra