feat: webauthn: support discoverable credentials

[dbarrosop]

PR Type

Enhancement

---

Description

• Implement discoverable credentials for WebAuthn

• Remove userHandle from signin request

• Update WebAuthn login flow

• Add discoverable login support

---

Changes walkthrough 📝

	Relevant files
Enhancement	7 files server.gen.go Update generated API server code                                                  +66/-67  types.gen.go Remove userHandle from SignInWebauthnRequest                          +0/-122  post_signin_webauthn.go Implement discoverable login for WebAuthn                                +20/-13  post_signin_webauthn_verify.go Add support for discoverable credentials verification        +69/-2    post_signup_webauthn.go Add discoverable flag to WebauthnUser                                        +5/-4      webauthn.go Implement discoverable login functionality                              +66/-12  openapi.yaml Remove userHandle from SignInWebauthnRequest                          +0/-12
Tests	3 files post_signin_webauthn_test.go Update tests for discoverable WebAuthn login                          +34/-146 post_signin_webauthn_verify_test.go Add test for discoverable WebAuthn login                                  +145/-2  post_signup_webauthn_test.go Update tests with discoverable flag                                            +15/-12
Documentation	1 files index.html Add WebAuthn signin and signup demo                                            +194/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 5facbf4)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns Sensitive information exposure: The new implementation includes client-side handling of WebAuthn credentials in the index.html file. While the code appears to use appropriate methods for encoding and decoding credentials, it's crucial to ensure that sensitive information such as private keys or user identifiers are not inadvertently exposed or stored insecurely on the client side. A thorough review of the credential handling process, especially in the arrayBufferToBase64URL and base64URLToArrayBuffer functions, is recommended to prevent any potential leakage of sensitive data.

