Commit Graph

11211 Commits

Author SHA1 Message Date
Anders Kaseorg 4f748fb627 markdown: Stop setting target="_blank".
This setting is being overridden by the frontend since the last
commit, and the security model is clearer and more robust if we don't
make it appear as though the markdown processor is handling this
issue.

Co-authored-by: Tim Abbott <tabbott@zulipchat.com>
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-04-01 14:01:45 -07:00
Tim Abbott e3a4aeeffa CVE-2020-9445: Remove unused and insecure modal_link feature.
Zulip's modal_link markdown feature has not been used since 2017; it
was a hack used for a 2013-era tutorial feature and was never used
outside that use case.

Unfortunately, it's sloppy implementation was exposed in the markdown
processor for all users, not just the tutorial use case.

More importantly, it was buggy, in that it did not validate the link
using the standard validation approach used by our other code
interacting with links.

The right solution is simply to remove it.
2020-04-01 14:01:45 -07:00
Tim Abbott 17723b1617 version: Move minimum desktop version configuration to version.py.
This makes it relatively easy for a system administrator to
temporarily override these values after a desktop app security
release that they want to ensure all of their users take.

We're not putting this in settings, since we don't want to encourage
accidental long-term overrides of these important-to-security values.
2020-04-01 13:23:08 -07:00
Rohitt Vashishtha 4a0fe4606a tests: Print test-case name in all test failures in fixtures.
Previously, we only printed the test-case when we had an assertion error.
With this change, we also include timeout errors as well as any other
causes for failure.
2020-03-31 14:57:18 -07:00
Steve Howell 2fe21de733 dev login: Sort users. 2020-03-30 10:16:27 -04:00
Steve Howell a37c183728 bot tests: Add test_multiple_services().
This test would have failed before we fixed the
bug in #14384.
2020-03-29 15:12:27 -07:00
Steve Howell 123604e4ba Improve test_pm_to_outgoing_webhook_bot().
We now have Hamlet, not Othello, send the message
to Othello's bot, since that's a more interesting
test and less likely to lead to a false positive.

And then we simplify the recipient check to avoid
the strange mypy mess as well as possible false
negatives.
2020-03-29 15:12:27 -07:00
Steve Howell fd77cfc4df tests: Inline TestOutgoingWebhookMessaging.setUp().
It's more readable if you just have the two
lines at the top of each function telling you
who's the bot owner and how you're creating the
bot.
2020-03-29 15:12:27 -07:00
Udit107710 ef741bf317 messages: Return shallow copy of message object.
When more than one outgoing webhook is configured,
the message which is send to the webhook bot passes
through finalize_payload function multiple times,
which mutated the message dict in a way that many keys
were lost from the dict obj.

This commit fixes that problem by having
`finalize_payload` return a shallow copy of the
incoming dict, instead of mutating it.  We still
mutate dicts inside of `post_process_dicts`, though,
for performance reasons.

This was slightly modified by @showell to fix the
`test_both_codepaths` test that was added concurrently
to this work.  (I used a slightly verbose style in the
tests to emphasize the transformation from `wide_dict`
to `narrow_dict`.)

I also removed a deepcopy call inside
`get_client_payload`, since we now no longer mutate
in `finalize_payload`.

Finally, I added some comments here and there.

For testing, I mostly protect against the root
cause of the bug happening again, by adding a line
to make sure that `sender_realm_id` does not get
wiped out from the "wide" dictionary.

A better test would exercise the actual code that
exposed the bug here by sending a message to a bot
with two or more services attached to it.  I will
do that in a future commit.

Fixes #14384
2020-03-29 15:12:27 -07:00
Steve Howell 4c51a94bcd message: Move transitional shim for delivery email.
If we have an old event that's missing the field
`sender_delivery_email`, we now patch it at the top
of `process_message_event`, rather than for each call
to `get_client_payload`.  This will make an upcoming
commit a bit easier to reason about.  Basically, it's
simpler to shim the incoming event one time rather
than doing it up to four times.  We know that
`get_client_payload` is non-destructive, because it
does a deepcopy.
2020-03-29 15:12:27 -07:00
Steve Howell 8002f48099 tests: Validate payload in test_build_bot_request.
We now validate the message data explicitly, rather
than comparing it to the event data.  This protects
us from false positives where we were only validating
that the request data was a mutated version of the
event message data.  (We'll have a commit soon that
fixes a mutation-related bug.)
2020-03-29 15:12:27 -07:00
Steve Howell 50c197c1fc tests: Move bot event code out of setUp().
This code is only used in one test, and having
the indirection of setUp partly obscured a
problem with the fact that our event message
is actually a wide dict that gets mutated
by `build_bot_request`.  We'll fix that soon,
but this is a pure code move for now.
2020-03-29 15:12:27 -07:00
Steve Howell f329fe465d minor: Use local instead of class `self.bot_user`. 2020-03-29 15:12:27 -07:00
Steve Howell e29ddd0ce0 outgoing_webhook: Remove `event` from process_success.
The `event` parameter is never used by `process_success`,
and eliminating it allows us to greatly simplify tests
that are just confusingly passing in events that are
totally ignored.
2020-03-29 15:12:27 -07:00
Steve Howell bacfadbc61 minor: Use explicit params in build_bot_request.
I also tweaked the block comment to mention
gravatars.
2020-03-29 15:12:27 -07:00
orientor b9d0d6edcb openapi: Use response schema for describing simple success response.
In zulip.yaml simple json success response which only contains 'msg'
and 'result' properties has been described repeatedly in multiple
endpoints. Instead, use SimpleSuccess template for such responses
to increase code modularity and reusablility.
2020-03-29 19:25:14 +05:30
shubhamgupta2956 12474a3deb api docs: Migrate REAL-TIME /events to OpenAPI.
Migrate "call_on_each_event" from api/arguments.json to
/events:real-time in OpenAPI.

This is a bit of a hack, but it lets us eliminate this secondary
arguments.json file, which is probably worth it.

Tweaked by tabbott to fix various formatting issues in the original
documentation while I was looking at it.
2020-03-27 17:43:35 -07:00
shubhamgupta2956 a578234fdc api docs: Migrate /message/{message_id}:patch.
Most part of "/message/{message_id}" is migrated to OpenAPI. This commit
migrated the remaning payload "update-message-edit-permission-error"
from "api/fixtures.json" to OpenAPI. This commit also fixes an error
schema in "zulip.yaml" for this payload.
2020-03-27 17:29:29 -07:00
Stefan Weil d2fa058cc1
text: Fix some typos (most of them found and fixed by codespell).
Signed-off-by: Stefan Weil <sw@weilnetz.de>
2020-03-27 17:25:56 -07:00
orientor 6eb39c8668 openapi: Migrate remaining properties from zulip-2.0.yaml to zulip.yaml.
This moves all useful work from the temporary/testing file
zulip-2.0.yaml to zulip.yaml, allowing us to remove the old file.
2020-03-27 17:09:10 -07:00
arpit551 8f7733cb20 emails: Added placeholders strings in FormAddress.
We've had a bug for a while that if any ScheduledEmail objects get
created with the wrong email sender address, even after the sysadmin
corrects the problem, they'll still get errors because of the objects
stored with the wrong format.

We solve this by using FromAddress placeholders strings in
send_future_email function, so that ScheduledEmail objects end up
setting the final `from_address` value when mail is actually sent
using the setting in effect at that time.

Fixes #11008.
2020-03-27 16:41:02 -07:00
Kartik Srivastava 1388a1d2b1
api docs: Refactor get_single_user to use get_user_by_id.
This refactors get_single_user to use get_user_by_id instead of
call_endpoint.  Doing so is only possible now that we've upgraded
python-zulip-api to a version with the new function.
2020-03-27 15:49:20 -07:00
Steve Howell f62f8c9238 message tests: Clean up edit-message tests.
Overall, this change eliminates a lot of
optional parameters and conditionals, plus
some legacy logic related to caches.

For all the places we are just editing topics,
we now just call `check_topic` to see that
the topic got updated.

For places where the topic edit failed, we
just inline the checks that message still
has the old topic and content.

And then for successful **content** edits,
we now do a more rigorous, more sane check
that the messages are properly cached.  The
old code here had evolved from 2013 into
something that didn't really make much sense
in the context of editing topics.

Now we are literally pulling data from the
cache and making sure it's valid, rather
than trying to poorly simulate the two
codepaths related to dispatching message
events and fetching messages.  Some of the
history here was that when I introduced
`MessageDict` several years ago, I did a
lot of code sweeping and didn't analyze every
single test to make sure it's still valid,
plus some of the tests still had some value
for catching regressions.  A recent commit
now gets us coverage on that a lot more
explicitly, rather than in passing.
2020-03-27 13:51:50 -07:00
Steve Howell db4ae7fc39 tests: Test equivalency of the two message codepaths.
See the comment in the test for a thorough explanation.

In brief, this test makes sure that the events codepath
for messages produces the same results as the fetch
codepath.

And this sets us up to simplify another test that kind
of poorly tried to do the same thing in passing.  (In
fairness the test was really ancient and preceded a lot
of later work that we did here.)
2020-03-27 17:20:34 +00:00
Steve Howell c2b3269420 message perf: Streamline stream name lookups.
When we are fetching messages, we need to hydrate
stream names into the messages for legacy reasons.

(Ideally, we could skip this step for the webapp
and modern mobile clients, since they really only
need stream_ids, but we're not there yet.)

We keep a recipient cache that maps recipient ids
to stream names.

When we populate that cache, we now use `values(...)`
to avoid fat objects and extra DB work.

Note that we are already using a similar technique
for hydrating PM/huddle recipients.
2020-03-27 17:20:34 +00:00
Tim Abbott cb1fb94ac8 webhooks: Fix Jira webhook constructing subject still too early.
For event types that we don't yet support, like worklog_created (and
likely many more in the future), it doesn't make sense to call a
function that only parses issue events correctly.
2020-03-27 06:19:00 -07:00
Tim Abbott 737a36a9f8 portico: Use a backend variable to determine desktop version.
This makes it possible to change this parameter without rebuilding all
the server's static assets.
2020-03-27 01:37:56 -07:00
Ryan Rehman b4ade7b6d8 migrations: Refactor the enum type fields.
Accessing attributes is the preferred design
pattern, as it is more readable.
2020-03-27 00:21:21 -07:00
Tim Abbott 06c97b5be2 api docs: Render example responses as with JSON codehilite.
This makes the example responses a lot prettier visually.
2020-03-27 00:03:36 -07:00
Tim Abbott 820f0e275e api docs: Redesign visuals for documenting arguments.
The previous system for documenting arguments was very ugly if any of
the examples or descriptions were wrong.  After thinking about this
for a while, I concluded the core problem was that a table was the
wrong design element to use for API parameters, and we'd be much
better off with individual card-type widgets instead.

This rewrites the API arguments documentation implementation to use a
basic sort of card-like system with some basic styling; I think the
result is a lot more readable, and it's a lot more clear how we would
add additional OpenAPI details (like parameter types) to the
documentation.
2020-03-27 00:03:36 -07:00
Anders Kaseorg 7ff9b22500 docs: Convert many http URLs to https.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-03-26 21:35:32 -07:00
Anders Kaseorg 56aadf6503 tests: Fix CI apocalypse perpetrated by previous commit.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-03-26 21:25:25 -07:00
Tim Abbott 9b9efdfa0f docs: Update API documention to note changes in Zulip 2.1.0. 2020-03-26 20:06:39 -07:00
Tim Abbott b29213ef90 docs: Document changes in the API made since Zulip 2.1.x.
This is important documentation for anyone working on writing clients
for the Zulip APIs that wants to support older releases.
2020-03-26 17:45:03 -07:00
Tim Abbott 9d346673f1 webhooks: Fix handling of additional old-format Jira events.
This should hopefully get the Jira webhook working correctly again for
all event types we handle.
2020-03-26 16:39:03 -07:00
Tim Abbott d52a891858 jira: Fix handling of sprint_started events.
Now that we inspect more attributes of events in determining subjects,
we need to filter events like sprint_started before fetching the subject.
2020-03-26 16:34:02 -07:00
Steve Howell 697b00dd6e default streams: Change remove api to use stream_id.
This is a full-stack change:

    - server
    - JS code
    - templates

It's all pretty simple--just use stream_id instead
of stream_name.

I am 99% sure we don't document this API nor use it
in mobile, so it should be a safe change.
2020-03-25 17:11:25 -07:00
Graham Bleaney fd5ee9a831 bots: Decouple user input from imported module.
This commit modifies 'zerver/lib/bot_lib.py' to decouple the
user-controllable 'service_name' parameter from the value that is
passed in to 'import_module'. This is done as a precautionary
hardening.
2020-03-25 16:39:17 -07:00
Graham Bleaney 2fe9d85a5f redirects: Refactor redirect code to use central helper function.
This commit introduces two new functions in 'url_encoding.py' which
centralize two common patterns for constructing redirect URLs. It
also migrates the files using those patterns to use the new
functions.
2020-03-25 16:39:17 -07:00
Graham Bleaney 5dca599481 export: Harden s3 export against directory traversal.
This commit modifies 'zerver/lib/export.py' to raise an exception
in the presence of a suspected attempt at directory traversal.
2020-03-25 16:39:17 -07:00
Graham Bleaney 3e602a9bd4 integrations: Harden development views against path traversal.
This commit hardens the code in the developer integrations views by
ensuring that file paths can only be constructed with valid
integration names.
2020-03-25 16:39:17 -07:00
Emilio López d3c841d587 email_mirror: also check for Envelope-To
After subscribing a stream email address to a Mailman email list
and receiving a message from it (using the polling configuration
with an Exim + Dovecot mailserver), the following error message
is emitted by Zulip:

    Logger zerver.lib.email_mirror, from module zerver.lib.email_mirror line 77:
    Error generated by Anonymous user (not logged in) on zulip deployment

    Sender: "Foo Bar" <foo@example.com>
    To: No recipient found
    Missing recipient in mirror email

This is because the To: header on the received email corresponds
to the email list, and there are no other headers to indicate the
final recipient, apart from the "Envelope-To" header added by
Exim. To resolve this problem, the commit adds "Envelope-To" to
the list of headers to check for a match.
2020-03-25 16:28:46 -07:00
Dinesh f526ae9377 tests: Change `is_signup` argument to boolean in `test_auth_backends.py`.
The function `prepare_login_url_and_headers` returns a register
link for any value of `is_signup` unless it's not none.
This commit changes it to a boolean for that function and other
functions using it so that it becomes much clearer when a
register link will be returned.

Also, all occurrences of `is_signup='1'` are changed to
`is_signup=True` to make the code consistent with the above change.
2020-03-25 15:59:37 -07:00
Vishnu KS f8ddab58ba billing: Downgrade plan to Limited during realm deactivation.
The realm would be instantly downgraded to Limited plan when
deactivated. Any extra users that were added in the final month
would not be charged.
2020-03-25 10:54:10 -07:00
Tim Abbott d9bb6d0081 compatibility: Add more strict desktop app blocking.
This allows us to block use of the desktop app with insecure versions
(we simply fail to load the Zulip webapp at all, instead rendering an
error page).

For now we block only versions that are known to be both insecure and
not auto-updating, but we can easily adjust these parameters in the
future.
2020-03-24 20:33:11 -07:00
Tim Abbott 85c9ffd91c message: Validate propagate_mode parameters.
This improves the error handling for invalid values of the
propagate_mode parameter to our message editing endpoints.
Previously, invalid values would just work like change_one rather than
doing nothing.
2020-03-24 12:36:45 -07:00
Anders Kaseorg 39f9abeb3f python: Convert json.loads(f.read()) to json.load(f).
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-03-24 10:46:32 -07:00
Mateusz Mandera 5ae6f4f0dd tornado: Put port in logging_data before setup_event_queue in runtornado.
setup_event_queue() generates some logs about loaded event queues, and
it's good for the logging system to have access to the port at that
point already.
2020-03-24 10:25:01 -07:00
Mateusz Mandera eb0216c5a8 middleware: Log <user.id>@subdomain instead of subdomain/<user.id>.
It was decided that the new format is preferable.
2020-03-24 10:25:01 -07:00
Mateusz Mandera a1daf0cf83 middleware: Log 'root/<user.id>' when realm string_id is ''. 2020-03-24 10:25:01 -07:00
Tim Abbott 6df86dab3e jira: Handle comment_created events without issue details.
I'm not sure what causes some Jira webhook events to not include the
metadata that other events do, but it's definitely a format sent by
real installations of Jira (likely a very old version, since this has
fields missing from what modern Jira does) and we've seen it in
production.

