Commit Graph

358 Commits

Author SHA1 Message Date
Mateusz Mandera dcbcb05655 saml: Make SP-initiated SLO work after signup. 2023-05-23 13:01:15 -07:00
Mateusz Mandera 5dd4dcdebb saml: Make SP-initiated SLO work in the desktop application. 2023-05-23 13:01:15 -07:00
Mateusz Mandera 3f55c10685 saml: Rework SP-initiated logout config to support IdP-level config.
This gives more flexibility on a server with multiple organizations and
SAML IdPs. Such a server can have some organizations handled by IdPs
with SLO set up, and some without it set up. In such a scenario, having
a generic True/False server-wide setting is insufficient and instead
being able to specify the IdPs/orgs for SLO is needed.
2023-05-23 13:01:15 -07:00
Mateusz Mandera 0bb0220ebb saml: Implement SP-initiated Logout.
Closes #20084

This is the flow that this implements:
1. A logged-in user clicks "Logout".
2. If they didn't auth via SAML, just do normal logout. Otherwise:
3. Form a LogoutRequest and redirect the user to
https://idp.example.com/slo-endpoint?SAMLRequest=<LogoutRequest here>
4. The IdP validates the LogoutRequest, terminates its own user session
and redirects the user to
https://thezuliporg.example.com/complete/saml/?SAMLRequest=<LogoutResponse>
with the appropriate LogoutResponse. In case of failure, the
LogoutResponse is expected to express that.
5. Zulip validates the LogoutResponse and if the response is a success
response, it executes the regular Zulip logout and the full flow is
finished.
2023-05-23 13:01:15 -07:00
Mateusz Mandera dda4603f94 auth: Extract EXPIRABLE_SESSION_VAR_DEFAULT_EXPIRY_SECS. 2023-05-23 13:01:15 -07:00
Mateusz Mandera 3ed2a30e01 maybe_send_to_registration: Remove password_required arg.
This argument was added with the default incorrectly set to `True` in
bb0eb76bf3 - despite
`maybe_send_to_registration` only ever being called in production code
in a single place, with `password_required=False` explicitly. And then
it just got carried forward through refactors.

`maybe_send_to_registration` was/is also called twice in tests, falling
back to the default, but the `password_required` value is irrelevant to
the tests - and if anything letting it use the `True` has been wrong,
due to not matching how this function is actually used.
2023-05-19 16:13:00 -04:00
Mateusz Mandera 254ea4b0c8 social_auth: Save authentication method information in the session.
The immediate application of this will be for SAML SP-initiated logout,
where information about which IdP was used for authenticating the
session needs to be accessed. Aside of that, this seems like generally
valuable session information to keep that other features may benefit
from in the future.
2023-05-12 16:21:26 -07:00
Mateusz Mandera af9d1a7dfb register_remote_user: Use explicit kwargs list.
This is nicer that .pop()ing specified keys - e.g. we no longer will
have to update this chunk of code whenever adding a new key to
ExternalAuthDataDict.
2023-05-12 16:21:25 -07:00
Prakhar Pratyush c8a9c0ee04 realm_redirect: Redirect always to the login page with the next parameter.
Previously, entering an organization via 'accounts/go' with the
web-public stream enabled took the user to the web-public view
even if the user was not logged in.

Now, a user is always redirected to the 'login_page' with
the next parameter, if present.

The 'login_page' view is updated to redirect an authenticated
user based on the 'next' parameter instead of always redirecting
to 'realm.uri'.

Fixes #23344.
2023-04-27 16:50:10 -07:00
AcKindle3 98c9a0366a settings: Replace occurrences of `uri` with `url`.
In #23380 we want to change all occurrences of `uri` with `url`.
This commit changes the names of two variables `external_uri_scheme`
and `main_site_uri`, who are constructed using `settings` constants.
2023-04-26 16:37:16 -07:00
Tanmay Munjal 80c01e9fc3
auth: Refactor JWT code to push REQ to top level functions.
Fixes #25015.
2023-04-26 10:57:22 -07:00
Lauryn Menard e95b784f6e backend-auth: Add user ID to fetch api key responses.
Adds the user ID to the return values for the `/fetch_api_key` and
`/dev_fetch_api_key` endpoints. This saves clients like mobile a
round trip to the server to get the user's unique ID as it is now
returned as part of the log in flow.

