From 154e23f6d76277cc8012dc7a5dfa5f22d62b9133 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Date: Mon, 3 Mar 2025 09:40:48 +0000
Subject: [PATCH] Add `redirect_uri` option to `oidc_providers` entries
(#18197)
Allows overriding the `redirect_uri` parameter sent to both the
authorization and token endpoints of the IdP. Typically this parameter
is hardcoded to `<public_baseurl>/_synapse/client/oidc/callback`.
Yet it can be useful in certain contexts to allow a different callback
URL. For instance, if you would like to intercept the authorization code
returned from the IdP and do something with it, before eventually
calling Synapse's OIDC callback URL yourself.
This change enables enterprise use cases but does not change the default
behaviour.
---
Best reviewed commit-by-commit.
---------
Co-authored-by: Eric Eastwood <erice@element.io>
---
changelog.d/18197.feature | 1 +
.../configuration/config_documentation.md | 7 +++
synapse/config/oidc.py | 16 ++++++
synapse/handlers/oidc.py | 7 ++-
tests/handlers/test_oidc.py | 50 +++++++++++++++++++
5 files changed, 80 insertions(+), 1 deletion(-)
create mode 100644 changelog.d/18197.feature
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 (`<public_baseurl>/_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
+ (`<public_baseurl>/_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": {
--
GitLab