fix(auth): sign out other sessions after password updates#1801
Conversation
📝 WalkthroughWalkthroughThree Vue authentication pages were updated to change post-action flows: each now performs an explicit sign-out call with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Vue Component
participant Auth as Supabase Auth
participant Nav as Router/Navigation
participant Toast
User->>UI: submit password change / reset
UI->>Auth: update password / complete reset
alt update/reset error
Auth-->>UI: error
UI-->>User: show form error (abort)
else update/reset success
Auth-->>UI: success
UI->>Auth: signOut({ scope: 'others' })
alt signOut error
Auth-->>UI: signOut error
UI-->>User: set form error (abort)
else signOut success
Auth-->>UI: signed out
UI->>Toast: show success toast
UI->>Nav: navigate to /dashboard
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b3c8e34de
ℹ️ 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".
| if (!organizationStore.currentOrganization?.password_has_access) { | ||
| await organizationStore.fetchOrganizations() | ||
| } | ||
| await supabase.auth.signOut({ scope: 'others' }) |
There was a problem hiding this comment.
After a successful password update, this flow no longer refreshes organization data, so users who were blocked by password policy can remain stuck with stale password_has_access === false in the client store until a manual reload. ChangePassword.vue and pages like dashboard.vue derive access gating directly from organizationStore.currentOrganization.password_has_access, so removing the organizationStore.fetchOrganizations() call here regresses the “regain access immediately after changing password” path for locked-out users.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/forgot_password.vue`:
- Around line 142-143: The current flow calls supabase.auth.signOut({ scope:
'others' }) and immediately router.push('/dashboard') without checking the
signOut result; change this to await the signOut response, handle errors (e.g.,
check for error in the returned object or thrown exception from
supabase.auth.signOut) and only call router.push('/dashboard') on success,
otherwise surface/log the error and show an appropriate user-facing message or
retry path; update the logic around supabase.auth.signOut and router.push to
conditionally redirect based on success.
In `@src/pages/onboarding/set_password.vue`:
- Around line 46-48: After showing toast.success, await and check the result of
supabase.auth.signOut({ scope: 'others' }) (or wrap it in try/catch) and only
call router.replace('/dashboard') when signOut completed without error; if
signOut returns an error or throws, surface an error toast (or processLogger)
and do not navigate. Update the code around toast.success,
supabase.auth.signOut, and router.replace to handle the signOut response/error
path explicitly.
In `@src/pages/settings/account/ChangePassword.vue`:
- Line 277: The call to supabase.auth.signOut({ scope: 'others' }) in the
password change flow must have its outcome checked and any error surfaced to the
user form; update the handler around supabase.auth.signOut in ChangePassword.vue
to await the result, inspect the returned error or status (e.g., the
payload/error fields from supabase.auth.signOut), and if revocation fails set
the form error/state (or throw) so the UI shows failure instead of reporting
success—ensure you reference the exact call site (supabase.auth.signOut) and
propagate or display the error message to the component's form error handling
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c2d664a-a9df-4526-8249-2f78994fa1c5
📒 Files selected for processing (3)
src/pages/forgot_password.vuesrc/pages/onboarding/set_password.vuesrc/pages/settings/account/ChangePassword.vue
| toast.success(t('changed-password-suc')) | ||
| await supabase.auth.signOut({ scope: 'others' }) | ||
| router.replace('/dashboard') |
There was a problem hiding this comment.
Don’t navigate on successful password update if session revocation fails.
The flow currently proceeds to /dashboard even if revoking other sessions fails.
Proposed fix
toast.success(t('changed-password-suc'))
- await supabase.auth.signOut({ scope: 'others' })
+ const { error: signOutError } = await supabase.auth.signOut({ scope: 'others' })
+ if (signOutError) {
+ setErrors('set-password', [signOutError.message], {})
+ return
+ }
router.replace('/dashboard')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/onboarding/set_password.vue` around lines 46 - 48, After showing
toast.success, await and check the result of supabase.auth.signOut({ scope:
'others' }) (or wrap it in try/catch) and only call router.replace('/dashboard')
when signOut completed without error; if signOut returns an error or throws,
surface an error toast (or processLogger) and do not navigate. Update the code
around toast.success, supabase.auth.signOut, and router.replace to handle the
signOut response/error path explicitly.
|
I checked the current failing CI job before making changes. The red check is in the Cloudflare backend suite, not the frontend auth files:\n\n- failing suite: \n- failing assertion: \n- location from CI log: \n\nI did not push a fix here because this branch currently contains a much broader backend/security diff in addition to the password-session UI changes, so the failure is not isolated to a simple one-line patch from the PR title alone. The next step is to inspect the subkey ownership behavior in the Cloudflare path on this branch and compare it against the same test on the latest base. |
|
Correcting the exact failing CI details from the Cloudflare backend job:
I still did not push a fix here because this branch currently includes a broader backend/security diff beyond the password-session UI changes, so the failure is not isolated to a simple one-line frontend patch. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/settings/account/ChangePassword.vue (1)
262-288:⚠️ Potential issue | 🟡 MinorDo not drop submit lock before post-update security steps finish.
At Line 264,
isLoadingis reset before the organization refresh and Line 281 sign-out call, which re-enables submit too early.Proposed fix
async function submit(form: { current_password?: string, password: string, password_confirm: string }) { if (isLoading.value) return isLoading.value = true - - if (needsReauthentication.value) { - const currentPasswordValid = await verifyCurrentPassword(form.current_password ?? '') - if (!currentPasswordValid) { - isLoading.value = false - return - } - } - - const mfaOk = await ensureMfaIfNeeded() - if (!mfaOk) { - isLoading.value = false - return - } - const { error: updateError } = await supabase.auth.updateUser({ password: form.password }) - - isLoading.value = false - if (updateError) { + try { + if (needsReauthentication.value) { + const currentPasswordValid = await verifyCurrentPassword(form.current_password ?? '') + if (!currentPasswordValid) + return + } + + const mfaOk = await ensureMfaIfNeeded() + if (!mfaOk) + return + + const { error: updateError } = await supabase.auth.updateUser({ password: form.password }) + if (updateError) { if (updateError.code === 'reauthentication_needed' || updateError.code === 'reauthentication_not_valid') { needsReauthentication.value = true - isLoading.value = false return } setErrors('change-pass', [t('account-password-error')], {}) - } - else { - needsReauthentication.value = false - resetCaptcha() + } + else { + needsReauthentication.value = false + resetCaptcha() - if (!organizationStore.currentOrganization?.password_has_access) { - await organizationStore.fetchOrganizations() - } + if (!organizationStore.currentOrganization?.password_has_access) { + await organizationStore.fetchOrganizations() + } - const { error: signOutError } = await supabase.auth.signOut({ scope: 'others' }) - if (signOutError) { - setErrors('change-pass', [signOutError.message], {}) - return - } + const { error: signOutError } = await supabase.auth.signOut({ scope: 'others' }) + if (signOutError) { + setErrors('change-pass', [signOutError.message], {}) + return + } - toast.success(t('changed-password-suc')) - } - if (!updateError) { - form.password = '' - form.password_confirm = '' - form.current_password = '' + toast.success(t('changed-password-suc')) + form.password = '' + form.password_confirm = '' + form.current_password = '' + } + } + finally { + isLoading.value = false } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/settings/account/ChangePassword.vue` around lines 262 - 288, The submit lock (isLoading.value) is released too early—remove the early isLoading.value = false placed immediately after supabase.auth.updateUser and instead set isLoading.value = false only after all post-update security steps finish: keep the existing early release in the reauthentication branch (needsReauthentication.value = true) but move the general error path to release isLoading after calling setErrors('change-pass', ...) and ensure the signOutError path releases isLoading before returning; finally, after successful flow (resetCaptcha, optional organizationStore.fetchOrganizations, supabase.auth.signOut, toast.success) release isLoading.value = false. Reference functions/vars: isLoading, updateError, needsReauthentication, setErrors('change-pass', ...), resetCaptcha, organizationStore.fetchOrganizations, supabase.auth.signOut, toast.success.src/pages/onboarding/set_password.vue (1)
37-52:⚠️ Potential issue | 🟡 MinorKeep loading active until revocation finishes.
At Line 40,
isLoadingis cleared before the Line 46 sign-out call completes, so users can re-submit while session revocation is still running.Proposed fix
async function submit(form: { password: string }) { isLoading.value = true - - const { error: updateError } = await supabase.auth.updateUser({ password: form.password }) - isLoading.value = false - if (updateError) { - setErrors('set-password', [updateError.message], {}) - return - } - - const { error: signOutError } = await supabase.auth.signOut({ scope: 'others' }) - if (signOutError) { - setErrors('set-password', [signOutError.message], {}) - return - } - toast.success(t('changed-password-suc')) - router.replace('/dashboard') + try { + const { error: updateError } = await supabase.auth.updateUser({ password: form.password }) + if (updateError) { + setErrors('set-password', [updateError.message], {}) + return + } + + const { error: signOutError } = await supabase.auth.signOut({ scope: 'others' }) + if (signOutError) { + setErrors('set-password', [signOutError.message], {}) + return + } + + toast.success(t('changed-password-suc')) + router.replace('/dashboard') + } + finally { + isLoading.value = false + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/onboarding/set_password.vue` around lines 37 - 52, Keep the loading indicator active until the session revocation finishes by moving isLoading.value = false out of the middle of the flow: remove the early isLoading.value = false after await supabase.auth.updateUser, and instead set isLoading.value = false in each return path (when updateError or signOutError occur) and after a successful sign-out (just before calling toast.success and router.replace). Update the block around supabase.auth.updateUser, supabase.auth.signOut, setErrors, toast, and router.replace so loading stays true during signOut and is explicitly cleared in both error branches and the success branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/pages/onboarding/set_password.vue`:
- Around line 37-52: Keep the loading indicator active until the session
revocation finishes by moving isLoading.value = false out of the middle of the
flow: remove the early isLoading.value = false after await
supabase.auth.updateUser, and instead set isLoading.value = false in each return
path (when updateError or signOutError occur) and after a successful sign-out
(just before calling toast.success and router.replace). Update the block around
supabase.auth.updateUser, supabase.auth.signOut, setErrors, toast, and
router.replace so loading stays true during signOut and is explicitly cleared in
both error branches and the success branch.
In `@src/pages/settings/account/ChangePassword.vue`:
- Around line 262-288: The submit lock (isLoading.value) is released too
early—remove the early isLoading.value = false placed immediately after
supabase.auth.updateUser and instead set isLoading.value = false only after all
post-update security steps finish: keep the existing early release in the
reauthentication branch (needsReauthentication.value = true) but move the
general error path to release isLoading after calling setErrors('change-pass',
...) and ensure the signOutError path releases isLoading before returning;
finally, after successful flow (resetCaptcha, optional
organizationStore.fetchOrganizations, supabase.auth.signOut, toast.success)
release isLoading.value = false. Reference functions/vars: isLoading,
updateError, needsReauthentication, setErrors('change-pass', ...), resetCaptcha,
organizationStore.fetchOrganizations, supabase.auth.signOut, toast.success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c9dee05-c835-40a5-8ff1-a220b9892225
📒 Files selected for processing (3)
src/pages/forgot_password.vuesrc/pages/onboarding/set_password.vuesrc/pages/settings/account/ChangePassword.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/forgot_password.vue
|
1 participant
Summary (AI generated)
scope: 'others'after successful password updates in the account change-password, forgot-password, and onboarding set-password flows.Motivation (AI generated)
Business Impact (AI generated)
Test Plan (AI generated)
bun run lint:backend && bun run lintScreenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.accordingly.
my tests
Generated with AI
Summary by CodeRabbit