Improve Boilerplate project template identity (#10960) #10961
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 enhances the boilerplate project template's identity management by storing Data Protection API keys in the database, introduces restrictions to prevent modification or deletion of built-in roles, and updates localized resource strings for role management. Several method and resource key names are also clarified for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RolesPage
participant RoleManagementController
participant AppDbContext
User->>RolesPage: Click "Remove all users from role"
RolesPage->>RoleManagementController: RemoveAllUsersFromRole(roleId)
RoleManagementController->>AppRoles: IsBuiltInRole(role.Name)
alt If built-in role
RoleManagementController-->>RolesPage: Throw BadRequestException
else
RoleManagementController->>AppDbContext: Remove users from role
AppDbContext-->>RoleManagementController: Complete
RoleManagementController-->>RolesPage: Success
end
sequenceDiagram
participant AppStartup
participant Program.Services
participant AppDbContext
AppStartup->>Program.Services: ConfigureServices()
Program.Services->>Program.Services: AddDataProtection()
Program.Services->>AppDbContext: PersistKeysToDbContext<AppDbContext>()
Assessment against linked issues
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the identity boilerplate by adding database-backed data protection, tightening built-in role protections, and clarifying method names.
- Persist DataProtection keys to the database and implement the key context
- Introduce
AppRoles.IsBuiltInRolechecks and update related error messages - Rename
RemoveRoleFromAllUserstoRemoveAllUsersFromRolein API and client
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/.../Shared/Services/AppRoles.cs | Add Demo constant and IsBuiltInRole helper |
| src/.../Shared/Resources/AppStrings.resx | Update built-in role messages and add new keys |
| src/.../Shared/Resources/AppStrings.fa.resx | Sync localized messages and resource keys |
| src/.../Controllers/Identity/IRoleManagementController.cs | Rename RemoveRoleFromAllUsers to clarify intent |
| src/.../Program.Services.cs | Configure AddDataProtection().PersistKeysToDbContext |
| src/.../Data/AppDbContext.cs | Implement IDataProtectionKeyContext |
| src/.../Controllers/Identity/RoleManagementController.cs | Enforce built-in role guards and rename remove action |
| src/.../Pages/Authorized/Management/RolesPage.razor(.cs) | Align client method/event names with API change |
...ilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx
Outdated
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
279-280: Excellent Data Protection configuration with important security reminder.The implementation correctly configures Data Protection to persist keys to the database using the AppDbContext. The comment about using
ProtectKeysWithCertificateis valuable security guidance for production deployments.For enhanced security in production environments, consider adding certificate-based key protection:
services.AddDataProtection() - .PersistKeysToDbContext<AppDbContext>(); // It's advised to secure database-stored keys with a certificate by invoking ProtectKeysWithCertificate. + .PersistKeysToDbContext<AppDbContext>() + .ProtectKeysWithCertificate(certificateThumbprint); // Add certificate protection in productionsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs (1)
79-80: Consider consolidating error message keys for better maintainability.While the security checks are well-implemented, consider whether all built-in role operations (including SuperAdmin) could use the same error message key for consistency, since SuperAdmin is likely a built-in role.
If SuperAdmin is considered a built-in role, you could unify the error handling:
- if (role.Name == AppRoles.SuperAdmin) - throw new BadRequestException(Localizer[nameof(AppStrings.UserCantChangeSuperAdminRoleClaimsErrorMessage)]); + if (AppRoles.IsBuiltInRole(role.Name!)) + throw new BadRequestException(Localizer[nameof(AppStrings.CanNotChangeBuitlInRole), role.Name!]);This would require updating the localization string to be more generic for both role and claim operations.
Also applies to: 101-102, 118-119, 136-137, 159-160
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (12)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Management/RolesPage.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Management/RolesPage.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs(6 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/AppDbContext.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Identity/IRoleManagementController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppRoles.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/AppDbContext.cs (6)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/User.cs (1)
User(9-55)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserClaim.cs (1)
UserClaim(3-6)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserRole.cs (1)
UserRole(3-8)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserLogin.cs (1)
UserLogin(3-6)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/RoleClaim.cs (1)
RoleClaim(3-6)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserToken.cs (1)
UserToken(3-6)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (20)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppRoles.cs (1)
13-16: LGTM! Clean implementation of built-in role identification.The method correctly uses pattern matching to identify SuperAdmin and Demo roles. This provides a centralized way to check for built-in roles that need special protection.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs (1)
377-397: Excellent enhancement to form field management based on panel type.The conditional logic correctly clears irrelevant fields based on the sign-in panel type:
- Clears password for OTP-only authentication
- Clears OTP for password-only authentication when OTP hasn't been sent
- Uses null-conditional operators for safer field change notifications
This improves user experience by preventing validation conflicts between different authentication methods.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Management/RolesPage.razor (1)
232-232: Good method name update for semantic clarity.The renamed method
RemoveAllUsersFromRoleis more semantically accurate than the previous name, clearly indicating that users are being removed from the role.src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Identity/IRoleManagementController.cs (1)
43-43: Interface method renamed for better semantic clarity.The new method name
RemoveAllUsersFromRolemore accurately describes the operation and aligns with the implementation updates across the application.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Management/RolesPage.razor.cs (1)
385-394: Method renamed consistently with interface updates.The implementation correctly updates both the method name and the internal controller call to maintain consistency across the application layers. The logic and error handling remain appropriately unchanged.
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx (1)
1298-1300: LGTM! Improved specificity for SuperAdmin claims protection.The key renaming from
UserCantChangeSuperAdminRoleErrorMessagetoUserCantChangeSuperAdminRoleClaimsErrorMessagebetter reflects its specific use case for preventing SuperAdmin role claims modification.src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx (2)
1298-1300: LGTM! Improved message clarity.The updated message "You can't remove this user from the SuperAdmin user group" is more descriptive and user-friendly.
1301-1303: LGTM! Better specificity for claims protection.The key renaming and message update provide clearer context for SuperAdmin claims protection scenarios.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs (7)
118-120: LGTM! Excellent SuperAdmin claims protection.The implementation correctly prevents modification of SuperAdmin role claims across all claim management methods, using the appropriately renamed resource key.
Also applies to: 136-138, 159-161
210-210: LGTM! Better method naming.The rename from
RemoveRoleFromAllUserstoRemoveAllUsersFromRoleis semantically more accurate and improves code readability.
101-102: LGTM! Consistent built-in role protection.The same built-in role check is properly applied to the Delete method, maintaining consistency with the Update method. This prevents accidental deletion of system roles.
136-137: LGTM! Consistent SuperAdmin claim protection.The same SuperAdmin claim protection is properly applied to both UpdateClaims and DeleteClaims methods, maintaining consistency across all claim modification operations.
Also applies to: 159-160
210-210: LGTM! Improved method naming for clarity.The method rename from
RemoveRoleFromAllUserstoRemoveAllUsersFromRoleis more descriptive and clearly indicates the operation being performed.
118-119:✅ Verification successful
Verify the localization string exists and consider SuperAdmin detection consistency.
The SuperAdmin claim protection is a good security measure. However, verify that the referenced localization string exists and consider if the SuperAdmin detection should be consistent with the built-in role check logic.
🏁 Script executed:
#!/bin/bash # Search for the localization string in resource files rg "UserCantChangeSuperAdminRoleClaimsErrorMessage" # Check if SuperAdmin is considered a built-in role rg -A 5 -B 5 "SuperAdmin.*IsBuiltInRole\|IsBuiltInRole.*SuperAdmin"Length of output: 1190
Localization string verified; review SuperAdmin detection
- The
UserCantChangeSuperAdminRoleClaimsErrorMessagekey is present in:
src/Shared/Resources/AppStrings.resxsrc/Shared/Resources/AppStrings.fa.resx- No existing
IsBuiltInRole(or equivalent) check was detected around the SuperAdmin logic inRoleManagementController.cs. Consider invoking a centralized built-in role check method—if one exists—to keep SuperAdmin detection consistent across the codebase.
79-80:✅ Verification successful
Verify the
AppRoles.IsBuiltInRolemethod implementation.The built-in role protection logic is a good security measure. However, ensure that the
AppRoles.IsBuiltInRolemethod is properly implemented and handles edge cases correctly.
🏁 Script executed:
#!/bin/bash # Search for the AppRoles.IsBuiltInRole method implementation ast-grep --pattern 'class AppRoles { $$$ IsBuiltInRole($_) { $$$ } $$$ }' # Also search for any static method with this name rg -A 10 "IsBuiltInRole"Length of output: 4131
Built-in role check implementation confirmed
I’ve reviewed
AppRoles.IsBuiltInRolein
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppRoles.cs—it correctly returnstrueonly for theSuperAdminandDemoconstants and safely returnsfalsefornullor any other values. No changes are needed here; just remember to update this method whenever new built-in roles are added.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/AppDbContext.cs (2)
20-20: LGTM: Correct using statement for Data Protection EntityFrameworkCore integration.The using statement properly imports the namespace needed for
IDataProtectionKeyContextandDataProtectionKeytypes.
25-25: LGTM: Proper implementation of IDataProtectionKeyContext interface.Implementing
IDataProtectionKeyContextcorrectly enables the AppDbContext to store ASP.NET Core Data Protection keys in the database, which is essential for multi-instance deployments.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj (1)
32-32: LGTM: Correct package reference for Data Protection EntityFrameworkCore.The package reference is properly added and will use the version specified in Directory.Packages.props, maintaining consistency with the project's central package management approach.
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
23-23:✅ Verification successful
Verify package version currency and compatibility.
The package version entry is correctly added for central package management. However, let's verify that version 9.0.5 is the latest stable version available.
🌐 Web query:
What is the latest stable version of Microsoft.AspNetCore.DataProtection.EntityFrameworkCore package for .NET 9.0?💡 Result:
Based on the search results, the latest stable version of Microsoft.AspNetCore.DataProtection.EntityFrameworkCore package for .NET 9.0 is 9.0.5.
The Microsoft.AspNetCore.DataProtection.EntityFrameworkCore package provides a mechanism for storing data protection keys to a database using Entity Framework Core[5]. This package must be added separately to your project as it's not part of the Microsoft.AspNetCore.App metapackage[5].
The package targets .NET 9.0 and is compatible with this framework or higher[1]. Version 9.0.5 is the current stable release, which aligns with the latest patch version of .NET 9 itself, which was released on May 13, 2025[10].
This package allows you to store data protection keys in a database by using Entity Framework Core, which enables sharing keys across multiple instances of a web application[5]. To use this functionality, you would typically:
- Add the package to your project
- Create a DbContext that implements IDataProtectionKeyContext
- Configure your services to use this DbContext for key persistence
The package has dependencies on Microsoft.AspNetCore.DataProtection (>= 9.0.3) and Microsoft.EntityFrameworkCore (>= 9.0.3)[1], ensuring compatibility with the broader .NET 9 ecosystem.
Citations:
- 1: https://www.nuget.org/packages/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore/
- 2: https://www.nuget.org/packages/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore/9.0.0-preview.1.24081.5
- 3: https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.dataprotection.entityframeworkcore?view=aspnetcore-9.0
- 4: https://learn.microsoft.com/en-us/aspnet/core/security/data-protection/configuration/overview?view=aspnetcore-9.0
- 5: https://learn.microsoft.com/en-us/aspnet/core/security/data-protection/implementation/key-storage-providers?view=aspnetcore-9.0
- 6: https://learn.microsoft.com/en-us/aspnet/core/security/data-protection/introduction?view=aspnetcore-9.0
- 7: https://www.nuget.org/packages/microsoft.entityframeworkcore
- 8: https://learn.microsoft.com/en-us/ef/core/what-is-new/ef-core-9.0/whatsnew
- 9: https://www.nuget.org/packages/microsoft.aspnetcore.dataprotection/
- 10: https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core
Approve addition of Microsoft.AspNetCore.DataProtection.EntityFrameworkCore 9.0.5
Version 9.0.5 is the latest stable release for .NET 9.0 and aligns with the current patch level. No further changes are needed.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
17-17: LGTM: Required using statement for Data Protection services.The using statement correctly imports the namespace needed for the
AddDataProtection()extension method.
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx
Outdated
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx
Outdated
Show resolved
Hide resolved
...ilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs
Show resolved
Hide resolved
...ilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs
Show resolved
Hide resolved
...Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/AppDbContext.cs
Show resolved
Hide resolved
1 participant
closes #10960
Summary by CodeRabbit