Commit Graph

4989 Commits

Author SHA1 Message Date
arpit551 d60efa1478 thumbor: Fix __file__ typo.
Replaced '__file__' typo with __file__ which used to add
wrong path to sys.path.
2020-04-12 11:23:03 -07:00
Mateusz Mandera 770086f983 url_preview: Discard url in oembed if server returns invalid json.
This fixes the scenario where we'd get errors in the
FetchLinksEmbedData queue processor if oembed got invalid json from the
URL.
2020-04-11 11:54:54 -07:00
Anders Kaseorg c734bbd95d python: Modernize legacy Python 2 syntax with pyupgrade.
Generated by `pyupgrade --py3-plus --keep-percent-format` on all our
Python code except `zthumbor` and `zulip-ec2-configure-interfaces`,
followed by manual indentation fixes.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-04-09 16:43:22 -07:00
Anders Kaseorg fff2d3958a timeout: Use Python 3 raise syntax.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-04-09 16:42:13 -07:00
Rohitt Vashishtha f9caf522f0 markdown: Allow setting a default language for code blocks.
This adds a new realm setting: default_code_block_language.

This PR also adds a new widget to specify a language, which
behaves somewhat differently from other widgets of the same
kind; instead of exposing methods to the whole module, we
just create a single IIFE that handles all the interactions
with the DOM for the widget.

We also move the code for remapping languages to format_code
function since we want to preserve the original language to
decide if we override it using default_code_clock_language.

Fixes #14404.
2020-04-09 16:02:02 -07:00
Rohitt Vashishtha 3f6541b306 bugdown: Add 'none' as alias for no syntax highlighting in codeblocks.
This is a precursor to #14404.
2020-04-09 16:02:02 -07:00
Abhinav 41fc7b2ae1 webhooks/semaphore: Add support for Semaphore 2.0 notifications.
Semaphore has currently has two different versions of their product -
Classic and 2.0. This commit adds support for Semaphore 2.0, along side
Semaphore Classic, using the same webhook. This would let the integration
work seamlessly for users who have already configured a Zulip integration in
their Semaphore 2.0 projects.

Semaphore 2.0 currently only supports GitHub and their payloads do not
contain URLs for common entities like commits, pull requests and tags. We
construct URLs for them using templates, but also try to support other
services by providing notifications without URLs.

Closes #14171

Co-authored-by: Puneeth Chaganti <punchagan@muse-amuse.in>
2020-04-09 12:41:40 -07:00
Mateusz Mandera 218be002f1 rate_limiter: Add more detailed automated tests.
Extracted by tabbott from the original commit to support testing
without the Tornado version merged yet.
2020-04-08 10:40:26 -07:00
Mateusz Mandera 46a02e70b0 rate_limiter: Fix inconsistency in an edge case in redis limiter.
If we had a rule like "max 3 requests in 2 seconds", there was an
inconsistency between is_ratelimited() and get_api_calls_left().
If you had:
request #1 at time 0
request #2 and #3 at some times < 2

Next request, if exactly at time 2, would not get ratelimited, but if
get_api_calls_left was called, it would return 0. This was due to
inconsistency on the boundary - the check in is_ratelimited was
exclusive, while get_api_calls_left uses zcount, which is inclusive.
2020-04-08 10:29:18 -07:00
Mateusz Mandera 4b567d8edd rate_limiter: Fix secs_to_freedom being set to a timestamp.
time_reset returned from api_calls_left() was a timestamp, but
mistakenly treated as delta seconds. We change the return value of
api_calls_left() to be delta seconds, to be consistent with the return
value of rate_limit().
2020-04-08 10:29:18 -07:00
Mateusz Mandera fc2b6c9c06 rate_limiter: Remove incorrect comment in RedisRateLimiterBackend. 2020-04-08 10:29:18 -07:00
Mateusz Mandera 0155193140 rate_limiter: Change type of the RateLimitResult.remaining to int.
This is cleaner than it being Optional[int], as the value of None for
this object has been synonymous to 0.
2020-04-08 10:29:18 -07:00
Mateusz Mandera e86cfbdbd7 rate_limiter: Store data in request._ratelimits_applied list.
The information used to be stored in a request._ratelimit dict, but
there's no need for that, and a list is a simpler structure, so this
allows us to simplify the plumbing somewhat.
2020-04-08 10:29:18 -07:00
Mateusz Mandera 9911c6a0f0 rate_limiter: Put secs_to_freedom as message when raising RateLimited.
That's the value that matters to the code that catches the exception,
and this change allows simplifying the plumbing somewhat, and gets rid
of the get_rate_limit_result_from_request function.
2020-04-08 10:29:18 -07:00
Wyatt Hoodes 13f86f35d9 zcommand: Add `/fluid-width` and `/fixed-width` slash commands. 2020-04-07 20:54:34 -07:00
Wyatt Hoodes 4d6755a807 zcommand.py: Clean up backend logic.
This commit contains a few clean ups:

* In order to scale better for adding multiple commands,
the message formatting and setting switch logic was
extracted to its own function.

* The command lists were removed, as the frontend parses
the slash command from the compose box, and only sends
a single command to the backend for any given command
alias typed.

* The `switch_command` logic was removed because, given
the aforementioned fact, the index of the command will
always be the same. Thus the switch command will always
be the same.

* Switched to using early returns as opposed to nested
conditionals.  Along with removing single use variable
declarations.
2020-04-07 20:54:34 -07:00
Tim Abbott 843345dfee message_edit: Add backend for moving a topic to another stream.
This commit reuses the existing infrastructure for moving a topic
within a stream to add support for moving topics from one stream to
another.

Split from the original full-feature commit so that we can merge just
the backend, which is finished, at this time.

This is a large part of #6427.

The feature is incomplete, in that we don't have real-time update of
the frontend to handle the event, documentation, etc., but this commit
is a good mergable checkpoint that we can do further work on top of.
We also still ideally would have a test_events test for the backend,
but I'm willing to leave that for follow-up work.

This appears to have switched to tabbott as the author during commit
squashing sometime ago, but this commit is certainly:

Co-Authored-By: Wbert Adrián Castro Vera <wbertc@gmail.com>
2020-04-07 14:19:19 -07:00
Tim Abbott 5aa6aeb303 typing: Stop using Collection[str].
Apparently, Collection is unavailable in Python 3.5, so we can't use
it.  Reverting to a Union for now.
2020-04-03 15:53:37 -07:00
Tim Abbott a745e533fe settings: Use cleaner validators for display settings.
This simplifies the update_display_settings endpoint to use REQ for
validation, rather than custom if/else statements.

The test changes just take advantage of the now more consistent
syntax.
2020-04-03 15:09:14 -07:00
Steve Howell 1ae07b93d8 presence: Simplify payload for webapp.
This changes the payload that is used
to populate `page_params` for the webapp,
as well as responses to the once-every-50-seconds
presence pings.

Now our dictionary of users only has these
two fields in the value:

    - activity_timestamp
    - idle_timestamp

Example data:

    {
        6: Object { idle_timestamp: 1585746028 },
        7: Object { active_timestamp: 1585745774 },
        8: Object { active_timestamp: 1585745578,
                    idle_timestamp: 1585745400}
    }

We only send the slimmer type of payload
to clients that have set `slim_presence`
to True.

Note that this commit does not change the format
of the event data, which still looks like this:

    {
        website: {
            client: 'website',
            pushable: false,
            status: 'active',
            timestamp: 1585745225
        }
    }
2020-04-03 11:44:56 -07:00
shubhamgupta2956 793c3f25e7 api_docs: Migrate POST /zulip-outgoing-webhook.
This commit migrates zulip outging webhook payload to
/zulip-outgoing-webhook:post in OpenAPI.

Since this migrates the last payloads from api/fixtures.json to
OpenAPI, this commit removes api/fixtures.json file and the functions
accessing the file.

Tweaked by tabbott to further remove an unnecessary conditional.
2020-04-02 14:55:32 -07:00
Tim Abbott 5eb5b6a5ad import: Make sure the internal realm is created before import.
This is critical for importing the very first realm into an empty
server, since in 27b15a9722, we changed
the model to create the internal realm when the first real realm would
be created, but neglected the data import code path.
2020-04-02 14:34:32 -07:00
Mateusz Mandera 5252b081bd queue_processors: Gather statistics on queue worker operations. 2020-04-01 16:44:06 -07:00
Steve Howell f6503a4061 validation: Use JsonableError for extractors.
The distinction between ValueError and TypeError
is not useful in these functions:

    - extract_stream_indicator
    - extract_private_recipients (or its callees)

These are always invoked in views to validate
user input.

When we use REQ to wrap the validators, any
Exception gets turned into a JsonableError, so
the distinction is not important.

And if we don't use REQ to wrap the validators,
the errors aren't caught.

Now we just let these functions directly produce
the desired end result for both codepaths.

Also, we now flag the error strings for translation.
2020-04-01 15:01:19 -07:00
Anders Kaseorg 2d45308546 CVE-2020-10935: Fix XSS vulnerability in local link rewriting.
Make sure rewrite_local_links_to_relative does not accidentally change
the meaning of links.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-04-01 14:01:45 -07:00
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
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 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
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
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
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 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
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
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
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 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
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 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 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
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 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 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 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
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
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
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
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
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
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 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 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 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
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
Mateusz Mandera 3922fb3a92 events: Clean up delete_message even processing code. 2020-03-03 15:52:42 -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
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
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