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: