From 44c9c6d1b7278570e500eda18e7511545f6c5b52 Mon Sep 17 00:00:00 2001 From: Scott Trinh Date: Fri, 1 Mar 2024 08:49:05 -0500 Subject: [PATCH] Allow passing WebAuthn user_handle in request body (#6942) --- edb/server/protocol/auth_ext/http.py | 55 +++++++----------------- edb/server/protocol/auth_ext/webauthn.py | 25 +++++------ tests/test_http_ext_auth.py | 25 ++++++++--- 3 files changed, 47 insertions(+), 58 deletions(-) diff --git a/edb/server/protocol/auth_ext/http.py b/edb/server/protocol/auth_ext/http.py index cdb0be27d2c..86be3537ee7 100644 --- a/edb/server/protocol/auth_ext/http.py +++ b/edb/server/protocol/auth_ext/http.py @@ -947,9 +947,8 @@ async def handle_magic_link_email(self, request: Any, response: Any): "Error redirect URL does not match any allowed URLs.", ) - if ( - maybe_redirect_to and - not self._is_url_allowed(maybe_redirect_to) + if maybe_redirect_to and not self._is_url_allowed( + maybe_redirect_to ): raise errors.InvalidData( "Redirect URL does not match any allowed URLs.", @@ -1090,18 +1089,22 @@ async def handle_webauthn_register(self, request: Any, response: Any): user_handle_cookie = request.cookies.get( "edgedb-webauthn-registration-user-handle" - ).value - if user_handle_cookie is None: + ) + user_handle_base64url: Optional[str] = ( + user_handle_cookie.value + if user_handle_cookie + else data.get("user_handle") + ) + if user_handle_base64url is None: raise errors.InvalidData( - "Missing 'edgedb-webauthn-registration-user-handle' cookie" + "Missing user_handle from cookie or request body" ) try: - user_handle = base64.urlsafe_b64decode(user_handle_cookie) + user_handle = base64.urlsafe_b64decode( + f"{user_handle_base64url}===" + ) except Exception as e: - raise errors.InvalidData( - "Failed to decode 'edgedb-webauthn-registration-user-handle'" - " cookie" - ) from e + raise errors.InvalidData("Failed to decode user_handle") from e require_verification = webauthn_client.provider.require_verification pkce_code: Optional[str] = None @@ -1163,18 +1166,12 @@ async def handle_webauthn_authenticate_options( ) webauthn_client = webauthn.Client(self.db) - (user_handle, registration_options) = ( + (_, registration_options) = ( await webauthn_client.create_authentication_options_for_email( email=email, webauthn_provider=webauthn_provider ) ) - _set_cookie( - response, - "edgedb-webauthn-authentication-user-handle", - user_handle, - path="/", - ) response.status = http.HTTPStatus.OK response.content_type = b"application/json" response.body = registration_options @@ -1192,25 +1189,9 @@ async def handle_webauthn_authenticate(self, request: Any, response: Any): assertion: str = data["assertion"] pkce_challenge: str = data["challenge"] - user_handle_cookie = request.cookies.get( - "edgedb-webauthn-authentication-user-handle" - ).value - if user_handle_cookie is None: - raise errors.InvalidData( - "Missing 'edgedb-webauthn-authentication-user-handle' cookie" - ) - try: - user_handle = base64.urlsafe_b64decode(user_handle_cookie) - except Exception as e: - raise errors.InvalidData( - "Failed to decode 'edgedb-webauthn-authentication-user-handle'" - " cookie" - ) from e - identity = await webauthn_client.authenticate( assertion=assertion, email=email, - user_handle=user_handle, ) require_verification = webauthn_client.provider.require_verification @@ -1226,12 +1207,6 @@ async def handle_webauthn_authenticate(self, request: Any, response: Any): self.db, identity.id, pkce_challenge ) - _set_cookie( - response, - "edgedb-webauthn-authentication-user-handle", - "", - path="/", - ) response.status = http.HTTPStatus.OK response.content_type = b"application/json" response.body = json.dumps( diff --git a/edb/server/protocol/auth_ext/webauthn.py b/edb/server/protocol/auth_ext/webauthn.py index 47385608a9e..578fb5379e3 100644 --- a/edb/server/protocol/auth_ext/webauthn.py +++ b/edb/server/protocol/auth_ext/webauthn.py @@ -376,14 +376,14 @@ async def is_email_verified( async def _get_authentication_challenge( self, email: str, - user_handle: bytes, + credential_id: bytes, ): result = await execute.parse_execute_json( self.db, """ with email := $email, - user_handle := $user_handle, + credential_id := $credential_id, select ext::auth::WebAuthnAuthenticationChallenge { id, created_at, @@ -407,10 +407,10 @@ async def _get_authentication_challenge( } }, } -filter .factor.email = email and .factor.user_handle = user_handle;""", +filter .factor.email = email and .factor.credential_id = credential_id;""", variables={ "email": email, - "user_handle": user_handle, + "credential_id": credential_id, }, cached_globally=True, ) @@ -428,19 +428,19 @@ async def _get_authentication_challenge( async def _delete_authentication_challenges( self, email: str, - user_handle: bytes, + credential_id: bytes, ): await execute.parse_execute_json( self.db, """ with email := $email, - user_handle := $user_handle, + credential_id := $credential_id, delete ext::auth::WebAuthnAuthenticationChallenge -filter .factor.email = email and .factor.user_handle = user_handle;""", +filter .factor.email = email and .factor.credential_id = credential_id;""", variables={ "email": email, - "user_handle": user_handle, + "credential_id": credential_id, }, ) @@ -448,21 +448,22 @@ async def authenticate( self, *, email: str, - user_handle: bytes, assertion: str, ) -> data.LocalIdentity: + credential = parse_authentication_credential_json(assertion) + authentication_challenge = await self._get_authentication_challenge( email=email, - user_handle=user_handle, + credential_id=credential.raw_id, ) await self._delete_authentication_challenges( email=email, - user_handle=user_handle, + credential_id=credential.raw_id, ) try: webauthn.verify_authentication_response( - credential=assertion, + credential=credential, expected_challenge=authentication_challenge.challenge, credential_public_key=( authentication_challenge.factor.public_key diff --git a/tests/test_http_ext_auth.py b/tests/test_http_ext_auth.py index 1d724a38a58..defe9a81707 100644 --- a/tests/test_http_ext_auth.py +++ b/tests/test_http_ext_auth.py @@ -31,7 +31,7 @@ import re import hashlib -from typing import Any, Callable +from typing import Any, Callable, Optional from jwcrypto import jwt, jwk from edgedb import QueryAssertionError @@ -567,15 +567,20 @@ async def extract_jwt_claims(self, raw_jwt: str): claims = json.loads(jwt_token.claims) return claims - def maybe_get_auth_token(self, headers: dict[str, str]) -> str | None: + def maybe_get_cookie_value( + self, headers: dict[str, str], name: str + ) -> Optional[str]: set_cookie = headers.get("set-cookie") if set_cookie is not None: - (k, v) = set_cookie.split(";")[0].split("=") - if k == "edgedb-session": + (k, v) = set_cookie.split(";", 1)[0].split("=", 1) + if k == name: return v return None + def maybe_get_auth_token(self, headers: dict[str, str]) -> Optional[str]: + return self.maybe_get_cookie_value(headers, "edgedb-session") + async def extract_session_claims(self, headers: dict[str, str]): maybe_token = self.maybe_get_auth_token(headers) assert maybe_token is not None @@ -3548,7 +3553,7 @@ async def test_http_auth_ext_webauthn_register_options(self): email = f"{uuid.uuid4()}@example.com" query_params = urllib.parse.urlencode({"email": email}) - body, _, status = self.http_con_request( + body, headers, status = self.http_con_request( http_con, path=f"webauthn/register/options?{query_params}", ) @@ -3599,6 +3604,14 @@ async def test_http_auth_ext_webauthn_register_options(self): user_handle = base64.urlsafe_b64decode( f'{body_json["user"]["id"]}===' ) + user_handle_cookie = self.maybe_get_cookie_value( + headers, "edgedb-webauthn-registration-user-handle" + ) + user_handle_cookie_value = base64.urlsafe_b64decode( + f'{user_handle_cookie}===' + ) + self.assertEqual(user_handle_cookie_value, user_handle) + self.assertTrue( await self.con.query_single( ''' @@ -3752,7 +3765,7 @@ async def test_http_auth_ext_webauthn_authenticate_options(self): public_key=public_key, ) - body, _, status = self.http_con_request( + body, headers, status = self.http_con_request( http_con, path=f"webauthn/authenticate/options?email={email}", )