From 5310808d3bebd17275355ecd474bc013e8c7462d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Jan 2021 18:19:42 +0000 Subject: Give the user a better error when they present bad SSO creds If a user tries to do UI Auth via SSO, but uses the wrong account on the SSO IdP, try to give them a better error. Previously, the UIA would claim to be successful, but then the operation in question would simply fail with "auth fail". Instead, serve up an error page which explains the failure. --- synapse/handlers/sso.py | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index d096e0b091..69ffc9d9c2 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -23,6 +23,7 @@ from typing_extensions import NoReturn, Protocol from twisted.web.http import Request from synapse.api.errors import Codes, RedirectException, SynapseError +from synapse.handlers.ui_auth import UIAuthSessionDataConstants from synapse.http import get_request_user_agent from synapse.http.server import respond_with_html from synapse.http.site import SynapseRequest @@ -147,6 +148,7 @@ class SsoHandler: self._server_name = hs.hostname self._registration_handler = hs.get_registration_handler() self._error_template = hs.config.sso_error_template + self._bad_user_template = hs.config.sso_auth_bad_user_template self._auth_handler = hs.get_auth_handler() # a lock on the mappings @@ -577,19 +579,40 @@ class SsoHandler: auth_provider_id, remote_user_id, ) + user_id_to_verify = await self._auth_handler.get_session_data( + ui_auth_session_id, UIAuthSessionDataConstants.REQUEST_USER_ID + ) # type: str + if not user_id: logger.warning( "Remote user %s/%s has not previously logged in here: UIA will fail", auth_provider_id, remote_user_id, ) - # Let the UIA flow handle this the same as if they presented creds for a - # different user. - user_id = "" + elif user_id != user_id_to_verify: + logger.warning( + "Remote user %s/%s mapped onto incorrect user %s: UIA will fail", + auth_provider_id, + remote_user_id, + user_id, + ) + else: + # success! + await self._auth_handler.complete_sso_ui_auth( + user_id, ui_auth_session_id, request + ) + return + + # the user_id didn't match: mark the stage of the authentication as unsuccessful + await self._store.mark_ui_auth_stage_complete( + ui_auth_session_id, LoginType.SSO, "" + ) - await self._auth_handler.complete_sso_ui_auth( - user_id, ui_auth_session_id, request + # render an error page. + html = self._bad_user_template.render( + server_name=self._server_name, user_id_to_verify=user_id_to_verify, ) + respond_with_html(request, 200, html) async def check_username_availability( self, localpart: str, session_id: str, -- cgit 1.4.1 From 420031906a04f7b5462347bf47730d4bc6cc8870 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Jan 2021 11:12:28 +0000 Subject: Move `complete_sso_ui_auth` into SSOHandler since we're hacking on this code anyway, may as well move it out of the cluttered AuthHandler. --- synapse/handlers/auth.py | 25 ------------------------- synapse/handlers/sso.py | 16 +++++++++++++--- 2 files changed, 13 insertions(+), 28 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 4f881a439a..18cd2b62f0 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -263,10 +263,6 @@ class AuthHandler(BaseHandler): # authenticating for an operation to occur on their account. self._sso_auth_confirm_template = hs.config.sso_auth_confirm_template - # The following template is shown after a successful user interactive - # authentication session. It tells the user they can close the window. - self._sso_auth_success_template = hs.config.sso_auth_success_template - # The following template is shown during the SSO authentication process if # the account is deactivated. self._sso_account_deactivated_template = ( @@ -1394,27 +1390,6 @@ class AuthHandler(BaseHandler): description=session.description, redirect_url=redirect_url, ) - async def complete_sso_ui_auth( - self, registered_user_id: str, session_id: str, request: Request, - ): - """Having figured out a mxid for this user, complete the HTTP request - - Args: - registered_user_id: The registered user ID to complete SSO login for. - session_id: The ID of the user-interactive auth session. - request: The request to complete. - """ - # Mark the stage of the authentication as successful. - # Save the user who authenticated with SSO, this will be used to ensure - # that the account be modified is also the person who logged in. - await self.store.mark_ui_auth_stage_complete( - session_id, LoginType.SSO, registered_user_id - ) - - # Render the HTML and return. - html = self._sso_auth_success_template - respond_with_html(request, 200, html) - async def complete_sso_login( self, registered_user_id: str, diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index 69ffc9d9c2..dcc85e9871 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -22,6 +22,7 @@ from typing_extensions import NoReturn, Protocol from twisted.web.http import Request +from synapse.api.constants import LoginType from synapse.api.errors import Codes, RedirectException, SynapseError from synapse.handlers.ui_auth import UIAuthSessionDataConstants from synapse.http import get_request_user_agent @@ -147,9 +148,13 @@ class SsoHandler: self._store = hs.get_datastore() self._server_name = hs.hostname self._registration_handler = hs.get_registration_handler() + self._auth_handler = hs.get_auth_handler() self._error_template = hs.config.sso_error_template self._bad_user_template = hs.config.sso_auth_bad_user_template - self._auth_handler = hs.get_auth_handler() + + # The following template is shown after a successful user interactive + # authentication session. It tells the user they can close the window. + self._sso_auth_success_template = hs.config.sso_auth_success_template # a lock on the mappings self._mapping_lock = Linearizer(name="sso_user_mapping", clock=hs.get_clock()) @@ -598,9 +603,14 @@ class SsoHandler: ) else: # success! - await self._auth_handler.complete_sso_ui_auth( - user_id, ui_auth_session_id, request + # Mark the stage of the authentication as successful. + await self._store.mark_ui_auth_stage_complete( + ui_auth_session_id, LoginType.SSO, user_id ) + + # Render the HTML confirmation page and return. + html = self._sso_auth_success_template + respond_with_html(request, 200, html) return # the user_id didn't match: mark the stage of the authentication as unsuccessful -- cgit 1.4.1