Simplify WebAuthn process for users of Boilerplate (#10262) #10263
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update refactors the WebAuthn passwordless authentication flow by switching from using usernames to user IDs (Guids) across both client and server components. Adjustments include UI modifications, updated method signatures, enhanced error handling, and the introduction of a new DTO for assertion options requests. Changes span Razor components, controller logic, client-side scripts, and JSON serialization settings to ensure consistent handling of user identifiers and improved robustness in credential operations. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SP as SignInPage
participant JS as JSRuntime
participant IC as IdentityController
participant DB as Database
U->>SP: Initiates Passwordless Sign-In
SP->>JS: Retrieve WebAuthn configuration (userId)
JS-->>SP: Return userId (or null)
alt userId exists
SP->>IC: Request Assertion Options (with DTO containing userId)
IC->>DB: Fetch existing credentials for userId
DB-->>IC: Return credential descriptors
IC-->>SP: Provide assertion options (with updated verification settings)
SP->>JS: Verify credential with provided options
JS-->>SP: Return verification result
else userId missing
SP-->>U: Abort passwordless sign-in
end
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/PasswordlessSection.razor (1)
23-25: Consider documenting the reason for commenting out this featureThe "Delete all credentials" button has been commented out, presumably to simplify the user interface. Consider adding a brief comment explaining why this feature is disabled in the UI but preserved in the codebase for potential future use.
-@* <BitButton AutoLoading OnClick="WrapHandled(DeleteAll)" Variant="BitVariant.Outline" Color="BitColor.Warning"> - Delete all credentials -</BitButton> *@ +@* +<!-- Disabled to simplify user experience. May be re-enabled in future versions --> +<BitButton AutoLoading OnClick="WrapHandled(DeleteAll)" Variant="BitVariant.Outline" Color="BitColor.Warning"> + Delete all credentials +</BitButton> +*@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebAuthn.ts (5)
8-11: Avoid redundant double-negationThere's redundant double-negation that can be simplified.
public static isConfigured(userId: string | undefined) { const storedCredentials = JSON.parse(localStorage.getItem(WebAuthn.STORE_KEY) || '[]') as string[]; - return !!userId ? storedCredentials.includes(userId) : storedCredentials.length > 0; + return userId ? storedCredentials.includes(userId) : storedCredentials.length > 0; }🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
13-16: Consider adding error handling togetConfiguredThis new method retrieves the last credential from the array, but there's no handling for empty arrays which could lead to undefined values.
public static getConfigured() { const storedCredentials = JSON.parse(localStorage.getItem(WebAuthn.STORE_KEY) || '[]') as string[]; - return storedCredentials[storedCredentials.length - 1]; + return storedCredentials.length > 0 ? storedCredentials[storedCredentials.length - 1] : null; }
24-27: Same double-negation issue as inisConfiguredThere's redundant double-negation that can be simplified.
public static removeConfigured(userId: string) { const storedCredentials = JSON.parse(localStorage.getItem(WebAuthn.STORE_KEY) || '[]') as string[]; - localStorage.setItem(WebAuthn.STORE_KEY, JSON.stringify(!!userId ? storedCredentials.filter(c => c !== userId) : [])); + localStorage.setItem(WebAuthn.STORE_KEY, JSON.stringify(userId ? storedCredentials.filter(c => c !== userId) : [])); }🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
31-74: Good addition of error handling tocreateCredentialAdding try-catch blocks is a good defensive programming practice that prevents uncaught exceptions from breaking the application flow. However, returning an empty object on error might cause silent failures downstream.
Consider enhancing the error handling to return a more explicit error status:
try { // existing code return result; } catch (err) { console.error(err); - return {}; + return { error: err.message || 'Failed to create credential', id: null }; }
77-112: Similar improvement needed forverifyCredentialerror handlingThe same error handling improvement suggested for
createCredentialapplies here as well.try { // existing code return result; } catch (err) { console.error(err); - return {}; + return { error: err.message || 'Failed to verify credential', id: null }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/PasswordlessSection.razor(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/PasswordlessSection.razor.cs(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPage.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/IJSRuntimeWebAuthnExtensions.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebAuthn.ts(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.WebAuthn.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.WebAuthn.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Identity/IIdentityController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/AppJsonContext.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/WebAuthnAssertionOptionsRequestDto.cs(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebAuthn.ts
[error] 10-10: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 26-26: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (23)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/WebAuthnAssertionOptionsRequestDto.cs (1)
1-6: New DTO looks good for WebAuthn user identificationThis new DTO class with a nullable Guid UserId property is a good approach for identifying users during the WebAuthn assertion process. Using GUIDs rather than usernames is a more stable identifier approach since usernames can change while user IDs typically remain constant.
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/AppJsonContext.cs (1)
60-60: Properly added serialization support for the new DTOThe JSON serialization configuration has been correctly updated to support the new
WebAuthnAssertionOptionsRequestDtoclass, which is necessary for proper API communication with the new user ID-based approach.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
313-313: Configuration looks goodThe WebAuthn server domain configuration is correctly maintained, ensuring the WebAuthn server properly identifies itself to authenticators.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/PasswordlessSection.razor (1)
9-9: Improves code readabilityAdding a blank line here improves the visual separation between the title section and the action buttons.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.WebAuthn.cs (3)
24-24: Code simplification looks good.The existingKeys variable initialization has been simplified to a single line, which improves readability.
53-53: Extensions property commented out.The Extensions property is no longer being used in the credential creation request. This simplifies the request but removes any extended functionality that was previously enabled.
34-38:✅ Verification successful
WebAuthn authenticator selection policy changes.
The authenticator selection policy has been modified to:
- Set RequireResidentKey to false
- Change ResidentKey from Required to Discouraged
- Change UserVerification from Preferred to Required
- Set AuthenticatorAttachment to Platform
This configuration favors platform authenticators (like Windows Hello, Touch ID) over cross-platform options like security keys.
🌐 Web query:
What is the difference between ResidentKey requirement Discouraged vs Required in WebAuthn?💡 Result:
ResidentKey requirements in WebAuthn dictate how relying parties (RPs) manage cryptographic credentials during authentication. Here's the key distinction between Discouraged and Required:
Requirement Behavior Use Case Discouraged Prefers non-resident keys but allows fallback to discoverable credentials if the authenticator can't comply[2][5][15]. Traditional username-based logins where servers store credential IDs for 2FA scenarios[4][15]. Required Enforces discoverable credential creation. Fails authentication/registration if unsupported by the authenticator[2][5][13]. Passwordless/username-less flows where credentials must be self-contained on devices[1][4][10]. Key Technical Differences
Credential Storage
- Discouraged: Credential IDs typically stored server-side (non-resident)[4][6]
- Required: Credentials stored permanently on authenticators (resident keys)[1][7]
User Experience
- Discouraged: Requires username input for credential lookup[1][15]
- Required: Enables "tap-to-login" without usernames via stored user metadata[7][10]
Security Implications
- Discouraged: Limits exposure of credentials but requires server-side credential management[4][15]
- Required: Eliminates server-side credential storage but depends on authenticator security[1][7]
Implementation Code
// Required Configuration authenticatorSelection: { residentKey: 'required', requireResidentKey: true } // Discouraged Configuration authenticatorSelection: { residentKey: 'discouraged', requireResidentKey: false } [3][6][16]When to Use Each
- Choose Required for passwordless systems prioritizing user convenience over authenticator storage limitations[4][10]
- Use Discouraged when maintaining existing username-based flows or working with authenticators with limited storage (e.g., security keys)[6][15]
The choice impacts authentication architecture - Required enables true passwordless experiences but requires modern authenticators, while Discouraged maintains compatibility with legacy systems[4][10][15].
Citations:
- 1: https://www.corbado.com/glossary/resident-key
- 2: https://web.dev/articles/webauthn-discoverable-credentials
- 3: https://developers.yubico.com/WebAuthn/WebAuthn_Developer_Guide/WebAuthn_Client_Registration.html
- 4: https://www.corbado.com/blog/webauthn-resident-key-discoverable-credentials-passkeys
- 5: https://developers.google.com/android/reference/com/google/android/gms/fido/fido2/api/common/ResidentKeyRequirement
- 6: https://www.corbado.com/blog/passkeys-cheat-sheet
- 7: https://developers.yubico.com/WebAuthn/WebAuthn_Developer_Guide/Resident_Keys.html
- 8: https://www.corbado.com/glossary/authenticatorselection
- 9: https://docs.goauthentik.io/docs/add-secure-apps/flows-stages/stages/authenticator_webauthn/
- 10: https://duo.com/labs/tech-notes/resident-keys-and-the-future-of-webauthn-fido2
- 11: https://developers.yubico.com/java-webauthn-server/JavaDoc/webauthn-server-core/2.0.0/com/yubico/webauthn/data/ResidentKeyRequirement.html
- 12: https://quarkus.io/guides/security-webauthn
- 13: https://chromestatus.com/feature/5701094648840192
- 14: https://developers.yubico.com/WebAuthn/Concepts/WebAuthn_Level_2_Features_and_Enhancements.html
- 15: https://thecopenhagenbook.com/webauthn
- 16: https://www.w3.org/TR/webauthn-2/
- 17: Indicate resident key credential "preferred" during registration and find out what the authenticator offered w3c/webauthn#991
Update on WebAuthn Authenticator Policy
The changes in this update are as follows:
- RequireResidentKey: Now set to
false, which aligns with the "discouraged" mode—resident keys aren’t enforced, allowing for non-resident credentials.- ResidentKey: Changed from
RequiredtoDiscouraged. As a result, while discoverable (resident) credentials can be used if available, the system favors using non-resident keys managed on the server. This matches traditional username-based flows where the username is needed for credential lookup.- UserVerification: Upgraded from
PreferredtoRequiredto ensure that strong verification (such as biometrics or PIN) is enforced during authentication.- AuthenticatorAttachment: Now explicitly set to
Platform, thereby favoring built-in authenticators (e.g., Windows Hello, Touch ID) over external security keys.These adjustments correctly implement the intended WebAuthn configuration changes, in line with current best practices. Please verify that this configuration integrates well with the rest of the authentication flow, especially if any previous dependency on discoverable credentials exists.
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Identity/IIdentityController.cs (1)
52-52:✅ Verification successful
Updated method signature to include user ID parameter.
The GetWebAuthnAssertionOptions method now accepts a WebAuthnAssertionOptionsRequestDto parameter, which contains the UserId. This change aligns with the shift from using usernames to user IDs for WebAuthn operations.
🏁 Script executed:
#!/bin/bash # Check if the implementation of GetWebAuthnAssertionOptions in IdentityController has been updated to match this interface change rg -A 5 "GetWebAuthnAssertionOptions.*WebAuthnAssertionOptionsRequestDto" --type csLength of output: 2026
Method signature update verified – approved
The revised signature now correctly accepts a
WebAuthnAssertionOptionsRequestDtothat includes theUserId, and the implementation inIdentityController.WebAuthn.cs(evidenced by theif (request.UserId is not null)check) confirms this update aligns with transitioning from usernames to user IDs for WebAuthn operations.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPage.razor.cs (1)
172-175: Improved passwordless sign-in with user ID.The HandleOnPasswordlessSignIn method now:
- Retrieves the user ID using JSRuntime.GetWebAuthnConfigured()
- Returns early if no user ID is found
- Passes the user ID to GetWebAuthnAssertionOptions
This change enhances the security and specificity of the WebAuthn flow by using user IDs instead of usernames.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/PasswordlessSection.razor.cs (7)
24-24: Updated WebAuthn configuration check to use User.Id.The code now checks if the WebAuthn is configured for the user ID instead of the username, aligning with the broader shift to user IDs for authentication.
46-50: Enhanced error handling for null attestation ID.Added a null check for attestationResponse.Id with appropriate error handling, improving robustness of the WebAuthn credential creation process.
54-54: Updated WebAuthn configuration storage to use User.Id.Changed from StoreWebAuthnConfigured(User.UserName) to SetWebAuthnConfigured(User.Id), consistent with the shift to using user IDs.
65-65: Updated WebAuthn assertion options request to include User.Id.The call to GetWebAuthnAssertionOptions now includes the User.Id in the request, aligning with the interface changes.
79-83: Enhanced error handling for null assertion ID.Added a null check for assertion.Id with appropriate error handling, improving robustness of the WebAuthn assertion verification process.
89-89: Updated WebAuthn configuration removal to use User.Id.Changed from RemoveWebAuthnConfigured(User.UserName) to RemoveWebAuthnConfigured(User.Id), consistent with other changes.
96-104: Added commented debugging code.Added a commented-out DeleteAll method for debugging purposes. This is a good practice as it provides developers with a way to test functionality without requiring them to write the code themselves.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.WebAuthn.cs (4)
19-19: Good change to use a DTO for request parametersThe updated method signature now accepts a structured
WebAuthnAssertionOptionsRequestDtoinstead of just a cancellation token, which provides better organization and extensibility for future parameter additions.
23-27: Great improvement for user-specific credential filteringThis new conditional block efficiently filters WebAuthn credentials by user ID, allowing for personalized assertion options instead of returning all credentials. This improves both security and performance by only providing relevant credentials to the client.
39-39: Security improvement by requiring user verificationChanging from
UserVerificationRequirement.DiscouragedtoUserVerificationRequirement.Requiredenhances security by ensuring that the user is verified during the authentication process, which helps prevent unauthorized access.
37-37:❓ Verification inconclusive
Why is the Extensions property commented out?
The
Extensionsproperty is commented out rather than removed. Is this intentional for debugging purposes or a temporary change?
🌐 Web query:
Are WebAuthn extensions recommended to be enabled or disabled for security reasons?💡 Result:
WebAuthn extensions should be enabled with careful configuration when they provide specific security benefits, but unnecessary or improperly configured extensions may introduce risks. Here's a breakdown of key considerations:
Security Benefits of WebAuthn Extensions
Enhanced Authentication Flexibility
Extensions likehmac-secret(PRF) enable pseudo-random function outputs for use cases such as end-to-end encryption key derivation, leveraging secure hardware enclaves[1].
- Example: Deriving encryption keys tied to WebAuthn credentials ensures secrets never leave the authenticator[1][18].
Anti-Phishing Protections
Extensions likeappidExcludeenforce origin checks, preventing credential misuse across domains[7][16].
- Critical for blocking reverse-proxy phishing attacks by validating the
rpIdagainst the request origin[2][4].Credential Management
credPropshelps identify discoverable credentials, improving user experience without compromising security[7].credentialProtectionPolicyallows enforcing user verification (e.g., PIN/biometrics) for high-security scenarios[7][16].
Risks and Mitigations
Implementation Complexity
Poorly configured extensions (e.g., incorrectrpIdvalidation) can weaken security. For example:
- KeePassXC faced issues with improper domain validation in WebAuthn, risking credential leaks[11].
- Mitigation: Strictly validate
rpIdand use HTTPS[6][12].Privacy Concerns
Some extensions (e.g., attestation data) could expose device metadata.
- Use
attestation: "none"unless necessary[3][18].Cross-Origin Risks
Extensions likeappidrequire careful handling to avoid credential confusion across domains[7][10].
- Ensure
appidExcludeis used when migrating from U2F to WebAuthn[3][13].
Recommended Practices
- Enable Security-Critical Extensions
Usehmac-secretfor encryption key derivation[1],appidExcludefor U2F compatibility[7], andcredentialProtectionPolicyfor MFA[3].- Disable Unnecessary Extensions
Reduce attack surface by disabling unused features (e.g.,largeBlobif not needed)[9].- Enforce User Verification
SetuserVerification: "required"for sensitive operations to mandate biometric/PIN checks[3][4].- Validate Server-Side
Always verify extension outputs (e.g., PRF results, attestation metadata) to prevent spoofing[1][7].
Browser and Authenticator Support
- CTAP2-compliant devices (e.g., YubiKeys, iOS Secure Enclave) support most extensions securely[3][13].
- Avoid extensions on older browsers (e.g., IE, Opera Mini) lacking WebAuthn compliance[5][12].
In summary, WebAuthn extensions are recommended when configured correctly to address specific security needs, but their use requires rigorous validation to avoid introducing vulnerabilities. Prioritize extensions that enhance anti-phishing, encryption, and credential management while adhering to best practices for implementation[1][3][7].
Citations:
- 1: https://github.com/w3c/webauthn/wiki/Explainer:-PRF-extension
- 2: https://www.slashid.dev/blog/webauthn-antiphishing/
- 3: https://developers.yubico.com/WebAuthn/WebAuthn_Developer_Guide/WebAuthn_Readiness_Checklist.html
- 4: https://www.slashid.dev/blog/webauthn-compliance/
- 5: https://www.beyondidentity.com/resource/webauthn-capabilities-and-limitations
- 6: https://docs.joomla.org/WebAuthn_Passwordless_Login
- 7: https://developer.mozilla.org/en-US/docs/Web/API/Web_Authentication_API/WebAuthn_extensions
- 8: https://quarkus.io/guides/security-webauthn
- 9: https://developer.chrome.com/docs/devtools/webauthn
- 10: https://www.w3.org/TR/webauthn-2/
- 11: Security, privacy and other issues with WebAuthn/passkeys support keepassxreboot/keepassxc#10287
- 12: https://fusionauth.io/articles/authentication/webauthn-explained
- 13: https://www.yubico.com/blog/webauthn-implementation-whats-what-why-should-you-care-and-new-updates-from-yubico/
- 14: https://issues.chromium.org/40850860
- 15: https://developer.chrome.com/docs/identity/webauthn
- 16: https://www.w3.org/TR/webauthn-3/
- 17: devicePubKey extension MUST be supported if multi-device WebAuthn credentials are used w3c/webauthn#1691
- 18: https://webauthn.guide
- 19: https://community.bitwarden.com/t/disable-auth-app-after-adding-yubikeys/50701
- 20: https://forums.developer.apple.com/forums/thread/733413
Clarification Needed on Commented-Out Extensions Property
The
Extensionsproperty is currently commented out. Given that WebAuthn extensions can be beneficial when configured correctly—but might introduce risks if left misconfigured—please clarify your intent:
- If this is a temporary debugging measure or if the configuration is incomplete, please add a brief comment (or a TODO) explaining why it’s commented out and under what conditions it should be re-enabled.
- If the property is no longer needed, consider removing the commented code entirely to avoid confusion.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/IJSRuntimeWebAuthnExtensions.cs (3)
13-16: Good refactoring to use GUIDs instead of usernamesChanging from using usernames to user IDs (GUIDs) is a more robust approach since GUIDs are stable unique identifiers that don't change, unlike usernames which users might update.
18-21: Good addition of method to retrieve configured user IDThis new method provides a clean way to retrieve the currently configured WebAuthn user ID, allowing for improved flow control in authentication scenarios.
23-31: Clean separation of WebAuthn configuration methodsBreaking down the functionality into distinct methods for setting and removing WebAuthn configurations improves readability and maintainability. The optional parameter for
RemoveWebAuthnConfiguredprovides flexibility in usage.
closes #10262
Summary by CodeRabbit
New Features
Bug Fixes
Style