Skip to content

Commit

Permalink
Handle WebAuthn challenge having multiple factors
Browse files Browse the repository at this point in the history
We made some big changes late in the game for allowing multiple WebAuthn
factors for a given (email, user_handle) pair, but didn't update this in
the authentication challenge flow.

Since we cannot remove the old link, this adds an additional link and
ignores the old one.
  • Loading branch information
scotttrinh committed Mar 1, 2024
1 parent 44c9c6d commit d7a17c7
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 16 deletions.
3 changes: 3 additions & 0 deletions edb/lib/ext/auth.edgeql
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ CREATE EXTENSION PACKAGE auth VERSION '1.0' {
create required link factor: ext::auth::WebAuthnFactor {
create constraint exclusive;
};
create required multi link factors: ext::auth::WebAuthnFactor {
create constraint exclusive;
};
};

create type ext::auth::PKCEChallenge extending ext::auth::Auditable {
Expand Down
9 changes: 5 additions & 4 deletions edb/server/protocol/auth_ext/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,14 @@ class WebAuthnAuthenticationChallenge:
created_at: datetime.datetime
modified_at: datetime.datetime
challenge: bytes
factor: WebAuthnFactor
factors: list[WebAuthnFactor]

def __init__(self, *, id, created_at, modified_at, challenge, factor):
def __init__(self, *, id, created_at, modified_at, challenge, factors):
self.id = id
self.created_at = created_at
self.modified_at = modified_at
self.challenge = base64.b64decode(challenge)
self.factor = (
self.factors = [
WebAuthnFactor(**factor) if isinstance(factor, dict) else factor
)
for factor in factors
]
33 changes: 21 additions & 12 deletions edb/server/protocol/auth_ext/webauthn.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,18 +315,19 @@ async def create_authentication_options_for_email(
challenge := <bytes>$challenge,
user_handle := <bytes>$user_handle,
email := <str>$email,
factor := (
assert_exists(assert_single((
factors := (
assert_exists((
select ext::auth::WebAuthnFactor
filter .user_handle = user_handle
and .email = email
)))
))
)
insert ext::auth::WebAuthnAuthenticationChallenge {
challenge := challenge,
factor := factor,
factor := assert_exists(assert_single((select factors limit 1))),
factors := factors,
}
unless conflict on .factor
unless conflict on .factors
else (
update ext::auth::WebAuthnAuthenticationChallenge
set {
Expand Down Expand Up @@ -389,7 +390,7 @@ async def _get_authentication_challenge(
created_at,
modified_at,
challenge,
factor: {
factors: {
id,
created_at,
modified_at,
Expand All @@ -407,7 +408,7 @@ async def _get_authentication_challenge(
}
},
}
filter .factor.email = email and .factor.credential_id = credential_id;""",
filter .factors.email = email and .factors.credential_id = credential_id;""",
variables={
"email": email,
"credential_id": credential_id,
Expand Down Expand Up @@ -437,7 +438,7 @@ async def _delete_authentication_challenges(
email := <str>$email,
credential_id := <bytes>$credential_id,
delete ext::auth::WebAuthnAuthenticationChallenge
filter .factor.email = email and .factor.credential_id = credential_id;""",
filter .factors.email = email and .factors.credential_id = credential_id;""",
variables={
"email": email,
"credential_id": credential_id,
Expand All @@ -461,13 +462,21 @@ async def authenticate(
credential_id=credential.raw_id,
)

factor = next(
(
f
for f in authentication_challenge.factors
if f.credential_id == credential.raw_id
),
None,
)
assert factor is not None, "Missing factor for the given credential."

try:
webauthn.verify_authentication_response(
credential=credential,
expected_challenge=authentication_challenge.challenge,
credential_public_key=(
authentication_challenge.factor.public_key
),
credential_public_key=factor.public_key,
credential_current_sign_count=0,
expected_rp_id=self.provider.relying_party_id,
expected_origin=self.provider.relying_party_origin,
Expand All @@ -477,4 +486,4 @@ async def authenticate(
"Invalid authentication response. Please retry authentication."
)

return authentication_challenge.factor.identity
return factor.identity

0 comments on commit d7a17c7

Please sign in to comment.