Skip to content

fix: disable password change when password auth is disabled#2546

Open
xingzihai wants to merge 1 commit intokarakeep-app:mainfrom
xingzihai:fix/784-disable-change-password-when-password-auth-disabled
Open

fix: disable password change when password auth is disabled#2546
xingzihai wants to merge 1 commit intokarakeep-app:mainfrom
xingzihai:fix/784-disable-change-password-when-password-auth-disabled

Conversation

@xingzihai
Copy link
Copy Markdown
Contributor

Summary

Fixes #784 by making password-change behavior consistent with auth config when password login is disabled.

Root Cause

users.changePassword could still be called even when auth.disablePasswordAuth=true, and the settings UI still rendered the Change Password section.

Changes

  • Web: hide ChangePassword settings section when disablePasswordAuth is enabled.
  • tRPC: add guard in users.changePassword to reject with FORBIDDEN when password auth is disabled.
  • Tests: add regression test ensuring changePassword throws when password auth is disabled.

Verification

  • Passed pre-commit preflight (typecheck, lint, format).
  • OpenAPI check passed.
  • Added/updated unit tests in packages/trpc/routers/users.test.ts.

Risk

Low. Small, issue-scoped change with explicit backend guard + test coverage.

Rollback

Revert commit ca378d5e15e5658d449595fcef0c8db19d892c49.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85e63ae and ca378d5.

📒 Files selected for processing (3)
  • apps/web/components/settings/ChangePassword.tsx
  • packages/trpc/routers/users.test.ts
  • packages/trpc/routers/users.ts

Walkthrough

This PR adds configuration-driven password authentication disabling. It includes a frontend UI component guard to conditionally hide the password change form, a backend mutation guard to reject password changes when disabled, and test coverage validating this behavior.

Changes

Cohort / File(s) Summary
Frontend UI Component
apps/web/components/settings/ChangePassword.tsx
Imports useClientConfig and adds early return to hide password change UI when disablePasswordAuth is true in client config.
Backend Router
packages/trpc/routers/users.ts
Adds guard in changePassword mutation to throw FORBIDDEN error and reject password changes when serverConfig.auth.disablePasswordAuth is enabled.
Router Tests
packages/trpc/routers/users.test.ts
Introduces authConfigOverrides to dynamically control auth config in tests. Adds test case to verify password change is rejected when password auth is disabled. Note: duplicate test case exists.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: disabling password change when password auth is disabled.
Description check ✅ Passed The description comprehensively covers the changes, root cause, modifications across web/tRPC/tests, and includes verification details.
Linked Issues check ✅ Passed The PR fulfills all requirements from issue #784: hiding ChangePassword UI when disablePasswordAuth is true and preventing password changes via backend guard.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #784: ChangePassword UI hiding, tRPC guard, and regression tests with no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 2, 2026

Greptile Summary

This PR successfully prevents password changes when password authentication is disabled, fixing issue #784.

Major changes:

  • Frontend: ChangePassword component returns null when disablePasswordAuth is enabled, preventing UI from rendering
  • Backend: users.changePassword mutation guards against calls when password auth is disabled, throwing FORBIDDEN error
  • Tests: Added regression test ensuring proper error is thrown when password auth is disabled

The implementation is clean and follows existing patterns in the codebase (similar to how CredentialsForm.tsx handles the same config). The backend guard ensures security even if the frontend check is bypassed.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Well-scoped change with both frontend and backend guards, consistent implementation patterns, comprehensive test coverage, and no identified security or logic issues
  • No files require special attention

Important Files Changed

Filename Overview
apps/web/components/settings/ChangePassword.tsx Hides password change UI when disablePasswordAuth is enabled, consistent with similar patterns in codebase
packages/trpc/routers/users.ts Adds backend guard to reject changePassword with FORBIDDEN when password auth is disabled
packages/trpc/routers/users.test.ts Adds regression test ensuring changePassword throws when password auth is disabled

Last reviewed commit: ca378d5

@MohamedBassem
Copy link
Copy Markdown
Collaborator

Thanks for the PR! I think it's fine the keep the password form when password auth is disabled. Maybe instead, we can just hide it if the user is !localUser as it means that they just authed with oauth and no password set for their account.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disable password change if password login is disabled

2 participants