Fixes #24980.
2023-04-04 10:54:49 -07:00
Sahil Batra 00ffa3e870 registration: Remove redundant code.
Since we have updated the registration code to use
PreregistrationRealm objects for realm creation in
previous commits, some of the code has become
redundant and this commit removes it.

We remove the following code -
- The modification to PreregistrationUser objects in
process_new_human_user can now be done unconditionally
because prereg_user is passed only during user creation
and not realm creation. And we anyway do not expect
any PreregistrationUser objects inside the realm
during the creation.
- There is no need of "realm_creation" parameter in
create_preregistration_user function, since we now
use create_preregistration_realm during realm creation.

Fixes part of #24307.
2023-03-27 15:44:42 -07:00
Sahil Batra 54771cfe94 registration: Create PreregistrationRealm object when creating realm.
This commit adds code to create PreregistrationRealm object when
creating realm and set it to the content_object field of
Confirmation object.
2023-03-27 15:44:42 -07:00
Anders Kaseorg afa218fa2a semgrep: Detect some unsafe uses of markupsafe.Markup.
Use the built-in HTML escaping of Markup("…{var}…").format(), in order
to allow Semgrep to detect mistakes like Markup("…{var}…".format())
and Markup(f"…{var}…").

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-03-22 11:23:27 -07:00
Anders Kaseorg 738667b39e templates: Convert config errors to HTML.
Markdown and Jinja don’t mix correctly, and templating is not an
appropriate use of Markdown.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-02-28 16:54:15 -08:00
Aman Agrawal 9965ad2ea3 registration: Track create organization page in GA.
This will help us track if users actually clicked on the
email confirmation link while creating a new organization.

Replaced all the `reder` calls in `accounts_register` with
`TemplateResponse` to comply with `add_google_analytics`
decorator.
2023-02-05 10:24:32 -08:00
Anders Kaseorg da3cf5ea7a ruff: Fix RSE102 Unnecessary parentheses on raised exception.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-02-04 16:34:55 -08:00
Anders Kaseorg 5b7c4206d7 ruff: Fix SIM300 Yoda conditions are discouraged.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-02-03 16:36:54 -08:00
Alessandro Toppi ff89590558 auth: Add JWT-based user API key fetch.
This adds a new endpoint /jwt/fetch_api_key that accepts a JWT and can
be used to fetch API keys for a certain user. The target realm is
inferred from the request and the user email is part of the JWT.

A JSON containing an user API key, delivery email and (optionally)
raw user profile data is returned in response.
The profile data in the response is optional and can be retrieved by
setting the POST param "include_profile" to "true" (default=false).

Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
2023-02-03 15:23:35 -08:00
Tim Abbott 7c7ca61e9f auth: Remove now unnecessary return_data parameter. 2023-02-03 15:23:35 -08:00
Mateusz Mandera 75b44337a8 auth: Extract get_api_key_fetch_authenticate_failure. 2023-02-03 15:23:35 -08:00
Mateusz Mandera 3a1d974cee auth: Extract process_api_key_fetch_authenticate_result function.
This will be useful for re-use in the implementation of another JWT
endpoint in the upcoming commits.
2023-02-03 15:23:35 -08:00
Mateusz Mandera 100f4a7152 auth: Add @require_post to remote_user_jwt. 2023-02-03 15:23:35 -08:00
Mateusz Mandera d466da1064 auth: Use REQ for getting token value and rename param in jwt paths.
This makes us use REQ properly instead of fetching from request.POST
manually - also renaming the param to "token" which is more standard.
2023-02-03 15:23:35 -08:00
Mateusz Mandera c9c255b3a8 auth: Improve JsonableError in get_..._jwt_authentication_request. 2023-02-03 15:23:35 -08:00
Mateusz Mandera 6c638a1057 auth: Extract token-check logic of remote_user_jwt.
This will be useful for re-use for implementation of an endpoint for
obtaining the API by submitting a JWT in the next commits.

