Add local account management controls#978
Open
CoderLuii wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds server-side JWT token generation invalidation, a new change-password backend route with supporting repository methods, and a client-facing Account settings tab enabling password changes and logout, wired through updated auth context, API utilities, and settings navigation. ChangesAccount Settings and Change Password
Sequence Diagram(s)sequenceDiagram
participant AccountSettingsTab
participant AuthContext
participant ChangePasswordRoute
participant UserDb
participant AppConfigDb
AccountSettingsTab->>AuthContext: changePassword(currentPassword, newPassword)
AuthContext->>ChangePasswordRoute: POST /api/auth/change-password
ChangePasswordRoute->>UserDb: getUserAuthById(userId)
ChangePasswordRoute->>ChangePasswordRoute: verify currentPassword
ChangePasswordRoute->>UserDb: updatePasswordHash(userId, newHash)
ChangePasswordRoute->>AppConfigDb: write new AUTH_TOKEN_GENERATION_KEY
ChangePasswordRoute-->>AuthContext: success response
AuthContext->>AuthContext: clearSession()
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
server/routes/auth.js (1)
38-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMessage text drifts from
PASSWORD_MIN_LENGTH.The validations use
PASSWORD_MIN_LENGTH, but the error strings hardcode"6 characters"(Line 39 and Line 148). If the constant changes, the messages will be misleading. Consider interpolating the constant.♻️ Interpolate the constant
- return res.status(400).json({ error: 'New password must be at least 6 characters' }); + return res.status(400).json({ error: `New password must be at least ${PASSWORD_MIN_LENGTH} characters` });Line 39 (registration) can be updated similarly.
Also applies to: 147-149
🤖 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 `@server/routes/auth.js` around lines 38 - 40, The validation error messages in the auth flow are hardcoding the password length instead of reflecting PASSWORD_MIN_LENGTH. Update the response strings in the registration and related checks within auth.js to interpolate PASSWORD_MIN_LENGTH so the message stays aligned with the actual validation logic. Use the existing validation branches around the username/password checks and keep the wording consistent across both occurrences.src/components/settings/view/tabs/AccountSettingsTab.tsx (1)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse a shared minimum-length constant instead of hardcoding
6.The length check and its message hardcode
6, while the server validates againstPASSWORD_MIN_LENGTH(server/routes/auth.js). If that constant changes, this client validation and copy silently drift out of sync. Consider importing/deriving a shared constant and interpolating it into the message.🤖 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/components/settings/view/tabs/AccountSettingsTab.tsx` around lines 28 - 30, The password validation in AccountSettingsTab is hardcoding the minimum length, which can drift from the server’s PASSWORD_MIN_LENGTH. Update the logic in the validation flow that checks formState.newPassword so it uses a shared minimum-length constant instead of a literal 6, and make the returned error message interpolate that same constant. Keep the client-side check and copy aligned with the server-side password rule.
🤖 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/components/settings/types/types.ts`:
- Line 6: The settings tab normalization is missing the new 'account' tab, so
URL/command-palette navigation falls back to 'agents' instead of opening
account. Update the KNOWN_MAIN_TABS array in useSettingsController.ts to include
'account' so normalizeMainTab recognizes it, and keep it aligned with the
SettingsMainTab union in types.ts.
In `@src/components/settings/view/tabs/AccountSettingsTab.tsx`:
- Around line 153-157: The error message in AccountSettingsTab is only visible
and not announced to assistive tech. Update the error container rendered in the
error && block to use an alert announcement pattern, such as adding role="alert"
or an appropriate aria-live region, so validation and submission failures are
surfaced by screen readers. Keep the change localized to the error UI in
AccountSettingsTab.
---
Nitpick comments:
In `@server/routes/auth.js`:
- Around line 38-40: The validation error messages in the auth flow are
hardcoding the password length instead of reflecting PASSWORD_MIN_LENGTH. Update
the response strings in the registration and related checks within auth.js to
interpolate PASSWORD_MIN_LENGTH so the message stays aligned with the actual
validation logic. Use the existing validation branches around the
username/password checks and keep the wording consistent across both
occurrences.
In `@src/components/settings/view/tabs/AccountSettingsTab.tsx`:
- Around line 28-30: The password validation in AccountSettingsTab is hardcoding
the minimum length, which can drift from the server’s PASSWORD_MIN_LENGTH.
Update the logic in the validation flow that checks formState.newPassword so it
uses a shared minimum-length constant instead of a literal 6, and make the
returned error message interpolate that same constant. Keep the client-side
check and copy aligned with the server-side password rule.
🪄 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: 8d89ef01-1cf3-4fa2-927e-8de224179af0
📒 Files selected for processing (13)
server/middleware/auth.jsserver/modules/database/repositories/app-config.tsserver/modules/database/repositories/users.tsserver/routes/auth.jssrc/components/auth/context/AuthContext.tsxsrc/components/auth/types.tssrc/components/settings/constants/constants.tssrc/components/settings/types/types.tssrc/components/settings/view/Settings.tsxsrc/components/settings/view/SettingsMainTabs.tsxsrc/components/settings/view/SettingsSidebar.tsxsrc/components/settings/view/tabs/AccountSettingsTab.tsxsrc/utils/api.js
| import type { ProviderAuthStatus } from '../../provider-auth/types'; | ||
|
|
||
| export type SettingsMainTab = 'agents' | 'appearance' | 'git' | 'api' | 'voice' | 'tasks' | 'browser' | 'notifications' | 'plugins' | 'about'; | ||
| export type SettingsMainTab = 'account' | 'agents' | 'appearance' | 'git' | 'api' | 'voice' | 'tasks' | 'browser' | 'notifications' | 'plugins' | 'about'; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
KNOWN_MAIN_TABS in useSettingsController.ts does not include 'account', making the tab unreachable via URL/command-palette normalization.
The SettingsMainTab type now includes 'account', but the upstream controller's KNOWN_MAIN_TABS array (in src/components/settings/hooks/useSettingsController.ts:54) is still ['agents', 'appearance', 'git', 'api', 'tasks', 'browser', 'notifications', 'plugins', 'about'] — 'account' is missing. The normalizeMainTab function falls back to 'agents' for any tab not in that array. This means:
- Opening settings with
?tab=account(e.g., via the command palette, which callsonOpenSettings('account')) will silently redirect to the'agents'tab. - The account tab will only work when activated by clicking the sidebar/main-tab button directly (bypassing normalization), creating inconsistent behavior.
Update KNOWN_MAIN_TABS to include 'account':
-const KNOWN_MAIN_TABS: SettingsMainTab[] = ['agents', 'appearance', 'git', 'api', 'tasks', 'browser', 'notifications', 'plugins', 'about'];
+const KNOWN_MAIN_TABS: SettingsMainTab[] = ['account', 'agents', 'appearance', 'git', 'api', 'tasks', 'browser', 'notifications', 'plugins', 'about'];🤖 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/components/settings/types/types.ts` at line 6, The settings tab
normalization is missing the new 'account' tab, so URL/command-palette
navigation falls back to 'agents' instead of opening account. Update the
KNOWN_MAIN_TABS array in useSettingsController.ts to include 'account' so
normalizeMainTab recognizes it, and keep it aligned with the SettingsMainTab
union in types.ts.
Comment on lines
+153
to
+157
| {error && ( | ||
| <div className="rounded-md border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-700 dark:border-red-900/50 dark:bg-red-950/30 dark:text-red-200"> | ||
| {error} | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Announce the error to assistive tech.
The error container is rendered visually but isn't announced to screen readers. Add role="alert" (or an aria-live region) so validation and submission failures are surfaced.
♿ Proposed a11y fix
- {error && (
- <div className="rounded-md border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-700 dark:border-red-900/50 dark:bg-red-950/30 dark:text-red-200">
+ {error && (
+ <div role="alert" className="rounded-md border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-700 dark:border-red-900/50 dark:bg-red-950/30 dark:text-red-200">
{error}
</div>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {error && ( | |
| <div className="rounded-md border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-700 dark:border-red-900/50 dark:bg-red-950/30 dark:text-red-200"> | |
| {error} | |
| </div> | |
| )} | |
| {error && ( | |
| <div role="alert" className="rounded-md border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-700 dark:border-red-900/50 dark:bg-red-950/30 dark:text-red-200"> | |
| {error} | |
| </div> | |
| )} |
🤖 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/components/settings/view/tabs/AccountSettingsTab.tsx` around lines 153 -
157, The error message in AccountSettingsTab is only visible and not announced
to assistive tech. Update the error container rendered in the error && block to
use an alert announcement pattern, such as adding role="alert" or an appropriate
aria-live region, so validation and submission failures are surfaced by screen
readers. Keep the change localized to the error UI in AccountSettingsTab.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds local account controls for self-hosted CloudCLI installs:
POST /api/auth/change-passwordfor local password rotationWhy
Local installs can create an account during setup, but there is no UI path to log out or rotate that password afterward. The practical workaround becomes deleting local auth state, which is risky and easy to get wrong.
This keeps the existing single-user local model intact while adding the missing account-management path.
Validation
npm cinpm run typechecknpm run buildnpm run lintexits 0 with existing warningsFixes #797.
Summary by CodeRabbit
New Features
Bug Fixes