The best we can do is encourage users to upgrade Jira for better data.
2020-03-22 21:43:21 -07:00
Tim Abbott 180d8abed6 messages: Fix unlikely exception when trying to delete a message. 2020-03-22 21:35:27 -07:00
Tim Abbott 481d351cee events: Fix buggy apply_events handling of starred_messages.
The previous starred_messages race handling did not correctly consider
the possibility that an event queue might have been registered without
starred_messages.
2020-03-22 21:30:23 -07:00
Mateusz Mandera 5da2f80140 queue_processors: Extract a duplicated logic block into do_consume. 2020-03-22 18:45:46 -07:00
Mateusz Mandera 27c19b081b rate_limit: Remove inaccurate docstring on clear_history methods. 2020-03-22 18:42:35 -07:00
Mateusz Mandera b9e5103d0c rate_limit: Refactor RateLimiterBackend to operate on keys and rules.
Instead of operating on RateLimitedObjects, and making the classes
depend on each too strongly. This also allows getting rid of get_keys()
function from RateLimitedObject, which was a redis rate limiter
implementation detail. RateLimitedObject should only define their own
key() function and the logic forming various necessary redis keys from
them should be in RedisRateLimiterBackend.
2020-03-22 18:42:35 -07:00
Mateusz Mandera 8069133f88 rate_limit: Remove __str__ methods of RateLimitedObjects.
These were clunky from the start and are no longer used, as keys are now
used directly for logging purposes.
2020-03-22 18:42:35 -07:00
Mateusz Mandera 4e9f77a6c4 rate_limit: Adjust keys() of some RateLimitedObjects.
type().__name__ is sufficient, and much readable than type(), so it's
better to use the former for keys.
We also make the classes consistent in forming the keys in the format
type(self).__name__:identifier and adjust logger.warning and statsd to
take advantage of that and simply log the key().
2020-03-22 18:42:35 -07:00
Mateusz Mandera 2c6b1fd575 rate_limit: Rename key_fragment() method to key(). 2020-03-22 18:42:35 -07:00
Mateusz Mandera 9c9f8100e7 rate_limit: Add the concept of RateLimiterBackend.
This will allow easily swapping and using various implementations of
rate-limiting, and separate the implementation logic from
RateLimitedObjects.
2020-03-22 18:42:35 -07:00
Mateusz Mandera 85df6201f6 rate_limit: Move functions called by external code to RateLimitedObject. 2020-03-22 18:42:35 -07:00
Mateusz Mandera 2b51b3c6c5 middleware: Also log request subdomain when logging "unauth" request.
This returns us to a consistent logging format regardless of whether
the request is authenticated.

We also update some log examples in docs to be consistent with the new
style.
2020-03-22 18:32:04 -07:00
Mateusz Mandera 3b5b19fde8 tornado: Log shard id in all logs coming from tornado processes.
This will make it easier to investigate using logs which requests are
being processed by which Tornado process.
2020-03-22 18:26:35 -07:00
Dinesh 5cb476e03d auth: Handle confirm registration page in `stage_two_of_registration`.
When a user in login flow using github auth chooses a email that is
not associated with an existing account, it leads to a "continue to
registration" choice. This cannot be tested with the earlier version
of `stage_two_of_registration`.
Also added the test.
Thanks to Mateusz Mandera for the solution.

Co-authored-by: Mateusz Mandera <mateusz.mandera@protonmail.com>
2020-03-22 17:31:01 -07:00
Dinesh 3de646d2cf auth: Improve GitHub auth with multiple verified emails.
The previous model for GitHub authentication was as follows:

* If the user has only one verified email address, we'll generally just log them in to that account
* If the user has multiple verified email addresses, we will always
  prompt them to pick which one to use, with the one registered as
  "primary" in GitHub listed at the top.

This change fixes the situation for users going through a "login" flow
(not registration) where exactly one of the emails has an account in
the Zulip oragnization -- they should just be logged in.

Fixes part of #12638.
2020-03-22 17:31:01 -07:00
Dinesh 5888d7c0f5 auth: Change how config error URLs are configured.
URLs for config errors were configured seperately for each error
which is better handled by having error name as argument in URL.
A new view `config_error_view` is added containing context for
each error that returns `config_error` page with the relevant
context.
Also fixed tests and some views in `auth.py` to be consistent with
changes.
2020-03-22 17:15:18 -07:00
Steve Howell a041d9e4aa minor: Clean up lstrip() for help article titles.
Saying `foo.lstrip('# ')` does more than just remove
a '# ' prefix.  It removes any combination of '#' and
spaces.

We now make the intention slightly more clear.

We would strip these as you'd expect:

    # foo
    ## foo
    ### foo

but for this we now only strip the first "#":

    # # # # # foo
2020-03-22 11:32:29 -07:00
Steve Howell edf1b1e5e8 minor: Fix buggy lstrip() call in integrations dev panel.
Thanks to @minusworld for catching this--see #14264, which
points out that lstrip() doesn't do what your intuition
might tell you it does.

Now we properly remove the "HTTP_" prefix.

It's not clear to me why we need these prefixes for Django
purposes in the fixtures, but I didn't want to go down
the rabbit hole of fixing those.

To test:

    got to http://YOUR-DEV_SERVER/devtools/integrations/
    select "bitbucket3" for the integration.
    select "diagnostics_ping.json" for the fixture.
    see "X_EVENT_KEY" in "Custom HTTP Headers"

Fixes #14264
2020-03-22 11:32:29 -07:00
Steve Howell 8c1244d0b4 tests: Kill off find_one() helper.
This was only recently added.  Using tuple
assignment raises the same errors, so the
indirection probably isn't worth it.
2020-03-20 13:40:20 -07:00
Steve Howell b5cba4aafe test_narrow: Use tuple unpacking to get messages.
This is a bit more rigorous than just
dereferencing the first element of
a list comprehension, as it will give a
ValueError if more matches are found than
the test was expecting.
2020-03-20 13:40:20 -07:00
Steve Howell ef772ee12f bot events: Prevent duplicate add-bot notifications.
We don't need `do_create_user` to send a partial
event here for bots.  The only caller to `do_create_user`
that actually creates bots (apart from some tests that
just need data setup) is `add_bot_backend`, which
sends the more complete event including bot "extras"
like service info.

The modified event tests show the simplification
here (2 events instead of 3).

Also, the bot tests now use tuple unpacking, which
will force a ValueError if we duplicate events
again.
2020-03-20 13:40:19 -07:00
Steve Howell eb9a252ec9 populate_db, tests: Restrict emails in zulip realm.
We now restrict emails on the zulip realm, and now
`email` and `delivery_email` will be different for
users.

This change should make it more likely to catch
errors where we leak delivery emails or use the
wrong field for lookups.
2020-03-19 16:21:31 -07:00
Steve Howell f647587675 bulk_create: Handle realms that hide delivery emails. 2020-03-19 16:04:05 -07:00
Steve Howell ecbbc3e365 performance: Simplify bulk_create_users().
We were going back to the database to get all
the users in the realm, when we had them right
there already.  I believe this is a legacy
of us running on a very old version of Django
(back in early days), where `bulk_create`
didn't give you back ids in a nice way.

In the interim we added the `RealmAuditLog`
code, which does take advantage of the
existing profiles (and proves we can rely
on them).

But meanwhile we were still
doing a query to get all N users in the
realm.  With `selected_related`!

To be fair, bulk_create_users() is by
its very nature a pretty infrequent
operation.  This change is more motivated
by code cleanup.

Now we just loop through user_ids for
the Recipient/Subscriber foreign key rows.

I also removed some fairly convoluted code mapping
emails to user_ids and just work in user_id
space.
2020-03-19 16:04:05 -07:00
Steve Howell 1306239c16 tests: Use email/delivery_email more explicitly.
We try to use the correct variation of `email`
or `delivery_email`, even though in some
databases they are the same.

(To find the differences, I temporarily hacked
populate_db to use different values for email
and delivery_email, and reduced email visibility
in the zulip realm to admins only.)

In places where we want the "normal" realm
behavior of showing emails (and having `email`
be the same as `delivery_email`), we use
the new `reset_emails_in_zulip_realm` helper.

A couple random things:

    - I fixed any error messages that were leaking
      the wrong email

    - a test that claimed to rely on the order
      of emails no longer does (we sort user_ids
      instead)

    - we now use user_ids in some place where we used
      to use emails

    - for IRC mirrors I just punted and used
      `reset_emails_in_zulip_realm` in most places

    - for MIT-related tests, I didn't fix email
      vs. delivery_email unless it was obvious

I also explicitly reset the realm to a "normal"
realm for a couple tests that I frankly just didn't
have the energy to debug.  (Also, we do want some
coverage on the normal case, even though it is
"easier" for tests to pass if you mix up `email`
and `delivery_email`.)

In particular, I just reset data for the analytics
and corporate tests.
2020-03-19 16:04:03 -07:00
Steve Howell b1f8141200 tests: Prevent false positives for duplicate signups.
We specifically give the existing user different
delivery_email and email addresses, to prevent false
positives during the test that checks that users
signing up with an already-existing email get
an error message.

(We also rename the test.)
2020-03-19 14:32:18 -07:00
Steve Howell d71111f3dc presence api: Use email to look up presence.
We don't want to use delivery_email to look up
presence on email-restricted realms.
2020-03-19 14:32:18 -07:00
Steve Howell 42ee2f5e86 tests: Fix test coverage on recent commit.
I guess `test_classes` has 100% line coverage
enforcement, which is a bit tricky for error
handling.

This fixes that, as well as making the name
snake_case and improving the format of the
errors.
2020-03-19 11:37:31 -04:00
Steve Howell 80acbb9fdf Clean up `test_get_all_profiles_avatar_urls`.
This test was using the anti-pattern of doing an
assertion inside a conditional.

I added the `findOne` helper to make it easier
to write robust tests for scenarios like this.
2020-03-19 10:34:35 -04:00
Mateusz Mandera f5e95c4fc1 requirements: Bump python-social-auth version.
We had a bunch of ugly hacks to monkey patch things due to upstream
being temporarily unmaintained and not merging PRs. Now the project is
active again and the fixes have been merged and included in the latest
version - so we clean up all that code.
2020-03-18 12:14:31 -07:00
Steve Howell ca74cd6e37 bug fix: Fix unread counts for certain API messages.
If I send a message from a normal Zulip client, it is
considered to be "read" by me.  But if I send it via
an API program (using my human account), the message
is not immediately "read" by me.

Now we handle this correctly in `get_raw_unread_data`.

The symptom of this was that these messages would get
"stuck" in "Private Messages" narrows until the next
time you reloaded your app.
2020-03-17 16:26:42 -07:00
Tim Abbott 1b95a1dea7 hello: Focus on distributed teams as use case.
I've always thought of distributed teams as the place where Zulip
really shines over other tools, because chat is much more important in
that context.

And I've always been kinda unhappy with "most productive team chat" as
a line.

There's a lot more we should do here, but this is a start.
2020-03-17 14:49:17 -07:00
Mateusz Mandera 5e47f2975e actions: Optimize query in get_occupied_streams.
Using an Exists subquery to avoid scanning the entire Subscription
table seems to speed things up greatly.
Set up with:
 ./manage.py populate_db --extra_users 2000 --extra-streams 1000

Tested on my computer, the original function was taking ~1.2seconds,
the optimized version only ~0.05-0.06.

Likely fixes #13874; we can re-open if after production testing we
feel more work is warranted.
2020-03-17 05:44:05 -07:00
Mateusz Mandera 884ff425da cache: Remove dead code for caching recipients.
With recipient column denormalized into all three of Stream, UserProfile
and Huddle, there is no more use for this caching.
2020-03-17 05:41:11 -07:00
Mateusz Mandera b4ce167a88 models: Add recipient foreign key to Huddle.
This follows the already tested approach from
8acfa17fe6.
2020-03-17 05:41:11 -07:00
Mateusz Mandera 08780fcb95 test_import_export: Fix how stream.recipient_id is verified. 2020-03-17 05:41:11 -07:00
Tim Abbott b064559652 zephyr: Add strict assertion about username format.
This ensures that even if it were possible to create an MIT Kerberos
account with a malicious username and/or hack webathena to pretend
that's the case, one couldn't do anything malicious.

This security improvement only impacts a single installation of Zulip
where Zephyr mirroring is in use that has already had the fix applied,
so there's no reason to do a security notice for it.

Found by Graham Bleaney using pysa.
2020-03-17 05:37:25 -07:00
Steve Howell ff4b5d8ce6 minor: Fix list/set test flake. 2020-03-15 09:11:14 -04:00
Steve Howell fcc5ae5247 invites: Fix regression w/email vs. delivery_email.
In 220c2a5ff3 I
introduced a query to find invites by delivery_email
but was still using email as the key.

For most realms `email` and `delivery_email` are
synonymous, so this temporary bug would not affect
them.  For realms that restrict emails, the invite
would have probably failed for other reasons, but
the symptom would have been less clear.
2020-03-12 10:13:08 -04:00
Steve Howell 1b16693526 tests: Limit email-based logins.
We now have this API...

If you really just need to log in
and not do anything with the actual
user:

    self.login('hamlet')

If you're gonna use the user in the
rest of the test:

    hamlet = self.example_user('hamlet')
    self.login_user(hamlet)

If you are specifically testing
email/password logins (used only in 4 places):

    self.login_by_email(email, password)

And for failures uses this (used twice):

    self.assert_login_failure(email)
2020-03-11 17:10:22 -07:00
Steve Howell c235333041 test performance: Pass in users to api_* helpers.
This reduces query counts in some cases, since
we no longer need to look up the user again. In
particular, it reduces some noise when we
count queries for O(N)-related tests.

The query count is usually reduced by 2 per
API call.  We no longer need to look up Realm
and UserProfile.  In most cases we are saving
these lookups for the whole tests, since we
usually already have the `user` objects for
other reasons.  In a few places we are simply
moving where that query happens within the
test.

In some places I shorten names like `test_user`
or `user_profile` to just be `user`.
2020-03-11 14:18:29 -07:00
Steve Howell 626ad0078d tests: Add uuid_get and uuid_post.
We want a clean codepath for the vast majority
of cases of using api_get/api_post, which now
uses email and which we'll soon convert to
accepting `user` as a parameter.

These apis that take two different types of
values for the same parameter make sweeps
like this kinda painful, and they're pretty
easy to avoid by extracting helpers to do
the actual common tasks.  So, for example,
here I still keep a common method to
actually encode the credentials (since
the whole encode/decode business is an
annoying detail that you don't want to fix
in two places):

    def encode_credentials(self, identifier: str, api_key: str) -> str:
        """
        identifier: Can be an email or a remote server uuid.
        """
        credentials = "%s:%s" % (identifier, api_key)
        return 'Basic ' + base64.b64encode(credentials.encode('utf-8')).decode('utf-8')

But then the rest of the code has two separate
codepaths.

And for the uuid functions, we no longer have
crufty references to realm.  (In fairness, realm
will also go away when we introduce users.)

For the `is_remote_server` helper, I just inlined
it, since it's now only needed in one place, and the
name didn't make total sense anyway, plus it wasn't
a super robust check.  In context, it's easier
just to use a comment now to say what we're doing:

    # If `role` doesn't look like an email, it might be a uuid.
    if settings.ZILENCER_ENABLED and role is not None and '@' not in role:
        # do stuff
2020-03-11 14:18:29 -07:00
Steve Howell 00dc976379 tests: Use users for common_subscribe_to_streams.
We also use users for get_streams().
2020-03-11 14:18:29 -07:00
Sourabh Singh 1b3cfecf2a
webhooks: Add team reviewers support in github webhook.
The github webhook implementation previously ignored the "team reviewers"
part of pull_request events, resulting in inaccurate output.