⚡ Recommended focus areas for review Security Concern The new implementation for discoverable credentials might introduce security risks if not properly validated. Ensure that the user authentication process is still secure when using discoverable credentials. func (ctrl *Controller) PostSigninWebauthnVerifyUserHandle( ctx context.Context, response *protocol.ParsedCredentialAssertionData, logger *slog.Logger, ) webauthn.DiscoverableUserHandler { return func(_, userHandle []byte) (webauthn.User, error) { // we need to encoide it back because the client treats it as a string including the hyphens b, err := json.Marshal(protocol.URLEncodedBase64(userHandle)) if err != nil { return nil, fmt.Errorf("failed to marshal user handle: %w", err) } userID, err := uuid.Parse(string(b)) if err != nil { return nil, fmt.Errorf("failed to parse user ID: %w", err) } keys, apiErr := ctrl.wf.GetUserSecurityKeys(ctx, userID, logger) if apiErr != nil { return nil, apiErr } creds, apiErr := webauthnCredentials(keys, logger) if apiErr != nil { return nil, apiErr } // we don't track the flags so we just copy them for i, userCreds := range creds { if bytes.Equal(response.RawID, userCreds.ID) { userCreds.Flags = webauthn.CredentialFlags{ UserPresent: response.Response.AuthenticatorData.Flags.UserPresent(), UserVerified: response.Response.AuthenticatorData.Flags.UserVerified(), BackupEligible: response.Response.AuthenticatorData.Flags.HasBackupEligible(), BackupState: response.Response.AuthenticatorData.Flags.HasBackupState(), } creds[i] = userCreds } } response.Response.UserHandle = []byte(userID.String()) return WebauthnUser{ ID: userID, Name: "", Email: "", Credentials: creds, Discoverable: true, }, nil } } Potential Bug The changes in the FinishLogin and FinishDiscoverableLogin functions may lead to unexpected behavior. Verify that the logic for handling different types of logins (discoverable and non-discoverable) is correct and doesn't introduce any regressions. func (w *Webauthn) FinishLogin( response *protocol.ParsedCredentialAssertionData, userHandler webauthn.DiscoverableUserHandler, logger *slog.Logger, ) (*webauthn.Credential, WebauthnUser, *APIError) { challenge, ok := w.Storage[response.Response.CollectedClientData.Challenge] if !ok { logger.Info("webauthn challenge not found") return nil, WebauthnUser{}, ErrInvalidRequest } if challenge.User.Discoverable { return w.FinishDiscoverableLogin(response, userHandler, logger) } // we don't track the flags so we just copy them for i, userCreds := range challenge.User.Credentials { if bytes.Equal(response.RawID, userCreds.ID) { userCreds.Flags = webauthn.CredentialFlags{ UserPresent: response.Response.AuthenticatorData.Flags.UserPresent(), UserVerified: response.Response.AuthenticatorData.Flags.UserVerified(), BackupEligible: response.Response.AuthenticatorData.Flags.HasBackupEligible(), BackupState: response.Response.AuthenticatorData.Flags.HasBackupState(), } challenge.User.Credentials[i] = userCreds } } // we do this in case the userHandle hasn't been urlencoded by the library b, err := json.Marshal(protocol.URLEncodedBase64(response.Response.UserHandle)) if err == nil { potentialUUID, err := uuid.Parse(string(b)) if err == nil && bytes.Equal(potentialUUID[:], challenge.User.ID[:]) { response.Response.UserHandle = challenge.User.WebAuthnID() } } cred, err := w.wa.ValidateLogin(challenge.User, challenge.Session, response) if err != nil { logger.Info("failed to validate webauthn login", logError(err)) return nil, WebauthnUser{}, ErrInvalidRequest } w.cleanCache() return cred, challenge.User, nil } func (w *Webauthn) BeginDiscoverableLogin( logger *slog.Logger, ) (*protocol.CredentialAssertion, *APIError) { w.cleanCache() challenge, sessionData, err := w.wa.BeginDiscoverableLogin() if err != nil { logger.Error("failed to begin discoverable webauthn login", logError(err)) return nil, ErrInternalServerError } w.Storage[challenge.Response.Challenge.String()] = WebauthnChallenge{ Session: *sessionData, User: WebauthnUser{ ID: uuid.Nil, Name: "", Email: "", Credentials: []webauthn.Credential{}, Discoverable: true, }, Options: nil, } return challenge, nil } func (w *Webauthn) FinishDiscoverableLogin( response *protocol.ParsedCredentialAssertionData, userHandler webauthn.DiscoverableUserHandler, logger *slog.Logger, ) (*webauthn.Credential, WebauthnUser, *APIError) { challenge, ok := w.Storage[response.Response.CollectedClientData.Challenge] if !ok { logger.Info("webauthn challenge not found") return nil, WebauthnUser{}, ErrInvalidRequest } cred, err := w.wa.ValidateDiscoverableLogin(userHandler, challenge.Session, response) if err != nil { logger.Info("failed to validate webauthn discoverable login", logError(err)) return nil, WebauthnUser{}, ErrInvalidRequest } Security Best Practices The new client-side implementation for WebAuthn signin and signup should be reviewed for adherence to security best practices, especially in handling of credentials and error scenarios. <!- python3 -m http.server 3000 open http://localhost:3000/index.html -> <!DOCTYPE html> <html> <head> <title>WebAuthn Signin</title> </head> <body> <h1>WebAuthn Signin</h1> <button onclick="startSignin()">Start Signin</button> <div id="emailForm" style="display: none; margin-top: 20px;"> <p>No credentials found. Please enter your email to register:</p> <input type="email" id="emailInput" placeholder="Enter your email"> <button onclick="startSignup()">Register</button> </div> <script> async function startSignin() { try { // First POST request to /signin/webauthn const initialResponse = await fetch('http://localhost:4000/signin/webauthn', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: '{}' }); if (!initialResponse.ok) { throw new Error('Initial request failed'); } // Get the options from the response let options = await initialResponse.json(); // Convert base64 strings to ArrayBuffer where needed if (options.challenge) { options.challenge = base64URLToArrayBuffer(options.challenge); } if (options.allowCredentials) { options.allowCredentials = options.allowCredentials.map(credential => ({ ...credential, id: base64URLToArrayBuffer(credential.id) })); } // Call navigator.credentials.get with the options const credential = await navigator.credentials.get({ publicKey: options }); console.log(arrayBufferToBase64URL(credential.response.userHandle)) // Prepare the credential data for sending to server const verifyData = { id: credential.id, rawId: arrayBufferToBase64URL(credential.rawId), response: { authenticatorData: arrayBufferToBase64URL(credential.response.authenticatorData), clientDataJSON: arrayBufferToBase64URL(credential.response.clientDataJSON), signature: arrayBufferToBase64URL(credential.response.signature), userHandle: credential.response.userHandle ? arrayBufferToBase64URL(credential.response.userHandle) : null }, type: credential.type }; // Second POST request to /signin/webauthn/verify const verifyResponse = await fetch('http://localhost:4000/signin/webauthn/verify', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ credential: verifyData }) }); if (!verifyResponse.ok) { throw new Error('Verification failed'); } const result = await verifyResponse.json(); console.log('Signin successful:', result); document.getElementById('emailForm').style.display = 'none'; } catch (error) { console.error('Error during signin:', error); // Show email form for registration document.getElementById('emailForm').style.display = 'block'; } } async function startSignup() { const email = document.getElementById('emailInput').value; if (!email) { alert('Please enter an email address'); return; } try { // First POST request to /signup/webauthn const initialResponse = await fetch('http://localhost:4000/signup/webauthn', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ email }) }); if (!initialResponse.ok) { throw new Error('Initial signup request failed'); } // Get the options from the response let options = await initialResponse.json(); // Convert base64 strings to ArrayBuffer where needed if (options.challenge) { options.challenge = base64URLToArrayBuffer(options.challenge); } if (options.user && options.user.id) { options.user.id = base64URLToArrayBuffer(options.user.id); } // Call navigator.credentials.create with the options const credential = await navigator.credentials.create({ publicKey: options }); // Prepare the credential data for sending to server const verifyData = { id: credential.id, rawId: arrayBufferToBase64URL(credential.rawId), response: { attestationObject: arrayBufferToBase64URL(credential.response.attestationObject), clientDataJSON: arrayBufferToBase64URL(credential.response.clientDataJSON) }, type: credential.type }; // Second POST request to /signup/webauthn/verify const verifyResponse = await fetch('http://localhost:4000/signup/webauthn/verify', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ credential: verifyData }) }); if (!verifyResponse.ok) { throw new Error('Signup verification failed'); } const result = await verifyResponse.json(); console.log('Signup successful:', result); document.getElementById('emailForm').style.display = 'none'; } catch (error) { console.error('Error during signup:', error); alert('Registration failed. Please try again.'); } } // Helper function to convert base64URL to ArrayBuffer function base64URLToArrayBuffer(base64URL) { const padding = '='.repeat((4 - base64URL.length % 4) % 4); const base64 = base64URL .replace(/-/g, '+') .replace(/_/g, '/') + padding; const binaryString = window.atob(base64); const bytes = new Uint8Array(binaryString.length); for (let i = 0; i < binaryString.length; i++) { bytes[i] = binaryString.charCodeAt(i); } return bytes.buffer; } // Helper function to convert ArrayBuffer to base64URL function arrayBufferToBase64URL(buffer) { const bytes = new Uint8Array(buffer); let binary = ''; for (let i = 0; i < bytes.byteLength; i++) { binary += String.fromCharCode(bytes[i]); } const base64 = window.btoa(binary); return base64 .replace(/\+/g, '-') .replace(/\//g, '_') .replace(/=/g, ''); } </script> </body> </html>

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 5facbf4
Explore these optional code suggestions:

Category	Suggestion	Score
Security	Add a check to ensure WebAuthn is enabled before proceeding with discoverable login Implement a check to ensure that WebAuthn is enabled for the user before proceeding with the login process. go/controller/post_signin_webauthn.go [63-65] if request.Body.Email == nil { + if !ctrl.config.WebauthnEnabled { + return ctrl.sendError(ErrWebauthnDisabled), nil + } return ctrl.postSigninWebauthnDiscoverableLogin(logger) } Suggestion importance[1-10]: 9 Why: This suggestion enhances security by ensuring WebAuthn is enabled before proceeding with the login process, preventing unauthorized access attempts when the feature is disabled.	9
	Enhance error handling and logging in WebAuthn login validation Add proper error handling and logging for the WebAuthn login validation process. This will help in debugging and improving the security of the authentication flow. go/controller/webauthn.go [220-223] cred, err := w.wa.ValidateLogin(challenge.User, challenge.Session, response) if err != nil { + logger.Error("Failed to validate WebAuthn login", "error", err) return nil, WebauthnUser{}, ErrInvalidRequest } Suggestion importance[1-10]: 8 Why: This suggestion adds crucial error logging for the WebAuthn login validation process, which is critical for security and debugging. It's a high-impact improvement for a security-sensitive area.	8
Possible issue	Add specific error handling for disabled or non-existent users during WebAuthn verification Consider adding error handling for the case where the user is not found or disabled. This will provide more specific feedback to the client. go/controller/post_signin_webauthn_verify.go [99-102] user, apiErr := ctrl.wf.GetUser(ctx, userID, logger) if apiErr != nil { return ctrl.sendError(apiErr), nil } +if !user.Disabled.Valid || user.Disabled.Bool { + return ctrl.sendError(ErrUserDisabled), nil +} Suggestion importance[1-10]: 8 Why: This suggestion adds important error handling for disabled or non-existent users, which improves security and user experience by providing more specific feedback.	8
	Improve error handling and logging for user handle parsing in WebAuthn login process Handle potential errors when parsing the user handle as a UUID. Consider using a more robust error handling approach or a different method to validate the user handle. go/controller/webauthn.go [211-218] b, err := json.Marshal(protocol.URLEncodedBase64(response.Response.UserHandle)) -if err == nil { +if err != nil { + logger.Warn("Failed to marshal user handle", "error", err) +} else { potentialUUID, err := uuid.Parse(string(b)) - if err == nil && bytes.Equal(potentialUUID[:], challenge.User.ID[:]) { + if err != nil { + logger.Warn("Failed to parse user handle as UUID", "error", err) + } else if bytes.Equal(potentialUUID[:], challenge.User.ID[:]) { response.Response.UserHandle = challenge.User.WebAuthnID() } } Suggestion importance[1-10]: 7 Why: The suggestion improves error handling and adds logging, which enhances debugging capabilities and robustness of the code. This is important for security-related features like WebAuthn.	7

---

Previous suggestions

Suggestions up to commit 71f708e

Category	Suggestion	Score
Possible issue	Add proper error handling for JSON marshaling operation to prevent potential issues with unhandled errors Consider adding error handling for the json.Marshal operation in the FinishLogin function. If the marshaling fails, the current code continues execution without addressing the error, which could lead to unexpected behavior. go/controller/webauthn.go [215-222] b, err := json.Marshal(protocol.URLEncodedBase64(response.Response.UserHandle)) -if err == nil { - potentialUUID, err := uuid.Parse(string(b)) - if err == nil && bytes.Equal(potentialUUID[:], challenge.User.ID[:]) { - response.Response.UserHandle = challenge.User.WebAuthnID() - } +if err != nil { + logger.Error("failed to marshal UserHandle", logError(err)) + return nil, WebauthnUser{}, ErrInternalServerError +} +potentialUUID, err := uuid.Parse(string(b)) +if err == nil && bytes.Equal(potentialUUID[:], challenge.User.ID[:]) { + response.Response.UserHandle = challenge.User.WebAuthnID() } Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential issue with error handling, which could lead to unexpected behavior. Proper error handling is crucial for robust and reliable code, especially in security-sensitive operations like WebAuthn.	8
	Add validation to handle cases where both Email and UserHandle are nil in the request body Consider adding error handling for the case when both Email and UserHandle are nil in the request body. This would prevent potential nil pointer dereferences and improve the robustness of the function. go/controller/post_signin_webauthn.go [63-65] +if request.Body.Email == nil && request.Body.UserHandle == nil { + return ctrl.sendError(ErrInvalidRequest), nil +} if request.Body.Email == nil { return ctrl.postSigninWebauthnDiscoverableLogin(logger) } Suggestion importance[1-10]: 7 Why: This suggestion improves error handling by explicitly checking for invalid input where both Email and UserHandle are nil. It prevents potential issues and improves the function's robustness.	7
	Add a check for empty userHandle to prevent potential errors when parsing invalid user handles Consider adding a check for empty userHandle before attempting to parse it as a UUID. This would prevent potential panics or errors when dealing with invalid user handles. go/controller/post_signin_webauthn_verify.go [28-31] +if len(userHandle) == 0 { + return nil, fmt.Errorf("empty user handle") +} userID, err := uuid.Parse(string(b)) if err != nil { return nil, fmt.Errorf("failed to parse user ID: %w", err) } Suggestion importance[1-10]: 6 Why: This suggestion adds a safety check for empty userHandle before parsing it as a UUID. It helps prevent potential panics or errors when dealing with invalid user handles, improving the function's reliability.	6
General	Check for WebAuthn support in the browser before attempting to use it to prevent errors in unsupported environments In the startSignin function, consider adding a check for browser support of WebAuthn before attempting to use it. This can prevent potential errors in unsupported browsers and provide a better user experience. index.html [20-30] async function startSignin() { + if (!window.PublicKeyCredential) { + console.error('WebAuthn is not supported in this browser'); + alert('Your browser does not support WebAuthn. Please use a modern browser.'); + return; + } try { // First POST request to /signin/webauthn const initialResponse = await fetch('http://localhost:4000/signin/webauthn', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: '{}' }); Suggestion importance[1-10]: 7 Why: This suggestion improves user experience by checking for WebAuthn support before attempting to use it. It prevents potential errors in unsupported browsers and provides clear feedback to users, enhancing the overall reliability of the application.	7

[github-actions]

Persistent review updated to latest commit 5facbf4