Fix WebAuthn issues in Boilerplate (#10228) #10229
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 adds functionality for managing WebAuthn credentials. A new icon-only button in the diagnostic modal triggers the deletion of all WebAuthn credentials when clicked. On the client side, new methods and dependency injection support this feature. The changes also include updates to the passwordless settings UI with added auto-loading attributes and conditional assignment in the settings page. Additionally, conversion methods in the WebAuthn script have been renamed and adjusted, and corresponding server-side methods and documentation for handling WebAuthn credentials have been updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal as AppDiagnosticModal
participant Ctrl as IUserController
participant Server as UserController.WebAuthn
User->>Modal: Click delete credentials button
Modal->>Modal: Execute DeleteAllWebAuthnCredentials()
Modal->>Ctrl: Call DeleteAllWebAuthnCredentials(cancellationToken)
Ctrl->>Server: Send HTTP DELETE request for credentials
Server-->>Ctrl: Return deletion result
Ctrl-->>Modal: Respond with operation status
Modal-->>User: Update UI state
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 (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppDiagnosticModal.razor (1)
148-153: Add a Title attribute to the WebAuthn credential delete buttonThe new button for deleting WebAuthn credentials is missing a Title attribute, which is present on all other buttons in this modal. Adding a title would improve accessibility and provide users with context about the button's function.
<BitButton IconOnly AutoLoading Color="BitColor.Info" Variant="BitVariant.Text" IconName="@BitIconName.Delete" + Title="Delete WebAuthn credentials" OnClick="DeleteAllWebAuthnCredentials" />Also, consider adding a confirmation dialog before deleting credentials since this is a destructive action that can't be undone.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.WebAuthn.cs (1)
40-45: Consider documenting the reason for disabling extensionsYou've commented out the Extensions property which disables credential properties, extensions, and user verification methods. Consider adding a comment explaining why these features were disabled.
//Extensions = new AuthenticationExtensionsClientInputs //{ // CredProps = true, // Extensions = true, // UserVerificationMethod = true, //} +// Extensions are disabled to simplify the WebAuthn implementation and avoid compatibility issuessrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebAuthn.ts (1)
26-26: Consider using conditional logging for productionAdding console logging of WebAuthn options is helpful for debugging but may expose sensitive information in production. Consider making this log conditional based on environment.
-console.log(options) +if (process.env.NODE_ENV !== 'production') { + console.log('WebAuthn credential options:', options); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppDiagnosticModal.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppDiagnosticModal.razor.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/PasswordlessSection.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SettingsPage.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebAuthn.ts(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.WebAuthn.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/WebAuthnCredential.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Identity/IUserController.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (12)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/WebAuthnCredential.cs (1)
5-9: Documentation addition looks goodThe added XML documentation for the WebAuthnCredential class is clear, informative, and follows good practices by including a reference link to the underlying library.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SettingsPage.razor.cs (1)
37-40: Good fix to prevent JavaScript interop during prerenderingThis change correctly prevents calling JavaScript methods during prerendering by checking the
InPrerenderSessionflag. This avoids potential errors since JavaScript interop is not available during prerendering in Blazor.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/PasswordlessSection.razor (2)
11-11: UI improvement with AutoLoading attributeAdding the AutoLoading attribute provides visual feedback during the async operation, improving user experience.
17-17: UI improvement with AutoLoading attributeAdding the AutoLoading attribute provides visual feedback during the async operation, improving user experience.
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Identity/IUserController.cs (1)
61-62: API extension for bulk credential deletion looks goodThe new
DeleteAllWebAuthnCredentialsmethod is a sensible addition that complements the existingDeleteWebAuthnCredentialmethod, providing a way to delete all WebAuthn credentials at once rather than individually.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppDiagnosticModal.razor.cs (3)
6-6: Appropriate import addedThe import for the Identity controllers namespace is correctly added to support the new WebAuthn functionality.
45-45: Dependency injection properly implementedThe
IUserControlleris correctly injected using the[AutoInject]attribute, following the established pattern in this class.
159-164: Proper authentication check in WebAuthn operationThe implementation correctly checks if the user is authenticated before attempting to delete WebAuthn credentials, which is a good security practice.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.WebAuthn.cs (2)
38-38: Security enhancement: Restricting to platform authenticatorsChanging the authenticator selection to platform-only is a security enhancement that restricts WebAuthn to built-in authenticators like Windows Hello or Touch ID, rather than allowing cross-platform authenticators.
116-130: Efficient implementation for bulk deletionThe implementation correctly handles user verification, empty result cases, and uses efficient bulk removal with
RemoveRange. Good error handling with theResourceNotFoundExceptionfor user not found scenarios.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebAuthn.ts (2)
28-28: Improved method naming enhances code clarityThe renaming of conversion methods (
urlToArray→stringToBinary,arrayToUrl→binaryToString,base64ToUrl→base64ToString) better reflects their actual functionality, making the code more maintainable and easier to understand.Also applies to: 32-32, 42-42, 49-50, 54-55, 63-63, 70-70, 79-79, 83-86
93-103: Consistent naming convention for conversion methodsThe renamed private methods now have a more consistent and descriptive naming convention that clearly indicates their purpose - converting between binary data and string representations.
closes #10228
Summary by CodeRabbit