From 964ca65ebb110203b449dccb64d7669e68dfbb3e Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Thu, 16 Apr 2026 11:19:28 +0000 Subject: [PATCH] Support MSC4450: Identity Provider selection for User-Interactive Authentication with Legacy Single Sign-On. (#19693) Closes: #19688 Part of: MSC4450 whose Experimental Feature tracking issue is #19691 Add an unstable, namespaced `idp_id` query parameter to `fallback/web` \ This allows clients to specify the identity provider they'd like to log in with for SSO when they have multiple upstream IdPs associated with their account. Previously, Synapse would just pick one arbitrarily. But this was undesirable as you may want to use a different one at that point in time. When logging in, the user is able to choose when IdP they use - during UIA (which uses fallback auth mechanism) they should be able to do the same. ----- Signed-off-by: Olivier 'reivilibre Co-authored-by: Andrew Morgan Co-authored-by: Eric Eastwood --- changelog.d/19693.feature | 1 + synapse/config/experimental.py | 6 ++ synapse/handlers/auth.py | 35 ++++++--- synapse/rest/client/auth.py | 21 ++++- tests/rest/client/test_auth.py | 140 +++++++++++++++++++++++++++++++++ 5 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 changelog.d/19693.feature diff --git a/changelog.d/19693.feature b/changelog.d/19693.feature new file mode 100644 index 0000000000..efe65b8c49 --- /dev/null +++ b/changelog.d/19693.feature @@ -0,0 +1 @@ +Support [MSC4450: Identity Provider selection for User-Interactive Authentication with Legacy Single Sign-On](https://github.com/matrix-org/matrix-spec-proposals/pull/4450). \ No newline at end of file diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index e77412a797..702c7e3246 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -604,3 +604,9 @@ class ExperimentalConfig(Config): # Note that sticky events persisted before this feature is enabled will not be # considered sticky by the local homeserver. self.msc4354_enabled: bool = experimental.get("msc4354_enabled", False) + + # MSC4450: Identity Provider selection for User-Interactive Authentication + # with Legacy Single Sign-On (`m.login.sso`) + # Tracked in: https://github.com/element-hq/synapse/issues/19691 + # Note that this is only applicable to legacy auth, not MAS integration (OAuth 2.0). + self.msc4450_enabled: bool = experimental.get("msc4450_enabled", False) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index b5c0cbdba2..1c8e108ea8 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -1738,13 +1738,17 @@ class AuthHandler: else: return False - async def start_sso_ui_auth(self, request: SynapseRequest, session_id: str) -> str: + async def start_sso_ui_auth( + self, request: SynapseRequest, session_id: str, preferred_idp_id: str | None + ) -> str: """ Get the HTML for the SSO redirect confirmation page. Args: request: The incoming HTTP request session_id: The user interactive authentication session ID. + preferred_idp_id: The ID of the identity provider to use. If `None` one will + be picked unpredictably from those the user has already signed in with. Returns: The HTML to render. @@ -1768,15 +1772,26 @@ class AuthHandler: # 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())) - 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, - ) + if preferred_idp_id is not None: + # Use the idp specified by the client. + sso_auth_provider = idps.get(preferred_idp_id) + if sso_auth_provider is None: + raise SynapseError( + 400, + f"Unknown preferred Identity Provider: '{preferred_idp_id}'", + errcode=Codes.INVALID_PARAM, + ) + else: + idp_id, sso_auth_provider = next(iter(idps.items())) + if len(idps) > 0: + # We arbitrarily picked an IdP from multiple potential + # candidates. This may be undesirable for the user. + 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 diff --git a/synapse/rest/client/auth.py b/synapse/rest/client/auth.py index f325499044..566c9c98c5 100644 --- a/synapse/rest/client/auth.py +++ b/synapse/rest/client/auth.py @@ -20,13 +20,13 @@ # import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Optional from twisted.web.server import Request from synapse.api.auth.mas import MasDelegatedAuth from synapse.api.constants import LoginType -from synapse.api.errors import LoginError, SynapseError +from synapse.api.errors import Codes, LoginError, SynapseError from synapse.api.urls import CLIENT_API_PREFIX from synapse.http.server import HttpServer, respond_with_html, respond_with_redirect from synapse.http.servlet import RestServlet, parse_string @@ -61,12 +61,27 @@ class AuthRestServlet(RestServlet): hs.config.registration.registration_token_template ) self.success_template = hs.config.registration.fallback_success_template + self._msc4450_enabled = hs.config.experimental.msc4450_enabled async def on_GET(self, request: SynapseRequest, stagetype: str) -> None: session = parse_string(request, "session") if not session: raise SynapseError(400, "No session supplied") + # MSC4450 query parameter which allows clients to specify the Identity Provider + # they wish to use for legacy SSO during User-Interactive Authentication. + idp_id: Optional[str] = None + + if self._msc4450_enabled: + idp_id = parse_string(request, "io.element.idp_id") + + if idp_id is not None and stagetype != LoginType.SSO: + raise SynapseError( + 400, + Codes.INVALID_PARAM, + "idp_id can only be specified for the `m.login.sso` auth type", + ) + # We support the unstable (`org.matrix.cross_signing_reset`) name from MSC4312 until # enough clients have adopted the stable name (`m.oauth`). # Note: `org.matrix.cross_signing_reset` *is* the stable name of the *action* in the @@ -118,7 +133,7 @@ class AuthRestServlet(RestServlet): elif stagetype == LoginType.SSO: # Display a confirmation page which prompts the user to # re-authenticate with their SSO provider. - html = await self.auth_handler.start_sso_ui_auth(request, session) + html = await self.auth_handler.start_sso_ui_auth(request, session, idp_id) elif stagetype == LoginType.REGISTRATION_TOKEN: html = self.registration_token_template.render( diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index ffaf0e5a32..c266937a65 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -43,6 +43,62 @@ from tests.rest.client.utils import TEST_OIDC_CONFIG, TEST_OIDC_ISSUER from tests.server import FakeChannel from tests.unittest import override_config, skip_unless +TEST_MULTIPLE_OIDC_IDP_ID1 = "friendface" +TEST_MULTIPLE_OIDC_IDP_ID2 = "potatocloud" +TEST_MULTIPLE_OIDC_IDP_ID3 = "svnswitch" + +TEST_MULTIPLE_OIDC_ISSUER1 = "https://friendface.invalid/" +TEST_MULTIPLE_OIDC_ISSUER2 = "https://potato.invalid/" +TEST_MULTIPLE_OIDC_ISSUER3 = "https://subversionswitch.invalid/" + +TEST_MULTIPLE_OIDC_PROVIDERS = [ + { + "idp_id": TEST_MULTIPLE_OIDC_IDP_ID1, + "idp_name": "FriendFace", + "idp_icon": "mxc://example.invalid/friendface", + "idp_brand": "friendface", + "issuer": TEST_MULTIPLE_OIDC_ISSUER1, + "client_id": "id-for-friendface", + "client_secret": "secret-for-friendface", + "scopes": ["openid"], + "discover": False, + "authorization_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER1}oidc/auth", + "token_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER1}oidc/token", + "jwks_uri": f"{TEST_MULTIPLE_OIDC_ISSUER1}oidc/jwks", + }, + { + "idp_id": TEST_MULTIPLE_OIDC_IDP_ID2, + "idp_name": "Potato Cloud", + "idp_icon": "mxc://example.invalid/potatocloud", + "idp_brand": "potato", + "issuer": TEST_MULTIPLE_OIDC_ISSUER2, + "client_id": "id-for-potato", + "client_secret": "secret-for-potato", + "scopes": ["openid"], + "discover": False, + "authorization_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER2}oidc/auth", + "token_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER2}oidc/token", + "jwks_uri": f"{TEST_MULTIPLE_OIDC_ISSUER2}oidc/jwks", + }, + { + "idp_id": TEST_MULTIPLE_OIDC_IDP_ID3, + "idp_name": "SubversionSwitch", + "idp_icon": "mxc://example.invalid/svnswitch", + "idp_brand": "svnswitch", + "issuer": TEST_MULTIPLE_OIDC_ISSUER3, + "client_id": "id-for-svnswitch", + "client_secret": "secret-for-svnswitch", + "scopes": ["openid"], + "discover": False, + "authorization_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER3}oidc/auth", + "token_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER3}oidc/token", + "jwks_uri": f"{TEST_MULTIPLE_OIDC_ISSUER3}oidc/jwks", + }, +] +""" +`oidc_providers` config example for multiple OIDC providers. +""" + class DummyRecaptchaChecker(UserInteractiveAuthChecker): def __init__(self, hs: HomeServer) -> None: @@ -506,6 +562,90 @@ class UIAuthTests(unittest.HomeserverTestCase): body={"auth": {"session": session_id}}, ) + @skip_unless(HAS_OIDC, "requires OIDC") + @override_config( + { + "oidc_providers": TEST_MULTIPLE_OIDC_PROVIDERS, + "experimental_features": {"msc4450_enabled": True}, + } + ) + def test_msc4450_select_idp_id(self) -> None: + """ + Test for MSC4450: Identity Provider selection for + User-Interactive Authentication with Legacy Single Sign-On. + + We configure 3 OIDC providers and then check that we can select + which one to redirect to for User-Interactive Authentication. + """ + + # Attach the user to the OIDC providers manually + for idp_id in ( + TEST_MULTIPLE_OIDC_IDP_ID1, + TEST_MULTIPLE_OIDC_IDP_ID2, + ): + self.get_success( + self.hs.get_datastores().main.record_user_external_id( + # `oidc-` is a magic prefix needed for OIDC providers + f"oidc-{idp_id}", + # arbitrary/opaque provider-specific external ID, doesn't really matter + "some.ext.user.id", + self.user, + ) + ) + + # Start a user-interactive authentication session by + # calling the device deletion API + channel = self.delete_device( + self.user_tok, self.device_id, HTTPStatus.UNAUTHORIZED + ) + auth_session = channel.json_body["session"] + flows = channel.json_body["flows"] + self.assertCountEqual( + flows, [{"stages": ["m.login.sso"]}, {"stages": ["m.login.password"]}] + ) + + # Try to start the User-Interactive Auth against both identity providers. + # Make sure we get the identity provider that we ask for. + # + # We need to try against both to be sure that Synapse doesn't just conveniently happen to + # arbitrarily select the identity provider we test. + for idp_id, provider_config in ( + ( + TEST_MULTIPLE_OIDC_IDP_ID1, + TEST_MULTIPLE_OIDC_PROVIDERS[0], + ), + ( + TEST_MULTIPLE_OIDC_IDP_ID2, + TEST_MULTIPLE_OIDC_PROVIDERS[1], + ), + ): + endpoint = f"/_matrix/client/v3/auth/m.login.sso/fallback/web?session={auth_session}&io.element.idp_id=oidc-{idp_id}" + channel = self.make_request("GET", endpoint) + self.assertEqual( + channel.code, + HTTPStatus.OK, + f"Failed to use the {endpoint} endpoint as part of the UIA flow for idp_id={idp_id} : response_body={channel.text_body}", + ) + + # This 'Continue with ...' text is templated by `synapse/res/templates/sso_auth_confirm.html` + self.assertIn( + f"Continue with {provider_config['idp_name']}", + channel.text_body, + ) + + self.assertIn(provider_config["authorization_endpoint"], channel.text_body) + + # Test that we can't use the 3rd OIDC provider as we're not + # registered with it + channel = self.make_request( + "GET", + f"/_matrix/client/v3/auth/m.login.sso/fallback/web?session={auth_session}&io.element.idp_id=oidc-{TEST_MULTIPLE_OIDC_IDP_ID3}", + ) + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.json_body) + self.assertEqual( + channel.json_body["errcode"], Codes.INVALID_PARAM, channel.json_body + ) + @skip_unless(HAS_OIDC, "requires OIDC") @override_config({"oidc_config": TEST_OIDC_CONFIG}) def test_does_not_offer_password_for_sso_user(self) -> None: