Support PKCE for OAuth 2.0

changelog.d/14750.feature

@@ -0,0 +1 @@
+Support [RFC7636](https://datatracker.ietf.org/doc/html/rfc7636) Proof Key for Code Exchange for OAuth single sign-on.

docs/usage/configuration/config_documentation.md

@@ -3053,8 +3053,12 @@ Options for each entry include:
    values are `client_secret_basic` (default), `client_secret_post` and
    `none`.
 
+* `code_challenge_method`: proof key for code exchange method used when requesting
+  and exchanging the token. Valid values are `null` (default, disables support),
+  `plain`, and `S256`.
+
 * `scopes`: list of scopes to request. This should normally include the "openid"
-   scope. Defaults to ["openid"].
+   scope. Defaults to `["openid"]`.
 
 * `authorization_endpoint`: the oauth2 authorization endpoint. Required if
    provider discovery is disabled.

synapse/config/oidc.py

@@ -117,6 +117,18 @@ def oidc_enabled(self) -> bool:
             # to avoid importing authlib here.
             "enum": ["client_secret_basic", "client_secret_post", "none"],
         },
+        "code_challenge_method": {
+            "oneOf": [
+                {
+                    "type": "string",
+                    # the following list is the same as the keys of
+                    # authlib.oauth2.auth.rfc7636.challenge.CodeChallenge.SUPPORTED_CODE_CHALLENGE_METHOD.
+                    # We inline it to avoid importing authlib here.
+                    "enum": ["plain", "S256"],
+                },
+                {"type": "null"},
+            ],
+        },
         "scopes": {"type": "array", "items": {"type": "string"}},
         "authorization_endpoint": {"type": "string"},
         "token_endpoint": {"type": "string"},
@@ -289,6 +301,7 @@ def _parse_oidc_config_dict(
         client_secret=oidc_config.get("client_secret"),
         client_secret_jwt_key=client_secret_jwt_key,
         client_auth_method=oidc_config.get("client_auth_method", "client_secret_basic"),
+        code_challenge_method=oidc_config.get("code_challenge_method"),
         scopes=oidc_config.get("scopes", ["openid"]),
         authorization_endpoint=oidc_config.get("authorization_endpoint"),
         token_endpoint=oidc_config.get("token_endpoint"),
@@ -357,6 +370,10 @@ class OidcProviderConfig:
     # 'none'.
     client_auth_method: str
 
+    # code challenge method to use when exchanging the authorization & token.
+    # Valid values are None (to disable use of code challenges), 'plain', and 'S256'.
+    code_challenge_method: Optional[str]
+
     # list of scopes to request
     scopes: Collection[str]
 

synapse/handlers/oidc.py

@@ -36,6 +36,7 @@
 from authlib.jose.errors import InvalidClaimError, JoseError, MissingClaimError
 from authlib.oauth2.auth import ClientAuth
 from authlib.oauth2.rfc6749.parameters import prepare_grant_uri
+from authlib.oauth2.rfc7636.challenge import create_s256_code_challenge
 from authlib.oidc.core import CodeIDToken, UserInfo
 from authlib.oidc.discovery import OpenIDProviderMetadata, get_well_known_url
 from jinja2 import Environment, Template
@@ -403,6 +404,7 @@ def __init__(
             provider.client_auth_method,
         )
         self._client_auth_method = provider.client_auth_method
+        self._code_challenge_method = provider.code_challenge_method
 
         # cache of metadata for the identity provider (endpoint uris, mostly). This is
         # loaded on-demand from the discovery endpoint (if discovery is enabled), with
@@ -475,6 +477,20 @@ def _validate_metadata(self, m: OpenIDProviderMetadata) -> None:
                     )
                 )
 
+        # Validate code_challenge_methods_supported only if it is enabled.
+        if (
+            m.get("code_challenge_methods_supported") is not None
+            and self._code_challenge_method is not None
+        ):
+            m.validate_code_challenge_methods_supported()
+            if self._code_challenge_method not in m["code_challenge_methods_supported"]:
+                raise ValueError(
+                    '"{code_challenge_method}" not in "code_challenge_methods_supported" ({supported!r})'.format(
+                        code_challenge_method=self._code_challenge_method,
+                        supported=m["code_challenge_methods_supported"],
+                    )
+                )
+
         if m.get("response_types_supported") is not None:
             m.validate_response_types_supported()
 
@@ -653,7 +669,7 @@ async def _load_jwks(self) -> JWKS:
 
         return jwk_set
 
