Skip to content

Fix vault import current password verification#31

Merged
sidhujag merged 3 commits intomasterfrom
fix/vault-import-current-password-auth
Apr 25, 2026
Merged

Fix vault import current password verification#31
sidhujag merged 3 commits intomasterfrom
fix/vault-import-current-password-auth

Conversation

@sidhujag
Copy link
Copy Markdown
Member

Summary

  • Requires first vault imports to verify the typed current account password before encrypting the new vault.
  • Removes local new-password policy gating from the import current-password field so the server credential check is authoritative.
  • Adds frontend coverage for empty password feedback, mismatch feedback, verify callback wiring, and VaultContext anti-divergence guards.

Test plan

  • npm test -- --watchAll=false src/components/VaultImportModal.test.js src/context/VaultContext.test.js
  • ReadLints on changed frontend files

Dependency

  • Requires the backend /auth/verify-password endpoint PR before deployment.

Made with Cursor

Ensure first vault imports prove the typed password against the active account before encrypting, preventing account/vault password divergence.

Made-with: Cursor
@sidhujag
Copy link
Copy Markdown
Member Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6631351e55

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/authService.js Outdated
Comment on lines +156 to +159
if (status === 401) {
const e = new Error('invalid_credentials');
e.code = 'invalid_credentials';
throw e;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Do not coerce all verify 401s to invalid_credentials

verifyPassword() currently rewrites every HTTP 401 from /auth/verify-password to invalid_credentials, but 401 can also mean an expired/revoked session (unauthorized) rather than a wrong password. In the new first-write path, VaultContext.save() remaps invalid_credentials to password_mismatch, so an expired session is misreported as “wrong password” and the auth-loss flow is skipped, leaving users unable to complete import until they manually re-authenticate. Preserve backend error codes (or only remap when the code is explicitly invalid_credentials) instead of blanket status-based remapping.

Useful? React with 👍 / 👎.

Only treat explicit invalid-credentials responses as password mismatches so expired sessions still follow the normal auth-loss path.

Made-with: Cursor
@sidhujag
Copy link
Copy Markdown
Member Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b61935736e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/authService.js
(responseData &&
typeof responseData === 'object' &&
(responseData.error || responseData.code));
if (status === 401 && code === 'invalid_credentials') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle session-expiry 401s from verifyPassword

The new verifyPassword path only special-cases 401 invalid_credentials; any other 401 is rethrown as-is. I checked src/lib/apiClient.js and its auth-loss hook is intentionally skipped for URLs under /auth/*, so in the EMPTY first-write flow an expired session on /auth/verify-password will no longer trigger global logout/redirect (the old flow reached /vault/*, which did). Users stay in a stale "logged-in" UI and just see a generic import error, so this should explicitly route non-credential 401s through auth-loss handling.

Useful? React with 👍 / 👎.

Let non-credential verify-password 401s trigger the global auth-loss handler while preserving password mismatch handling.

Made-with: Cursor
@sidhujag
Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@sidhujag sidhujag merged commit 5639683 into master Apr 25, 2026
4 checks passed
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.

1 participant