It's not a pure refactor, as it requires some tweaks to remote_user_jwt
behavior:
1. The expected format of the request is changed a bit. It used to
   expect "user" and "realm" keys, from which the intended email was
   just generated by joining with @. Now it just expects "email"
   straight-up. The prior design was a bt strange to begin with, so this
   might be an improvement actually.
2. In the case of the codepath of new user signup, this will no longer
   pre-populate the Full Name in the registration form with the value
   from the "user" key. This should be a very minor lost of
   functionality, because the "user" value was not going to be a proper
   Full Name anyway. This functionality can be restored in a future
   commit if desired.

This is an API change, but this endpoint is nearly unused as far as
we're aware.
2023-02-03 15:23:35 -08:00
Anders Kaseorg ff1971f5ad ruff: Fix SIM105 Use `contextlib.suppress` instead of try-except-pass.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-01-23 11:18:36 -08:00
Anders Kaseorg b0e569f07c ruff: Fix SIM102 nested `if` statements.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-01-23 11:18:36 -08:00
Anders Kaseorg 73c4da7974 ruff: Fix N818 exception name should be named with an Error suffix.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-11-17 16:52:00 -08:00
Anders Kaseorg 924d530292 ruff: Fix N813 camelcase imported as lowercase.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-11-16 09:29:11 -08:00
Mateusz Mandera 23a776c144 maybe_send_to_registration: Don't reuse pre-existing PreregistraionUser.
There was the following bug here:
1. Send an email invite to a user.
2. Have the user sign up via social auth without going through that
   invite, meaning either going via a multiuse invite link or just
   straight-up Sign up if the org permissions allow.

That resulted in the PreregistrationUser that got generated in step (1)
having 2 Confirmations tied to it - because maybe_send_to_registration
grabbed the object and created a new confirmation link for it. That is a
corrupted state, Confirmation is supposed to be unique.

One could try to do fancy things with checking whether a
PreregistrationUser already have a Confirmation link, but to avoid races
between ConfirmationEmailWorker and maybe_send_to_registration, this
would require taking locks and so on - which gets needlessly
complicated. It's simpler to not have them compete for the same object.

