This fixes a regression in ae5ba7f4fd,
where Zulip would 500 if the newly added system bots didn't exist on
the server.
This also fixes a moderate size performance problem where we'd fetch 5
users from memcached or the database in a loop.
This fixes a regression in 25c669df52.
We were draining the queue in both the superclass and the subclass,
so by the time the subclass started processing events, there were
no events to process. Now the subclass properly uses the events
passed in from the superclass.
These are new:
new-user-bot
emailgateway
Our cross-realm bots are hard coded to have email addresses
in the `zulip.com` domain, and they're not part of ordinary
realms.
These have always been cross-realm, but new enforcement in the
frontend code of all messages having been sent by a known user means
that it's important to add these properly.
Previously, these push notification events were being generated, but
then ignored in handle_push_notification because there was no
user_message object.
Generally emails are not written with markdown in mind and hence
sometimes render in strange ways. This commit fixes a particular
issue that was causing whitespace before paragraphs to be treated
as code block due to which email content was being rendered in a
box that scrolls in right direction a lot.
Fixes: #7045.
In templates/zerver/api/main.html, since the current context isn't
passed to render_markdown_path when rendering an article,
render_markdown_path doesn't have the context to render values such
as api_url. This commit makes sure that it does by passing a dict
called api_uri_context to render_markdown_path when rendering an
article.
This change affects realm_users and realm_non_active_users.
Note that we still send full avatar urls in realm_user/add
events, so apply_events has to do something mildly hacky to
turn the avatar_url to None in that case.
Fixing the event is probably not worth the trouble, as single
urls are not bandwidth hogs; we only need this optimization
for bulk data.
This change affects these values:
* page_params.avatar_url
* page_params.avatar_url_medium
It requires passing the client_gravatar flag through this
codepath:
* home_real
* do_events_register
* fetch_initial_state_data
* avatar_url
Seems like the more logical check. Also, the previous code makes it feel
like there is a potential vulnerability where one could get an email change
object in a realm where email changes are disabled, and then open that link
while logged in to a different realm.
While we're at it, remove the unnecessary check that the user is
logged in when clicking the confirmation link; that creates
unnecessary trouble for users who use multiple browsers.
This commit allows clients to register client_gravatar=True, and
then we recognize that flag for message events. If the flag is
True, we will not calculate gravatar URLs and let the clients do
it themselves. (Clients can calculate gravatar URLs based on
emails with just a little bit of code.)
This refactoring doesn't change behavior, but it sets us up
to more easily handle a register setting for `client_gravatar`,
which will allow clients to tell us they're going to compute
their own gravatar URLs.
The `client_gravatar` flag already exists in our code, but it
is only used for Django views (users/messages) but not for
Zulip events.
The main change is to move the call to `set_sender_avatar` into
`finalize_payload`, which adds the boolean `client_gravatar`
parameter to that function. And then we update various callers
to supply that flag.
One small performance benefit of this change is that we now
lazily compute the client message payloads in
`event_queue.process_message_event` now, so this will improve
performance if all interested clients have the same value of
`apply_markdown`. But the change here is really preparing us
for the additional boolean parameter, which will cause us to
have four variations of the payload.
This change:
* Prevents weird potential attacks like taking a valid confirmation link
(say an unsubscribe link), and putting it into the URL of a multiuse
invite link. I don't know of any such attacks one could do right now, but
reasoning about it is complicated.
* Makes the code easier to read, and in the case of confirmation/views.py,
exposes something that needed refactoring anyway (USER_REGISTRATION and
INVITATION should have different endpoints, and both of those endpoints
should be in zerver/views/registration, not this file).
Before this commit, ResponseMock() was initialized
with a data attribute, which isn't used in the tests
and does not occur in the outgoing webhook code.
The main limitation of this version is that it's controlled entirely
from settings, with nothing in the database and no web UI or even
management command to control it. That makes it a bit more of a
burden for the server admins than it'd ideally be, but that's fine
for now.
Relatedly, the web flow for realm creation still requires choosing a
subdomain even if the realm is destined to live at an alias domain.
Specific to the dev environment, there is an annoying quirk: the
special dev login flow doesn't work on a REALM_HOSTS realm. Also,
in this version the `add_new_realm` and `add_new_user` management
commands, which are intended for use in development environments only,
don't support this feature.
In manual testing, I've confirmed that a REALM_HOSTS realm works for
signup and login, with email/password, Google SSO, or GitHub SSO.
Most of that was in dev; I used zulipstaging.com to also test
* logging in with email and password;
* logging in with Google SSO... far enough to correctly determine
that my email address is associated with some other realm.
The original PR to allow generic bots to be mentioned had
some merge issues that we detected about a week after the
fact. This commit restores the logic from the original PR.
The reason we didn't detect this bug earlier is that the
merge issues didn't break any existing behavior. Instead,
they made it so that only UserMessage rows got written for
bots, but no events were being set. The part of the commit
that got lost is restored here, so now events get sent as
well.
Thanks to @derAnfaenger for reporting this and being patient
as we tracked it down.
Fixes#7140
This adds the data model and bugdown support for the new UserGroup
mention feature.
Before it'll be fully operational, we'll still need:
* A backend API for making these.
* A UI for interacting with that API.
* Typeahead on the frontend.
* CSS to make them look pretty and see who's in them.
tsearch_extras returns search offsets in bytes but our highlight
function treated them as character offsets. Added a check to subtract
extra bytes if the tsearch search backend is being used.
Fixes#4084.
Fixes#7021.
This commit allows for the /api-new/ page to rendered similarly to our
/help pages. It's based on the old content for /api, but we're not
replacing the old content yet, to give a bit of time to restructure
things reasonably.
Tweaked by eeshangarg and tabbott.
Because this is for tests, a heuristic like this that's right in most
situations is actually fine; we can override it in the few cases where
a test might set up a situation where it fails.
So just make it clear for the next reader that that's what's going on,
and also adjust the helper's interface slightly so that its callers
do have that flexibility.
The "subdomain" label is redundant, to the extent it's even
accurate -- this is really just the URL we want to display,
which may or may not involve a subdomain. Similarly "external".
The former `external_api_path_subdomain` was never a path -- it's a
host, followed by a path, which together form a scheme-relative URL.
I'm not quite convinced that value is actually the right thing in
2 of the 3 places we use it, but fixing that can start by giving an
accurate name to the thing we have.
We extract get_bulk_stream_subscriber_info() from this
function to remove some of the complexity. Also, in that
new function we avoid a hop to the database by querying
on stream ids instead of recipient ids. The query that
gets changed here does require a join to the recipient
table (to get the stream id), so it's a little bit of a
tradeoff.
We were using Google's diff-match-patch library to diff HTML. The
problem with that approach is that it is a text differ, not an HTML
differ and so it ends up messing up the HTML tags. `lxml` is a safer
option.
Fixes: #7219.
The first method we extract to this library is
get_active_subscriptions_for_stream_id().
We also move num_subscribers_for_stream_id() to here, which
is slightly annoying (having the method on Stream was nice)
but avoids some circular dependency issues.
Do you call get_recipient(Recipient.STREAM, stream_id) or
get_recipient(stream_id, Recipient.STREAM)? I could never
remember, and it was not very type safe, since both parameters
are integers.
Almost all callers to do_create_user were trying to
create active users, except for one test. The
active=False codepath was kind of broken (things
like sending welcome messages had sort of undefined
behavior there), so instead of trying to maintain it,
we just update the one test (`test_people`) to flip the
`is_active` flag manually.
Fixes#7197
We mostly introduce these functions (as part of a big
code sweep):
send_stream_message
send_personal_message
send_huddle_message
In two cases, where we want to specifically manipulate
queue ids, we now call check_send_message directly. (The
above three functions deliberately don't support kwargs
to ensure simple code and better type safety.)
If a Zulip install at example.org got a request at an HTTP `Host`
like foo.example.org.evil.com (or even foo.example.orgevil.com),
we would accept it as subdomain foo. This isn't likely to happen
in practice because it shouldn't pass ALLOWED_HOSTS, and it's not
obvious to me that anything untoward could be done with it even
if ALLOWED_HOSTS were set wide open, but if nothing else it
multiplies the cases in analyzing this logic.
The reason we had a loose match like this, I assume, is to allow
the user to come from arbitrary ports -- especially in development.
So tighten the pattern to allow just that, and add some tests for
that behavior and a comment explaining why this complication is
needed.
The cookie mechanism only works when passing the login token to a
subdomain. URLs work across domains, which is why they're the
standard transport for SSO on the web. Switch to URLs.
Tweaked by tabbott to add a test for an expired token.
This makes the tests a little cleaner in itself, and also prepares
them to adjust with less churn when we change how
redirect_and_log_into_subdomain passes the signed token.
Most of these have more to do with authentication in general than with
registering a new account. `create_preregistration_user` could go
either way; we move it to `auth` so we can make the imports go only in
one direction.
Lets administrators view a list of open(unconfirmed) invitations and
resend or revoke a chosen invitation.
There are a few changes that we can expect for the future:
* It is currently possible to invite an email that you have already
invited, it might make sense to change this behavior.
* Resend currently sends an invite reminder instead of resending the
original invite, this is because 'custom_body' was not stored when
the first invite was sent.
Tweaked in various minor ways, primarily in the backend, by tabbott,
mostly for style consistency with the rest of the codebase.
Fixes: #1180.
Tweaked by tabbott to have the field before the invitation is
completed be called invite_as_admins, not invited_as_admins, for
readability.
Fixes#6834.
The tighter interface prevents the need to specify
Recipient.PERSONAL (which can often be inaccurate in the
huddle case, anyway), and it prevents tests from confusingly
specifying a "subject" field for PMs.
Having send_stream_message() avoids the need to supply
Recipient.STREAM as a parameter, and it also uses the more
modern name of `topic_name` for topics. Under the hood, it
avoids some annoying steps for re-formatting the recipients,
since we just have a single stream name.
When possible, we want to use direct APIs for sending
stream messages.
This changes the codepath slightly, by not using
forwarded_user_profile, but it doesn't impact the number
of queries, and it's a simple check.
We also remove a couple "subject" references here.
This change allows normal bots to get UserMessage rows when
they are mentioned on a stream, even if they are not actually
subscribed to the stream.
Fixes#7140.
We now find all (possibly) relevant service bots for a message
in the call to get_recipient_info. This allows us to eliminate
some code that would patch them after we rendered.
The get_service_bot_events() function will ignore any service
bots that weren't actually mentioned in the message (due to
backticks) or part of the active user ids.
In do_send_messages, we only produce one dictionary for
the event queues, instead of different flavors for text
vs. html. This prevents two unnecessary queries to the
database.
It also means we only put one dictionary on the "message"
event queue instead of two, albeit a wider one that has
some values that won't be sent to the actual clients.
This wider dictionary from MessageDict.wide_dict is also
used for the `feedback_messages` queue and service bot
queues. Since the extra fields are possibly useful down
the road, and they'll just be ignored for now, we don't
bother to remove them. Also, those queue processors won't
have access to `content_type`, which they shouldn't need.
Fixes#6947
Before this change, we populated two cache entries for each
message that we sent. The entries were largely redundant,
with the only difference being whether we sent the content
as raw markdown or as the rendered HTML.
This commit makes it so we only have one cache entry per
message, and it includes both content and rendered_content.
One legacy source on confusion here is that `content`
changes meaning when you're on the front end. Here is the
situation going forward:
database:
content = raw
rendered_contented = rendered
cache entry:
content = raw
rendered_contented = rendered
payload for the frontend:
content = raw (for apply_markdown=False)
content = rendered (for apply_markdown=True)
Adds support to add "Embedded bot" Service objects. This service
handles every embedded bot.
Extracted from "Embedded bots: Add support to add embedded bots from
UI" by Robert Honig.
Tweaked by tabbott to be disabled by default.
Every time we updated a UserProfile object, we were calling
delete_display_recipient_cache(), which churns the cache and
does an extra database hop to find subscriptions. This was
due to saying `updated_fields` instead of `update_fields`.
This made us prone to cache churn for fields like UserProfile.pointer
that are fairly volatile.
Now we use the helper function changed(). To prevent the
opposite problem, we use all the fields that could invalidate
the cache.
This test had a little bug, where we weren't actually
verifying `realm_bots` before, because we weren't using
`field` to look it up.
This commit fixes that bug and adds additional checks,
particularly for the recently added `realm_non_active_users'.
We now add `realm_non_active_users` to the result of
`do_events_register` (and thus `page_params`). It has
the same structure as `realm_users`, but it's for
non-active users. Clients need data on non-active users
when they process old messages that were sent by those
users when they were active. Clients can currently get
most of the data they need in the message events, but it
makes for ugly client code.
Fixes#4322
We make a few things cleaner for populating `realm_users`
in `do_event_register` and `apply_events`:
* We have a `raw_users` intermediate dictionary that
makes event updates O(1) and cleaner to read.
* We extract an `is_me` section for all updates that
apply to the current user.
* For `update` events, we do a more surgical copying
of fields from the event into our dict. This
prevents us from mutating fields in the event,
which was sketchy (at least in test mode). In
particular, this allowed us to remove some ugly
`del` code related to avatars.
* We introduce local vars `was_admin` and `now_admin`.
The cleanup had two test implications:
* We no longer need to normalize `realm_users`, since
`apply_events` now sees `raw_users` instead. Since
`raw_users` is a dict, there is no need to normalize
it, unlike lists with possibly random order.
* We updated the schema for avatar updates to include
the two fields that we used to hackily delete from
an event.
If an organization doesn't have the EmailAuthBackend (which allows
password auth) enabled, then our password reset form doesn't do
anything, so we should hide it in the UI.
While our recent changing to hide /register means we don't need a nice
pretty error message here, eventually we'll want to clean up the error
message.
Fixes#7047.
Umair Khan
Tim Abbott
Brock Whittaker
Steve Howell
Vishnu Ks
derAnfaenger
Eeshan Garg
Tommy Ip
YJDave
rht
Harshit Bansal
Rishi Gupta
neiljp (Neil Pilgrim)
Greg Price
Sampriti Panda
Aditya Bansal
Henrik Pettersson
Abhijeet Kaur
Shekh Ataul