Commit Graph

34125 Commits

Author SHA1 Message Date
Anders Kaseorg 319e2231b8 thumbnail: Tighten fix for CVE-2019-19775 open redirect.
Due to a known but unfixed bug in the Python standard library’s
urllib.parse module (CVE-2015-2104), a crafted URL could bypass the
validation in the previous patch and still achieve an open redirect.

https://bugs.python.org/issue23505

Switch to using django.utils.http.is_safe_url, which already contains
a workaround for this bug.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-01-16 12:36:24 -08:00
Tim Abbott ef1f6b1c33 filter: Allow marking is:mentioned messages as read.
We may revisit this in the future, but similar to is:private, the
current Zulip user experience makes users expect that in the
is:mentioned view, they should really be able to mark messages as
read.

Further, the practice use case for not marking them as read is very
low, since it's rare for someone to have so many mentions that
revisiting the mentions view isn't sufficient to see everything that
needs their attention.
2020-01-16 11:10:36 -08:00
Tim Abbott 409e320d9d filter: Add streams:public to sorted_term_types.
This is for consistency with in:, has:, and similar values where
there's a fixed set of RHS entries.
2020-01-16 11:05:07 -08:00
Rohitt Vashishtha d4ef50634c filter: Remove is_exactly().
Previously, is_exactly() had already been repalced with can_bucket_by().
This commit removes is_exactly() and replaces its usage in our tests
with can_bucket_by().
2020-01-16 10:54:59 -08:00
Rohitt Vashishtha 8a1299d076 filter: Add 'in:*' to sorted_term_types.
This simplifies our handling of in-home and in-all cases in
can_mark_messages_read().
2020-01-16 10:54:59 -08:00
Steve Howell 316eda071d refactor: Clean up can_mark_messages_read.
We now explicitly enumerate various cases, which
should make it easier to change this code.
2020-01-16 10:54:59 -08:00
Rohitt Vashishtha bb579f8823 unreads: Remove is_reading_mode().
This was a part of an experiment we ran on chat.zulip.org in Jul 2018
and surrounding code that used it never got merged to master.

See: https://chat.zulip.org/#narrow/stream/2-general/topic/un-narrow.20view/near/609506
and c407ba5175.
2020-01-16 10:54:59 -08:00
Steve Howell 741da61407 node: Flatten pm_list testing code a bit.
The `messages` part of the data is going away in
the next commits.  This pre-factoring just makes
the next diff a bit more clean.
2020-01-16 10:32:36 -08:00
Anders Kaseorg 8c733a3f68 create-db.sql: Start by dropping the zulip database if needed.
At some point the PostgreSQL Docker image started creating the zulip
database for us, which caused our CREATE DATABASE to fail.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-01-15 18:04:34 -08:00
Anders Kaseorg 298d45b46a create-db.sql: Handle exception if zulip user already exists.
Fixes #13530.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-01-15 18:04:34 -08:00
Steve Howell 3cc6ff5480 templates: Cache translations.
For Manage Streams, when we render the subscriptions
template, a significant amount of time is taken
by the "t" helper.

Obviously for the first call, we expect "t" to be
somewhat expensive, but subsuquent calls should be
fast, but i18next seems to have some overhead.
Also, we can save a tiny bit of overhead (marking it
as a safe string) that comes from our helper.

As an aside, are we sure it's ok to mark translations
as safe strings?

To test before and after, use blueslip.timings before
and after this commit.  When I tested with about 300
streams, the difference is pretty striking:

    without cache: 100ms
    with cache: 20ms

This is particularly interesting, since the subscriptions
templates have long strings for things like the SVG-based
checkmarks, but they're not really the bottleneck.

Unfortunately, this doesn't seem to be a huge win
elsewhere.  In some places we don't call "t", but of
course those might change in the future and benefit from
the cache.  And in other places we have smart widgets
that avoid rendering all N objects at one (e.g. buddy
list and list_render).