Fixes: #14096.
2020-03-10 16:29:59 -07:00
Mateusz Mandera 2000608a9e report_error: Fix inaccurate docstring.
do_report_error isn't actually below.
2020-03-09 13:54:58 -07:00
Mateusz Mandera 89394fc1eb middleware: Use request.user for logging when possible.
Instead of trying to set the _requestor_for_logs attribute in all the
relevant places, we try to use request.user when possible (that will be
when it's a UserProfile or RemoteZulipServer as of now). In other
places, we set _requestor_for_logs to avoid manually editing the
request.user attribute, as it should mostly be left for Django to manage
it.
In places where we remove the "request._requestor_for_logs = ..." line,
it is clearly implied by the previous code (or the current surrounding
code) that request.user is of the correct type.
2020-03-09 13:54:58 -07:00
Mateusz Mandera 0255ca9b6a middleware: Log user.id/realm.string_id instead of _email. 2020-03-09 13:54:58 -07:00
akashaviator 700123a30b api: Document DELETE ../messages/{message_id}/reactions endpoint.
This refactors remove_reaction in python_examples.py to validate the
result with validate_against_openapi_schema.  Minor changes and some
additions have been made to the OpenAPI format data for
/messages/{message_id}/reactions endpoint.
2020-03-08 19:12:45 -07:00
akashaviator 5dd1a1fc83 api: Document POST ../messages/{message_id}/reactions endpoint.
This refactors add_reaction in python_examples.py to use the
openapi_test_function decorator and validate result with
validate_against_openapi_schema. Minor changes have been made to the
OpenAPI format data for /messages/{message_id}/reactions endpoint.

This also adds add-emoji.md to templates/zerver/api and adds
add-emoji to rest-endpoints.md (templates/zerver/help/include).
2020-03-08 19:04:15 -07:00
akashaviator 9c63976da5 api: Refactor get_members_backend in zerver/views/users.py.
This refactors get_members_backend to return user data of a single
user in the form of a dictionary (earlier being a list with a single
dictionary).

This also refactors it to return the data with an appropriate key
(inside a dictionary), "user" or "members", according to the type of
data being returned.

Tweaked by tabbott to use somewhat less opaque code and simple OpenAPI
descriptions.
2020-03-08 18:43:30 -07:00
Tim Abbott 2c75b39078 templates: Delete show_debug feature.
As far as I know, this hasn't been used in at least 5 years, and I'm
not sure there's a real use case for it with the current app.
2020-03-08 18:34:59 -07:00
Tim Abbott ccf63ac66b decorators: Restructure get_client_name interface.
Previously, get_client_name was responsible for both parsing the
User-Agent data as well as handling the override behavior that we want
to use "website" rather than "Mozilla" as the key for the Client object.

Now, it's just responsible for User-Agent, and the override behavior
is entirely within process_client (the function concerned with Client
objects).

This has the side effect of changing what `Client` object we'll use
for HTTP requests to /json/ endpoints that set the `client` attribute.
I think that's in line with our intent -- we only have a use case for
API clients overriding the User-Agent parsing (that feature is a
workaround for situations where the third party may not control HTTP
headers but does control the HTTP request payload).

This loses test coverage on the `request.GET['client']` code path; I
disable that for now since we don't have a real use for that behavior.

(We may want to change that logic to have Client recognize individual
browsers; doing so requires first using a better User-Agent parsing
library).

Part of #14067.
2020-03-08 14:19:50 -07:00
Tim Abbott 53cc00c21c messages: Ban the sender property when not mirroring.
The "sender" property in `send_message_backend` is meant to only do
something when doing Zephyr mirroring (or similar).  We should help
clients behave correctly by banning this property in requests that are
not specifically requesting mirroring behavior.

This commit requires changes to a number of tests that incorrectly
passed this parameter or didn't use the right setup for mirroring.
2020-03-08 14:09:32 -07:00
Tim Abbott cf897cc4b6 test_messages: Convert Zephyr mirror tests to use API.
The special Zephyr mirroring logic is only intended to be used via the
API, so this sets up a more effective test.  It also allows us to
remove certain Client parsing logic for the /json/ views using session
authentication.
2020-03-08 13:38:20 -07:00
Mateusz Mandera fe0f381914 populate_db: Don't restrict email domains by default in tests and dev.
The email domain restriction to @zulip.com is annoying in development
environment when trying to test sign up. For consistency, it's best to
have tests use the same default, and the tests that require domain
restriction can be adjusted to set that configuration up for themselves
explicitly.
2020-03-07 18:38:59 -08:00
Tim Abbott 5835023021 tests: Use user IDs internally in send message helpers.
This uses the better, modern, user ID based API for sending messages
internally in the test suite, something that's convenient to do as a
follow-up to the migration to pass UserProfile objects to these
functions.
2020-03-07 18:31:13 -08:00
Steve Howell 5e2a32c936 tests: Use users in send_*_message.
This commit mostly makes our tests less
noisy, since emails are no longer an important
detail of sending messages (they're not even
really used in the API).

It also sets us up to have more scrutiny
on delivery_email/email in the future
for things that actually matter.  (This is
a prep commit for something along those
lines, kind of hard to explain the full
plan.)
2020-03-07 18:30:13 -08:00
Tim Abbott 35b444d59c api docs: Document historical changes to typing API.
Along with other recent changes, this fixes #13286.
2020-03-06 17:49:53 -08:00
Vishnu KS 1c6435d4cc validator: Optionally record a type_structure attribute.
We plan to use these records to check and record the schema of Zulip's
events for the purposes of API documentation.

Based on an original messier commit by tabbott.

In theory, a nicer version of this would be able to work directly off
the mypy type system, but this will be good enough for our use case.
2020-03-06 17:07:14 -08:00
Tim Abbott 9230213bde settings: Add EMAIL_ADDRESS_VISIBILITY_NOBODY.
This extends our email address visibility settings to deny access to
user email addresses even to organization administrators.

At the moment, they can of course change the setting (which leaves an
audit trail), but in the future only organization owners will be able
to change that setting.

While we're at this, we rewrite the settings_data.js test to cover all
the cases in a more consistent way.

Fixes #14111.
2020-03-06 16:34:08 -08:00
Tim Abbott 914cda9e2d test_classes: Fix api credentials with email_address_visibility setting.
This isn't the only bug in our testing libraries with
EMAIL_ADDRESS_VISIBILITY; but we don't have a lot of tests that need
to deal with that set of settings.
2020-03-06 16:33:16 -08:00
Steve Howell 1b4cac6734 models: Cache failures to find user in get_user_by_api_key.
We will cache failed lookups with None.  The
use case here is that broken API clients may
continually ask for the same wrong API key, and
we want to handle that as quickly as possible.
2020-03-06 12:02:02 -08:00
Steve Howell f2b8eef21a refactor: Avoid hacky use of ValidationError.code.
We were using `code` to pass around messages.

The `code` field is designed to be a code, not
a human-readable message.

It's possible that we don't actually need two
flavors of messages for these type of validations,
but I didn't want to change that yet.

We **definitely** don't need to put two types of
message in the exception, so I fix that.  Instead,
I just have the caller ask what level of detail
it needs.

I added a non-verbose message for the case of
system bots.

I removed the non-translated version of the message
for deactivated accounts, which didn't have test
coverage and is slightly more prone to leaking
email info that we don't want to leak.
2020-03-06 11:53:22 -08:00
Steve Howell 62fb3ad801 refactor: Move validate_email_not_already_in_realm.
We move this to email_validation.py.
2020-03-06 11:53:22 -08:00
Steve Howell 7e55cab429 invite performance: Reduce queries to find existing users.
In the prep commits leading up to this, we split
out two new helpers:

    validate_email_is_valid
    get_errors_for_new_emails

Now when we validate invites we use two separate
loops to filter our emails.

Note that the two extracted functions map to two
of the data structures that used to be handled
in a single loop, and now we break them out:

    errors = validate_email_is_valid
    skipped = get_errors_for_new_emails

The first loop checks that emails are even valid
to begin with.

The second loop finds out whether emails are already
in use.

The second loop takes advantage of this helper:

    get_errors_for_new_emails

The second helper can query all potential new emails
with a single round trip to the database.

This reduces our query count.
2020-03-06 11:53:22 -08:00
Steve Howell 220c2a5ff3 performance: Add get_users_by_delivery_email().
The main purpose of this new function is to allow
us to validate emails in bulk, which we don't do
yet (still setting the stage for that).

This is still a speedup, though, since in our
caller we grab only three fields now.

And other than that, we're essentially doing
the same query for the single-email case, just
outside the loop.
2020-03-06 11:53:22 -08:00
Steve Howell b35ffde5fb tests: Avoid calling actions.validate_email().
We are trying to kill off `validate_email`, so
we no longer call it from these tests.

These tests are already kind of low-level in
nature, so testing the more specific helpers
here should be fine.

Note that we also make the third parameter
to `validate_email` non-optional in this commit,
to preserve 100% coverage.  This is really just
refactoring noise--we will soon eliminate the
entire function, but I didn't want to do everything
in a huge commit.
2020-03-06 11:53:22 -08:00
Steve Howell 6f62c993a6 refactor: Extract get_existing_user_errors.
This is a prep commit that will allow us
to more efficiently validate a bunch of
emails in the invite UI.

This commit does not yet change any
behavior or performance.

A secondary goal of this commit is to
prepare us to eliminate some hackiness
related to how we construct
`ValidationError` exceptions.

It preserves some quirks of the prior
implementation:

   - the strings we decided to translate
     here appear haphazard (and often
     get ignored anyway)

   - we use `msg` in most codepaths,
     but use `code` for invites

Right now we never actually call this with
more than one email, but that will change
soon.

Note that part of the rationale for the inner
method here is to avoid a test coverage bug
with `continue` in loops.
2020-03-06 11:53:22 -08:00
Steve Howell ad85e286de user settings: Inline call to validate_email.
We are trying to elminate the version of
`validate_email` that lives in `actions.py`.

Inlining it barely increases the code size, and
it removes some noise related the three-item
tuple that `check_incoming_email` returns.
2020-03-06 11:53:22 -08:00
Steve Howell 689aca9140 refactor: Extract validate_email_is_valid().
This has two goals:

    - sets up a future commit to bulk-validate
      emails

    - the extracted function is more simple,
      since it just has errors, and no codes
      or deactivated flags

This commit leaves us in a somewhat funny
intermediate state where we have
`action.validate_email` being a glorified
two-line function with strange parameters,
but subsequent commits will clean this up:

    - we will eliminate validate_email
    - we will move most of the guts of its
      other callee to lib/email_validation.py

To be clear, the code is correct here, just
kinda in an ugly, temporarily-disorganized
intermediate state.
2020-03-06 11:53:22 -08:00
Steve Howell 4f5b07a7e6 refactor: Extract zerver/lib/email_validation.py. 2020-03-06 11:53:22 -08:00
Steve Howell 30b43605c3 invite performance: Reduce RealmDomain queries.
We now use the `get_realm_email_validator()`
helper to build an email validator outside
the loop of emails in our invite list.

This allows us to perform RealmDomain queries
only once per request, instead of once per
email.
2020-03-06 11:53:22 -08:00
Steve Howell ce8f6797c7 performance: Optimize get_realm_email_validator.
We now query RealmDomain objects up front.  This
change is minor in most circumstances--it sometimes
saves a round trip to the database; other times,
it actually brings back slightly more data
(optimistically).

The big win will come in a subsequent commit,
where we avoid running these queries in a loop
for every callback.

Note that I'm not sure if we intentionally
omitted checks for emails with "+" in them
for some circumstances, but I just preserved
the behavior.
2020-03-06 11:53:22 -08:00
Steve Howell ddbc536739 refactor: Extract get_realm_email_validator.
This change sets us up to use the same realm
data for multiple email validations.
2020-03-06 11:53:22 -08:00
Steve Howell 57f1aa722c refactor: Rename validate_email_for_realm.
Now called:

    validate_email_not_already_in_realm

We have a separate validation function that
makes sure that the email fits into a realm's
domain scheme, and we want to avoid naming
confusion here.
2020-03-06 11:53:22 -08:00
Steve Howell c43a29ff54 invites: Fix bug with inviting cross realm bots.
Without the fix here, you will get an exception
similar to below if you try to invite one of the
cross realm bots.  (The actual exception is
a bit different due to some rebasing on my branch.)

	  File "/home/zulipdev/zulip/zerver/lib/request.py", line 368, in _wrapped_view_func
		return view_func(request, *args, **kwargs)
	  File "/home/zulipdev/zulip/zerver/views/invite.py", line 49, in invite_users_backend
		do_invite_users(user_profile, invitee_emails, streams, invite_as)
	  File "/home/zulipdev/zulip/zerver/lib/actions.py", line 5153, in do_invite_users
		email_error, email_skipped, deactivated = validate_email(user_profile, email)
	  File "/home/zulipdev/zulip/zerver/lib/actions.py", line 5069, in validate_email
		return None, (error.code), (error.params['deactivated'])
	TypeError: 'NoneType' object is not subscriptable

Obviously, you shouldn't try to invite a cross
realm bot to your realm, but we want a reasonable
error message.

RESOLUTION:

Populate the `code` parameter for `ValidationError`.

BACKGROUND:

Most callers to `validate_email_for_realm` simply catch
the `ValidationError` and then report a more generic error.

That's also what `do_invite_users` does, but it has the
somewhat convoluted codepath through `validate_email`
that triggers this code:

    try:
        validate_email_for_realm(user_profile.realm, email)
    except ValidationError as error:
        return None, (error.code), (error.params['deactivated'])

The way that we're using the `code` parameter for
`ValidationError` feels hacky to me.  The intention
behind `code` is to provide a descriptive error to
calling code, and it's not intended for humans, and
it feels strange that we actually translate this in
other places.  Here are the Django docs:

    https://docs.djangoproject.com/en/3.0/ref/forms/validation/

And then here's an example of us actually translating
a code (not part of this commit, just providing context):

    raise ValidationError(_('%s already has an account') %
                          (email,), code = _("Already has an account."),
                          params={'deactivated': False})

Those codes eventually get put into InvitationError, which
inherits from JsonableError, and we do actually display
these errors in the webapp:

    if skipped and len(skipped) == len(invitee_emails):
        # All e-mails were skipped, so we didn't actually invite anyone.
        raise InvitationError(_("We weren't able to invite anyone."),
                              skipped, sent_invitations=False)

I will try to untangle this somewhat in upcoming commits.
2020-03-06 11:53:22 -08:00
Steve Howell 923e6dcd5d tests: Add test for mirror_dummy user invites.
We allow folks to invite emails that are
associated with a mirror_dummy account.

We had a similar test already for registration,
but not invites.

This logic typically affects MIT realms in the
real world, but the logic should apply to any
realm, so I use accounts from the zulip realm
for convenient testing.  (For example, we might
run an IRC mirror for a non-MIT account.)
2020-03-06 11:53:22 -08:00
Steve Howell 332f8e7dde tests: Add query count check for invites.
I use a range here because there's some leak
from another test that causes the count to
vary.  Once we get this a bit more under control,
we should be able to analyze the leak better.
2020-03-06 11:53:22 -08:00
Steve Howell 32e1c22c88 tests: Improve test_invite_existing_user.
The substantive improvement here is to use
a strange casing for Hamlet's email, which
will prevent future casing bugs.

I also log in as Cordelia to prevent confusion
that the test has something to do with
inviting yourself.  It's more typical for
somebody to invite another person to a realm
(not realizing they're already there).

I also made two readability tweaks.
2020-03-06 11:53:22 -08:00
Vishnu KS 6fec2e03e6 docs: Recommend user_id instead of email in send-message doc.
Also stop documenting the legacy method of sending emails
in CSV format.
2020-03-06 11:39:43 -08:00
Rohitt Vashishtha 2fab45e530 bugdown: Use AtomicString in UserMentionPattern.
This fixes the user-mention counterpart of #14080.
2020-03-06 11:35:56 -08:00
Rohitt Vashishtha 7f9d8e1907 bugdown: Use AtomicString in UserGroupMentionPattern.
This fixes the user-group counterpart of #14080.
2020-03-06 11:35:56 -08:00
orientor 7d2bb707e7 integrations: Improved Taiga Integration.
This adds links to the users and other small improvements.

Progress towards #13698.
2020-03-04 16:52:52 -08:00
Mateusz Mandera 2d544250b7 events: Add block for compatibility with old delete_message events. 2020-03-03 15:52:42 -08:00
Mateusz Mandera 3922fb3a92 events: Clean up delete_message even processing code. 2020-03-03 15:52:42 -08:00
Mateusz Mandera e506dbcdad auth: Monkey patch a fix for Github deprecation notice spam.
This is a way to monkey-patch a fix for
https://github.com/python-social-auth/social-core/issues/430
Changes from this commit should be reverted once the issue is fixed
upstream.
2020-03-03 15:51:40 -08:00
Tim Abbott 783a77c532 queue processors: Flush per-request caches after each item.
Several of our queues are capable of doing work that includes
rendering markdown (outgoing_webhook, embedded_bots, embed_links, and
email_mirror).  As a result, it's essential that these don't cache
per-request data (specifically, realm filters) longer than they
should, making editing/deleting linkifiers potentially use old
settings until the relevant process was restarted.

Flushing these caches is extremely cheap (just clearing two
dictionaries) and thus is reasonable to do after every queue event,
rather than trying to do it only the ~1/3 of queues that specifically
do markdown processing.  We do the same in our middleware for
reset_queries.

It's not worth writing a test for this because it's very difficult to
create the test setup situation for this bug with a single test worker
process; one needs to edit the linkifier configuration in a different
process than the one sending the message in order to see the bug.

This was a much larger visible bug on Zulip 2.1.x, where the presence
of the message_sender queue meant that this would apply to messages
sent via a browser.

Fixes #14095.
2020-03-03 15:29:11 -08:00
Rohitt Vashishtha ff5e2b6eb7 bugdown: Avoid hanging list paragraphs being processed as codeblocks.
Previously, the input:

====================
- One
  - Two

    Two continued
====================

Would produce the same output as:

====================
- One
  - Two

```
Two continued
```
====================

This was because our CodeBlockProcessor had a higher priority than
the ListIndentProcessor. This issue was discussed here:
https://chat.zulip.org/#narrow/stream/9-issues/topic/continuation.20paragraphs.20in.20list.20items.
2020-03-03 12:08:19 -08:00
Rohitt Vashishtha cd7396e732 bugdown: Update outdated comment about Zulip's heading support. 2020-03-03 11:54:18 -08:00
Steve Howell 862515b7a4 presence: Avoid failures with obsolete events.
We only recently added `user_id` to presence
events.
2020-03-03 11:45:45 -08:00
Rohitt Vashishtha 62a7e464fb bugdown: Use AtomicString in StreamPattern.
This fixes the stream counterpart of #14080.
2020-03-02 00:03:33 -08:00
Rohitt Vashishtha 245de9e1e2 bugdown: Use AtomicString in StreamTopicPattern.
Fixes #14080.
2020-03-02 00:03:33 -08:00
Mateusz Mandera 05e7214690 do_delete_messages: Handle empty set of messages passed as input.
/delete_topic endpoint could be used to request the deletion of a topic,
that would cause do_delete_messages to be called with an empty set in
these cases:
1. Requesting deletion of an empty stream.
2. Requesting deletion of a topic in a private stream with history not
   public to subscribers, if the requesting admin doesn't have access to
   any of the messages in that topic.
2020-03-02 00:01:35 -08:00
Steve Howell 94192395fb perf: Extract Stream.get_client_data.
This function slims down the data that we get
from the database in order to create the
streams part of our client payload.

We also fix a typo.

We also clearly distinguish between queries
and lists here.
2020-03-01 22:38:03 -08:00
Steve Howell 49b8218463 perf: Extract get_subscribed_stream_ids_for_user.
This new method prevents us from getting fat
objects from the database.

Instead, now we just get ids from the database
to build our subqueries.

Note that we could also technically eliminate
the `set(...)` wrappers in this code to have
Django make a subquery and save a round trip.
I am postponing that for another commit (since
it's still somewhat coupled to some other
complexity in `do_get_streams` that I am trying
to cut through, plus it's not the main point
of this commit.)

BEFORE:

    # old, still in use for other codepaths
    def get_stream_subscriptions_for_user(user_profile: UserProfile) -> QuerySet:
        # TODO: Change return type to QuerySet[Subscription]
        return Subscription.objects.filter(
            user_profile=user_profile,
            recipient__type=Recipient.STREAM,
        )

    user_subs = get_stream_subscriptions_for_user(user_profile).filter(
        active=True,
    ).select_related('recipient')
    recipient_check = Q(id__in=[sub.recipient.type_id for sub in user_subs])

AFTER:

    # newly added
    def get_subscribed_stream_ids_for_user(user_profile: UserProfile) -> QuerySet:
        return Subscription.objects.filter(
            user_profile_id=user_profile,
            recipient__type=Recipient.STREAM,
            active=True,
        ).values_list('recipient__type_id', flat=True)

    subscribed_stream_ids = get_subscribed_stream_ids_for_user(user_profile)
    recipient_check = Q(id__in=set(subscribed_stream_ids))
2020-03-01 22:38:03 -08:00
Steve Howell eb368c9c92 performance: Optimize max_message_id calculation.
We calculate `max_message_id` for the mobile client.

Our query now no longer joins to the Message table
and just grabs one value instead of fat objects.
2020-03-01 22:38:03 -08:00
Mateusz Mandera 6e4dcc714e auth: Fix bug in backend configuration checks in start_social_login.
We shouldn't check if *every* backend in the backends list is
configured, but only the one the user is attempting to log in with.
2020-03-01 22:30:38 -08:00
Steve Howell 4fba227898 backend tests: Test include_all_active for streams.
We were only checking error handling before, not
the happy path.  The structure of the code
made it so that we effectively tested most of the
logic for this use case (since all the other flags
are sort of just filters on top of this), but
obviously we want explicit coverage here.  Also,
we weren't testing the is-admin-but-not-api-super-user
error checking until this commit.
2020-03-01 07:49:38 -05:00
Mateusz Mandera 2d55c7e0d5 home: Don't assume user agent header is set for insecure_desktop_app.
The header may not be set - this leads to CI failures on 2.1.x branch,
but in any case is a real bug.
2020-02-28 16:51:23 -08:00
Chris Bobbe 23ba2b63c5 push_notifications: In dev, make APNs or GCM config suffice. 2020-02-28 16:49:35 -08:00
Steve Howell 504ec9d489 typing: Remove recipient-related complexity.
For historical reasons we were creating Recipient
objects at some point in the typing-notifications
codepath.  Now we just work with UserProfiles.
This removes some queries, as indicated by
the change to `len(queries)` in a couple of the
tests.

The one subtle thing that changes here is huddles.
If user 10 sends a typing notification that they
are talking to users 20 and 30, there might not
actually be a huddle for users 10/20/30, but
we were actually creating huddles on the fly!
There is no need to create huddles just for
typing notifications, since we don't even
share huddle ids with our clients.  The clients
just infer the huddles.

Some of the code that gets killed off here as
somewhat "collateral damage" is some
defensive code related to formerly supporting streams
in typing indicators.  The support for streams
was killed off almost as soon as we released
the feature, and the codepath is pretty clearly
user-centric at this point.
2020-02-28 12:46:20 -08:00
Steve Howell f224f215c1 refactor: Simplify handling of emails for typing endpoint.
Instead of duplicating code for the email case, just
convert emails to user_ids and then run the same code.
2020-02-28 12:39:36 -08:00
Steve Howell bed6d5a789 typing: Inline check_typing_notification.
I actually like this pattern:

    def check_send_typing_notification(...):
        typing_notification = check_typing_notification(...)
        do_send_typing_notification(...)

It can help divide responsibilities nicely and make it easy
to write detailed unit tests against each of the two helpers.

Unfortunately, the good things didn't really happen here, and
instead we got the worst aspects of the pattern:

    - The responsibilities for validation leaked into
      the second function.

    - Both functions were doing sane things individually
      that became not-so-sane in the big picture (namely,
      we ended up making Recipient objects for no reason,
      but if you read each of the helpers, it was just one
      step that seemed reasonable).

    - Passing around dictionaries for results can be annoying.

Also, the pattern made a lot more sense when the validation
for typing was a lot more complicated.  My prior commit makes
it so that we only ever deal with a list of user_ids.

Anyway, now I'm inlining it. :)

Subsequent commits will clean up the more substantive issue
here, which is that we are building Recipients for no reason.
2020-02-28 12:39:36 -08:00
Steve Howell b26f2dcd4b typing: Deprecate emails in typing endpoint.
The only clients that should use the typing
indicators endpoint are our internal clients,
and they should send a JSON-formatted list
of user_ids.

Unfortunately, we still have some older versions
of mobile that still send emails.

In this commit we fix non-user-facing things
like docs and tests to promote the user_ids
interface that has existed since about version
2.0 of the server.

One annoyance is that we documented the
typing endpoint with emails, instead of the
more modern user_ids, which may have delayed
mobile converting to user_ids (and which
certainly caused confusion).  It's trivial
to update the docs, but we need to short
circuit one assertion in the openapi tests.

We also clean up the test structure for the
typing tests:

    TypingHappyPathTest.test_start_to_another_user
    TypingHappyPathTest.test_start_to_multiple_recipients
    TypingHappyPathTest.test_start_to_self
    TypingHappyPathTest.test_start_to_single_recipient
    TypingHappyPathTest.test_stop_to_another_user
    TypingHappyPathTest.test_stop_to_self

    TypingValidateOperatorTest.test_invalid_parameter
    TypingValidateOperatorTest.test_missing_parameter

    TypingValidateUsersTest.test_argument_to_is_not_valid_json
    TypingValidateUsersTest.test_bogus_user_id
    TypingValidateUsersTest.test_empty_array
    TypingValidateUsersTest.test_missing_recipient

    TypingValidationHelpersTest.test_recipient_for_user_ids
    TypingValidationHelpersTest.test_recipient_for_user_ids_non_existent_id

    TypingLegacyMobileSupportTest.test_legacy_email_interface
2020-02-28 12:39:36 -08:00
Tim Abbott d79a7a8c35 panels: Show a banner for users with legacy desktop apps.
Users who are using ZulipDesktop or haven't managed to auto-update to
ZulipElectron should be strongly encouraged to upgrade.

We'll likely want to move to something even stricter that blocks
loading the app at all, but this is a good start.
2020-02-28 01:54:46 -08:00
Mateusz Mandera 7db3d4560f do_delete_messages: Archive the messages in bulk.
The test added in this commit shows 37 queries - compared to 181 without
the change to the function. That seems very much worth it.
2020-02-27 23:12:32 -08:00
Mateusz Mandera b4186fb680 do_delete_messages: Remove unused message_ids list. 2020-02-27 23:12:32 -08:00
Wyatt Hoodes 6ed944c761 test_runner: Update database ids to be human readable.
Before the Django 2.x upgrade, the DatabaseCreation
argument took an integer value.  To deal with running
mulitple test instances, we created a random start
range that could count up 100 workers until the next
random id.  Arbitrarily limiting the number of workers
to 100.

Post upgrade, we can now use string values. Enabling
the database + worker numbers to be more readable, as
well as removing the cap on the worker count.
2020-02-27 23:01:29 -08:00
Mateusz Mandera efb3065158 social_auth: Take user to find_account if invalid subdomain is given.
This allows to also clean up some code that's not really useful.
2020-02-27 17:27:55 -08:00
Tim Abbott 2fb967b735 do_update_message: Remove sender field from update_message events.
This field wasn't accessed by any clients and was a less robust
version of the user_id field.  Any client hoping to be interested in
who did message edits should be able to handle working with user IDs
rather than email addresses.
2020-02-26 16:16:01 -08:00
Tim Abbott 588bcb37cf do_update_message: Avoid using a direct query to fetch a Stream.
We have a helper designed for the purpose, and it fixes potentially
misbehavior where the previous code did not do `.select_related()`.
2020-02-26 16:14:34 -08:00
Tim Abbott 49ca7cf717 topic: Add recipient_id to fields for message edit saves.
This is preparation for supporting moving messages between streams in
some cases.

It doesn't actually have any functional effect, since flush_message
clears the message unconditionally anyway.
2020-02-26 16:12:07 -08:00
Steve Howell b75fb579e3 typing tests: Test unwanted Huddle side effect.
This test shows that we are, among other
things, creating Huddle records every
time somebody starts typing a message
to multiple people.
2020-02-25 16:17:47 -08:00
Steve Howell 600fcd6c52 typing tests: Add query_count checks.
We should not need so many queries here,
although a couple of the queries are just
standard things that apply to all requests.

I will reduce the number of queries in a
later commit.
2020-02-25 16:17:47 -08:00
Steve Howell 995353fb28 message validation: Clean up extract_private_recipients.
This is mostly refactoring, but we also prevent a new
type of value error (list of non-int-or-string).  The
new test code helps enforce that.

Cleanup includes:

    - Use early-exit for email case.
    - Rename helpers to get_validate_*.
    - Avoid clumsy rebuilding of lists in helpers.
    - Avoid the confusing `recipient` name (which
      can be confused with the model by the same
      name).
    - Just delegate duplicate-id/email-removal to
      the helpers.

The cleaner structure allows us to elminate a couple
mypy workarounds.
2020-02-25 16:17:47 -08:00
Vishnu KS 303cd9bb9e actions: Make do_change_plan_type support changing plan to SELF_HOSTED.
Credits to @xpac1985 for reporting, debugging and proposing fix to the
issue. The proposed fix was modified slightly by @hackerkid to set the
correct value for max_invites and upload_quota_gb. Tests added by
@hackerkid.

Fixes #13974
2020-02-25 16:14:45 -08:00
Anders Kaseorg 1cdab5ae61 emoji: Resolve emoji sprite sheets and stylesheets through Webpack.
This gives them cache-compatible URLs, and also avoids some extra
copies of the sprite sheet images.

Comments on the Octopus emoji added by tabbott.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-25 14:43:46 -08:00
Tim Abbott 27edc18330 test_classes: Use realistic web and mobile User-Agent strings.
This fixes a confusing aspect of how our automated tests worked
previously, where we'd almost all HTTP requests in the unlikely
configuration with no User-Agent string specified.

We need to adjust query counts in a few tests that now are a bit
cheaper because they now can take advantage of a Client object created
in server_initialization.py in `process_client`.
2020-02-24 23:19:43 -08:00
Tim Abbott 27b267026e test_classes: Rename set_http_host to set_http_headers.
This supports the goal of setting other headers like User-Agent in the
future.
2020-02-24 23:19:43 -08:00
Tim Abbott d80175d29e server_initialization: Create Client objects for mobile/desktop.
This replaces the "API" client, which isn't used by any real clients,
with the "ZulipMobile" and "ZulipElectron" client strings, which are.
2020-02-24 23:19:43 -08:00
Steve Howell 29dd7d2267 minor: Fix comment in send_message_backend.
The `send_message_backend` function no longer
calls `recipient_for_emails`.
2020-02-24 15:32:29 -08:00
Mateusz Mandera 2343f80d72 auth: Remove redundant get_subdomain(request) call in remote_user_sso.
subdomain = get_subdomain(request) is already called earlier in the
function.
2020-02-24 12:39:48 -08:00
Mateusz Mandera f2903e9c70 auth: Refactor - convert remote_username to email in remote_user_sso.
So far the conversion was in a very random place -
register_remote_user(). All other codepaths that use
login_or_register_remote_user() call it with the user's email address.
Making remote_user_sso convert remote_username to the email address
before calling login_or_register_remote_user makes this usage consistent
across the board.
2020-02-24 12:39:48 -08:00
Mateusz Mandera 98ae2fb940 auth: Remove redundant realm argument to finish_desktop_flow.
finish_desktop_flow is called with the assumption that the request
successfully proved control over the user_profile and generates a
special link to log into the user_profile account. There's no reason to
pass the realm param, as user_profile.realm can be assumed.
2020-02-24 12:39:48 -08:00
Dinesh 1308544a70 auth: Remove `if` blocks to redirect to config error page.
In `auth.py` there are three `if` blocks for different backends
to redirect to config error page with similar code. It is better
handled with common code using `get_attr()` function on
constructed setting names.
2020-02-24 12:19:45 -08:00
Dinesh 144304c798 auth: Move `ConfigErrorTest` from `test_docs` to `test_auth_backends`.
There was some duplicated code to test config error pages for
different auths which could be handled with less duplicated code
by adding those functions to `SocialAuthBase`.
Also moving the other tests makes it easier to access tests related
to a backend auth when they are in the same file.
2020-02-24 12:19:45 -08:00
harshavardhanpb cac4feb263 openapi: Move openapi.py into zerver/openapi.py.
Fixes #14006
2020-02-24 12:21:26 -05:00
Steve Howell ed859617e4 minor: Add test for extract_stream_indicator. 2020-02-24 07:40:31 -05:00
Mateusz Mandera ac041956d9 test_cache: Test caching of None values is handled correctly. 2020-02-21 09:05:46 -08:00
Mateusz Mandera a9794ec001 cache: Delete unused function cache(). 2020-02-21 09:05:46 -08:00
Mateusz Mandera bf0f1274fa saml: Make the bad idp param KeyError log message more verbose.
Original idea was that KeyError was only going to happen there in case
of user passing bad input params to the endpoint, so logging a generic
message seemed sufficient. But this can also happen in case of
misconfiguration, so it's worth logging more info as it may help in
debugging the configuration.
2020-02-20 14:49:27 -08:00
vsvipul 020a263a67 auth: Create a new page hop for desktop auth.
Create a new page for desktop auth flow, in which
users can select one from going to the app or
continue the flow in the browser.

Co-authored-by: Mateusz Mandera <mateusz.mandera@protonmail.com>
2020-02-20 11:59:55 -08:00
Mateusz Mandera c78d0712f7 tests: For ldap tests, give each ldap user a unique password.
To avoid some hidden bugs in tests caused by every ldap user having the
same password, we give each user a different password, generated based
on their uids (to avoid some ugly hard-coding in a bunch of places).
2020-02-19 14:46:29 -08:00
Vishnu KS 51f5701879 export: Canonicalize the email of cross realm bot to default value.
Fixes #13496
2020-02-19 14:44:50 -08:00
Vishnu KS 0d4bf86130 management: Make backup command work when DB is not in localhost.
This is useful preparatory work for supporting the backup management
command inside docker-zulip.
2020-02-19 12:41:05 -08:00
Vishnu KS e1a7716578 emails: Translate from_name of account security emails. 2020-02-18 17:45:33 -08:00
Tim Abbott 0075c6cd56 do_update_message: Clean up timestamp code.
By moving this logic to the topic of the functon, we make the code a
lot more readable.
2020-02-18 16:38:34 -08:00
Ryan Rehman 1bda3babbe tests: Minor refactoring of test_messages.
This avoids using `.save()` directly for editing stream properties,
and also uses the API in _send_and_verify_message to avoid confusing
logic around which user is doing what request.

Fixes part of #13823
2020-02-18 11:07:41 -08:00
Mateusz Mandera 6a0b68bc7f models: Delete get_stream_recipient function and its uses.
With recipient being now a Stream field, there's no more use for
this helper function.
2020-02-18 10:49:14 -08:00
Mateusz Mandera 0d6f78b381 models: Delete get_personal_recipient function and its uses.
With recipient being now a UserProfile field, there's no more use for
this helper function.
2020-02-18 10:49:14 -08:00
Mateusz Mandera 920d22524b import: Use re_map_foreign_keys on the realm column of UserPresence.
We forgot to make this adjustment in the recent denormalization of realm
into UserPresence. It's needed for imports to work correctly.
2020-02-18 10:45:38 -08:00
akashaviator 6a36edef9e api: Document PATCH ../users/{user_id} endpoint.
This adds update_user to python_examples.py in zerver/openapi.
This also adds update-user.md to templates/zerver/api and adds
update-user to rest-endpoints.md (templates/zerver/help/include).
2020-02-15 23:08:13 -08:00
akashaviator dc6a5e3ca2 api: Document DELETE ../users/{user_id} endpoint.
This adds deactivate_user to python_examples.py in zerver/openapi.
This also adds delete-user.md to templates/zerver/api and adds
delete-user to rest-endpoints.md (templates/zerver/help/include).
2020-02-15 23:08:13 -08:00
akashaviator 08efb00321 api: Document GET ../users/{user_id} endpoint.
This adds get_single_user to python_examples.py in zerver/openapi.
This also adds get-single-user.md to templates/zerver/api and adds
get-single-user to rest-endpoints.md (templates/zerver/help/include).
2020-02-15 23:08:13 -08:00
akashaviator 7bc470c699 openapi: Add OpenAPI format data for /users/{user_id} endpoint.
This adds the OpenAPI format data for /users/{user_id} endpoint
and also removes 'users/{user_id}' from 'pending_endpoints' in
zerver/tests/test_openapi.py .
2020-02-15 23:08:10 -08:00
Anders Kaseorg b2ec8e157b has_request_variables: Remove query_params dict.
‘req_var in request.GET’ was previously believed to be slow from
profiling results.  However, the real explanation for those profiling
results is that WSGIRequest.GET is a lazy cached property, so there’s
no reason to avoid it if we’re accessing request.GET anyway.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-02-15 11:37:18 -08:00
Chris Heald 18e3982acd integrations: Add AlertManager webhook. 2020-02-14 17:43:15 -08:00
Mateusz Mandera cbdfef28a8 retention: Update to account for the zulipinternal realm.
In https://github.com/zulip/zulip/pull/12823 some changes to the realms
structure have been made, so now both in production and development
cross-realm bots live in the realm with string_id "zulipinternal".
There was a TODO in retention code to eliminate a conditional in a query
that became redundant with this change, and also the zulipinternal realm
should be omitted from the archiving process in archive_messages().
2020-02-14 17:15:26 -08:00
Tim Abbott 229090a3a5 middleware: Avoid running APPEND_SLASH logic in Tornado.
Profiling suggests this saves about 600us in the runtime of every GET
/events request attempting to resolve URLs to determine whether we
need to do the APPEND_SLASH behavior.

It's possible that we end up doing the same URL resolution work later
and we're just moving around some runtime, but I think even if we do,
Django probably doesn't do any fancy caching that would mean doing
this query twice doesn't just do twice the work.

In any case, we probably want to extend this behavior to our whole API
because the APPEND_SLASH redirect behavior is essentially a bug there.
That is a more involved refactor, however.
2020-02-14 16:15:57 -08:00
Tim Abbott 10e7e15088 user_agent: Compile the regular expression.
We use this single regular expression for processing essentially every
request, so it's definitely worth hinting to Python that we're going
to do so by compiling it.  Saves about 40us per request.
2020-02-14 10:26:37 -08:00