fix(invitations): extend invite link lifetime to 30 days and revoke on cancel#229
Conversation
…n cancel Invite emails minted a magic-link token that expired after 10 minutes, the same default used for a user-initiated sign-in link. A recipient who did not click within 10 minutes hit a dead link even though the invitation itself stayed valid for days, which made invites hard to act on (#228). Invite links now live as long as the invitation record (30 days for both team and portal invites). Sign-in and recovery links keep the 10-minute default. The token lifetime is derived from the invite lifetime, so the two cannot drift apart. Longer-lived tokens exposed a revocation gap: cancelling an invite left the emailed link able to mint a session until the token's own expiry. Invites now record their current magic-link token, and the cancel, resend, and copy-link paths delete or rotate the backing verification row so a cancelled or superseded link can no longer sign anyone in. Copy-link caps its token at the invite's remaining lifetime so a re-minted link cannot outlive the invite. - Add invitation.magic_link_token column (migration 0111) - mintMagicLinkUrl returns { url, token }; add revokeMagicLinkToken - Add generateInvitationMagicLink + rotateInviteMagicLinkToken helpers - Wire token persistence/rotation into team and portal send, resend, cancel, and copy-link
![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: 7c65196704
ℹ️ 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".
Codex review flagged a TOCTOU race in rotateInviteMagicLinkToken: the
unconditional token UPDATE was last-write-wins, so a resend/copy-link racing a
cancel could re-arm a live link on a cancelled invite, and two concurrent
rotations could orphan a token (live but no longer tracked, so never revoked).
Rotation is now a compare-and-swap on (status='pending', magicLinkToken=prior).
If the row no longer matches, the just-minted token is revoked and the call
throws so the caller doesn't email or return a dead link as success. The cancel
paths now revoke the token returned atomically from their own UPDATE rather than
an earlier read, so a rotation racing a cancel can't leave a live token behind.
Also fixes the CI test failure: mintMagicLinkUrl now returns { url, token }, but
the recovery-codes and email-signin test mocks still returned a bare string, so
the destructured url was undefined. Updated both mocks to the new shape.
- rotateInviteMagicLinkToken: compare-and-swap + revoke-orphan-on-miss
- cancel (team + portal): revoke the token the UPDATE returns, not a stale read
- tests: rotate race-guard coverage; sync mintMagicLinkUrl mocks to { url, token }
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7a5eb0299
ℹ️ 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".
…ery fails Second round of Codex review flagged three cases where rotating the token (revoking the old one) before the new link is safely delivered could strand the recipient: - Team and portal resend revoked the prior token, then sent the email. If the provider threw, the new link was neither delivered nor returned, so a failed resend killed the invitee's still-valid link. Now the prior token is revoked only after the send succeeds; on a hard send failure the just-minted token is revoked (orphan cleanup) and the existing link is left intact. - Copy-link revoked the prior token server-side, but if the browser clipboard write was blocked the UI discarded the returned link, leaving the admin with nothing. The clipboard write is now separated from the fetch: on a clipboard failure the minted link is surfaced for manual copy instead of being dropped. - resend (team + portal): rotate after send; revoke orphan + rethrow on failure - invite-row: fall back to showing the link when clipboard write is blocked - test: portal resend send-failure preserves the old token, drops the new one
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef7518b1f3
ℹ️ 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".
…after Codex flagged that the post-send rotation could email a link and then revoke it: under concurrent resend/copy the compare-and-swap could fail after the email was already sent, and the handler then revoked the just-delivered token, handing the recipient a dead link. Reworked so the emailed/returned token is always recorded on the invite BEFORE delivery, and the prior token is revoked only AFTER delivery succeeds: - rotateInviteMagicLinkToken is replaced by recordInviteMagicLinkToken, a pure compare-and-swap (status='pending' AND magicLinkToken=expected -> next) that returns whether it matched, with no side effects. - resend (team + portal): record the new token, then send; on a send failure roll the record back to the prior token and drop the undelivered new one, so the invitee's existing link still works; revoke the prior token only after a successful send. - copy-link: record then revoke the prior; the link is returned synchronously, so there's no post-record delivery step that can fail. Caught a concurrent resend before it emails (record fails the CAS first), never revokes a token whose link was delivered, and never strands the invitee on a send failure. - tests updated for the record/revoke split + the send-failure rollback
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ac94d5b35
ℹ️ 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".
…otating Clicking "Copy link" minted a fresh token and revoked the prior one, so copying silently killed the link the invitee had already been emailed. Copy is a read-ish action and shouldn't rotate. Industry norm is a single active token: resend rotates (supersedes the prior link), but copy should just hand back the current link. So getPortalInviteLinkFn now returns the invite's current link when its token is still live (rebuilt from the stored token via the shared buildVerifyMagicLinkUrl), and only mints a fresh token when there isn't a live one (never sent, or already used/expired). It no longer revokes anything. - add isMagicLinkTokenLive(token) — verification row exists and not expired - getPortalInviteLinkFn: reuse-or-mint; non-destructive on the reuse path - share the /portal-invite/:id callback path between mint and copy - tests for isMagicLinkTokenLive + copy-link reuse-vs-mint
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99243cca72
ℹ️ 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".
Replaces the single rotating magic-link token with a per-invite token set. The rotation model could never fully close the cancel race: it replaced the one tracked pointer, so during a resend's email-send window (or after a worker restart) the prior token was live but untracked, and a later cancel missed it. Now send/resend/copy each APPEND a token the instant it's minted, and cancel revokes the whole set. A token can never be live-but-untracked, so cancellation is robust to concurrency, send failures, and worker restarts. Resend and copy are additive (prior links keep working until accept/cancel/expiry), which also gives the non-destructive copy-link behavior we want. This is the grant-style bulk-revocation model rather than single-token rotation. - migration 0112: replace invitation.magic_link_token with magic_link_tokens[] - appendInviteMagicLinkToken (status-pinned) + removeInviteMagicLinkToken - findLiveMagicLinkToken + revokeMagicLinkTokens (bulk) in magic-link-mint - send seeds the set; resend appends (drops the new token only if send fails); copy reuses a live token from the set else mints+appends; cancel revokes all - removes the compare-and-swap rotation + rollback machinery
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e29fd8d7e
ℹ️ 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".
…gest-lived token Two follow-ups from the token-set model: - P1: with resend/copy appending tokens, an accepted invite could still have sibling tokens (from a resend or copy) live for their 30-day TTL, so a second link could sign in as that email after acceptance. Both accept paths now revoke the invite's whole token set on success — symmetric with cancel, so acceptance and cancellation are the two terminal states that kill every link. - P2: findLiveMagicLinkToken used limit(1) with no ordering, so copy-link could reuse an older token that expires before the invite. It now orders by expiry descending and returns the longest-lived live token. - accept (team + portal): revokeMagicLinkTokens over the returned token set - findLiveMagicLinkToken: order by verification.expiresAt desc - test: accept revokes the set; mock chain updated for orderBy
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13fe2f0c9a
ℹ️ 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".
… type
The token-set revoke added to the accept paths sat inside the try whose catch
rolls a team accept back to pending on error. If revokeMagicLinkTokens threw
after the membership was already created, cleanup failure would undo the accept
while leaving the membership. The accept is the committed operation; revoking
sibling tokens is cleanup, so it now runs in its own try/catch and only logs on
failure (the stray tokens still expire with the invite). Applied to both the
team and portal accept paths.
Also fixes the typecheck failure CI caught: the accept test's mock return type
was { id: string }[], which rejected the magicLinkTokens field the new
revoke-on-accept test sets — widened it to include the optional field.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b62ac7c93
ℹ️ 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".
…ay default
Two more Codex P2s on the token-set model:
- Copy-link reused the longest-lived token in the set, but after a resend whose
email send failed (the new token removed, only an older shorter-lived token
left) it could hand out a link that expires well before the invite's pending
window. Rather than add coverage checks to the reuse path, copy now simply
mints a fresh token each time and appends it. Minting is additive in the set
model, so it never invalidates a link the invitee already holds, and a fresh
token always covers the invite's remaining lifetime. Removes
findLiveMagicLinkToken and the whole reuse surface.
- invitation.magic_link_tokens used a JS-value array default (.default([]));
switched to the SQL literal .default(sql`'{}'::text[]`) so the schema matches
the hand-written DEFAULT '{}' in migration 0112 and won't drift.
- portal copy-link: mint+append (additive), no reuse
- remove findLiveMagicLinkToken + its tests; copy tests assert additive mint
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
What
Invite email links expired after 10 minutes (the same default used for a user-initiated sign-in link). A recipient who didn't click right away hit a dead link even though the invitation itself stayed valid for days, which made invites hard to act on. Closes #228.
Changes
Notes
invitation.magic_link_token(migration 0111), additive and nullable. Existing pending invites get NULL and their tokens simply self-expire.mintMagicLinkUrlnow returns{ url, token }; the two non-invite callers ignore the token.Testing