This should help make it explicit whenever we add a new table to Zulip
that we need to correctly categorize it for whether it will be
included in the data export, or not.
The user can now specify the value while creating a stream.
An admin can later change it via `Change stream permissions`
modal. Add is_announcement_only to subscription type text.
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.
I spend a lot of time on this. One of our users had reported that
this webhook wasn't working at all. So I tested this with a local
ngrok instance and made sure that it was working. I also took this
opportunity to rewrite the docs for this, which were quite outdated.
With a few changes by Rishi Gupta!
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.
We don't want to keep around a declaration of
PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS forever, so we should just move
this to a getattr; if the user has set it on their server, we'll use
the value; otherwise, we just use False.
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 should have no effect for now, but it'll make things a bit
simpler in case we make future changes to support public streams
without history public to subscribers (and other organization
members).
Significantly tweaked by tabbott because:
* Argparse was already handling the early checks
* Splitting the bottom loop into two loops means we validate all the
input before trying to run actual import code on anything.
* The argparse documentation was confusing about whether the paths
should be files or directories.
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.
In 'zerver_reaction', the emoji_code should be updated
with the RealmEmoji allocated id when the 'reaction_type'
is 'realm_emoji'. Hence we add an extra field 'reaction_field'
in 're_map_foreign_keys', to process the above mentioned
condition.
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 commit adds a view which will be used to process login requests,
adds an AuthenticationTokenForm so that we can use TextField widget for
tokens, and activates two factor authentication code path whenever user
tries to login.
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.
This improves test coverage for a lot of our webhooks that relied
on ad-hoc methods to handle unexpected event types.
Note that I have deliberately skipped github_legacy, it isn't
advertised and is officially deprecated.
Also, I have refrained from making further changes to Trello, I
believe further improvements to test coverage should be covered
in separate per-webhook commits/PRs.
UnexpectedWebhookEventType is a generic exception that we may
now raise when we encounter a webhook event that is new or one
that we simply aren't aware of.
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.
We add conditional infinite sleep to this delivery job as a means to
handle case of multiple servers in service to a realm running this
job. In such a scenerio race conditions might arise leading to
multiple deliveries for same message. This way we try to match the
behaviour of what other jobs do in such a case.
Note: We should eventually do something to make such jobs work
while being running on multiple servers.
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).
We've for a long time been plagued by run-dev.py needing to be
restarted every time one does a rebase that has merge conflicts,
because the Tornado process restarts itself into a syntax error and
crashes.
This fixes the Tornado autoreload process to check explicitly for
whether files actually syntax-check before trying to actually reload
the Tornado process to run that code.
There are a few things that are a bit janky:
* Ideally, this would go into Tornado upstream
* We removed the `_watched_files` feature, which we weren't using.
* Ideally, we'd use something other than `importlib.reload` that just
does the syntax-check without adjusting the state within our current
process.
Fixes#4351.
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.
We ask our users to enable Snapshot notifications in Zulip via
Slack! But our Slack integration isn't exactly super robust and
I checked and our librato implementation isn't super smart about
handling snapshot payloads that come in via Slack.
Overall, this seems like a very poor solution, asking the user
to set up Slack in order to get the notifications in Zulip. So, I
thought we should get rid of at least the docs that suggest doing
this.
I also read librato/view.py and it wasn't clear to me how Slack
is supposed to act as an intermediate service here in a reliable
manner, which is another reason to not advertise this.
This should help avoid confusing error messages for anyone
accidentally running this twice.
In particular, this also makes it easier to run Zulip inside
Kubernetes, since one doesn't need to worry about duplicate calls.
The only slash command implemented in this initial
version is an extremely crippled version of a
"/stats" slash command that reports that you are
running 1 server.
Makes announce stream `is_announcement_only` for the dev db for easier
manual testing. The default value for `is_announcement_only` in
`bulk_create_streams` is False.
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.
We're never going to add tests for this block, which is fundamentally
well-tested code from Django with a since line changed which is hard
to screw up (long-polling will not work at all without it). The hope
is to remove it entirely and replace it with a cleaner monkey-patch,
but until then, unit tests for it would be redundant.
This has a cool structure, but it's written against the long-dead
South API, and we can always pull it out of the Git history if we want
to use this approach in the future.
This module doesn't exist, and never did; the name appears to be a
mistaken variant of the module that really does contain ZulipTestCase.
So, fix the import to use the real name.
This would never have worked at runtime, which is why it's in an
`if False:`. It's also an example of the kind of error that can be
hidden by `ignore_missing_imports`; we'd have caught the issue
immediately if we hadn't had a blanket application of that flag
in place.
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.
Cleaned up add_user_list_args(). The "help" and
"all_users_help" have all default values. As noted in
an earlier commit, "all_users_help" is always passed in,
so we can get rid of "all_users_arg". We keep the default
for "all_users_help" so we don't have to change variable order
in function definition.
We remove an unecessary "required" paramter from this function
because as seen in the get_users() function right below, you have
to pass either -u/--users or -a/--all-users, meaning there should
never be a reason to require --users.
This reflects the fact that these are just defensive programming (we
don't expect them to ever happen) and also nicely makes these lines
not show up in our missing test coverage reports.
We only use this in the direct management command, and it involves
some autoreload process setup stuff that we probably don't want to do
in our unit tests regardless.
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.
The "Short/Long Text" option for custom profile fields wasn't properly
capitalized (i.e. "Text" should have been all lowercase), and also
wasn't properly tagged for translation.
For the sake of consistency, the change to proper capitalization has
also been applied to the models and any tests involving this feature.
Due to a bug in Django, it complained about the models having changed
and thus not being consistent with the migrations. That isn't actually
true (since the database stores the numeric values for each key), but
the migrations have been modified to avoid this error. This does not
affect the migrations' behaviour in any way.
This fixes exceptions when sending PMs in development (where we were
trying to connect to the localhost push bouncer, which we weren't
authorized for, but even if we were, it wouldn't work, since there's
no APNS/GCM certs).
At the same time, we also set and order of operations that ensures one
has the opportunity to adjust the server URL before submitting
anything to us.
It makes sense to refactor out the last_reminder logic out of
send_pm_if_empty_stream and have a generic function that can send
rate-limited PM notifications to a bot owner and can be used by
methods other than send_pm_if_empty_stream.
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.
A typo in my reading of 6cc2e8bbff meant
that we were incorrectly doing database queries for each Service
object, just to get the user_profile.id, which we already had.
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.
Our query for Custom Profile fields was for no good reason passing the
list of all users in the realm (potentially many thousands) into a
database query, rather than letting the database do that join.
Fixing this saves 100ms-200ms in the loading time for / on
chat.zulip.org for all users, since we were previously doing a ton of
work even if the feature wasn't being used.
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.
The main thing here is writing check_string_fixed_length and
check_capped_string as returning a Validator, but we also fix issues
around passing default=None.
Tim Abbott
Sampriti Panda
Shubham Padia
Steve Howell
Rhea Parekh
Shubham Dhama
Robert Hönig
Eeshan Garg
Yashashvi Dave
Vishnu Ks
Yago González
Umair Khan
Aditya Bansal
Michael J. Sullivan
Joshua Pan
jkiely
Rishi Gupta
Lyla Fischer
Baron Chandler
Greg Price
=
RobbieClarken
Darshan Markandaiah
Raymond Akornor