Repository navigation
feat(app): sign-out revokes the session in Crove first - #178
Merged
Merged
Conversation
Every SPA sign-out control now goes through one helper that revokes the current session via sessions.revoke (3 s timeout, best effort) and then ends the Supabase session locally. The sessions screen keeps its own flow to avoid a double revoke. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit details
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust sign-out flow that attempts to revoke the current session in Crove before signing out of Supabase Auth, updating various routes to use the new signOut helper and adding comprehensive unit tests. Feedback on the implementation highlights a potential unhandled promise rejection in signOutWithRevoke if the revoke request fails after the timeout has already resolved Promise.race; attaching a .catch() handler to the revoke promise is recommended to safely handle this scenario.
Comment on lines
+21
to
+27
| const timeout = new Promise<void>((resolve) => { | ||
| timer = setTimeout(() => { | ||
| controller.abort(); | ||
| resolve(); | ||
| }, deps.timeoutMs ?? REVOKE_TIMEOUT_MS); | ||
| }); | ||
| await Promise.race([deps.revoke(session, controller.signal), timeout]); |
There was a problem hiding this comment.
If the timeout resolves first, Promise.race settles and the execution proceeds to finally and deps.signOut(). However, the pending deps.revoke promise continues running in the background. When controller.abort() is called, the aborted deps.revoke promise will eventually reject. Since Promise.race has already settled, this rejection occurs outside the try/catch block, leading to an unhandled promise rejection.
To prevent this, attach a .catch(() => {}) handler directly to the deps.revoke promise to safely catch and ignore any late rejections.
const timeout = new Promise<void>((resolve) => {
timer = setTimeout(() => {
controller.abort();
resolve();
}, deps.timeoutMs ?? REVOKE_TIMEOUT_MS);
});
const revokePromise = deps.revoke(session, controller.signal).catch(() => {});
await Promise.race([revokePromise, timeout]);Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Review follow-up: the 3 s budget now covers reading the session too. Known limit, tracked in #184: sessions.revoke needs an org header server side, so on onboarding (no org) the revoke fails and sign-out falls back to Supabase only (needs a protected-path server change). |
JOY (JOY)
pushed a commit
that referenced
this pull request
Oct 15, 2026
… step-up (#32) Rebased onto main with member invites, owner confirmations (#119), the sessions screen (#147, #178) and MFA_REQUIRED (#126), so the earlier commits of this PR are squashed into one. - auth-js uses PKCE. settleCallback() runs before the router: it explains failed links by known error codes only (a link's error_description is attacker-written), notes a link opened in another browser, sends recovery links to /reset-password, and removes the one-time code and error parameters from the URL and history. - /reset-password asks for the TOTP code first when the user has a factor; /mfa is the sign-in challenge, and every data route waits for aal2. MFA_REQUIRED from the kernel (#126) now sends the user to /mfa instead of showing an error; /mfa signs out through the shared sign-out that revokes the Crove session first (#178). - Settings: two-factor card (TOTP enroll, verify, disable). - Step-up (P11): approving, inviting, changing a role or removing a member (inside #119's confirmation) and rotating the inbound address show a prompt on STEP_UP_REQUIRED that names the pending action, asks for a fresh code or points to enabling two-factor, and repeats the refused request as sent (an approval prompted on one revision gets STALE_REVISION if someone saved another meanwhile). - Compose: GOTRUE_MAILER_URLPATHS_* include the /auth/v1 prefix (email links were broken on self-host), and AUTH_STEP_UP defaults to required. - e2e with email confirmation on (compose.e2e.yml, Mailpit, RFC 6238 TOTP, an inbound address for the rotation): confirmation and reused links, a forged link error, recovery, TOTP enrollment and the sign-in challenge, refresh and sign-out, the invite, owner confirmation, role change and removal step-up flows across two browsers, the rotation step-up, and the Inbox approval with its denied and aal2 audit rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LrvuThKGaPX1uDwUiwmGQ
JOY (JOY)
pushed a commit
that referenced
this pull request
Oct 16, 2026
… step-up (#32) Rebased onto main with member invites, owner confirmations (#119), the sessions screen (#147, #178) and MFA_REQUIRED (#126), so the earlier commits of this PR are squashed into one. - auth-js uses PKCE. settleCallback() runs before the router: it explains failed links by known error codes only (a link's error_description is attacker-written), notes a link opened in another browser, sends recovery links to /reset-password, and removes the one-time code and error parameters from the URL and history. - /reset-password asks for the TOTP code first when the user has a factor; /mfa is the sign-in challenge, and every data route waits for aal2. MFA_REQUIRED from the kernel (#126) now sends the user to /mfa instead of showing an error; /mfa signs out through the shared sign-out that revokes the Crove session first (#178). - Settings: two-factor card (TOTP enroll, verify, disable). - Step-up (P11): approving, inviting, changing a role or removing a member (inside #119's confirmation) and rotating the inbound address show a prompt on STEP_UP_REQUIRED that names the pending action, asks for a fresh code or points to enabling two-factor, and repeats the refused request as sent (an approval prompted on one revision gets STALE_REVISION if someone saved another meanwhile). - Compose: GOTRUE_MAILER_URLPATHS_* include the /auth/v1 prefix (email links were broken on self-host), and AUTH_STEP_UP defaults to required. - e2e with email confirmation on (compose.e2e.yml, Mailpit, RFC 6238 TOTP, an inbound address for the rotation): confirmation and reused links, a forged link error, recovery, TOTP enrollment and the sign-in challenge, refresh and sign-out, the invite, owner confirmation, role change and removal step-up flows across two browsers, the rotation step-up, and the Inbox approval with its denied and aal2 audit rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LrvuThKGaPX1uDwUiwmGQ
JOY (JOY)
pushed a commit
that referenced
this pull request
Oct 16, 2026
… step-up (#32) Rebased onto main with member invites, owner confirmations (#119), the sessions screen (#147, #178) and MFA_REQUIRED (#126), so the earlier commits of this PR are squashed into one. - auth-js uses PKCE. settleCallback() runs before the router: it explains failed links by known error codes only (a link's error_description is attacker-written), notes a link opened in another browser, sends recovery links to /reset-password, and removes the one-time code and error parameters from the URL and history. - /reset-password asks for the TOTP code first when the user has a factor; /mfa is the sign-in challenge, and every data route waits for aal2. MFA_REQUIRED from the kernel (#126) now sends the user to /mfa instead of showing an error; /mfa signs out through the shared sign-out that revokes the Crove session first (#178). - Settings: two-factor card (TOTP enroll, verify, disable). - Step-up (P11): approving, inviting, changing a role or removing a member (inside #119's confirmation) and rotating the inbound address show a prompt on STEP_UP_REQUIRED that names the pending action, asks for a fresh code or points to enabling two-factor, and repeats the refused request as sent (an approval prompted on one revision gets STALE_REVISION if someone saved another meanwhile). - Compose: GOTRUE_MAILER_URLPATHS_* include the /auth/v1 prefix (email links were broken on self-host), and AUTH_STEP_UP defaults to required. - e2e with email confirmation on (compose.e2e.yml, Mailpit, RFC 6238 TOTP, an inbound address for the rotation): confirmation and reused links, a forged link error, recovery, TOTP enrollment and the sign-in challenge, refresh and sign-out, the invite, owner confirmation, role change and removal step-up flows across two browsers, the rotation step-up, and the Inbox approval with its denied and aal2 audit rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LrvuThKGaPX1uDwUiwmGQ
JOY (JOY)
pushed a commit
that referenced
this pull request
Oct 16, 2026
… step-up (#32) Rebased onto main with member invites, owner confirmations (#119), the sessions screen (#147, #178) and MFA_REQUIRED (#126), so the earlier commits of this PR are squashed into one. - auth-js uses PKCE. settleCallback() runs before the router: it explains failed links by known error codes only (a link's error_description is attacker-written), notes a link opened in another browser, sends recovery links to /reset-password, and removes the one-time code and error parameters from the URL and history. - /reset-password asks for the TOTP code first when the user has a factor; /mfa is the sign-in challenge, and every data route waits for aal2. MFA_REQUIRED from the kernel (#126) now sends the user to /mfa instead of showing an error; /mfa signs out through the shared sign-out that revokes the Crove session first (#178). - Settings: two-factor card (TOTP enroll, verify, disable). - Step-up (P11): approving, inviting, changing a role or removing a member (inside #119's confirmation) and rotating the inbound address show a prompt on STEP_UP_REQUIRED that names the pending action, asks for a fresh code or points to enabling two-factor, and repeats the refused request as sent (an approval prompted on one revision gets STALE_REVISION if someone saved another meanwhile). - Compose: GOTRUE_MAILER_URLPATHS_* include the /auth/v1 prefix (email links were broken on self-host), and AUTH_STEP_UP defaults to required. - e2e with email confirmation on (compose.e2e.yml, Mailpit, RFC 6238 TOTP, an inbound address for the rotation): confirmation and reused links, a forged link error, recovery, TOTP enrollment and the sign-in challenge, refresh and sign-out, the invite, owner confirmation, role change and removal step-up flows across two browsers, the rotation step-up, and the Inbox approval with its denied and aal2 audit rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LrvuThKGaPX1uDwUiwmGQ
JOY (JOY)
pushed a commit
that referenced
this pull request
Oct 16, 2026
… step-up (#32) Rebased onto main with member invites, owner confirmations (#119), the sessions screen (#147, #178) and MFA_REQUIRED (#126), so the earlier commits of this PR are squashed into one. - auth-js uses PKCE. settleCallback() runs before the router: it explains failed links by known error codes only (a link's error_description is attacker-written), notes a link opened in another browser, sends recovery links to /reset-password, and removes the one-time code and error parameters from the URL and history. - /reset-password asks for the TOTP code first when the user has a factor; /mfa is the sign-in challenge, and every data route waits for aal2. MFA_REQUIRED from the kernel (#126) now sends the user to /mfa instead of showing an error; /mfa signs out through the shared sign-out that revokes the Crove session first (#178). - Settings: two-factor card (TOTP enroll, verify, disable). - Step-up (P11): approving, inviting, changing a role or removing a member (inside #119's confirmation) and rotating the inbound address show a prompt on STEP_UP_REQUIRED that names the pending action, asks for a fresh code or points to enabling two-factor, and repeats the refused request as sent (an approval prompted on one revision gets STALE_REVISION if someone saved another meanwhile). - Compose: GOTRUE_MAILER_URLPATHS_* include the /auth/v1 prefix (email links were broken on self-host), and AUTH_STEP_UP defaults to required. - e2e with email confirmation on (compose.e2e.yml, Mailpit, RFC 6238 TOTP, an inbound address for the rotation): confirmation and reused links, a forged link error, recovery, TOTP enrollment and the sign-in challenge, refresh and sign-out, the invite, owner confirmation, role change and removal step-up flows across two browsers, the rotation step-up, and the Inbox approval with its denied and aal2 audit rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LrvuThKGaPX1uDwUiwmGQ
JOY (JOY)
pushed a commit
that referenced
this pull request
Oct 16, 2026
… step-up (#32) Rebased onto main with member invites, owner confirmations (#119), the sessions screen (#147, #178) and MFA_REQUIRED (#126), so the earlier commits of this PR are squashed into one. - auth-js uses PKCE. settleCallback() runs before the router: it explains failed links by known error codes only (a link's error_description is attacker-written), notes a link opened in another browser, sends recovery links to /reset-password, and removes the one-time code and error parameters from the URL and history. - /reset-password asks for the TOTP code first when the user has a factor; /mfa is the sign-in challenge, and every data route waits for aal2. MFA_REQUIRED from the kernel (#126) now sends the user to /mfa instead of showing an error; /mfa signs out through the shared sign-out that revokes the Crove session first (#178). - Settings: two-factor card (TOTP enroll, verify, disable). - Step-up (P11): approving, inviting, changing a role or removing a member (inside #119's confirmation) and rotating the inbound address show a prompt on STEP_UP_REQUIRED that names the pending action, asks for a fresh code or points to enabling two-factor, and repeats the refused request as sent (an approval prompted on one revision gets STALE_REVISION if someone saved another meanwhile). - Compose: GOTRUE_MAILER_URLPATHS_* include the /auth/v1 prefix (email links were broken on self-host), and AUTH_STEP_UP defaults to required. - e2e with email confirmation on (compose.e2e.yml, Mailpit, RFC 6238 TOTP, an inbound address for the rotation): confirmation and reused links, a forged link error, recovery, TOTP enrollment and the sign-in challenge, refresh and sign-out, the invite, owner confirmation, role change and removal step-up flows across two browsers, the rotation step-up, and the Inbox approval with its denied and aal2 audit rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LrvuThKGaPX1uDwUiwmGQ
JOY (JOY)
pushed a commit
that referenced
this pull request
Oct 16, 2026
… step-up (#32) Rebased onto main with member invites, owner confirmations (#119), the sessions screen (#147, #178) and MFA_REQUIRED (#126), so the earlier commits of this PR are squashed into one. - auth-js uses PKCE. settleCallback() runs before the router: it explains failed links by known error codes only (a link's error_description is attacker-written), notes a link opened in another browser, sends recovery links to /reset-password, and removes the one-time code and error parameters from the URL and history. - /reset-password asks for the TOTP code first when the user has a factor; /mfa is the sign-in challenge, and every data route waits for aal2. MFA_REQUIRED from the kernel (#126) now sends the user to /mfa instead of showing an error; /mfa signs out through the shared sign-out that revokes the Crove session first (#178). - Settings: two-factor card (TOTP enroll, verify, disable). - Step-up (P11): approving, inviting, changing a role or removing a member (inside #119's confirmation) and rotating the inbound address show a prompt on STEP_UP_REQUIRED that names the pending action, asks for a fresh code or points to enabling two-factor, and repeats the refused request as sent (an approval prompted on one revision gets STALE_REVISION if someone saved another meanwhile). - Compose: GOTRUE_MAILER_URLPATHS_* include the /auth/v1 prefix (email links were broken on self-host), and AUTH_STEP_UP defaults to required. - e2e with email confirmation on (compose.e2e.yml, Mailpit, RFC 6238 TOTP, an inbound address for the rotation): confirmation and reused links, a forged link error, recovery, TOTP enrollment and the sign-in challenge, refresh and sign-out, the invite, owner confirmation, role change and removal step-up flows across two browsers, the rotation step-up, and the Inbox approval with its denied and aal2 audit rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LrvuThKGaPX1uDwUiwmGQ
This branch was previously deployed
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.
Closes #150
signOutWithRevokehelper (apps/app/src/lib/sign-out.ts): revokes the current session viasessions.revoke, aborting after 3 s, then signs out. A failed or slow revoke never blocks the sign-out.signOut(). The sessions screen (feat(app): device sessions screen and owner session revoke #147) keeps its own flow (no double revoke), and the 401 handlers keep a plain local sign-out (the token is already rejected).Unit tests: revoke ok, fails, times out (and aborts), no session, session read throws.
🤖 Generated with Claude Code