This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
UI Auth via SSO: redirect the user to an appropriate SSO. #9081
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
4f1477e
Collect identifiers for UIA session data together
richvdh 16a2af9
Rename user_id in validate_user_via_ui_auth
richvdh edaed4a
Store the requester's user id during UIA
richvdh 97c9147
UIAuth: only offer SSO for users with a mapping from a known IdP
richvdh 3e47573
UI Auth via SSO: redirect the user to an appropriate IdP.
richvdh 82808fb
changelog
richvdh 3dadd1b
Use the correct identifier for the session data
richvdh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add support for multiple SSO Identity Providers. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,7 +50,10 @@ | |
) | ||
from synapse.api.ratelimiting import Ratelimiter | ||
from synapse.handlers._base import BaseHandler | ||
from synapse.handlers.ui_auth import INTERACTIVE_AUTH_CHECKERS | ||
from synapse.handlers.ui_auth import ( | ||
INTERACTIVE_AUTH_CHECKERS, | ||
UIAuthSessionDataConstants, | ||
) | ||
from synapse.handlers.ui_auth.checkers import UserInteractiveAuthChecker | ||
from synapse.http import get_request_user_agent | ||
from synapse.http.server import finish_request, respond_with_html | ||
|
@@ -335,39 +338,42 @@ async def validate_user_via_ui_auth( | |
request_body.pop("auth", None) | ||
return request_body, None | ||
|
||
user_id = requester.user.to_string() | ||
requester_user_id = requester.user.to_string() | ||
|
||
# Check if we should be ratelimited due to too many previous failed attempts | ||
self._failed_uia_attempts_ratelimiter.ratelimit(user_id, update=False) | ||
self._failed_uia_attempts_ratelimiter.ratelimit(requester_user_id, update=False) | ||
|
||
# build a list of supported flows | ||
supported_ui_auth_types = await self._get_available_ui_auth_types( | ||
requester.user | ||
) | ||
flows = [[login_type] for login_type in supported_ui_auth_types] | ||
|
||
def get_new_session_data() -> JsonDict: | ||
return {UIAuthSessionDataConstants.REQUEST_USER_ID: requester_user_id} | ||
|
||
try: | ||
result, params, session_id = await self.check_ui_auth( | ||
flows, request, request_body, description | ||
flows, request, request_body, description, get_new_session_data, | ||
) | ||
except LoginError: | ||
# Update the ratelimiter to say we failed (`can_do_action` doesn't raise). | ||
self._failed_uia_attempts_ratelimiter.can_do_action(user_id) | ||
self._failed_uia_attempts_ratelimiter.can_do_action(requester_user_id) | ||
raise | ||
|
||
# find the completed login type | ||
for login_type in supported_ui_auth_types: | ||
if login_type not in result: | ||
continue | ||
|
||
user_id = result[login_type] | ||
validated_user_id = result[login_type] | ||
break | ||
else: | ||
# this can't happen | ||
raise Exception("check_auth returned True but no successful login type") | ||
|
||
# check that the UI auth matched the access token | ||
if user_id != requester.user.to_string(): | ||
if validated_user_id != requester_user_id: | ||
raise AuthError(403, "Invalid auth") | ||
|
||
# Note that the access token has been validated. | ||
|
@@ -399,13 +405,9 @@ async def _get_available_ui_auth_types(self, user: UserID) -> Iterable[str]: | |
|
||
# if sso is enabled, allow the user to log in via SSO iff they have a mapping | ||
# from sso to mxid. | ||
if self.hs.config.saml2.saml2_enabled or self.hs.config.oidc.oidc_enabled: | ||
if await self.store.get_external_ids_by_user(user.to_string()): | ||
ui_auth_types.add(LoginType.SSO) | ||
|
||
# Our CAS impl does not (yet) correctly register users in user_external_ids, | ||
# so always offer that if it's available. | ||
if self.hs.config.cas.cas_enabled: | ||
if await self.hs.get_sso_handler().get_identity_providers_for_user( | ||
user.to_string() | ||
): | ||
ui_auth_types.add(LoginType.SSO) | ||
|
||
return ui_auth_types | ||
|
@@ -424,6 +426,7 @@ async def check_ui_auth( | |
request: SynapseRequest, | ||
clientdict: Dict[str, Any], | ||
description: str, | ||
get_new_session_data: Optional[Callable[[], JsonDict]] = None, | ||
) -> Tuple[dict, dict, str]: | ||
""" | ||
Takes a dictionary sent by the client in the login / registration | ||
|
@@ -447,6 +450,13 @@ async def check_ui_auth( | |
description: A human readable string to be displayed to the user that | ||
describes the operation happening on their account. | ||
|
||
get_new_session_data: | ||
an optional callback which will be called when starting a new session. | ||
it should return data to be stored as part of the session. | ||
|
||
The keys of the returned data should be entries in | ||
UIAuthSessionDataConstants. | ||
|
||
Returns: | ||
A tuple of (creds, params, session_id). | ||
|
||
|
@@ -474,10 +484,15 @@ async def check_ui_auth( | |
|
||
# If there's no session ID, create a new session. | ||
if not sid: | ||
new_session_data = get_new_session_data() if get_new_session_data else {} | ||
|
||
session = await self.store.create_ui_auth_session( | ||
clientdict, uri, method, description | ||
) | ||
|
||
for k, v in new_session_data.items(): | ||
await self.set_session_data(session.session_id, k, v) | ||
|
||
else: | ||
try: | ||
session = await self.store.get_ui_auth_session(sid) | ||
|
@@ -639,7 +654,8 @@ async def set_session_data(self, session_id: str, key: str, value: Any) -> None: | |
|
||
Args: | ||
session_id: The ID of this session as returned from check_auth | ||
key: The key to store the data under | ||
key: The key to store the data under. An entry from | ||
UIAuthSessionDataConstants. | ||
value: The data to store | ||
""" | ||
try: | ||
|
@@ -655,7 +671,8 @@ async def get_session_data( | |
|
||
Args: | ||
session_id: The ID of this session as returned from check_auth | ||
key: The key to store the data under | ||
key: The key the data was stored under. An entry from | ||
UIAuthSessionDataConstants. | ||
default: Value to return if the key has not been set | ||
""" | ||
try: | ||
|
@@ -1329,12 +1346,12 @@ def _do_validate_hash(checked_hash: bytes): | |
else: | ||
return False | ||
|
||
async def start_sso_ui_auth(self, redirect_url: str, session_id: str) -> str: | ||
async def start_sso_ui_auth(self, request: SynapseRequest, session_id: str) -> str: | ||
""" | ||
Get the HTML for the SSO redirect confirmation page. | ||
|
||
Args: | ||
redirect_url: The URL to redirect to the SSO provider. | ||
request: The incoming HTTP request | ||
session_id: The user interactive authentication session ID. | ||
|
||
Returns: | ||
|
@@ -1344,6 +1361,35 @@ async def start_sso_ui_auth(self, redirect_url: str, session_id: str) -> str: | |
session = await self.store.get_ui_auth_session(session_id) | ||
except StoreError: | ||
raise SynapseError(400, "Unknown session ID: %s" % (session_id,)) | ||
|
||
user_id_to_verify = await self.get_session_data( | ||
session_id, UIAuthSessionDataConstants.REQUEST_USER_ID | ||
) # type: str | ||
|
||
idps = await self.hs.get_sso_handler().get_identity_providers_for_user( | ||
user_id_to_verify | ||
) | ||
|
||
if not idps: | ||
# we checked that the user had some remote identities before offering an SSO | ||
# flow, so either it's been deleted or the client has requested SSO despite | ||
# it not being offered. | ||
raise SynapseError(400, "User has no SSO identities") | ||
|
||
# for now, just pick one | ||
idp_id, sso_auth_provider = next(iter(idps.items())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This theoretically isn't stable before Python 3.7. I suspect we don't care since they can use any identity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indeed. the first is no better than the last. |
||
if len(idps) > 0: | ||
logger.warning( | ||
"User %r has previously logged in with multiple SSO IdPs; arbitrarily " | ||
"picking %r", | ||
user_id_to_verify, | ||
idp_id, | ||
) | ||
|
||
redirect_url = await sso_auth_provider.handle_redirect_request( | ||
request, None, session_id | ||
) | ||
|
||
return self._sso_auth_confirm_template.render( | ||
description=session.description, redirect_url=redirect_url, | ||
) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point in the flow should we be rendering JSON or HTML errors? (Should this use
sso_handler.render_error
?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that said... there are plenty of other places we are raising SynapseErrors in this method. Ideally they'd all be caught and sent to
sso_handler.render_error
instead, and while we're at it I thinkstart_sso_ui_auth
probably just wants to move intoSsoHandler
, but that feels like a bigger refactor.