-
-
Notifications
You must be signed in to change notification settings - Fork 363
doc(ITotpService): add sample code #5912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's Guide by SourceryThis pull request replaces the existing No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the BootstrapBlazor.Authenticator component with a new ITotpService implementation and updates the related samples, localization, and documentation accordingly.
- Renamed and updated parameters in the ITotpService API (step → period, totpSize → digits).
- Introduced an OtpServices sample and removed the old Authenticators sample.
- Updated service registration, localization files, and docs to reflect the changes.
Reviewed Changes
Copilot reviewed 7 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Services/ITotpService.cs | Updated parameter names and documentation in the TOTP service interface |
| src/BootstrapBlazor/Services/DefaultTotpService.cs | Updated parameter names and defaults in the TOTP service implementation |
| src/BootstrapBlazor.Server/Extensions/ServiceCollectionSharedExtensions.cs | Updated service registration for the new ITotpService |
| src/BootstrapBlazor.Server/Extensions/MenusLocalizerExtensions.cs | Added a new menu entry for TotpService |
| src/BootstrapBlazor.Server/Components/Samples/OtpServices.razor.cs | Renamed sample class and updated service usage with new OtpOptions |
| src/BootstrapBlazor.Server/Components/Samples/OtpServices.razor | Created a new sample page for ITotpService |
| src/BootstrapBlazor.Server/Components/Samples/Authenticators.razor | Removed the old Authenticator sample |
Files not reviewed (6)
- BootstrapBlazor.sln: Language not supported
- src/BootstrapBlazor.Server/BootstrapBlazor.Server.csproj: Language not supported
- src/BootstrapBlazor.Server/Locales/en-US.json: Language not supported
- src/BootstrapBlazor.Server/Locales/zh-CN.json: Language not supported
- src/BootstrapBlazor.Server/docs.json: Language not supported
- src/BootstrapBlazor/BootstrapBlazor.csproj: Language not supported
Comments suppressed due to low confidence (2)
src/BootstrapBlazor/Services/ITotpService.cs:29
- The default value for 'period' is set to 6, which is non-standard given that TOTP periods are typically 30 seconds. Please verify and update the default if needed.
string Compute(string secretKey, int period = 6, OtpHashMode mode = OtpHashMode.Sha1, int digits = 6, DateTime? timestamp = null);
src/BootstrapBlazor.Server/Components/Samples/OtpServices.razor:30
- The PackageTips component still references 'BootstrapBlazor.Authenticator'; update it to reference 'BootstrapBlazor.TotpService' to maintain consistency with the new implementation.
<PackageTips Name="BootstrapBlazor.Authenticator"></PackageTips>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider updating the PR title and description to better reflect the full scope, which includes replacing the
Authenticatorservice withITotpServicerather than just adding sample code.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5912 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 665 665
Lines 30485 30485
Branches 4345 4345
=========================================
Hits 30485 30485 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Link issues
fixes #5911
Summary By Copilot
This pull request introduces significant changes to the
BootstrapBlazorproject, focusing on replacing theBootstrapBlazor.Authenticatorcomponent with a newITotpServiceimplementation. The changes include updates to project references, renaming files and classes, modifying localization files, and improving the TOTP service's API. Below is a summary of the most important changes grouped by theme.Removal of
BootstrapBlazor.Authenticatorand Introduction ofITotpService:BootstrapBlazor.Authenticatorproject reference fromBootstrapBlazor.slnand replaced it withITotpServicefunctionality in theBootstrapBlazor.Serverproject. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-870ac4a4b1c5ee4def744573119c9717970bba3b8d2a460723e46a16e6f94d39L81-L82),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-acda8ffd7b0362074afe236cb631a348f2c8a44568a1f235743982c2a0ad6416R24),[[3]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-acda8ffd7b0362074afe236cb631a348f2c8a44568a1f235743982c2a0ad6416L72-L81),[[4]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-8ec92a1c789bcd9f30e2bf5f32650e11512a8d9818d4bb826d144b69f96a5181L33-R34))Authenticatorsample with a newOtpServicessample, including renaming the Razor and code-behind files, updating the implementation to use the newITotpService, and adding a new demo page. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-1870f0684661390279b8dabb1fc420f521ff8d2e98a9d30dbf973af1dbfa597cL1-L39),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-436a2abad2c2c0fe81cb6a4a49310b861d8a3656cda22a6e43016827dbb3bf92R1-R56),[[3]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-61a86a9cf933da975b47868eba7ec96cbb748ed2583f4c75789390c9a7f0da07L10-R16),[[4]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-61a86a9cf933da975b47868eba7ec96cbb748ed2583f4c75789390c9a7f0da07L37-R45),[[5]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-0da175523aa74d67c07cc125ac55664960a8e14e9b980fc8394ee1b9fff866f7R1541-R1546))API Enhancements for TOTP Service:
ITotpServiceinterface and its implementation to use more descriptive parameter names (step→period,totpSize→digits) and aligned the API with standard TOTP terminology. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-ccc7650d5da806a4fcb0148dc7fde082c94c49c7492f681c1ae7b96a26e1a427L12-R12),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-bb486b36d893ea93119d3c1e13e85f33e2ba5c15ce6f093b18e51cd7ad8b110cL24-R29))Localization Updates:
en-US.jsonandzh-CN.json) to reflect the changes fromAuthenticatortoITotpService, including new keys for theOtpServicesdemo. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-790d42ebea731775f0fee88a92b20fadb044e53706fd6d3025dfa095df9b1b41L4923-R4923),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-790d42ebea731775f0fee88a92b20fadb044e53706fd6d3025dfa095df9b1b41L7106-R7110),[[3]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-746b8cc3c80dd10d0a1bf77b8d6571652e15bcc7c33dcac0954d93b1a728cc7fL4923-R4923),[[4]](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-746b8cc3c80dd10d0a1bf77b8d6571652e15bcc7c33dcac0954d93b1a728cc7fL7106-R7110))Documentation and Versioning:
docs.jsonfile to replace references toauthenticatorwithotp-service. ([src/BootstrapBlazor.Server/docs.jsonL233-R233](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-7f22699a321cdff3a7bd4a3f85ae66d0fbe78560628329c39cca784d9d64dfb7L233-R233))BootstrapBlazorfrom9.5.11-beta04to9.5.12. ([src/BootstrapBlazor/BootstrapBlazor.csprojL4-R4](https://github.com/dotnetcore/BootstrapBlazor/pull/5912/files#diff-07918ce1b66955e76da5cd0ffa38512cce984fa2a09735a60e0db37b45235527L4-R4))These changes streamline the implementation of TOTP functionality, improve code clarity, and enhance the user experience with updated samples and documentation.
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce
ITotpServiceand a corresponding sample page, replacing the previousAuthenticatorcomponent and sample. Rename service API parameters for clarity.Enhancements:
ITotpService.Computeparameters (steptoperiod,totpSizetodigits) to align with standard TOTP terminology.Authenticatorsample page with a newOtpServicespage demonstratingITotpServiceusage.AddBootstrapBlazorTotpServiceinstead of the previousAddBootstrapBlazorAuthenticatormethod.OtpServicessample.