For some reason in my original version I was sending both
content and data to the client for submessage events,
where data === JSON.parse(content). There's no reason
to not just let the client parse it, since the client
already does it for data that comes on the original
message, and since we might eventually have non-JSON
payloads.
The server still continues to validate that the payload
is JSON, and the client will blueslip if the server
regressses and sends bad JSON for some reason.
We now have a simple algorithm: First, look at the URL path
(e.g. /de/, which is intended to be an override). Second, look at the
language the user has specified in their settings.
This adds a common function `access_bot_by_id` to access bot id within
same realm. It probably fixes some corner case bugs where we weren't
checking for deactivated bots when regenerating API keys.
Fixes the avatar/emoji part of #8177.
Does not address the issue with uploaded images, since we don't do
anything with them.
Also adds 3 images with different orientation exif tags to
test-images.
Previously, if you had LDAPAuthBackend enabled, we basically blocked
any other auth backends from working at all, by requiring the user's
login flow include verifying the user's LDAP password.
We still want to enforce that in the case that the account email
matches LDAP_APPEND_DOMAIN, but there's a reasonable corner case:
Having effectively guest users from outside the LDAP domain.
We don't want to allow creating a Zulip-level password for a user
inside the LDAP domain, so we still verify the LDAP password in that
flow, but if the email is allowed to register (due to invite or
whatever) but is outside the LDAP domain for the organization, we
allow it to create an account and set a password.
For the moment, this solution only covers EmailAuthBackend. It's
likely that just extending the list of other backends we check for in
the new conditional on `email_auth_backend` would be correct, but we
haven't done any testing for those cases, and with auth code paths,
it's better to disallow than allow untested code paths.
Fixes#9422.
This is the analog of the last commit, for the password reset flow.
For these users, they should be managing/changing their password in
the LDAP server.
The error message for users doing the wrong thing here is nonexistent
isn't great, but it should be a rare situation.
Previously, if both EmailAuthBackend and LDAPAuthBackend were enabled,
LDAP users could set a password using EmailAuthBackend and continue to
use that password, even if their LDAP account was later deactivated.
That configuration wasn't supported at all before, so this doesn't fix
a pre-existing security issue, but now that we're making that a valid
configuration, we need to cover this case.
This reflects the changes to the default URL publicly
displayed to the user. It also changes the default
URL of the default test server outgoing webhook, which
prevented the test server flaskbotrc from working out
of the box.
Export of RealmEmoji should also include the image
file of those emojis.
Here, we export emojis both for local and S3 backend
in a method with is similar to attachments and avatars.
Added tests for the same.
This adds the fields `trigger` and `service_email`
to each message event dispatched by outgoing webhook bots.
`trigger` will be used by the Botserver to determine if
a bot is mentioned in the message.
`service_email` will be used by the Botserver to determine
by which outgoing webhook bot the message should be handled.
This should make it easier for us to iterate on a less-dense Zulip.
We create two classes on body, less_dense_mode and more_dense_mode, so
that it's easy as we refactor to separate the two concepts from things
like colors that are independent.
API users, particularly bots, can now send a field
called "widget_content" that will be turned into
a submessage for the web app to look at. (Other
clients can still rely on "content" to be there,
although it's up to the bot author to make the
experience good for those clients as well.)
Right now widget_content will be a JSON string that
encodes a "zform" widget with "choices." Our first
example will be a trivia bot, where users will see
something like this:
Which fruit is orange in color?
[A] orange
[B] blackberry
[C] strawberry
The letters will be turned into buttons on the webapp
and have canned replies.
This commit has a few parts:
- receive widget_content in the request (simply
validating that it's a string)
- parse the JSON in check_message and deeply
validate its structure
- turn it into a submessage in widget.py
This should significantly improve the user experience for creating
additional accounts on zulipchat.com.
Currently, disabled in production pending some work on visual styling.
This is intended to support our upcoming feature to support copying a
user's customization settings from an existing account that user owns
in another organization.
We essentially stop running create_realm_internal_bots during
every provisioing and move its operations to run from populate db.
In fact to speed things up a bit we actually make populate db call the
funcs which create_realm_internal_bots calls behind the scenes.
Fixes: #9467.
We extract the entire operations of the management command to a
function create_if_missing_realm_internal_bots in the
zerver/lib/onboarding.py. The logic for determining if there are any realm
internal bots which have not been created is extracted to a function
missing_any_realm_internal_bots in actions.py.
This isn't a complete long-term fix, in that ideally we'd be doing
this check at the view layer, but various structural things make that
annoying, and we'll want this test either way.
We've had this sort of logic for GCM for a long time; it's worth
adding for APNS as well.
Writing this is a bit of a reminder that I'm not a fan of how our unit
tests for push notifications work.
This revised GitHub auth backend test is inspired by the end-to-end
flow model of the Google auth backend test. My hope is that we will
be able to migrate the rest of the important cases in the GitHub auth
backend tests to this model and then delete what is now
GitHubAuthBackendLegacyTest.
The next step after that will be to merge the GitHub and Google auth
tests (since actually, the actual test functions are basically
identical between the two).
Apparently, the bug here was that we were aliasing the user_profile
variable, so that the results depended on what the last iteration in
the loop landed on.
Since this is a logged-out view, need to actually write code for the
case of deactivated realms.
The change to get_active_user is more for clarity; the Django password
reset form already checks for whether the user is active earlier.
If a user's account has been deactivated, we want to provide a special
error message that makes clear what's going on.
Future work is to provide some administrative controls on whether a
user should be able to re-activate their account.
This query was incorreclty not checking whether a user was deactivated
before managing their subscriptions.
This isn't an important bug, but should prevent some weird corner
cases (like trying to send a notification PM to a deactivated user,
which fails).
Slow queries during backend tests sends messages to Error Bot
which affects the database state causing the tests to fail.
This fixes the occasional flakes due to that.
Most of this is just asserting that the sub_dict return value from
access_stream_by_id is not None in the cases where it shouldn't be,
but additionally, we also need to pass a function into
validate_user_access_to_subscribers_helper (in this case, just `lambda:
True` works fine)
While maybe these don't all belong in this test file, the overall
effect is that we now have quite good test coverage on
analytics/views.py.
It'd be nice to add some more assert statements for specific values
being present in the pages, but since we're not really working on that
part of the product, it's not a priority yet.
Refactor custom fields creation and deletion tests to assert
if created/deleted field exist or not, instead of asserting
total count of all realm fields.
- do_change_is_admin now raises AssertionError when a non-admin
permission is given.
- adds test to test_users to ensure admin asserts on invalid
permission values.
This system was written years ago and has been working well the whole
time, but having unit tests for it will help future developers in
understanding what the intent is.
This is primarily useful for the mobile app, but could also be used to
control whether we display push-notifications related settings to
users in the web UI.
We send add events on upload, update events when sending a message
referencing it, and delete updates on removal.
This should make it possible to do real-time sync for the attachments
UI.
Based in part on work by Aastha Gupta.
We only use this data in a rarely-used settings screen, and it can be
large after years of posting screenshots.
So optimize the performance of / by just loading these data when we
actually visit the page.
This saves about 300ms of runtime for loading the home view for my
user account on chat.zulip.org.
This eliminates the need to call user_ids_to_users inside the
get_service_dicts_for_bots code path, saving a database query.
This completes my refactor to fix backend performance issues in this
code path. Previously, our messy layering of queries that resulted in
Zulip doing work even if none of the bots actually had Services or
config_data.
These decorators will be part of the process for disabling access to
various features for guest users.
Adding this decorator to the subscribe endpoint breaks the guest users
test we'd just added for the subscribe code path; we address this by
adding a more base-level test on filter_stream_authorization.
This is helpful for cases where an argument is supposed to be a normal
string, and we want to use a Zulip validator function to do basic
things like check its length.
We were rejecting strings of length equal to the max.
While we're at it, fix the unnecessary period in the error message,
which doesn't align with similar validators.
This commit changes the tests based on the fact that pygments.css
will no longer be found in the template during testing. pygemnts.css
is being compiled by webpack under app-styles and so we look for the
stubentry for app-styles instead.
Tweaked by tabbott to do a cleaner test.
This removes a check on invite_only, that should have been a check on
history_public_to_subscribers. In addition to fixing a bug for zephyr
realms, it also makes "more topics" work correctly for realms using
the new settings for stream history being public to subscribers.
This is a general code cleanlyness improvement.
While we're at it, we combine together two test classes that didn't
have a particularly good reason for existing.
This commit adds a new field history_public_to_subscribers to the
Stream model, which serves a similar function to the old
settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS; we still use that
setting as the default value for new streams to avoid breaking
backwards-compatibility for those users before we are ready with an
actual UI for users to choose directly.
This also comes with a migration to set the value of the new field for
existing streams with an algorithm matching that used at runtime.
With significant changes by Tim Abbott.
This is an initial part of our efforts on #9232.
The handlebars error message is just for the manual development
environment; this prevents the state of compiling handlebars templates
from run-dev.py from potentially causing the unit tests to fail.
The removed code path was only needed due to buggy setup code in the
test_cross_realm_scenarios test. We address that with a less buggy
workaround, and which lets us remove unnecessary complexity from this
important validation function.
Thanks for Umair Waheed for some preliminary work on this.
Fixes#7561.
Add realm setting to set time limit for message deleitng.
Set default value of message_content_delete_limit_seconds
to 600 seconds(10 min).
Thanks to Shubham Dhama for rebasing and reworking this. Some final
edits also done by Tim Abbott.
Fixes#7344.
This verifies an important case. We still have an open bug for why in
some production environments, the email_gateway_bot seems to not be
tagged as an API super user (resulting in this code path not working).
This exception class was clearly missing the part where `role` gets
stored, which was intended to be inherited from
InvalidZulipServerError.
This fixes an unnecessary 500 error in the push notifications bouncer.
Tweaked by tabbott to add a test and fix a super subtle issue with the
relative_settings_link variable having been set once the first time a
/help article was rendered.
A common path is a new user goes to realm_uri, which redirects to
realm_uri/login, and clicks the google auth button thinking it is a
registration button.
This commit just changes the wording on the page they land on to be
friendlier for that use case.
This completes the effort to ensure that all of our webhooks that do
parsing of the third-party message format log something that we can
use to debug cases where we're not parsing the payloads correctly.
The main change here is to send a proper confirmation link to the
frontend in the `confirm_continue_registration` code path even if the
user didn't request signup, so that we don't need to re-authenticate
the user's control over their email address in that flow.
This also lets us delete some now-unnecessary code: The
`invalid_email` case is now handled by HomepageForm.is_valid(), which
has nice error handling, so we no longer need logic in the context
computation or template for `confirm_continue_registration` for the
corner case where the user somehow has an invalid email address
authenticated.
We split one GitHub auth backend test to now cover both corner cases
(invalid email for realm, and valid email for realm), and rewrite the
Google auth test for this code path as well.
Fixes#5895.
This test class is basically a poor version of the end-to-end tests
that we have in `test_auth_backends.py`, and didn't really add any
value other than making it difficult to refactor.
By moving all of the logic related to the is_signup flag into
maybe_send_to_registration, we make the login_or_register_remote_user
function quite clean and readable.
The next step is to make maybe_send_to_registration less of a
disaster.
The code in maybe_send_to_registration incorrectly used the
`get_realm_from_request` function to fetch the subdomain. This usage
was incorrect in a way that should have been irrelevant, because that
function only differs if there's a logged-in user, and in this code
path, a user is never logged in (it's the code path for logged-out
users trying to sign up).
This this bug could confuse unit tests that might run with a logged-in
client session. This made it possible for several of our GitHub auth
tests to have a totally invalid subdomain value (the root domain).
Fixing that bug in the tests, in turn, let us delete a code path in
the GitHub auth backend logic in `backends.py` that is impossible in
production, and had just been left around for these broken tests.
This code path has actually been dead for a while (since
`invalid_subdomain` gets set to True only when `user_profile` is
`None`). We might want to re-introduce it later, but for now, we
eliminate it and the artificial test that provided it with test
coverage.
This commit sends the event for renaming of a private stream to
organization admins of the realm, in addition to the obvious list of
subscribers of the private stream.
Normally, admins can manage a private stream (e.g. unsubscribing a
user). But when the admin tried to unsubscribes a user from a
previously renamed stream, we previously were throwing a JS error, as
the webapp hadn't been notified about the new stream name.
Fixes#9034.
This was stored as a fixture file under zerver/fixtures, which caused
problems, since we don't show that directory under production (as its
part of the test system).
The simplest emergency fix here would be to just move the file, but
when looking at it, it's clear that we don't need or want a fixture
file here; we want a Python object, so we just do that.
A valuable follow-up improvement to this block would be to create an
actual new Realm object (not saved to the database), and dump it the
same code we use in the export tool; that should handle the vast
majority of these correctly.
Fixes#9123.
We should still short-circuit the iteration in
`add_missing_messages` if the unsubscription was the last
thing to happen to the user before unsubscription and
soft deactivation.
This extends the /user_uploads API endpoint to support passing the
authentication credentials via the URL, not the HTTP_AUTHORIZATION
headers. This is an important workaround for the fact that React
Native's Webview system doesn't support setting HTTP_AUTHORIZATION;
the app will be responsible for rewriting URLs for uploaded files
directly to add this parameter.
This commit increases the rendered_content limit from 2x to 10x of the
original message length.
Earlier, we had placed a limit of MAX_MESSAGE_LENGTH * 2 for the
rendered content (explained in commit
77addc5456). That limit was based on
the assumption that in most cases, the rendered content wouldn't cause
a large increase in message length. However, quite prominently in
syntax highlighted codeblocks, that wasn't true and this caused the
limit condition to be hit for long messages composed primarily of code
blocks.
Example: The following message would render close to 10x it's original size.
```py
if:
def:
print("x", var)
x = y
```
Because the syntax highlighted logic is extremely compressible, having
rendered_content reach up to 100KB doesn't create a network
performance problem.
This is a mobile-specific endpoint used for logging into a dev server.
On mobile without this realm_uri it's impossible to send a login request
to the corresponding realm on the dev server and proceed further; we can
only guess, which doesn't work for using multiple realms.
Also rename the endpoint to reflect the additional data.
Testing Plan:
Sent a request to the endpoint, and inspected the result.
[greg: renamed function to match, squashed renames with data change,
and adjusted commit message.]
After some thinking, I don't think there's any actual value to doing
the ../ style relative links here, whereas there is actual harm from
the links being slightly broken in the current model. We fix this by
just using /#settings as the URL.
Fixes#8978.
For certain queries where both include_history and
use_first_unread_anchor are set to True, we were excluding
historical rows. Now we only use the use_first_unread_anchor
flag to filter rows that we use to find the anchor, without
having it filter the actual search results.
The bug went unreported for a long time, because it only
affected mobile users who had newly subscribed to streams.
Note that we make a small change to the test called
test_use_first_unread_anchor_with_muted_topics, which has
a very scary comment about being "arcane" and "be
absolutely sure you know what you're doing." I think it's
fine.
Also, the new test code would fail before this fix, so it
should help prevent future regressions.
Fixes#8958
This is a bit more than a pure refactor, because we duplicate a
chunk of code to calculate a query inside of
find_first_unread_anchor(), so we're doing a bit more work
than before.
We need this refactoring to start decoupling find_first_unread_anchor
from get_messages_backend for the case where include_history is
True. This will happen in a subsequent commit.
The only test that changes here is a direct test on
find_first_unread_anchor(). All other tests pass without
modification, and we have decent coverage on get_messages_backend.
Tim Abbott
Steve Howell
Shubham Padia
Rhea Parekh
Shubham Dhama
Robert Hönig
Yashashvi Dave
Sampriti Panda
Vishnu Ks
Eeshan Garg
Umair Khan
Aditya Bansal
Michael J. Sullivan
jkiely
Lyla Fischer
Baron Chandler
RobbieClarken
Joshua Pan
Darshan Markandaiah
Armaan Ahluwalia
novokrest
Rohitt Vashishtha
Angelika Serwa
Rishi Gupta
Preston Hansen
Roman Godov
Nikhil Kumar Mishra
Ben Reeves
Alyssa Wagenmaker
Vishwesh Jainkuniya
rht