-    async def _exchange_code(self, code: str) -> Token:
+    async def _exchange_code(self, code: str, code_verifier: str) -> Token:
         """Exchange an authorization code for a token.
 
         This calls the ``token_endpoint`` with the authorization code we
@@ -666,6 +682,7 @@ async def _exchange_code(self, code: str) -> Token:
 
         Args:
             code: The authorization code we got from the callback.
+            code_verifier: The code verifier to send, blank if unused.
 
         Returns:
             A dict containing various tokens.
@@ -696,6 +713,8 @@ async def _exchange_code(self, code: str) -> Token:
             "code": code,
             "redirect_uri": self._callback_url,
         }
+        if code_verifier:
+            args["code_verifier"] = code_verifier
         body = urlencode(args, True)
 
         # Fill the body/headers with credentials
@@ -935,17 +954,38 @@ async def handle_redirect_request(
 
         state = generate_token()
         nonce = generate_token()
+        code_verifier = ""
 
         if not client_redirect_url:
             client_redirect_url = b""
 
+        # If code challenge is supported and configured, use it.
+        extra_grant_values = {}
+        if self._code_challenge_method is not None:
+            code_verifier = generate_token(48)
+
+            # Default to the values for plain.
+            extra_grant_values = {"code_challenge_method": self._code_challenge_method}
+
+            if self._code_challenge_method == "S256":
+                extra_grant_values["code_challenge"] = create_s256_code_challenge(
+                    code_verifier
+                )
+            elif self._code_challenge_method == "plain":
+                extra_grant_values["code_challenge"] = code_verifier
+            else:
+                raise RuntimeError(
+                    f"Unexpeced code challenege method: {self._code_challenge_method!r}"
+                )
+
         cookie = self._macaroon_generaton.generate_oidc_session_token(
             state=state,
             session_data=OidcSessionData(
                 idp_id=self.idp_id,
                 nonce=nonce,
                 client_redirect_url=client_redirect_url.decode(),
                 ui_auth_session_id=ui_auth_session_id or "",
+                code_verifier=code_verifier,
             ),
         )
 
@@ -976,6 +1016,7 @@ async def handle_redirect_request(
             scope=self._scopes,
             state=state,
             nonce=nonce,
+            **extra_grant_values,
         )
 
     async def handle_oidc_callback(
@@ -1003,7 +1044,9 @@ async def handle_oidc_callback(
         # Exchange the code with the provider
         try:
             logger.debug("Exchanging OAuth2 code for a token")
-            token = await self._exchange_code(code)
+            token = await self._exchange_code(
+                code, code_verifier=session_data.code_verifier
+            )
         except OidcError as e:
             logger.warning("Could not exchange OAuth2 code: %s", e)
             self._sso_handler.render_error(request, e.error, e.error_description)

synapse/util/macaroons.py

@@ -110,6 +110,9 @@ class OidcSessionData:
     ui_auth_session_id: str
     """The session ID of the ongoing UI Auth ("" if this is a login)"""
 
+    code_verifier: str
+    """The random string used in code challenges ("" if code challenges are not used)."""
+
 
 class MacaroonGenerator:
     def __init__(self, clock: Clock, location: str, secret_key: bytes):
@@ -187,6 +190,7 @@ def generate_oidc_session_token(
         macaroon.add_first_party_caveat(
             f"ui_auth_session_id = {session_data.ui_auth_session_id}"
         )
+        macaroon.add_first_party_caveat(f"code_verifier = {session_data.code_verifier}")
         macaroon.add_first_party_caveat(f"time < {expiry}")
 
         return macaroon.serialize()
@@ -278,6 +282,7 @@ def verify_oidc_session_token(self, session: bytes, state: str) -> OidcSessionDa
         v.satisfy_general(lambda c: c.startswith("idp_id = "))
         v.satisfy_general(lambda c: c.startswith("client_redirect_url = "))
         v.satisfy_general(lambda c: c.startswith("ui_auth_session_id = "))
+        v.satisfy_general(lambda c: c.startswith("code_verifier = "))
         satisfy_expiry(v, self._clock.time_msec)
 
         v.verify(macaroon, self._secret_key)
@@ -287,11 +292,13 @@ def verify_oidc_session_token(self, session: bytes, state: str) -> OidcSessionDa
         idp_id = get_value_from_macaroon(macaroon, "idp_id")
         client_redirect_url = get_value_from_macaroon(macaroon, "client_redirect_url")
         ui_auth_session_id = get_value_from_macaroon(macaroon, "ui_auth_session_id")
+        code_verifier = get_value_from_macaroon(macaroon, "code_verifier")
         return OidcSessionData(
             nonce=nonce,
             idp_id=idp_id,
             client_redirect_url=client_redirect_url,
             ui_auth_session_id=ui_auth_session_id,
+            code_verifier=code_verifier,
         )
 
     def _generate_base_macaroon(self, type: MacaroonType) -> pymacaroons.Macaroon: