TestMaybeSendToRegistration needs tweaking here, because it wasn't
setting the subdomain for the dummy request, so
maybe_send_to_registration was actually running with realm=None, which
is not right for these tests.
Also, test_sso_only_when_preregistration_user_exists was creating
PreregistrationUser without setting the realm, which was also incorrect.
create_preregistration_user is a footgun, because it takes the realm
from the request. The calling code is supposed to validate that
registration for the realm is allowed
first, but can sometimes do that on "realm" taken from something else
than the request - and later on calls create_preregistration_user, thus
leading to prereg user creation on unvalidated request.realm.
It's safer, and makes more sense, for this function to take the intended
realm as argument, instead of taking the entire request. It follows that
the same should be done for prepare_activation_url.
The codepaths for joining an organization via a multi-use invitation
(accounts_home_from_multiuse_invite and maybe_send_to_registration)
weren't validating whether
the organization the invite was generated for matches the organization
the user attempts to join - potentially allowing an attacker with access
to organization A to generate a multi-use invite and use it to join
organization B within the same deployment, that they shouldn't have
access to.
Adds `realm_web_public_access_enabled` as a realm-specific server
setting potentially returned by the `/get-server-settings` endpoint
so that clients that support browsing of web-public stream content
without an account can generate a login page that supports that
type of access.
Adds request as a parameter to json_success as a refactor towards
making `ignored_parameters_unsupported` functionality available
for all API endpoints.
Also, removes any data parameters that are an empty dict or
a dict with the generic success response values.
Fixes “DeprecationWarning: 'jinja2.Markup' is deprecated and will be
removed in Jinja 3.1. Import 'markupsafe.Markup' instead.”
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The user id is a very useful piece of information that the mobile
client should have access to - instead of only getting the email. This
makes it much simpler to impleent clients that might be robust to
changes in email address.
SOCIAL_AUTH_SUBDOMAIN was potentially very confusing when opened by a
user, as it had various Login/Signup buttons as if there was a realm on
it. Instead, we want to display a more informative page to the user
telling them they shouldn't even be there. If possible, we just redirect
them to the realm they most likely came from.
To make this possible, we have to exclude the subdomain from
ROOT_SUBDOMAIN_ALIASES - so that we can give it special behavior.
This utilizes the generic `BaseNotes` we added for multipurpose
patching. With this migration as an example, we can further support
more types of notes to replace the monkey-patching approach we have used
throughout the codebase for type safety.
Return zulip_merge_base alongside zulip_version
in `/register`, `/event` and `/server_settings`
endpoint so that the value can be used by other
clients.
JsonableError has two major benefits over json_error:
* It can be raised from anywhere in the codebase, rather than
being a return value, which is much more convenient for refactoring,
as one doesn't potentially need to change error handling style when
extracting a bit of view code to a function.
* It is guaranteed to contain the `code` property, which is helpful
for API consistency.
Various stragglers are not updated because JsonableError requires
subclassing in order to specify custom data or HTTP status codes.
Raising jsonableError in the authentication form was non-ideal because
it took the user to an ugly page with the returned json.
We also add logging of this rare occurence of the scenario being
handled here.
django.utils.http.is_safe_url is a deprecated alias of
django.utils.http.url_has_allowed_host_and_scheme as of Django 3.0,
and will be removed in Django 4.0.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
django.utils.translation.ugettext is a deprecated alias of
django.utils.translation.gettext as of Django 3.0, and will be removed
in Django 4.0.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was used by the old native Zulip Android app
(zulip/zulip-android). That app has been undeveloped for enough years
that we believe it no longer functions; as a result, there's no reason
to keep a prototype API endpoint for it (that we believe never worked).
If a user visits a realm which has been deactivated and it's
deactivated_redirect field is set, we should have a message telling the
user that the realm has moved to the deactivated_redirect url.
We previously used to to redirect to config error page with
a different URL. This commit renders config error in the same
URL where configuration error is encountered. This way when
conifguration error is fixed the user can refresh to continue
normally or go back to login page from the link provided to
choose any other backend auth.
Also moved those URLs to dev_urls.py so that they can be easily
accessed to work on styling etc.
In tests, removed some of the asserts checking status code to be 200
as the function `assert_in_success_response` does that check.
There are three functional side effects:
• Correct an insignificant but mathematically offensive bias toward
repeated characters in generate_api_key introduced in commit
47b4283c4b4c70ecde4d3c8de871c90ee2506d87; its entropy is increased
from 190.52864 bits to 190.53428 bits.
• Use the base32 alphabet in confirmation.models.generate_key; its
entropy is reduced from 124.07820 bits to the documented 120 bits, but
now it uses 1 syscall instead of 24.
• Use the base32 alphabet in get_bigbluebutton_url; its entropy is
reduced from 51.69925 bits to 50 bits, but now it uses 1 syscall
instead of 10.
(The base32 alphabet is A-Z 2-7. We could probably replace all of
these with plain secrets.token_urlsafe, since I expect most callers
can handle the full urlsafe_b64 alphabet A-Z a-z 0-9 - _ without
problems.)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This redirect was never effective -- because of the
HostDomainMiddleware, all requests to invalid domains have their
actual results thrown away, and replaced by an "Invalid realm" 404.
These lines are nonetheless _covered_ by coverage, because they do
run; the redirect is simply ineffective. This can be seen by the test
that was added with them, in c8edbae21c, actually testing the contents
for the invalid realm wording, not the "find your accounts" wording.
Due to authentication restrictions, a deployment may need to direct
traffic for mobile applications to an alternate uri to take advantage
alternate authentication mechansism. By default the standard realm URI
will be usedm but if overridden in the settings file, an alternate uri
can be substituted.
`/api/v1/fetch_api_key`'s response had a key `email` with the user's
delivery email. But its JSON counterpart `/json/fetch_api_key`, which
has a completely different implementation, did not return `email` in
its success response.
So to avoid confusion, the non-API endpoint, `/json/fetch_api_key`
response has been made identical with it's `/api` counterpart by
adding the `email` key. Also it is safe to send as the calling user
will only see their own email.
The most import change here is the one in maybe_send_to_registration
codepath, as the insufficient validation there could lead to fetching
an expired PreregistrationUser that was invited as an administrator
admin even years ago, leading to this registration ending up in the
new user being a realm administrator.
Combined with the buggy migration in
0198_preregistrationuser_invited_as.py, this led to users incorrectly
joining as organizations administrators by accident. But even without
that bug, this issue could have allowed a user who was invited as an
administrator but then had that invitation expire and then joined via
social authentication incorrectly join as an organization administrator.
The second change is in ConfirmationEmailWorker, where this wasn't a
security problem, but if the server was stopped for long enough, with
some invites to send out email for in the queue, then after starting it
up again, the queue worker would send out emails for invites that
had already expired.
Fixes#14828.
Giving the /subdomain/<token>/ url there could feel buggy if the user
ended up using the token in the desktop app, and then tried clicking the
"continue in browser" link - which had the same token that would now be
expired. It's sufficient to simply link to /login/ instead.
Fixes#2665.
Regenerated by tabbott with `lint --fix` after a rebase and change in
parameters.
Note from tabbott: In a few cases, this converts technical debt in the
form of unsorted imports into different technical debt in the form of
our largest files having very long, ugly import sequences at the
start. I expect this change will increase pressure for us to split
those files, which isn't a bad thing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Automatically generated by the following script, based on the output
of lint with flake8-comma:
import re
import sys
last_filename = None
last_row = None
lines = []
for msg in sys.stdin:
m = re.match(
r"\x1b\[35mflake8 \|\x1b\[0m \x1b\[1;31m(.+):(\d+):(\d+): (\w+)", msg
)
if m:
filename, row_str, col_str, err = m.groups()
row, col = int(row_str), int(col_str)
if filename == last_filename:
assert last_row != row
else:
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
with open(filename) as f:
lines = f.readlines()
last_filename = filename
last_row = row
line = lines[row - 1]
if err in ["C812", "C815"]:
lines[row - 1] = line[: col - 1] + "," + line[col - 1 :]
elif err in ["C819"]:
assert line[col - 2] == ","
lines[row - 1] = line[: col - 2] + line[col - 1 :].lstrip(" ")
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This commit adds three `.pysa` model files: `false_positives.pysa`
for ruling out false positive flows with `Sanitize` annotations,
`req_lib.pysa` for educating pysa about Zulip's `REQ()` pattern for
extracting user input, and `redirects.pysa` for capturing the risk
of open redirects within Zulip code. Additionally, this commit
introduces `mark_sanitized`, an identity function which can be used
to selectively clear taint in cases where `Sanitize` models will not
work. This commit also puts `mark_sanitized` to work removing known
false postive flows.
set(redirect_host) is a set of characters, so the only non-relative
redirects being allowed were to certain one-character hostnames, which
certainly isn’t what was intended.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This implementation overrides some of PSA's internal backend
functions to handle `state` value with redis as the standard
way doesn't work because of apple sending required details
in the form of POST request.
Includes a mixin test class that'll be useful for testing
Native auth flow.
Thanks to Mateusz Mandera for the idea of using redis and
other important work on this.
Documentation rewritten by tabbott.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
Since owners have the highest privilege level, it made little sense to
sandwich them between administrators and guests.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Calling jwt.decode without an algorithms list raises a
DeprecationWarning. This is for protecting against
symmetric/asymmetric key confusion attacks.
This is a backwards-incompatible configuration change.
Fixes#15207.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
mock is just a backport of the standard library’s unittest.mock now.
The SAMLAuthBackendTest change is needed because
MagicMock.call_args.args wasn’t introduced until Python
3.8 (https://bugs.python.org/issue21269).
The PROVISION_VERSION bump is skipped because mock is still an
indirect dev requirement via moto.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This does not rely on the desktop app being able to register for the
zulip:// scheme (which is problematic with, for example, the AppImage
format).
It also is a better interface for managing changes to the system,
since the implementation exists almost entirely in the server/webapp
project.
This provides a smoother user experience, where the user doesn't need
to do the paste step, when combined with
https://github.com/zulip/zulip-desktop/pull/943.
Fixes#13613.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This new type eliminates a bunch of messy code that previously
involved passing around long lists of mixed positional keyword and
arguments, instead using a consistent data object for communicating
about the state of an external authentication (constructed in
backends.py).
The result is a significantly more readable interface between
zproject/backends.py and zerver/views/auth.py, though likely more
could be done.
This has the side effect of renaming fields for internally passed
structures from name->full_name, next->redirect_to; this results in
most of the test codebase changes.
Modified by tabbott to add comments and collaboratively rewrite the
initialization logic.
The purpose is to provide a way for (non-webapp) clients,
like the mobile and terminal apps, to tell whether the
server it's talking to is new enough to support a given
API feature -- in particular a way that
* is finer-grained than release numbers, so that for
features developed after e.g. 2.1.0 we can use them
immediately on servers deployed from master (like
chat.zulip.org and zulipchat.com) without waiting the
months until a 2.2 release;
* is reliable, unlike e.g. looking at the number of
commits since a release;
* doesn't lead to a growing bag of named feature flags
which the server has to go on sending forever.
Tweaked by tabbott to extend the documentation.
Closes#14618.
This used to show a blank page. Considering that the links remain valid
only for 15 seconds it's important to show something more informative to
the user.
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.
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.
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.
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.
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.
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.
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>
This makes it possible to create a Zulip account from the mobile or
desktop apps and have the end result be that the user is logged in on
their mobile device.
We may need small changes in the desktop and/or mobile apps to support
this.
Closes#10859.
Zixuan James Li
Anders Kaseorg
Mateusz Mandera
Lauryn Menard
jai2201
Alex Vandiver
Mateusz Mandera
Vishnu KS
Gaurav Pandey
Tim Abbott
Riken Shah
sushant52
Siddharth Asthana
Dinesh
Tim April
orientor
Anders Kaseorg
Graham Bleaney
sahil839
Hashir Sarwar
Steve Howell
Dinesh
vsvipul