Commit Graph

35290 Commits

Author SHA1 Message Date
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
Anders Kaseorg 68cfcd6446 CVE-2020-9444: Prevent reverse tabnabbing attacks.
While we could fix this issue by changing the markdown processor,
doing so is not a robust solution, because even a momentary bug in the
markdown processor could allow cached messages that do not follow our
security policy.

This change ensures that even if our markdown processor has bugs that
result in rendered content that does not properly follow our policy of
using rel="noopener noreferrer" on links, we'll still do something
reasonable.

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
Tim Abbott f572bf50f5 message_list_view: Suppress exceptions for missing mention users.
If we can't find data on a mentioned user to update its full_name to
the current value, we'll have to go with the value in the message
itself.

This can happen if e.g. we hard-deleted the originally mentioned user
from the database (which can sometimes happen after a "delete my
account completely" request).
2020-03-31 17:56:12 -07:00
Ryan Rehman 9cb2475b53 notification_settings: Display streams that differ.
The user has an option for setting global
notification settings as well as the same settings
for individual streams. Currently the user has to
keep track of each unmatched stream and then visit
each individual stream whose settings he wants to
update.

Thus this adds a dedicated UI table allowing the user
to view and update the notifications of the specific
streams which differs from the global settings.
It is located on the same page where the user defined
global notification settings can be modified.

Fixes #9228.
2020-03-31 15:57:47 -07:00
Ryan Rehman e845b2364f refactor: Remove redundant block in `stream_edit.stream_setting_clicked`.
Currently we are updating the checkbox UI as soon as the user clicks.
This block is removed to match with the pattern of rest of the
properties in the stream edit page where `stream_events.update_property`
is responsible for updating the UI after a successful server response.
2020-03-31 15:36:59 -07:00
Ryan Rehman c759083cb8 notification settings: Add helper function for stream notifications.
This function returns a list of objects to create a
list_render object, and each item contains the streams
whose atleast one notification setting differs from the
default set by the user.

This is done by comparing the global settings in the
`#settings/notifications` page with those settings
present in the subscribed streams.

Work towards #9228.
2020-03-31 15:36:59 -07:00
Tim Abbott 0f238f29fb unread: Remove suppress_unread_counts flag.
This flag was used to delay unread count updates while the bankruptcy
modal was visible.  Now that bankrupcty is no longer a modal, we don't
need this flag at all.
2020-03-31 15:30:52 -07:00
majordwarf e85e598e56 bankruptcy: Migrate from modal view to top-of-page panel.
Switched to top-of-page prompt to make it natural to fit in with other
notifications. As we switch to panel-based prompt, templates for the
bankruptcy modal are moved along with its usage in application's
homepage.

We include a bit of delay before reloading to make it easy for the
user to read the "Marking all messages as read" banner before it is
covered by the "Reloading..." notice in environments where the reload
is fast.

Fixes #3347.
2020-03-31 15:30:37 -07:00
sahil839 3cea9ce1c4 stream_edit: Fix closing settings overlay after closing modals.
When stream_post_policy modal is closed either after saving or using
cancel button or cross button, the pointer-events is set to none which
does not allow to close the stream settings overlay on one click.

Added overlay.close_modal on saving such that pointer-events:none is
removed.

Added line which removes pointer-events:none again on clicking cancel
button or close icon.
2020-03-31 15:15:11 -07:00
sahil839 e43614debd overlays: Extract methods for disabling and enabling mouse events.
This is a prep commit which extracts the part of the code in open_modal
and close_modal to separate methods which adds inline style of
pointer-events to enable/disable the background mouse events.

Block comments are added for easy understanding of reader.
2020-03-31 15:12:44 -07:00
arpit551 a2dd1fad12 provision: switch Focal to use PGroonga from PPA.
Since Groonga packages for Ubuntu 20.04 Focal are now available
in their ppa so stopped building pgroonga from source.
2020-03-31 15:02:31 -07:00
Anders Kaseorg 9d75eb0ad5 install-shellcheck: Check the tarball’s SHA-256 hash.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-03-31 15:01:54 -07:00
Siddharth Varshney 3b3f9b0b8a portico: Fix the position and size of overflow for small size devices.
Fixes: #8424
2020-03-31 15:00:26 -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 dd8958e0cd minor: Move then_log_in next to then_log_out. 2020-03-31 10:57:08 -04:00
Steve Howell 3dde4a70d2 capser: Extract wait_for_text().
Before this we were monkey-patching in the
function `waitForSelectorText` into the
`casper` namespace, but only if you called
`common.initialize_casper`.

This would cause confusion if you expected
that function to be documented by Casper.

Now we just add the helper to `common` in
the `common` namespace.

We also avoid having to reason about what
`this` means by just using `casper` inside
the implementation of `wait_for_text` now.
And we don't bother with a return code that
none of our callers were using, anyway.
2020-03-31 10:57:08 -04:00
Steve Howell 05f52f0de6 casper: Remove unused args/params in helpers.
We remove `credentials` or `viewport` as either
arguments or parameters in several places, based
on their callee or caller not using them.
2020-03-31 10:57:08 -04:00
Steve Howell 35d12c55d1 casper: Remove phantom_page_loaded code.
We removed the phantom_page_loaded logic in
b13265d135
(July 2017).

Now we just say that the page is loaded
to the console, which can possibly help
us debug glitches where the tests are
executing too early.
2020-03-31 10:57:08 -04:00
Steve Howell c0782f525a casper test: Refine --loop mode.
We added a really nice feature recently,
called `--interactive`, which lets you loop
through Casper tests without having to restart
it every time.

I am renaming it to `--loop` and adding a few
features:

- The first loop will just run without you having
  to tell it to start.  (This means you don't have
  to sit there while waiting for webpack to finish
  and for the server to start, just to launch
  the tests again.)

- You specify how many loops you want to run,
  which means in the success case, it won't
  just keep going forever--it will eventually
  stop, giving you an opportunity to refine
  the test further without re-launching.
2020-03-31 10:57:08 -04:00
Steve Howell 6b946e4d05 casper: Clean up message helpers.
We now trim the headers inside of
`get_rendered_messages`, since any
sane caller of that function just
wants nicely trimmed headers.

(Note that we're now doing the
string manipulation inside of
Zulip code, not Casper code, which
is why I didn't reuse normalize_spaces.)
2020-03-31 10:57:08 -04:00
Mateusz Mandera 34f1f577f2 forms: Show user.delivery_email in password reset form.
.email shows the dummy userXXXX@domain.com style email, which is
will be confusing for users and is most likely an unintentional bug.
2020-03-30 16:41:46 -07:00
Tim Abbott 724fcc74e8 help: Improve documentation on importing exported organizations.
This should help avoid confusion where some users used the Zulip
backup tool, not the import tool, to try to import their backups.
2020-03-30 13:25:03 -07:00
YashRE42 556a13b7d6 portico: Drop font-weight for links from landing-page.scss.
This commit removes "font-weight: 500;" from landing-page.scss so as
to fix a bug on landing pages that used the `markdown` class to
format content. The bug was caused by "a:hover" from landing-page.scss
overriding the font-weight (600) on links as set by the markdown
class, this caused the text to seem jumpy when one hovered over links.

Note from tabbott: The original code was added in
d7f5f31f6a, which doesn't explain it's
purpose, but it predates the more complete "markdown" CSS, was part of
an early prototype that had unfortunate hover behavior more generally,
and makes sense to remove.

Fixes: #14387.
2020-03-30 12:55:23 -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
Steve Howell 6b86d9055f tests: Only wait up to 90s for the server. 2020-03-29 12:17:16 -07:00
Steve Howell cef74e1fdd tests: Sleep a bit more between server-is-up checks.
A watched pot never boils.

If we get server startup time down enough where 400ms
becomes a significant chunk, we can re-tweak this.
2020-03-29 12:17:16 -07:00
Steve Howell 7e4d542df4 tests: Use zulipdev.com for server_is_up() check.
Folks can have issues connecting to Casper
as zulipdev.com when they are not connected to
the internet or just have a bad connection, since
the DNS record is on the internet.  Folks can
work around this by just creating an /etc/hosts
entry for zulipdev.com, but people don't always
know.

This fix moves the symptom slightly earlier in
the process--we don't advertise that the server
is "up" if you can't actually connect to it as
"zulipdev.com".
2020-03-29 12:17:16 -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
vaibhavrajsingh2001 51a8873579 widgets: Hide edit question icon in poll widget for non-author users.
If a non-author user clicked on view source in a poll and then close it,
the edit question icon would incorrectly get visible. This made changing
the question in local echo possible for non-author users.

Fixes: #14299
2020-03-28 15:04:36 -04:00
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
shubhamgupta2956 3096251995 api docs: Remove payload from fixtures.json.
Remove unused payload ("bad_event_queue_id_error",
"successful-response-empty") from api/fixtures.json.
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
Vishnu KS 96eb1bcd9d tests: Test the case where Customer exists but stripe_customer_id is None. 2020-03-27 16:31:04 -07:00
Vishnu KS 388384e894 tests: Check for object equality instead of type equality.
I don't see any strong reason to do only type equality
when we can do object equality here.
2020-03-27 16:31:04 -07:00