Fallback to PKCE RFC parameter names (#7034)

docs/guides/auth/built_in_ui.rst

@@ -55,6 +55,20 @@ end of the flow. Take this ``verifier`` string, hash it with SHA256, and then
 base64url encode the resulting string. This new string is called the
 ``challenge``.
 
+.. note::
+
+   If you are familiar with PKCE, you will notice some differences from how RFC
+   7636 defines PKCE. Our authentication flow is not an OAuth flow, but rather a
+   strict server-to-server flow with Proof Key of Code Exchange added for
+   additional security to avoid leaking the authentication token. Here are some
+   differences between PKCE as defined in RFC 7636 and our implementation:
+
+   - We do not support the ``plain`` value for ``code_challenge_method``, and
+     therefore do not read that value if provided in requests.
+   - Our parameters omit the ``code_`` prefix, however we do support
+     ``code_challenge`` and ``code_verifier`` as aliases, preferring
+     ``challenge`` and ``verifier`` if present.
+
 .. code-block:: javascript
 
    import http from "node:http";

docs/guides/auth/email_password.rst

@@ -58,6 +58,20 @@ end of the flow. Take this ``verifier`` string, hash it with SHA256, and then
 base64url encode the resulting string. This new string is called the
 ``challenge``.
 
+.. note::
+
+   If you are familiar with PKCE, you will notice some differences from how RFC
+   7636 defines PKCE. Our authentication flow is not an OAuth flow, but rather a
+   strict server-to-server flow with Proof Key of Code Exchange added for
+   additional security to avoid leaking the authentication token. Here are some
+   differences between PKCE as defined in RFC 7636 and our implementation:
+
+   - We do not support the ``plain`` value for ``code_challenge_method``, and
+     therefore do not read that value if provided in requests.
+   - Our parameters omit the ``code_`` prefix, however we do support
+     ``code_challenge`` and ``code_verifier`` as aliases, preferring
+     ``challenge`` and ``verifier`` if present.
+
 .. lint-off
 
 .. code-block:: javascript

docs/guides/auth/oauth.rst

@@ -58,6 +58,20 @@ end of the flow. Take this ``verifier`` string, hash it with SHA256, and then
 base64url encode the resulting string. This new string is called the
 ``challenge``.
 
+.. note::
+
+   If you are familiar with PKCE, you will notice some differences from how RFC
+   7636 defines PKCE. Our authentication flow is not an OAuth flow, but rather a
+   strict server-to-server flow with Proof Key of Code Exchange added for
+   additional security to avoid leaking the authentication token. Here are some
+   differences between PKCE as defined in RFC 7636 and our implementation:
+
+   - We do not support the ``plain`` value for ``code_challenge_method``, and
+     therefore do not read that value if provided in requests.
+   - Our parameters omit the ``code_`` prefix, however we do support
+     ``code_challenge`` and ``code_verifier`` as aliases, preferring
+     ``challenge`` and ``verifier`` if present.
+
 .. code-block:: javascript
 
    import http from "node:http";

edb/server/protocol/auth_ext/http.py

@@ -253,7 +253,9 @@ async def handle_authorize(self, request: Any, response: Any):
             raise errors.InvalidData(
                 "Redirect URL does not match any allowed URLs.",
             )
-        challenge = _get_search_param(query, "challenge")
+        challenge = _get_search_param(
+            query, "challenge", fallback_keys=["code_challenge"]
+        )
         oauth_client = oauth.Client(
             db=self.db, provider_name=provider_name, base_url=self.test_url
         )
@@ -386,7 +388,9 @@ async def handle_token(self, request: Any, response: Any):
             request.url.query.decode("ascii") if request.url.query else ""
         )
         code = _get_search_param(query, "code")
-        verifier = _get_search_param(query, "verifier")
+        verifier = _get_search_param(
+            query, "verifier", fallback_keys=["code_verifier"]
+        )
 
         verifier_size = len(verifier)
 
@@ -1338,7 +1342,9 @@ async def handle_ui_forgot_password(self, request: Any, response: Any):
             query = urllib.parse.parse_qs(
                 request.url.query.decode("ascii") if request.url.query else ""
             )
-            challenge = _get_search_param(query, "challenge")
+            challenge = _get_search_param(
+                query, "challenge", fallback_keys=["code_challenge"]
+            )
             app_details_config = self._get_app_details_config()
 
             response.status = http.HTTPStatus.OK
@@ -1660,9 +1666,7 @@ def _make_session_token(self, identity_id: str) -> str:
             claims=claims,
         )
         session_token.make_signed_token(signing_key)
-        metrics.auth_successful_logins.inc(
-            1.0, self.tenant.get_instance_name()
-        )
+        metrics.auth_successful_logins.inc(1.0, self.tenant.get_instance_name())
         return session_token.serialize()
 
     def _get_from_claims(self, state: str, key: str) -> str:
@@ -1987,8 +1991,18 @@ def _maybe_get_search_param(
     return params[0] if params else None
 
 
-def _get_search_param(query_dict: dict[str, list[str]], key: str) -> str:
+def _get_search_param(
+    query_dict: dict[str, list[str]],
+    key: str,
+    *,
+    fallback_keys: Optional[list[str]] = None,
+) -> str:
     val = _maybe_get_search_param(query_dict, key)
+    if val is None and fallback_keys is not None:
+        for fallback_key in fallback_keys:
+            val = _maybe_get_search_param(query_dict, fallback_key)
+            if val is not None:
+                break
     if val is None:
         raise errors.InvalidData(f"Missing query parameter: {key}")
     return val
@@ -2010,7 +2024,9 @@ def _get_pkce_challenge(
     query_dict: dict[str, list[str]],
 ) -> str | None:
     cookie_name = 'edgedb-pkce-challenge'
-    challenge: str | None = _maybe_get_search_param(query_dict, 'challenge')
+    challenge: str | None = _maybe_get_search_param(
+        query_dict, 'challenge'
+    ) or _maybe_get_search_param(query_dict, "code_challenge")
     if challenge is not None:
         _set_cookie(response, cookie_name, challenge)
     else:

tests/test_http_ext_auth.py

@@ -1071,7 +1071,7 @@ async def test_http_auth_ext_discord_authorize_01(self):
             query = {
                 "provider": provider_name,
                 "redirect_to": redirect_to,
-                "challenge": challenge,
+                "code_challenge": challenge,
             }
 
             _, headers, status = self.http_con_request(
@@ -3042,7 +3042,7 @@ async def test_http_auth_ext_token_01(self):
                 http_con,
                 {
                     "code": pkce.id,
-                    "verifier": base64.urlsafe_b64encode(os.urandom(43))
+                    "code_verifier": base64.urlsafe_b64encode(os.urandom(43))
                     .rstrip(b"=")
                     .decode(),
                 },