Add support for overriding id_token_signing_alg_values_supported for an OpenID identity provider (#18177)

Normally, when `discovery` is enabled,
`id_token_signing_alg_values_supported` comes from the OpenID Discovery
Document (`/.well-known/openid-configuration`). If nothing was
specified, we default to supporting `RS256` in the downstream usage.

This PR just adds support for adding a default/overriding the the
discovered value [just like we do for other things like the
`token_endpoint`](1525a3b4d4/docs/usage/configuration/config_documentation.md (oidc_providers)),
etc.

changelog.d/18177.feature

@@ -0,0 +1 @@
+Add support for specifying/overriding `id_token_signing_alg_values_supported` for an OpenID identity provider.

docs/usage/configuration/config_documentation.md

@@ -3579,6 +3579,24 @@ Options for each entry include:
    to `auto`, which uses PKCE if supported during metadata discovery. Set to `always`
    to force enable PKCE or `never` to force disable PKCE.
 
+* `id_token_signing_alg_values_supported`: List of the JWS signing algorithms (`alg`
+  values) that are supported for signing the `id_token`.
+
+  This is *not* required if `discovery` is disabled. We default to supporting `RS256` in
+  the downstream usage if no algorithms are configured here or in the discovery
+  document.
+
+  According to the spec, the algorithm `"RS256"` MUST be included. The absolute rigid
+  approach would be to reject this provider as non-compliant if it's not included but we
+  simply allow whatever and see what happens (you're the one that configured the value
+  and cooperating with the identity provider).
+
+  The `alg` value `"none"` MAY be supported but can only be used if the Authorization
+  Endpoint does not include `id_token` in the `response_type` (ex.
+  `/authorize?response_type=code` where `none` can apply,
+  `/authorize?response_type=code%20id_token` where `none` can't apply) (such as when
+  using the Authorization Code Flow).
+
 * `scopes`: list of scopes to request. This should normally include the "openid"
    scope. Defaults to `["openid"]`.
 

synapse/config/oidc.py

@@ -125,6 +125,10 @@ OIDC_PROVIDER_CONFIG_SCHEMA = {
             "enum": ["client_secret_basic", "client_secret_post", "none"],
         },
         "pkce_method": {"type": "string", "enum": ["auto", "always", "never"]},
+        "id_token_signing_alg_values_supported": {
+            "type": "array",
+            "items": {"type": "string"},
+        },
         "scopes": {"type": "array", "items": {"type": "string"}},
         "authorization_endpoint": {"type": "string"},
         "token_endpoint": {"type": "string"},
@@ -326,6 +330,9 @@ def _parse_oidc_config_dict(
         client_secret_jwt_key=client_secret_jwt_key,
         client_auth_method=client_auth_method,
         pkce_method=oidc_config.get("pkce_method", "auto"),
+        id_token_signing_alg_values_supported=oidc_config.get(
+            "id_token_signing_alg_values_supported"
+        ),
         scopes=oidc_config.get("scopes", ["openid"]),
         authorization_endpoint=oidc_config.get("authorization_endpoint"),
         token_endpoint=oidc_config.get("token_endpoint"),
@@ -402,6 +409,34 @@ class OidcProviderConfig:
     # Valid values are 'auto', 'always', and 'never'.
     pkce_method: str
 
+    id_token_signing_alg_values_supported: Optional[List[str]]
+    """
+    List of the JWS signing algorithms (`alg` values) that are supported for signing the
+    `id_token`.
+
+    This is *not* required if `discovery` is disabled. We default to supporting `RS256`
+    in the downstream usage if no algorithms are configured here or in the discovery
+    document.
+
+    According to the spec, the algorithm `"RS256"` MUST be included. The absolute rigid
+    approach would be to reject this provider as non-compliant if it's not included but
+    we can just allow whatever and see what happens (they're the ones that configured
+    the value and cooperating with the identity provider). It wouldn't be wise to add it
+    ourselves because absence of `RS256` might indicate that the provider actually
+    doesn't support it, despite the spec requirement. Adding it silently could lead to
+    failed authentication attempts or strange mismatch attacks.
+
+    The `alg` value `"none"` MAY be supported but can only be used if the Authorization
+    Endpoint does not include `id_token` in the `response_type` (ex.
+    `/authorize?response_type=code` where `none` can apply,
+    `/authorize?response_type=code%20id_token` where `none` can't apply) (such as when
+    using the Authorization Code Flow).
+
+    Spec:
+     - https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
+     - https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationExamples
+    """
+
     # list of scopes to request
     scopes: Collection[str]
 

synapse/handlers/oidc.py

@@ -640,6 +640,11 @@ class OidcProvider:
         elif self._config.pkce_method == "never":
             metadata.pop("code_challenge_methods_supported", None)
 
+        if self._config.id_token_signing_alg_values_supported:
+            metadata["id_token_signing_alg_values_supported"] = (
+                self._config.id_token_signing_alg_values_supported
+            )
+
         self._validate_metadata(metadata)
 
         return metadata

tests/handlers/test_oidc.py

@@ -70,12 +70,16 @@ DEFAULT_CONFIG = {
 }
 
 # extends the default config with explicit OAuth2 endpoints instead of using discovery
+#
+# We add "explicit" to things to make them different from the discovered values to make
+# sure that the explicit values override the discovered ones.
 EXPLICIT_ENDPOINT_CONFIG = {
     **DEFAULT_CONFIG,
     "discover": False,
-    "authorization_endpoint": ISSUER + "authorize",
+    "authorization_endpoint": ISSUER + "authorize-explicit",
-    "token_endpoint": ISSUER + "token",
+    "token_endpoint": ISSUER + "token-explicit",
-    "jwks_uri": ISSUER + "jwks",
+    "jwks_uri": ISSUER + "jwks-explicit",
+    "id_token_signing_alg_values_supported": ["RS256", "<explicit>"],
 }
 
 
@@ -259,12 +263,64 @@ class OidcHandlerTestCase(HomeserverTestCase):
         self.get_success(self.provider.load_metadata())
         self.fake_server.get_metadata_handler.assert_not_called()
 
+    @override_config({"oidc_config": {**EXPLICIT_ENDPOINT_CONFIG, "discover": True}})
+    def test_discovery_with_explicit_config(self) -> None:
+        """
+        The handler should discover the endpoints from OIDC discovery document but
+        values are overriden by the explicit config.
+        """
+        # This would throw if some metadata were invalid
+        metadata = self.get_success(self.provider.load_metadata())
+        self.fake_server.get_metadata_handler.assert_called_once()
+
+        self.assertEqual(metadata.issuer, self.fake_server.issuer)
+        # It seems like authlib does not have that defined in its metadata models
+        self.assertEqual(
+            metadata.get("userinfo_endpoint"),
+            self.fake_server.userinfo_endpoint,
+        )
+
+        # Ensure the values are overridden correctly since these were configured
+        # explicitly
+        self.assertEqual(
+            metadata.authorization_endpoint,
+            EXPLICIT_ENDPOINT_CONFIG["authorization_endpoint"],
+        )
+        self.assertEqual(
+            metadata.token_endpoint, EXPLICIT_ENDPOINT_CONFIG["token_endpoint"]
+        )
+        self.assertEqual(metadata.jwks_uri, EXPLICIT_ENDPOINT_CONFIG["jwks_uri"])
+        self.assertEqual(
+            metadata.id_token_signing_alg_values_supported,
+            EXPLICIT_ENDPOINT_CONFIG["id_token_signing_alg_values_supported"],
+        )
+
+        # subsequent calls should be cached
+        self.reset_mocks()
+        self.get_success(self.provider.load_metadata())
+        self.fake_server.get_metadata_handler.assert_not_called()
+
     @override_config({"oidc_config": EXPLICIT_ENDPOINT_CONFIG})
     def test_no_discovery(self) -> None:
         """When discovery is disabled, it should not try to load from discovery document."""
-        self.get_success(self.provider.load_metadata())
+        metadata = self.get_success(self.provider.load_metadata())
         self.fake_server.get_metadata_handler.assert_not_called()
 
+        # Ensure the values are overridden correctly since these were configured
+        # explicitly
+        self.assertEqual(
+            metadata.authorization_endpoint,
+            EXPLICIT_ENDPOINT_CONFIG["authorization_endpoint"],
+        )
+        self.assertEqual(
+            metadata.token_endpoint, EXPLICIT_ENDPOINT_CONFIG["token_endpoint"]
+        )
+        self.assertEqual(metadata.jwks_uri, EXPLICIT_ENDPOINT_CONFIG["jwks_uri"])
+        self.assertEqual(
+            metadata.id_token_signing_alg_values_supported,
+            EXPLICIT_ENDPOINT_CONFIG["id_token_signing_alg_values_supported"],
+        )
+
     @override_config({"oidc_config": DEFAULT_CONFIG})
     def test_load_jwks(self) -> None:
         """JWKS loading is done once (then cached) if used."""