Correct FinishPasskeyLogin return-value handling in passkey authentication flow

[Copilot]

PasskeyHandler.FinishAuthentication was using the wrong return value from go-webauthn FinishPasskeyLogin in v0.17.x context. The handler treated the first return as the updated credential, but the credential is returned as the second value.

• Return-value mapping fix

  • Updated FinishAuthentication to read the credential from the second return position of FinishPasskeyLogin.
  • Keeps existing behavior and error handling intact while ensuring credential persistence logic receives the correct object.

• Credential counter persistence correctness

  • The data passed to json.Marshal and PasskeyStore.UpdateCredentialData is now the actual updated WebAuthn credential, not the user object.

// before
updatedCred, _, err := h.WebAuthn.FinishPasskeyLogin(handler, challengeData.SessionData, r)

// after
_, updatedCred, err := h.WebAuthn.FinishPasskeyLogin(handler, challengeData.SessionData, r)

Greptile Summary

This PR fixes a return-value swap bug in FinishAuthentication: FinishPasskeyLogin returns (webauthn.User, *webauthn.Credential, error), but the handler was binding the credential variable to the first return position (the User), causing each login to serialize and persist the user object instead of the updated credential. The fix correctly reads the credential from the second position.

• Return-value fix (handler/passkey.go): swaps updatedCred, _, err to _, updatedCred, err so json.Marshal and UpdateCredentialData receive the actual *webauthn.Credential with the post-ceremony sign count.
• Interface extraction (handler/passkey.go): PasskeyHandler.WebAuthn is changed from the concrete *webauthn.WebAuthn to a new webAuthnProvider interface, enabling the mock-based test that directly verifies the persistence path.
• New regression test (handler/passkey_test.go): TestPasskey_finishAuthentication_updatesCredentialDataWithSignCount drives the discoverable-user handler through the mock and asserts that the marshaled JSON written to UpdateCredentialData carries the expected SignCount.

Confidence Score: 5/5

Safe to merge — the change is a targeted one-line swap that corrects which return value gets persisted, with a new test that directly validates the fixed path end-to-end.

The change touches a single assignment in FinishAuthentication and introduces an interface extraction for testability. The regression test drives the discoverable-user handler through the mock and asserts the exact SignCount value written to the store, leaving no ambiguity about the fix. No error-handling paths are altered, and the rest of the authentication flow (token issuance, cookie setting) is unchanged.

No files require special attention.

Important Files Changed

Filename	Overview
handler/passkey.go	Core bug fix: swaps FinishPasskeyLogin return-value bindings so the *webauthn.Credential (not the webauthn.User) is marshaled and persisted; also extracts a webAuthnProvider interface for testability.
handler/passkey_test.go	Adds mockWebAuthnProvider and a focused regression test that injects a known SignCount=42 credential and asserts the store receives it, directly covering the fixed code path.

Sequence Diagram

sequenceDiagram
    participant Client
    participant FinishAuthentication
    participant DiscoverableUserHandler
    participant PasskeyStore
    participant WebAuthnProvider

    Client->>FinishAuthentication: "POST /passkeys/auth/finish?session_id=..."
    FinishAuthentication->>PasskeyStore: GetAndDeleteChallenge(sessionID)
    PasskeyStore-->>FinishAuthentication: challengeData

    FinishAuthentication->>WebAuthnProvider: FinishPasskeyLogin(handler, sessionData, r)
    WebAuthnProvider->>DiscoverableUserHandler: handler(rawID, userHandle)
    DiscoverableUserHandler->>PasskeyStore: FindCredentialByCredentialID(credID)
    PasskeyStore-->>DiscoverableUserHandler: credential record
    DiscoverableUserHandler->>PasskeyStore: ListCredentialsByUser(userID)
    PasskeyStore-->>DiscoverableUserHandler: existing creds
    DiscoverableUserHandler-->>WebAuthnProvider: passkeyUser
    WebAuthnProvider-->>FinishAuthentication: (_, updatedCred, nil) FIXED

    FinishAuthentication->>PasskeyStore: UpdateCredentialData(authedUserID, authedCredentialID, json(updatedCred))
    PasskeyStore-->>FinishAuthentication: ok

    FinishAuthentication->>Client: 200 OK

Loading

_{Reviews (2): Last reviewed commit: "Merge branch 'main' into copilot/review-..." | Re-trigger Greptile}

[Copilot]

Pull request overview

Fixes the passkey authentication completion flow to persist the correct, updated WebAuthn credential data when using github.com/go-webauthn/webauthn v0.17.x (where FinishPasskeyLogin returns the updated credential as the second return value).

Changes:

• Corrected return-value handling in PasskeyHandler.FinishAuthentication to read the updated credential from the second return position of FinishPasskeyLogin.
• Ensures credential persistence (json.Marshal + PasskeyStore.UpdateCredentialData) operates on the updated credential object rather than the user object.

Show a summary per file

File	Description
handler/passkey.go	Fixes FinishPasskeyLogin return-value mapping so credential counter persistence uses the updated credential.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 1/1 changed files
• Comments generated: 1