So this might be too big a hammer to speed up one
screen (albeit a really slow one).  It's possible
that we should simply move the i18n.t step **outside**
of certain templates to avoid doing them in a loop.
2020-01-15 18:01:59 -08:00
Tim Abbott 5b30138a31 casper tests: Add an additional waitWhileVisible check.
This fix at least one race and help prevent more race issues in the
casper tests for the organization administration pages.
2020-01-15 17:51:46 -08:00
Anders Kaseorg a82032a182 generate_secrets: Enable Redis authentication in production.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-01-15 17:35:15 -08:00
Anders Kaseorg 3360df7ad1 generate_secrets: Enable memcached authentication in production.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-01-15 17:35:15 -08:00
Anders Kaseorg cdda983e90 settings: Support optional memcached authentication.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-01-15 17:35:15 -08:00
Tim Abbott d816a12db9 casper tests: Fix buggy submit_notifications_stream_settings.
This new function Casper testing function improperly used
`casper.then` in a nested fashion rather than in series, which doesn't
work how one expects.  This likely caused the test flakes we've
started seeing with this code path since adding
submit_notifications_stream_settings (though it's hard to prove).
2020-01-15 14:11:29 -08:00
Tim Abbott 3a9568b4d4 migrations: Fix zulipinternal migration corner case.
It's theoretically possible to have configured a Zulip server where
the system bots live in the same realm as normal users (and may have
in fact been the default in early Zulip releases?  Unclear.).  We
should handle these without the migration intended to clean up naming
for the system bot realm crashing.

Fixes #13660.
2020-01-15 13:59:31 -08:00
Steve Howell aea369f878 Refine user-related typeahead results for large realms.
We now incorporate people.get_message_people() in our
logic for compose/PM typeaheads.  This not only gives
users better results in some cases, but it will also
improve performance for large realms in some cases.
2020-01-15 12:22:23 -08:00
Steve Howell 8f35700da8 refactor: Extract get_message_people.
We'll use this in two places coming up, so it's
worth extracting, plus I wanted to add the
fairly lengthy comment here.  (Tim, feel free
to edit down the comment as you see fit).
2020-01-15 12:22:23 -08:00
Steve Howell c47fc36201 refactor: Extract filter_persons.
This extraction will make sense in the next commit.
2020-01-15 12:22:23 -08:00
Steve Howell 9ed5545abb Add test coverage for filter_and_sort_mentions.
A recent commit removed test coverage for the
actual filtering/sorting of mention typeaheads
when you did a non-silent method.  This commit
now tests that important step again.

Note that we also had (and still have) tests
that make sure the is_silent flag is set
correctly by get_candidates.

We don't have a true full-stack test, but those
can be quite tricky to set up and maintain.
2020-01-15 12:22:23 -08:00
Steve Howell e1213ca30a minor: Add person/group to composebox_typeahead tests. 2020-01-15 12:22:23 -08:00
Steve Howell 0aa9decd86 blueslip: Add feature to time common operations.
This is relatively unobtrusive, and we don't send
anything to the server.

But any user can now enter blueslip.timings in the
console to see a map of how long things take in
milliseconds.  We only record one timing per
event label (i.e. the most recent).

It's pretty easy to test this by just clicking
around.  For 300 users/streams most things are
fast except for:

    - initialize_everything
    - manage streams (render_subscriptions)

Both do lots of nontrivial work, although
"manage streams" is a bit surprising, since
we're only measuring how long to build the
HTML from the templates (whereas the real
time is probably browser rendering costs).
2020-01-15 12:01:16 -08:00
Steve Howell b8f9f6018a page_params: Record page_params_parse_time.
We put page_params_parse_time on the window object
to help diagnose customer issues.
2020-01-15 12:01:14 -08:00
Steve Howell 890a4b1247 refactor: Add filterer for user settings.
This change sets us up to optimize how we
filter users in the admin user settings.
See #13554 for more context on the user
facing issues.

This fix is basically three related things:

    - Add filterer options to list_render.
    - Add helper method to people.js.
    - Use filterer in settings_users.js.
2020-01-14 22:43:08 -08:00
Steve Howell 110c15737f Rename filter.callback to filter.predicate.
The filter "callback" was only a "callback" in the
most general sense of the word.

It's just a filter predicate that returns a bool.

This is to prepare for another filtering option,
where the caller can filter the whole list
themselves.  I haven't figured out what I will name
the new option yet, but I know I want to make the
two options have specific names.
2020-01-14 22:43:08 -08:00
Steve Howell 3f3b9c3b70 list_render: Make callbacks required.
We are already providing callbacks everywhere, so
it would be nice to eliminate some dead code.

This also speeds things up ever so slightly (no
longer type-checking the option every time through
the loop).

We also split out exports.filter to make unit testing
easier.  The function seems kinda silly now, being so
small, but I hope to add another filtering option soon.
2020-01-14 22:43:08 -08:00
Steve Howell 90ed18d01a minor: Add comment explaining list_render.get call.
It's a bit confusing when you read this code to know
where the original list was created.  I'm not a huge
fan of the cache scheme here, but it does seem to
work for live updates.
2020-01-14 22:43:08 -08:00
Anders Kaseorg ea6934c26d dependencies: Remove WebSockets system for sending messages.
Zulip has had a small use of WebSockets (specifically, for the code
path of sending messages, via the webapp only) since ~2013.  We
originally added this use of WebSockets in the hope that the latency
benefits of doing so would allow us to avoid implementing a markdown
local echo; they were not.  Further, HTTP/2 may have eliminated the
latency difference we hoped to exploit by using WebSockets in any
case.

While we’d originally imagined using WebSockets for other endpoints,
there was never a good justification for moving more components to the
WebSockets system.

This WebSockets code path had a lot of downsides/complexity,
including:

* The messy hack involving constructing an emulated request object to
  hook into doing Django requests.
* The `message_senders` queue processor system, which increases RAM
  needs and must be provisioned independently from the rest of the
  server).
* A duplicate check_send_receive_time Nagios test specific to
  WebSockets.
* The requirement for users to have their firewalls/NATs allow
  WebSocket connections, and a setting to disable them for networks
  where WebSockets don’t work.
* Dependencies on the SockJS family of libraries, which has at times
  been poorly maintained, and periodically throws random JavaScript
  exceptions in our production environments without a deep enough
  traceback to effectively investigate.
* A total of about 1600 lines of our code related to the feature.
* Increased load on the Tornado system, especially around a Zulip
  server restart, and especially for large installations like
  zulipchat.com, resulting in extra delay before messages can be sent
  again.

As detailed in
https://github.com/zulip/zulip/pull/12862#issuecomment-536152397, it
appears that removing WebSockets moderately increases the time it
takes for the `send_message` API query to return from the server, but
does not significantly change the time between when a message is sent
and when it is received by clients.  We don’t understand the reason
for that change (suggesting the possibility of a measurement error),
and even if it is a real change, we consider that potential small
latency regression to be acceptable.

If we later want WebSockets, we’ll likely want to just use Django
Channels.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-01-14 22:34:00 -08:00
Pragati Agrawal 6fc2a317e9 org settings: Use save/discard widget for notification stream settings.
Currently, if we change stream we see the immediate saving of stream, but
it is more convenient to have "Save" and "Discard" buttons as we use
everywhere else in the organization setting subsystem.
2020-01-14 17:16:23 -08:00
Pragati Agrawal ec8fdc5c3d org settings: Extract core logic to check changes and change widget status.
This is a preliminary commit for further commits where we will be using the
newly created function `save_discard_widget_status_handler` in click
handler for changing the notification stream.
2020-01-14 17:16:23 -08:00
Pragati Agrawal 3e62e59bfe org settings: Minor refactor to move property-specific statements above.
This refactors `discard_property_element_changes` and
`check_property_changed` function to move conditional statements of
properties that need to be handled separately. It's a preliminary commit in
the series of using save/discard widget for notification stream setting.
2020-01-14 17:16:23 -08:00
Pragati Agrawal 48b6734b73 org settings: Minor refactor to add notification stream id data in widget.
As the part of making notification stream settings to change using
"save/discard" widget instead of immediate saving, we need to access the
stream id which is being selected at the moment.
2020-01-14 17:16:23 -08:00
Pragati Agrawal a1426d78b1 org settings: De-duplicate the JS code for notifications stream handlers.
(This is another preliminary commit in the direction of having
"save/discard" widget show up rather than saving immediately.)

The code for selecting and processing the stream for both types of
notifications is almost the same, so de-duplicated.
2020-01-14 17:16:23 -08:00
Pragati Agrawal 9a6b3c1cde org settings: De-duplicate template code for notification streams settings. 2020-01-14 17:16:23 -08:00
Pragati Agrawal bde8838d7e org settings: Use id instead of class for a specific element.
This is a preliminary commit to do some deduplication with notification
stream dropdown widget.
2020-01-14 17:16:23 -08:00
Pragati Agrawal 8512106c64 org settings: Fix selecting in streams dropdown using the keyboard.
For "New stream notifications" and "New user notifications", if we select
using "enter", we always get stream selected of later one's dropdown.
2020-01-14 17:16:23 -08:00
Pragati Agrawal e7c40f69de org settings: Remove old method of success status for notification streams.
For "New stream notifications" and "New user notifications" it is more
intuitive to just use the new system for showing success/saving status
feedback.
2020-01-14 17:16:23 -08:00
Steve Howell 752d6dc6df tools: Remove find-add-class tool.
I added this tool a few years ago, and I did have
a vision for how it would improve our codebase, but
I can't remember exactly where I was going with it.

At this point the tool is just a little too noisy
to be helpful.  An example of it creating confusion
was a recent PR where somebody was patching
user_circle_class in the PM list, and we already
had similar code in the buddy list, because they
use the same CSS.  I mean, there was possibly a way
that the code could have been structured to remove
some of the duplication, but it probably would have
just moved the complexity around.

I just don't think it's worth maintaining the tool
at this point.
2020-01-14 15:45:49 -08:00
Steve Howell 29e63c0417 Fix type errors in LazySet.
I think the only place that was broken is where
we copy users from streams.
2020-01-14 15:40:40 -08:00
Steve Howell b65138c83f minor: Make type conversion explicit. 2020-01-14 15:40:40 -08:00
Mateusz Mandera 0beae44081 email_mirror: Use .walk() to search all MIME parts for attachments.
Fixes #13416

We used to search only one level in depth through the MIME structure,
and thus would miss attachments that were nested deeper (which can
happen with some email clients). We can take advantage of message.walk()
to iterate through each MIME part.
2020-01-14 15:37:39 -08:00
Mateusz Mandera c579b6858e send_to_email_mirror: Fix loop setting recipient-like headers.
return in that loop was a bug, which would lead to the To: header not
being set even though data['recipient'] = str(message['To']) is being
run next, thus requiring the header. We can remove the return
statement and now the loop will overwrite all the potentially
troublesome headers.
2020-01-14 15:37:39 -08:00
Mateusz Mandera 1561d144e0 email_mirror: Insert a new line before attachment links. 2020-01-14 15:37:39 -08:00
Tim Abbott 8226573af6 default stream groups: Fix broken registration UI.
The default stream groups feature (#6693) was never fully implemented;
this fixes a key detail (the registration UI being broken).
2020-01-14 14:50:18 -08:00
Tim Abbott 4562949f43 default stream groups: Fix buggy LDAP behavior.
With LDAP authentication, we don't currently have a good way to
support the default stream groups feature.

The old behavior was just to assume a user select every default stream
group, which seems wrong; since we didn't prompt the user about these,
we should just ignore the feature.
2020-01-14 14:50:18 -08:00
Pragati Agrawal 0eafa48ca1 org settings: Fix error of wrong type of argument passed to InDict.has().
This fixes the error where we pass `user_id` of 'string' type as the
argument instead of 'integer' to `exports.get_person_from_user_id` which
further passes `user_id` to InDict.has() function which accepts integer
argument only.
2020-01-14 14:38:26 -08:00
Tim Abbott 80b9acd745 compose: Update some comments on private stream warnings. 2020-01-14 13:23:27 -08:00
Steve Howell c2af2c1fd1 refactor: Extract is_subscriber_subset().
Extracting the function makes it a bit easier to
test and use in a generic way.

Also, I wanted this to live in stream_data, so that
it's easier to find if we change how we model
subscriber data.

Finally, I use _.every to do the subset check
instead of `_.difference`, since _.difference
is actually N-squared:

  _.difference = restArguments(function(array, rest) {
    rest = flatten(rest, true, true);
    return _.filter(array, function(value){
      return !_.contains(rest, value);
    });
  });

And we don't actually want to build a list only
to check that it's zero vs. nonzero length.

We now do this, which short circuits as soon
as it finds any key that is only in sub1:

    return _.every(sub1.subscribers.keys(), (key) => {
        return sub2_set.has(key);
    });
2020-01-14 13:19:49 -08:00
Steve Howell 34b21bc0ee refactor: Use is_broadcast flag for mention check.
I also clean up the noop tests here, which were
actually redundant (all three cases were short
circuiting on the "everyone" mention).
2020-01-14 13:19:49 -08:00