Warn about sign-out after security-stamp changes #12647#12648
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe update adds localized warnings for security-sensitive account changes, introduces a password-change tab, synchronizes profile state without forced navigation, and improves passwordless availability handling. It also adds multitenant current-tenant propagation and display, plus template configuration updates. ChangesAccount Security and Settings
Multitenant UI State
Template Configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsComponent
participant UserController
participant SnackBarService
participant PubSubService
User->>SettingsComponent: Change account security setting
SettingsComponent->>UserController: Submit operation
UserController-->>SettingsComponent: Return operation response
SettingsComponent->>SnackBarService: Show success and sign-out warning
SettingsComponent->>PubSubService: Publish PROFILE_UPDATED
SnackBarService-->>User: Display warning
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePhoneNumberTab.razor.cs`:
- Around line 90-93: In the successful phone-number update flow around
GetCurrentUser, isolate the best-effort profile refresh and PROFILE_UPDATED
publish from the subsequent post-success actions. Handle refresh failure locally
so SnackBarService.Warning, state cleanup, and navigation still execute after
the update succeeds, while preserving the existing refreshed-profile behavior
when the request succeeds.
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.ar.resx`:
- Around line 777-779: Update the localized value for
SignOutOfAllDevicesWarningMessage to describe sign-out occurring after the next
token refresh, including all devices and the current device, and requiring the
user to sign in again; remove the fixed “within a few minutes” timing.
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.de.resx`:
- Around line 777-779: Update the localized value for
SignOutOfAllDevicesWarningMessage to describe sign-out occurring after the next
token refresh, including all devices and the current device, and instruct the
user to sign in again. Remove the fixed “within a few minutes” timing promise
while preserving the German localization.
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.es.resx`:
- Around line 777-779: The SignOutOfAllDevicesWarningMessage localization
currently promises a fixed-time sign-out; update its Spanish value to state that
sign-out occurs after the next token refresh, affects all devices including the
current one, and requires signing in again.
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx`:
- Around line 777-778: Update the SignOutOfAllDevicesWarningMessage resource and
every localized copy to state that sign-out occurs on the next token refresh,
replacing the misleading “within a few minutes” wording while preserving the
existing security and reauthentication guidance.
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.zh.resx`:
- Around line 777-779: The SignOutOfAllDevicesWarningMessage localization
incorrectly promises a fixed delay; replace its Chinese value with wording that
ties sign-out to the next token refresh, includes all devices including the
current device, and instructs the user to sign in again.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6d7dcc73-87e6-4687-a25c-895005ad4882
📒 Files selected for processing (13)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePhoneNumberTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/TwoFactorSection.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Infrastructure/Services/SnackBarService.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.ar.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.de.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.es.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fr.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.hi.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.nl.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.sv.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.zh.resx
b7b2553 to
e087780
Compare
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangeEmailTab.razor.cs`:
- Around line 88-91: Update the email-change success flow around GetCurrentUser
and RefreshCurrentPage so synchronization failures do not enter the
mutation-failure catch path. After the email mutation succeeds, attempt profile
retrieval and publishing separately, then guarantee
NavigationManager.RefreshCurrentPage executes regardless of synchronization
failure.
- Around line 65-66: Remove the GetCurrentUser call and PROFILE_UPDATED
publication from the SendChangeEmailToken flow; sending the change-email token
must not fetch or publish profile updates.
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/TwoFactorSection.razor.cs`:
- Around line 49-58: Update DisableTwoFactorAuth() to clear or replace the
recovery-code state after a successful response, using response.RecoveryCodes
when appropriate or an explicit empty value, so previously generated codes are
no longer displayed when 2FA is disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3f14dd21-2f56-45f8-abf1-c8c3436b6306
⛔ Files ignored due to path filters (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.jsonsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/AccountSection.razorsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangeEmailTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePasswordTab.razorsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePasswordTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePhoneNumberTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/TwoFactorSection.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Infrastructure/Services/SnackBarService.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Features/Identity/Dtos/IdentityJsonContext.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.ar.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.de.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.es.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fr.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.hi.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.nl.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.sv.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.zh.resx
a307432 to
770a779
Compare
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Tenants/ManageMyTenantsPage.razor.cs (1)
86-94: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse
LoadTenants()for switches/leaves andRefresh()for renames
SwitchTo,CreateTenant, andLeaveTenantalready change the auth state;MainLayoutpicks up the new tenant from that path, so republishing here can race and send the old tenant back to the layout.UpdateTenantneedsRefresh()so the renamed tenant is published toMainLayout;LoadTenants()alone leaves the old name visible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Tenants/ManageMyTenantsPage.razor.cs` around lines 86 - 94, Update the tenant mutation flows so SwitchTo, CreateTenant, and LeaveTenant call LoadTenants() after successful auth-state changes instead of Refresh(), avoiding republishing stale tenant state; keep Refresh() in UpdateTenant so renamed tenant data is published to MainLayout.
🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.scss (1)
129-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStylelint reports an empty-line-before error on the
//#endifcomment.Stylelint flags
scss/double-slash-comment-empty-line-beforeat line 137. If CI enforces stylelint, this will fail the build.🔧 Proposed fix
.current-tenant { max-width: 60%; position: absolute; inset-block-end: 0; inset-inline-start: 0; } + //#endif🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.scss` around lines 129 - 137, Update the conditional SCSS block around .current-tenant so the //#endif directive satisfies the scss/double-slash-comment-empty-line-before rule, adding the required empty line before it while preserving the existing styles and template directives.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePhoneNumberTab.razor.cs`:
- Around line 94-95: Update the post-save flow around CurrentUser and
PubSubService.Publish to handle a null CurrentUser before assigning PhoneNumber
or publishing the profile update. Use an explicit null guard so a successful
server call still completes the tab’s cleanup path without dereferencing
CurrentUser!.
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/PasswordlessTab.razor`:
- Line 19: Update the passwordless disable flow around DisablePasswordless so
the disable action is unavailable when WebAuthn is not supported, not only when
enrollment is unavailable. Apply the existing isAvailable state to the
configured branch, or provide an explicit recovery state that prevents invoking
DisablePasswordless without WebAuthn capability.
---
Outside diff comments:
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Tenants/ManageMyTenantsPage.razor.cs`:
- Around line 86-94: Update the tenant mutation flows so SwitchTo, CreateTenant,
and LeaveTenant call LoadTenants() after successful auth-state changes instead
of Refresh(), avoiding republishing stale tenant state; keep Refresh() in
UpdateTenant so renamed tenant data is published to MainLayout.
---
Nitpick comments:
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.scss`:
- Around line 129-137: Update the conditional SCSS block around .current-tenant
so the //#endif directive satisfies the
scss/double-slash-comment-empty-line-before rule, adding the required empty line
before it while preserving the existing styles and template directives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ad74ae04-565e-4665-80ab-1fc8479bc70f
⛔ Files ignored due to path filters (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.jsonsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razorsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.scsssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/AppMenu.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razorsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/AccountSection.razorsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/AccountSection.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangeEmailTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePasswordTab.razorsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePasswordTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/ChangePhoneNumberTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/PasswordlessTab.razorsrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/PasswordlessTab.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/TwoFactorSection.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Tenants/ManageMyTenantsPage.razor.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Infrastructure/Services/ClientAppMessages.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Infrastructure/Services/SnackBarService.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Features/Identity/Dtos/IdentityJsonContext.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.ar.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.de.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.es.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fr.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.hi.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.nl.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.sv.resxsrc/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.zh.resx
💤 Files with no reviewable changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/AccountSection.razor.cs
770a779 to
54a08e2
Compare
Summary
Changing a security-sensitive setting regenerates the user's ASP.NET Core Identity security stamp, which the server re-validates on every token refresh (
IdentityController.Refresh→ValidateSecurityStampAsync). As a result, all of the user's sessions — including the current device — get signed out on the next refresh. This previously happened silently. This PR warns the user with a localized snackbar right after such an operation succeeds, adds a Change Password tab, and stops forcing a full app reload after account edits.Changes
Warn about the imminent sign-out (security-stamp-changing operations):
SnackBarService.Warning(usesBitColor.Warning).SignOutOfAllDevicesWarningMessageresource string to all 10 locale.resxfiles.TwoFactorSection: enabling 2FA, disabling 2FA, and resetting the authenticator key (also guards the success/warning snackbars so they no longer show when the request failed).ChangePhoneNumberTab: changing the phone number.ChangePasswordTab(new): changing the password.New Change Password tab:
ChangePasswordTab(backed by the existingChangePasswordendpoint) to the account settings, placed before the Passwordless tab. It shows a success snackbar plus the sign-out warning.ChangePasswordButtonTextandPasswordChangedSuccessfullyMessageresources across all 10 locales.No more full app reload on account edits:
ChangePhoneNumberTabandChangeEmailTabpreviously did a forced reload (NavigateTo(..., forceLoad: true)), which also wiped any snackbar. They now refresh the cached user viaPROFILE_UPDATEDand re-render the current page withNavigationManager.RefreshCurrentPage(). Changing the email does not regenerate the security stamp, so no sign-out warning is shown there.Notes
ChangeUserNamealso regenerates the stamp but currently has no UI;ResetPasswordis the unauthenticated forgot-password flow — so no snackbar location applies to them here.ChangeEmailimplementation sets the email through the store directly and does not regenerate the security stamp, so it does not sign the user out (arguably a separate inconsistency, left out of scope).Related Issue
closes #12647
Summary by CodeRabbit