The point of the PreregistrationUser re-use in
maybe_send_to_registration is that if an admin invites a user, setting
their initial streams and role, it'd be an annoying experience if the
user ends up signing up not via the invite and those initial streams
streams etc. don't get set up. But to handle this, we can just copy the
relevant values from the pre-existing prereg_user, rather than re-using
the object itself.
2022-11-10 17:46:53 -08:00
Mateusz Mandera b5b8cc06dc auth: Clean up redundant request.session.modified change.
This line was added in 94e099eaab,
presumably because of the
del request.session["multiuse_object_key"]
line that was just above it.
Looks like it should have been removed in
868a763cec, which eliminated that `del`
operation.
2022-11-10 17:46:53 -08:00
Anders Kaseorg fcd81a8473 python: Replace avoidable uses of __special__ attributes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-10-10 08:32:29 -07:00
Anders Kaseorg 47c5deeccd python: Mark dict parameters with defaults as read-only.
Found by semgrep 0.115 more accurately applying the rule added in
commit 0d6c771baf (#15349).

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-10-06 13:48:28 -07:00
Zixuan James Li 21fd62427d typing: Remove ViewFuncT.
This removes ViewFuncT and all the associated type casts with ParamSpec
and Concatenate. This provides more accurate type annotation for
decorators at the cost of making the concatenated parameters
positional-only. This change does not intend to introduce any other
behavioral difference. Note that we retype args in process_view as
List[object] because the view functions can not only be called with
arguments of type str.

Note that the first argument of rest_dispatch needs to be made
positional-only because of the presence of **kwargs.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-08-22 15:46:16 -07:00
Mateusz Mandera 2c693f3bd9 billing: Fix licenses amount check during user signup/invitation.
Our seat count calculation is different for guest user than normal users
(a number of initial guests are free, and additional marginal guests are
worth 1/5 of a seat) - so these checks we apply when a user is being
invited or signing up need to know whether it's a guest or non-guest
being added.
2022-08-18 11:56:54 -07:00
Zixuan James Li 14dd7f1692 auth: Make QueryDict immutable before assignment.
This modifies the QueryDict when it is mutable, and assign it to `.POST`
after it is turned immutable, as required by django-stubs for this
attribute.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-08-12 17:08:04 -07:00
Zixuan James Li a3ec420074 auth: Add type annotation for contexts.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-08-12 17:08:04 -07:00
Zixuan James Li adae8b6d42 request: Refactor has_request_variables with ParamSpec.
This makes `has_request_variables` more generic, in the sense of the return
value, and also makes it more accurate, in the sense of requiring the
first parameter of the decorated function to be `HttpRequest`, and
preserving the function signature without using `cast`.

This affects some callers of `has_request_variables` or the callers of its
decoratedfunctions in the following manners:

- Decorated non-view functions called directly in other functions cannot
use `request` as a keyword argument. Becasue `Concatenate` turns the
concatenated parameters (`request: HttpRequest` in this case) into
positional-only parameters. Callers of `get_chart_data` are thus
refactored.

- Functions to be decorated that accept variadic keyword arguments must
define `request: HttpRequest` as positional-only. Mypy in strict mode
rejects such functions otherwise because it is possible for the caller to
pass a keyword argument that has the same name as `request` for `**kwargs`.
No defining `request: HttpRequest` as positional-only breaks type safety
because function with positional-or-keyword parameters cannot be considered
a subtype of a function with the same parameters in which some of them are
positional-only.

Consider `f(x: int, /, **kwargs: object) -> int` and `g(x: int,
**kwargs: object) -> int`. `f(12, x="asd")` is valid but `g(12, x="asd")`
is not.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-08-06 16:19:48 -07:00
Anders Kaseorg b945aa3443 python: Use a real parser for email addresses.
Now that we can assume Python 3.6+, we can use the
email.headerregistry module to replace hacky manual email address
parsing.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-07-29 15:47:33 -07:00
Zixuan James Li 789b66ff3b decorator: Remove unnecessary flag for process_client.
We can express the same idea more simply by not passing `user` in
cases where it isn't valid for UserActivity.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-28 09:38:21 -07:00
Zixuan James Li 06d3f3cf64 2fa: Refactor is_2fa_verified to require type narrowing.
This makes it mandatory to narrow the type of the user to `UserProfile`
before calling this helper.

This effectively removes the `request.user` check. We do not call login_page
anywhere else without getting through the authentication middleware.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-27 14:28:31 -07:00
Zixuan James Li 7233ad454f auth: Add an assertion check for context_data.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-26 18:00:24 -07:00
Mateusz Mandera 39d8a81e51 registration: Tie PreregistrationUser to the original MultiUseInvite.
Fixes #21266.

We want to tie the prereg_user to the MultiUseInvite directly rather
than to the MultiUserInvite's confirmation object, because the latter is
not possible. This is because the flow is that after going through the
multiuse invite link, the PreregistrationUser is created together with a
Confirmation object, creating a confirmation link (via
create_confirmation_link) to which then the user is redirected to finish
account creation. This means that the PreregistrationUser is already
tied to a Confirmation, so that attribute is occupied.
2022-07-22 17:08:44 -07:00
Tim Abbott dcc03a453a confirmation: Tighten logic around the mark_object_used parameter. 2022-07-21 15:18:15 -07:00
Zixuan James Li 00bd7513f2 2fa: Verify 2FA authentication status with is_2fa_verified.
This replaces user.is_verified with is_2fa_verified.

The helper does extra checks such that the user being checked for 2fa
authentication status is valid.

`request.user.is_verified` is functionally the same as `is_verified`
from `django_otp.middleware`, except that the former is monkey-patched
onto the user object by the 2FA middleware. We use the latter wrapped
in `is_2fa_verified` instead to avoid accessing the patched attribute.

See also: 6b24d56e59/docs/source/overview.rst (authentication-and-verification)

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-13 14:49:41 -07:00
Zixuan James Li 0017f7a025 confirmation: Support more models as ConfirmationObjT.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-06-29 11:31:07 -07:00
Zixuan James Li 4c6f2ae7be typing: Add assertions for authentication.
Signed-off-by: Zixuan James Li <359101898@qq.com>
2022-05-31 09:43:55 -07:00
Anders Kaseorg 0043c0b6b2 django: Use HttpRequest.headers.
Fixes #14769.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-05-13 20:42:20 -07:00