diff --git a/changelog.d/18197.feature b/changelog.d/18197.feature new file mode 100644 index 0000000000..4572ac3bdb --- /dev/null +++ b/changelog.d/18197.feature @@ -0,0 +1 @@ +Add support for specifying/overriding `redirect_uri` in the authorization and token requests against an OpenID identity provider. \ No newline at end of file diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index ffee089304..d2d282f203 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3662,6 +3662,13 @@ Options for each entry include: not included in `scopes`. Set to `userinfo_endpoint` to always use the userinfo endpoint. +* `redirect_uri`: An optional string, that if set will override the `redirect_uri` + parameter sent in the requests to the authorization and token endpoints. + Useful if you want to redirect the client to another endpoint as part of the + OIDC login. Be aware that the client must then call Synapse's OIDC callback + URL (`/_synapse/client/oidc/callback`) manually afterwards. + Must be a valid URL including scheme and path. + * `additional_authorization_parameters`: String to string dictionary that will be passed as additional parameters to the authorization grant URL. diff --git a/synapse/config/oidc.py b/synapse/config/oidc.py index fc4bc35b30..8ba0ba2c36 100644 --- a/synapse/config/oidc.py +++ b/synapse/config/oidc.py @@ -141,6 +141,9 @@ OIDC_PROVIDER_CONFIG_SCHEMA = { "type": "string", "enum": ["auto", "userinfo_endpoint"], }, + "redirect_uri": { + "type": ["string", "null"], + }, "allow_existing_users": {"type": "boolean"}, "user_mapping_provider": {"type": ["object", "null"]}, "attribute_requirements": { @@ -344,6 +347,7 @@ def _parse_oidc_config_dict( ), skip_verification=oidc_config.get("skip_verification", False), user_profile_method=oidc_config.get("user_profile_method", "auto"), + redirect_uri=oidc_config.get("redirect_uri"), allow_existing_users=oidc_config.get("allow_existing_users", False), user_mapping_provider_class=user_mapping_provider_class, user_mapping_provider_config=user_mapping_provider_config, @@ -467,6 +471,18 @@ class OidcProviderConfig: # values are: "auto" or "userinfo_endpoint". user_profile_method: str + redirect_uri: Optional[str] + """ + An optional replacement for Synapse's hardcoded `redirect_uri` URL + (`/_synapse/client/oidc/callback`). This can be used to send + the client to a different URL after it receives a response from the + `authorization_endpoint`. + + If this is set, the client is expected to call Synapse's OIDC callback URL + reproduced above itself with the necessary parameters and session cookie, in + order to complete OIDC login. + """ + # whether to allow a user logging in via OIDC to match a pre-existing account # instead of failing allow_existing_users: bool diff --git a/synapse/handlers/oidc.py b/synapse/handlers/oidc.py index 76b692928d..18efdd9f6e 100644 --- a/synapse/handlers/oidc.py +++ b/synapse/handlers/oidc.py @@ -382,7 +382,12 @@ class OidcProvider: self._macaroon_generaton = macaroon_generator self._config = provider - self._callback_url: str = hs.config.oidc.oidc_callback_url + + self._callback_url: str + if provider.redirect_uri is not None: + self._callback_url = provider.redirect_uri + else: + self._callback_url = hs.config.oidc.oidc_callback_url # Calculate the prefix for OIDC callback paths based on the public_baseurl. # We'll insert this into the Path= parameter of any session cookies we set. diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index 5ffc5a90a8..cfd9969563 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -57,6 +57,7 @@ CLIENT_ID = "test-client-id" CLIENT_SECRET = "test-client-secret" BASE_URL = "https://synapse/" CALLBACK_URL = BASE_URL + "_synapse/client/oidc/callback" +TEST_REDIRECT_URI = "https://test/oidc/callback" SCOPES = ["openid"] # config for common cases @@ -586,6 +587,24 @@ class OidcHandlerTestCase(HomeserverTestCase): code_verifier = get_value_from_macaroon(macaroon, "code_verifier") self.assertEqual(code_verifier, "") + @override_config( + {"oidc_config": {**DEFAULT_CONFIG, "redirect_uri": TEST_REDIRECT_URI}} + ) + def test_redirect_request_with_overridden_redirect_uri(self) -> None: + """The authorization endpoint redirect has the overridden `redirect_uri` value.""" + req = Mock(spec=["cookies"]) + req.cookies = [] + + url = urlparse( + self.get_success( + self.provider.handle_redirect_request(req, b"http://client/redirect") + ) + ) + + # Ensure that the redirect_uri in the returned url has been overridden. + params = parse_qs(url.query) + self.assertEqual(params["redirect_uri"], [TEST_REDIRECT_URI]) + @override_config({"oidc_config": DEFAULT_CONFIG}) def test_callback_error(self) -> None: """Errors from the provider returned in the callback are displayed.""" @@ -953,6 +972,37 @@ class OidcHandlerTestCase(HomeserverTestCase): self.assertEqual(args["client_id"], [CLIENT_ID]) self.assertEqual(args["redirect_uri"], [CALLBACK_URL]) + @override_config( + { + "oidc_config": { + **DEFAULT_CONFIG, + "redirect_uri": TEST_REDIRECT_URI, + } + } + ) + def test_code_exchange_with_overridden_redirect_uri(self) -> None: + """Code exchange behaves correctly and handles various error scenarios.""" + # Set up a fake IdP with a token endpoint handler. + token = { + "type": "Bearer", + "access_token": "aabbcc", + } + + self.fake_server.post_token_handler.side_effect = None + self.fake_server.post_token_handler.return_value = FakeResponse.json( + payload=token + ) + code = "code" + + # Exchange the code against the fake IdP. + self.get_success(self.provider._exchange_code(code, code_verifier="")) + + # Check that the `redirect_uri` parameter provided matches our + # overridden config value. + kwargs = self.fake_server.request.call_args[1] + args = parse_qs(kwargs["data"].decode("utf-8")) + self.assertEqual(args["redirect_uri"], [TEST_REDIRECT_URI]) + @override_config( { "oidc_config": {