From 7c6519670476262c13d3e59222e208d56efe32a6 Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 12:58:02 +0100 Subject: [PATCH 1/9] fix(invitations): extend invite link lifetime to 30 days and revoke on 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 --- .../admin/users/invitations-view.tsx | 2 +- .../src/components/admin/users/invite-row.tsx | 6 +- .../auth/__tests__/magic-link-mint.test.ts | 34 ++++++- apps/web/src/lib/server/auth/email-signin.ts | 2 +- .../src/lib/server/auth/magic-link-mint.ts | 22 ++++- .../__tests__/invitation-magic-link.test.ts | 47 +++++++++ .../__tests__/portal-invites.test.ts | 98 ++++++++++++++++--- apps/web/src/lib/server/functions/admin.ts | 64 ++++++------ .../server/functions/invitation-magic-link.ts | 57 +++++++++++ .../src/lib/server/functions/portal-access.ts | 2 +- .../lib/server/functions/portal-invites.ts | 73 ++++++++++---- .../functions/recovery-codes-consume.ts | 2 +- .../0111_invitation_magic_link_token.sql | 10 ++ packages/db/drizzle/meta/_journal.json | 7 ++ packages/db/src/schema/auth.ts | 7 ++ 15 files changed, 359 insertions(+), 74 deletions(-) create mode 100644 apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts create mode 100644 apps/web/src/lib/server/functions/invitation-magic-link.ts create mode 100644 packages/db/drizzle/0111_invitation_magic_link_token.sql diff --git a/apps/web/src/components/admin/users/invitations-view.tsx b/apps/web/src/components/admin/users/invitations-view.tsx index bac12bb62..3179ac1c5 100644 --- a/apps/web/src/components/admin/users/invitations-view.tsx +++ b/apps/web/src/components/admin/users/invitations-view.tsx @@ -27,7 +27,7 @@ const EMPTY_COPY: Record = { }, expired: { title: 'No expired invitations', - body: 'Pending invitations expire after 14 days. Expired ones show here so you can resend.', + body: 'Pending invitations expire after 30 days. Expired ones show here so you can resend.', }, all: { title: 'No invitations yet', diff --git a/apps/web/src/components/admin/users/invite-row.tsx b/apps/web/src/components/admin/users/invite-row.tsx index 0d676e8be..11d1f174c 100644 --- a/apps/web/src/components/admin/users/invite-row.tsx +++ b/apps/web/src/components/admin/users/invite-row.tsx @@ -62,8 +62,8 @@ interface InviteRowProps { * * The Revoke button uses an inline "confirm" two-step (avoids a dialog * for a per-row destructive action). Copy-link mints a fresh magic-link - * URL valid for ~10 minutes so admins can hand it to invitees out of - * band when SMTP isn't reachable. + * URL (valid for the invite's lifetime) so admins can hand it to invitees + * out of band when SMTP isn't reachable. */ export function InviteRow({ invite, onRevoke, onResend, revoking, resending }: InviteRowProps) { const [confirmRevoke, setConfirmRevoke] = useState(false) @@ -113,7 +113,7 @@ export function InviteRow({ invite, onRevoke, onResend, revoking, resending }: I > {copyState === 'copying' && } {copyState === 'copied' - ? 'Copied — link valid 10 min' + ? 'Link copied' : copyState === 'error' ? 'Copy failed' : 'Copy link'} diff --git a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts index 5cbaabbaf..0b1cdf5d5 100644 --- a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts @@ -25,7 +25,18 @@ vi.mock('../index', async () => { } }) -const { mintMagicLinkUrl } = await import('../magic-link-mint') +const mockDeleteWhere = vi.fn().mockResolvedValue(undefined) +const mockDbDelete = vi.fn(() => ({ where: mockDeleteWhere })) +const mockEq = vi.fn((col: unknown, val: unknown) => ({ col, val })) +const mockVerificationTable = { identifier: 'verification.identifier' } + +vi.mock('@/lib/server/db', () => ({ + db: { delete: mockDbDelete }, + verification: mockVerificationTable, + eq: mockEq, +})) + +const { mintMagicLinkUrl, revokeMagicLinkToken } = await import('../magic-link-mint') beforeEach(() => { vi.clearAllMocks() @@ -76,12 +87,31 @@ describe('mintMagicLinkUrl', () => { }) it('returns a /verify-magic-link URL with the token embedded', async () => { - const url = await mintMagicLinkUrl({ + const { url, token } = await mintMagicLinkUrl({ email: 'a@b.com', callbackPath: '/admin', portalUrl: 'https://acme.test', }) expect(url).toMatch(/^https:\/\/acme\.test\/verify-magic-link\?token=/) expect(url).toContain('callbackURL=') + // The returned token is the verification identifier and the one in the URL. + expect(token).toBe(mockCreateVerificationValue.mock.calls[0][0].identifier) + expect(url).toContain(`token=${token}`) + }) +}) + +describe('revokeMagicLinkToken', () => { + it('deletes the verification row whose identifier is the token', async () => { + await revokeMagicLinkToken('tok_abc') + + expect(mockDbDelete).toHaveBeenCalledTimes(1) + expect(mockDbDelete).toHaveBeenCalledWith(mockVerificationTable) + expect(mockEq).toHaveBeenCalledWith(mockVerificationTable.identifier, 'tok_abc') + expect(mockDeleteWhere).toHaveBeenCalled() + }) + + it('is a no-op when the token is null (invite minted before tracking)', async () => { + await revokeMagicLinkToken(null) + expect(mockDbDelete).not.toHaveBeenCalled() }) }) diff --git a/apps/web/src/lib/server/auth/email-signin.ts b/apps/web/src/lib/server/auth/email-signin.ts index ea1c0faae..03422771d 100644 --- a/apps/web/src/lib/server/auth/email-signin.ts +++ b/apps/web/src/lib/server/auth/email-signin.ts @@ -30,7 +30,7 @@ export async function requestEmailSignin(opts: { // back to /auth/login (the public signup/login screen). const errorCallbackPath = opts.callbackURL.startsWith('/admin') ? '/admin/login' : '/auth/login' - const [signInUrl, , settings] = await Promise.all([ + const [{ url: signInUrl }, , settings] = await Promise.all([ mintMagicLinkUrl({ email: opts.email, callbackPath: opts.callbackURL, diff --git a/apps/web/src/lib/server/auth/magic-link-mint.ts b/apps/web/src/lib/server/auth/magic-link-mint.ts index cd9f9f623..188b47955 100644 --- a/apps/web/src/lib/server/auth/magic-link-mint.ts +++ b/apps/web/src/lib/server/auth/magic-link-mint.ts @@ -1,4 +1,5 @@ import { generateRandomString } from 'better-auth/crypto' +import { db, verification, eq } from '@/lib/server/db' import { getAuth } from './index' interface MintOptions { @@ -45,7 +46,7 @@ export function buildVerifyMagicLinkUrl(opts: { * for internal token-mint. Token format mirrors BA's magic-link * plugin so its `/magic-link/verify` endpoint reads our row. */ -export async function mintMagicLinkUrl(opts: MintOptions): Promise { +export async function mintMagicLinkUrl(opts: MintOptions): Promise<{ url: string; token: string }> { const auth = await getAuth() const token = generateRandomString(32, 'a-z', 'A-Z') const expiresInSeconds = opts.expiresInSeconds ?? DEFAULT_EXPIRES_IN_SECONDS @@ -59,10 +60,27 @@ export async function mintMagicLinkUrl(opts: MintOptions): Promise { expiresAt: new Date(Date.now() + expiresInSeconds * 1000), }) - return buildVerifyMagicLinkUrl({ + const url = buildVerifyMagicLinkUrl({ origin: opts.portalUrl, token, callbackPath: opts.callbackPath, errorCallbackPath: opts.errorCallbackPath, }) + // `token` is the verification-row identifier. Callers that need to be able + // to invalidate the link later (invitations) persist it; sign-in callers + // ignore it. + return { url, token } +} + +/** + * Delete the verification row backing a previously-minted magic link, so the + * link can no longer be verified. Used by invitations when they're cancelled + * or re-issued. No-op when `token` is null/undefined (e.g. an invite minted + * before the token was tracked, or a path that never stored one). + */ +export async function revokeMagicLinkToken(token: string | null | undefined): Promise { + if (!token) return + // `token` is the row's `identifier` — see `createVerificationValue` in + // mintMagicLinkUrl above, which stores the raw token as the identifier. + await db.delete(verification).where(eq(verification.identifier, token)) } diff --git a/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts new file mode 100644 index 000000000..4ab0e8a74 --- /dev/null +++ b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +const mockMintMagicLinkUrl = vi.fn() + +vi.mock('@/lib/server/auth/magic-link-mint', () => ({ + mintMagicLinkUrl: mockMintMagicLinkUrl, +})) + +const { generateInvitationMagicLink } = await import('../invitation-magic-link') + +beforeEach(() => { + vi.clearAllMocks() + mockMintMagicLinkUrl.mockResolvedValue({ + url: 'https://acme.test/verify-magic-link?token=abc', + token: 'tok_team', + }) +}) + +describe('generateInvitationMagicLink', () => { + it('mints a link that lives as long as the invitation record (30 days), not the 10-minute sign-in default', async () => { + await generateInvitationMagicLink( + 'invitee@example.com', + '/complete-signup/invite_1', + 'https://acme.test' + ) + + expect(mockMintMagicLinkUrl).toHaveBeenCalledTimes(1) + expect(mockMintMagicLinkUrl.mock.calls[0][0]).toMatchObject({ + email: 'invitee@example.com', + callbackPath: '/complete-signup/invite_1', + portalUrl: 'https://acme.test', + expiresInSeconds: 30 * 24 * 60 * 60, + }) + }) + + it('returns the minted url and token so the caller can persist the token for revocation', async () => { + const result = await generateInvitationMagicLink( + 'invitee@example.com', + '/complete-signup/invite_1', + 'https://acme.test' + ) + expect(result).toEqual({ + url: 'https://acme.test/verify-magic-link?token=abc', + token: 'tok_team', + }) + }) +}) diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts index 8c94360d6..5719d5620 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts @@ -54,6 +54,7 @@ const hoisted = vi.hoisted(() => ({ }, mockSendPortalInviteEmail: vi.fn(), mockMintMagicLinkUrl: vi.fn(), + mockRevokeMagicLinkToken: vi.fn(), mockGetEmailSafeUrl: vi.fn(), mockGetBaseUrl: vi.fn(), mockGenerateId: vi.fn(), @@ -123,6 +124,7 @@ vi.mock('@quackback/ids', () => ({ // Dynamic imports used inside handlers vi.mock('@/lib/server/auth/magic-link-mint', () => ({ mintMagicLinkUrl: hoisted.mockMintMagicLinkUrl, + revokeMagicLinkToken: hoisted.mockRevokeMagicLinkToken, })) vi.mock('@/lib/server/storage/s3', () => ({ @@ -169,9 +171,11 @@ beforeEach(async () => { // Sensible defaults hoisted.mockRequireAuth.mockResolvedValue(ADMIN_AUTH) hoisted.mockGetBaseUrl.mockReturnValue('https://acme.example.com') - hoisted.mockMintMagicLinkUrl.mockResolvedValue( - 'https://acme.example.com/verify-magic-link?token=abc' - ) + hoisted.mockMintMagicLinkUrl.mockResolvedValue({ + url: 'https://acme.example.com/verify-magic-link?token=abc', + token: 'tok_new', + }) + hoisted.mockRevokeMagicLinkToken.mockResolvedValue(undefined) hoisted.mockGetEmailSafeUrl.mockReturnValue(null) hoisted.mockSendPortalInviteEmail.mockResolvedValue({ sent: false }) hoisted.mockGenerateId.mockReturnValue('invite_test') @@ -280,6 +284,20 @@ describe('sendPortalInviteFn — success (single)', () => { ) }) + it('mints a link that lives as long as the invite (30 days), not the 10-minute sign-in default', async () => { + await sendHandler({ data: { emails: ['invitee@example.com'] } }) + + const opts = hoisted.mockMintMagicLinkUrl.mock.calls[0][0] as { expiresInSeconds?: number } + expect(opts.expiresInSeconds).toBe(30 * 24 * 60 * 60) // 30-day invite lifetime, not the 10-min sign-in default + }) + + it('persists the magic-link token on the invite row so cancel can revoke it', async () => { + await sendHandler({ data: { emails: ['invitee@example.com'] } }) + + const insertCall = hoisted.mockDbInsert.mock.calls[0][0] as Record + expect(insertCall.magicLinkToken).toBe('tok_new') + }) + it('records a portal.invite.sent audit event', async () => { await sendHandler({ data: { emails: ['invitee@example.com'] } }) @@ -356,9 +374,10 @@ describe('getPortalInviteLinkFn', () => { email: 'user@example.com', expiresAt: new Date(Date.now() + 14 * 24 * 60 * 60 * 1000), }) - hoisted.mockMintMagicLinkUrl.mockResolvedValue( - 'https://acme.example.com/verify-magic-link?token=xyz' - ) + hoisted.mockMintMagicLinkUrl.mockResolvedValue({ + url: 'https://acme.example.com/verify-magic-link?token=xyz', + token: 'tok_xyz', + }) const result = await getLinkHandler({ data: { inviteId: 'invite_abc' } }) const r = result as { inviteLink: string; expiresAt: Date } @@ -366,6 +385,29 @@ describe('getPortalInviteLinkFn', () => { expect(r.expiresAt).toBeInstanceOf(Date) }) + it("caps the copy-link token at the invite's remaining lifetime so it can't outlive the invite", async () => { + // Invite was created earlier and expires in ~3 days; copy-link must mint a + // token that dies with the invite, not a fresh full-lifetime one. + const threeDaysSeconds = 3 * 24 * 60 * 60 + hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ + id: 'invite_abc', + kind: 'portal', + status: 'pending', + email: 'user@example.com', + expiresAt: new Date(Date.now() + threeDaysSeconds * 1000), + magicLinkToken: 'tok_old', + }) + + await getLinkHandler({ data: { inviteId: 'invite_abc' } }) + + const opts = hoisted.mockMintMagicLinkUrl.mock.calls[0][0] as { expiresInSeconds?: number } + // Allow a few seconds of slack for execution time. + expect(opts.expiresInSeconds).toBeGreaterThan(threeDaysSeconds - 60) + expect(opts.expiresInSeconds).toBeLessThanOrEqual(threeDaysSeconds) + // And the prior token is revoked. + expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') + }) + it('rejects for a non-portal invite kind (returns null from DB)', async () => { // findFirst returns null because kind='team' is filtered out in the WHERE clause hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue(null) @@ -481,6 +523,20 @@ describe('cancelPortalInviteFn — success', () => { ) expect(auditCall).toBeDefined() }) + + it('revokes the magic-link token so the emailed link can no longer sign anyone in', async () => { + hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ + id: 'invite_1', + kind: 'portal', + status: 'pending', + email: 'user@example.com', + magicLinkToken: 'tok_live', + }) + + await cancelHandler({ data: { inviteId: 'invite_1' } }) + + expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_live') + }) }) // --------------------------------------------------------------------------- @@ -529,6 +585,26 @@ describe('resendPortalInviteFn — success', () => { expect((result as { inviteId: string }).inviteId).toBe('invite_1') }) + it('revokes the prior token and records the new one so cancel revokes the live link', async () => { + const futureDate = new Date(Date.now() + 14 * 24 * 60 * 60 * 1000) + hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ + id: 'invite_1', + kind: 'portal', + status: 'pending', + email: 'user@example.com', + expiresAt: futureDate, + magicLinkToken: 'tok_old', + }) + + await resendHandler({ data: { inviteId: 'invite_1' } }) + + expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') + const tokenWrite = hoisted.mockDbSet.mock.calls.find( + (c) => (c[0] as Record).magicLinkToken !== undefined + ) + expect(tokenWrite?.[0]).toMatchObject({ magicLinkToken: 'tok_new' }) + }) + it('emits portal.invite.resent (not portal.invite.sent) on resend', async () => { const futureDate = new Date(Date.now() + 14 * 24 * 60 * 60 * 1000) hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ @@ -834,7 +910,7 @@ describe('cancelPortalInviteFn — race guard', () => { // --------------------------------------------------------------------------- describe('resendPortalInviteFn — extends expiresAt', () => { - it('sets expiresAt to ~14 days from now on resend', async () => { + it('sets expiresAt to ~30 days from now on resend', async () => { const nearExpiry = new Date(Date.now() + 2 * 60 * 60 * 1000) // 2 hours left hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ id: 'invite_1', @@ -854,11 +930,11 @@ describe('resendPortalInviteFn — extends expiresAt', () => { expiresAt: Date } - // expiresAt must be ~14 days from the time of resend, not the old near-expiry. - const FOURTEEN_DAYS_MS = 14 * 24 * 60 * 60 * 1000 + // expiresAt must be ~30 days from the time of resend, not the old near-expiry. + const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000 const expiresAtMs = setPayload.expiresAt.getTime() - expect(expiresAtMs).toBeGreaterThanOrEqual(before + FOURTEEN_DAYS_MS - 5000) - expect(expiresAtMs).toBeLessThanOrEqual(after + FOURTEEN_DAYS_MS + 5000) + expect(expiresAtMs).toBeGreaterThanOrEqual(before + THIRTY_DAYS_MS - 5000) + expect(expiresAtMs).toBeLessThanOrEqual(after + THIRTY_DAYS_MS + 5000) }) it('also updates lastSentAt on resend', async () => { diff --git a/apps/web/src/lib/server/functions/admin.ts b/apps/web/src/lib/server/functions/admin.ts index f1c40fb3b..7ec650641 100644 --- a/apps/web/src/lib/server/functions/admin.ts +++ b/apps/web/src/lib/server/functions/admin.ts @@ -56,9 +56,11 @@ import { import type { UserAttributeId } from '@quackback/ids' import { sendInvitationEmail } from '@quackback/email' import { getBaseUrl } from '@/lib/server/config' - -/** Invitation expiry duration — 7 days in milliseconds */ -const INVITATION_EXPIRY_MS = 7 * 24 * 60 * 60 * 1000 +import { + INVITATION_EXPIRY_MS, + generateInvitationMagicLink, + rotateInviteMagicLinkToken, +} from './invitation-magic-link' /** * Server functions for admin data fetching. @@ -903,30 +905,6 @@ const invitationByIdSchema = z.object({ export type SendInvitationInput = z.infer export type InvitationByIdInput = z.infer -/** - * Generate a magic link for invitation authentication. - * Uses Better Auth's API to generate the token and stores it for later URL construction. - * - * @param email - The invitee's email address - * @param callbackPath - Relative path to redirect to after authentication (e.g., /complete-signup/{id}) - * @param portalUrl - The base portal URL (workspace domain) - * @returns The magic link URL with the correct workspace domain - */ -async function generateInvitationMagicLink( - email: string, - callbackPath: string, - portalUrl: string -): Promise { - console.log( - `[fn:admin] generateInvitationMagicLink: email=${email}, callbackPath=${callbackPath}, portalUrl=${portalUrl}` - ) - const { mintMagicLinkUrl } = await import('@/lib/server/auth/magic-link-mint') - // Invitations reuse the same path for success + error so an - // expired/consumed link sends the recipient back to the same - // invitation page (with its own expired-state copy). - return mintMagicLinkUrl({ email, callbackPath, portalUrl }) -} - /** * Send a team invitation */ @@ -977,6 +955,17 @@ export const sendInvitationFn = createServerFn({ method: 'POST' }) const expiresAt = new Date(Date.now() + INVITATION_EXPIRY_MS) const now = new Date() + // Mint the magic link before the insert so the row can record its token + // (lets cancel/resend revoke the link). invitationId is fixed above, so + // the callback path is already known. + const portalUrl = getBaseUrl() + const callbackURL = `/complete-signup/${invitationId}` + const { url: inviteLink, token: magicLinkToken } = await generateInvitationMagicLink( + email, + callbackURL, + portalUrl + ) + await db.insert(invitation).values({ id: invitationId, email, @@ -987,13 +976,9 @@ export const sendInvitationFn = createServerFn({ method: 'POST' }) lastSentAt: now, inviterId: auth.user.id, createdAt: now, + magicLinkToken, }) - // Generate magic link for one-click authentication - const portalUrl = getBaseUrl() - const callbackURL = `/complete-signup/${invitationId}` - const inviteLink = await generateInvitationMagicLink(email, callbackURL, portalUrl) - const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined const result = await sendInvitationEmail({ @@ -1065,6 +1050,10 @@ export const cancelInvitationFn = createServerFn({ method: 'POST' }) throw new Error('Invitation is no longer pending — refresh and try again') } + // Invalidate the emailed magic link so a cancelled invite can't sign anyone in. + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkToken(invitationRecord.magicLinkToken) + console.log(`[fn:admin] cancelInvitationFn: canceled`) return { invitationId } } catch (error) { @@ -1123,14 +1112,21 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) throw new Error('Invitation is no longer pending — refresh and try again') } - // Generate new magic link for one-click authentication + // Generate new magic link for one-click authentication. Revoke the + // invite's previous token and record the new one so a later cancel + // invalidates the link that's actually live. const portalUrl = getBaseUrl() const callbackURL = `/complete-signup/${invitationId}` - const inviteLink = await generateInvitationMagicLink( + const { url: inviteLink, token: magicLinkToken } = await generateInvitationMagicLink( invitationRecord.email, callbackURL, portalUrl ) + await rotateInviteMagicLinkToken( + invitationId, + invitationRecord.magicLinkToken, + magicLinkToken + ) const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined diff --git a/apps/web/src/lib/server/functions/invitation-magic-link.ts b/apps/web/src/lib/server/functions/invitation-magic-link.ts new file mode 100644 index 000000000..bb6918ab7 --- /dev/null +++ b/apps/web/src/lib/server/functions/invitation-magic-link.ts @@ -0,0 +1,57 @@ +/** + * Team-invitation magic link — the team counterpart to portal-invites.ts. + * Split out of admin.ts (and its large import surface) so the link's + * lifetime can be reasoned about and tested in isolation. Also hosts the + * shared token-rotation helper used by both team and portal invite paths. + */ +import type { InviteId } from '@quackback/ids' + +/** + * Team invitation lifetime — 30 days. Source of truth for both the + * invitation row's `expiresAt` and the emailed magic-link token TTL. + * + * The token deliberately lives this long rather than falling back to + * `mintMagicLinkUrl`'s 10-minute sign-in default: an invite is emailed and + * opened asynchronously — often days later — and the invitation row still + * governs long-term access either way. + */ +export const INVITATION_EXPIRY_MS = 30 * 24 * 60 * 60 * 1000 + +/** + * Mint the invite's one-click sign-in link (lives for INVITATION_EXPIRY_MS). + * Returns both the URL and its `token` — persist the token on the invite row + * so {@link revokeMagicLinkToken} can invalidate the link on cancel/re-send. + */ +export async function generateInvitationMagicLink( + email: string, + callbackPath: string, + portalUrl: string +): Promise<{ url: string; token: string }> { + console.log( + `[fn:invite] generateInvitationMagicLink: email=${email}, callbackPath=${callbackPath}, portalUrl=${portalUrl}` + ) + const { mintMagicLinkUrl } = await import('@/lib/server/auth/magic-link-mint') + return mintMagicLinkUrl({ + email, + callbackPath, + portalUrl, + expiresInSeconds: INVITATION_EXPIRY_MS / 1000, + }) +} + +/** + * Point an invite at a freshly-minted magic-link token: revoke the prior + * token (so the previously-emailed link dies) and record the new one on the + * row. Used by the resend and copy-link paths, which supersede an existing + * link; cancel only revokes, so it calls `revokeMagicLinkToken` directly. + */ +export async function rotateInviteMagicLinkToken( + inviteId: InviteId, + priorToken: string | null | undefined, + newToken: string +): Promise { + const { db, invitation, eq } = await import('@/lib/server/db') + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkToken(priorToken) + await db.update(invitation).set({ magicLinkToken: newToken }).where(eq(invitation.id, inviteId)) +} diff --git a/apps/web/src/lib/server/functions/portal-access.ts b/apps/web/src/lib/server/functions/portal-access.ts index 62c9fa158..0871348d0 100644 --- a/apps/web/src/lib/server/functions/portal-access.ts +++ b/apps/web/src/lib/server/functions/portal-access.ts @@ -145,7 +145,7 @@ export const resolvePortalAccessForRequest = createServerOnlyFn( eq(invitation.kind, 'portal'), // Accepted invites are permanent until revoked — expiry only governs // pending invites. Dropping the expires_at check here prevents a - // user losing access 14 days after the invite was sent. + // user losing access once the invite's pending window passes. eq(invitation.status, 'accepted') ), columns: { id: true }, diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index b46914f46..d6314598e 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -16,17 +16,21 @@ import type { InviteId, UserId } from '@quackback/ids' import { generateId } from '@quackback/ids' import { db, invitation, principal, user, eq, and, gt, or, sql } from '@/lib/server/db' import { requireAuth } from './auth-helpers' +import { rotateInviteMagicLinkToken } from './invitation-magic-link' import { actorFromAuth, recordAuditEvent } from '@/lib/server/audit/log' import { getBaseUrl } from '@/lib/server/config' import { sendPortalInviteEmail } from '@quackback/email' import { getSession } from '@/lib/server/auth/session' import { safeEmail } from '@/lib/shared/utils/string' -/** Portal invite lifetime — 14 days. */ -const PORTAL_INVITE_EXPIRY_MS = 14 * 24 * 60 * 60 * 1000 +/** Portal invite lifetime — 30 days. */ +const PORTAL_INVITE_EXPIRY_MS = 30 * 24 * 60 * 60 * 1000 -/** Magic-link token lifetime — 10 minutes. */ -const PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS = 10 * 60 +/** Magic-link token lifetime — matches the invite's full lifetime so a + * link emailed and opened days later still works. The 10-minute sign-in + * default would strand invitees who don't click immediately; the invite + * row's own `expiresAt` still governs long-term access. */ +const PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS = PORTAL_INVITE_EXPIRY_MS / 1000 // --------------------------------------------------------------------------- // Schemas @@ -53,17 +57,18 @@ const portalInviteByIdSchema = z.object({ async function mintPortalInviteMagicLink( email: string, inviteId: string, - portalUrl: string -): Promise { + portalUrl: string, + // Defaults to the full lifetime (see PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS). + // Copy-link passes the invite's *remaining* lifetime so a re-minted token + // can't outlive the invite row it belongs to. + expiresInSeconds: number = PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS +): Promise<{ url: string; token: string }> { const { mintMagicLinkUrl } = await import('@/lib/server/auth/magic-link-mint') return mintMagicLinkUrl({ email, callbackPath: `/portal-invite/${inviteId}`, portalUrl, - // Portal invite links live for the invite's full lifetime; a 10-minute - // magic-link token is enough since the invitee clicks it promptly after - // receiving the email. The invite row itself governs long-term access. - expiresInSeconds: PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS, + expiresInSeconds, }) } @@ -126,6 +131,15 @@ async function sendOnePortalInvite({ const inviteId = generateId('invite') const expiresAt = new Date(now.getTime() + PORTAL_INVITE_EXPIRY_MS) + // Mint before the insert so the row records its token (lets cancel/resend + // revoke the link). inviteId is fixed above, so the callback path is known. + const portalUrl = getBaseUrl() + const { url: inviteLink, token: magicLinkToken } = await mintPortalInviteMagicLink( + email, + inviteId, + portalUrl + ) + await db.insert(invitation).values({ id: inviteId, email, @@ -137,11 +151,9 @@ async function sendOnePortalInvite({ createdAt: now, lastSentAt: now, inviterId: auth.user.id as UserId, + magicLinkToken, }) - const portalUrl = getBaseUrl() - const inviteLink = await mintPortalInviteMagicLink(email, inviteId, portalUrl) - const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined await sendPortalInviteEmail({ @@ -278,6 +290,10 @@ export const cancelPortalInviteFn = createServerFn({ method: 'POST' }) return { inviteId, status: 'no_op_already_accepted' as const } } + // Invalidate the emailed magic link so a cancelled invite can't sign anyone in. + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkToken(inv.magicLinkToken) + await recordAuditEvent({ event: 'portal.invite.revoked', actor, @@ -367,7 +383,14 @@ export const resendPortalInviteFn = createServerFn({ method: 'POST' }) } const portalUrl = getBaseUrl() - const inviteLink = await mintPortalInviteMagicLink(inv.email, inviteId, portalUrl) + const { url: inviteLink, token: magicLinkToken } = await mintPortalInviteMagicLink( + inv.email, + inviteId, + portalUrl + ) + // Revoke the superseded token and record the new one so a later cancel + // invalidates the link that's actually live. + await rotateInviteMagicLinkToken(inviteId, inv.magicLinkToken, magicLinkToken) const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined @@ -444,7 +467,8 @@ export const fetchPortalInvitesFn = createServerFn({ method: 'GET' }).handler(as /** * Mint a fresh magic-link for a pending portal invite. * - * Admin-only. The link expires in 10 minutes. The invite row itself must + * Admin-only. The link lives as long as the invite (see + * PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS). The invite row itself must * be `kind='portal'`, `status='pending'`, and not past its own `expiresAt`. * Records a `portal.invite.link_minted` audit event on success. */ @@ -463,7 +487,19 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) if (inv.expiresAt && inv.expiresAt < new Date()) throw new Error('Invite has expired') const portalUrl = getBaseUrl() - const inviteLink = await mintPortalInviteMagicLink(inv.email, inv.id, portalUrl) + // Copy-link mints a fresh token but does NOT extend the invite row, so cap + // the token at the invite's remaining lifetime — otherwise a link copied + // late could outlive (and sign in past) the invite's own expiry. + const remainingSeconds = Math.floor((inv.expiresAt.getTime() - Date.now()) / 1000) + const { url: inviteLink, token: magicLinkToken } = await mintPortalInviteMagicLink( + inv.email, + inv.id, + portalUrl, + remainingSeconds + ) + // Each copy-link supersedes the prior token; revoke it and record the new + // one so the invite always tracks (and cancel can revoke) the live link. + await rotateInviteMagicLinkToken(inv.id, inv.magicLinkToken, magicLinkToken) await recordAuditEvent({ event: 'portal.invite.link_minted', @@ -474,8 +510,9 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) metadata: { email: inv.email }, }) - const expiresAt = new Date(Date.now() + PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS * 1000) - return { inviteLink, expiresAt } + // The token is capped to expire with the invite row, so the row's own + // expiry is the accurate cutoff to surface. + return { inviteLink, expiresAt: inv.expiresAt } }) // --------------------------------------------------------------------------- diff --git a/apps/web/src/lib/server/functions/recovery-codes-consume.ts b/apps/web/src/lib/server/functions/recovery-codes-consume.ts index e1c84b66a..2e81ad4c8 100644 --- a/apps/web/src/lib/server/functions/recovery-codes-consume.ts +++ b/apps/web/src/lib/server/functions/recovery-codes-consume.ts @@ -156,7 +156,7 @@ export const consumeRecoveryCodeFn = createServerFn({ method: 'POST' }) .set({ usedAt: new Date() }) .where(eq(ssoRecoveryCode.id, matchedId as SsoRecoveryCodeId)) - const redirectUrl = await mintMagicLinkUrl({ + const { url: redirectUrl } = await mintMagicLinkUrl({ email: data.email, callbackPath: '/admin', errorCallbackPath: '/admin/login', diff --git a/packages/db/drizzle/0111_invitation_magic_link_token.sql b/packages/db/drizzle/0111_invitation_magic_link_token.sql new file mode 100644 index 000000000..633b4d29f --- /dev/null +++ b/packages/db/drizzle/0111_invitation_magic_link_token.sql @@ -0,0 +1,10 @@ +-- Tracks the verification-table identifier (the magic-link token) currently +-- minted for this invitation. Lets the cancel / re-send paths delete the +-- backing `verification` row so a cancelled or superseded invite link can no +-- longer mint a session -- without it, the emailed token stays live for its +-- full 30-day TTL regardless of the invite's status. +-- +-- Additive + backfill-safe: existing pending invites get NULL (their tokens +-- self-expire at the row's expires_at and are simply not revocable). Deletes +-- by verification.identifier are served by the existing verification_identifier_idx. +ALTER TABLE "invitation" ADD COLUMN "magic_link_token" text; diff --git a/packages/db/drizzle/meta/_journal.json b/packages/db/drizzle/meta/_journal.json index c5031fbf1..64f264cef 100644 --- a/packages/db/drizzle/meta/_journal.json +++ b/packages/db/drizzle/meta/_journal.json @@ -778,6 +778,13 @@ "when": 1782172800000, "tag": "0110_oauth_refresh_token_family_idx", "breakpoints": true + }, + { + "idx": 111, + "version": "7", + "when": 1782259200000, + "tag": "0111_invitation_magic_link_token", + "breakpoints": true } ] } diff --git a/packages/db/src/schema/auth.ts b/packages/db/src/schema/auth.ts index fd83c6186..151dbaa95 100644 --- a/packages/db/src/schema/auth.ts +++ b/packages/db/src/schema/auth.ts @@ -464,6 +464,13 @@ export const invitation = pgTable( expiresAt: timestamp('expires_at', { withTimezone: true }).notNull(), createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(), lastSentAt: timestamp('last_sent_at', { withTimezone: true }), + /** + * The `verification.identifier` (magic-link token) currently minted for + * this invite. Lets cancel / re-send delete the backing verification row + * so a cancelled or superseded link can't mint a session. Null for invites + * sent before this was tracked (their tokens just self-expire at expiresAt). + */ + magicLinkToken: text('magic_link_token'), inviterId: typeIdColumn('user')('inviter_id') .notNull() .references(() => user.id, { onDelete: 'cascade' }), From f7a5eb0299780802266e9fac04cf845403f4a01a Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 13:51:24 +0100 Subject: [PATCH 2/9] fix(invitations): CAS token rotation and fix magic-link mock shape 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 } --- .../auth/__tests__/email-signin.test.ts | 5 +- .../__tests__/invitation-magic-link.test.ts | 51 ++++++++++++++++++- .../__tests__/portal-invites.test.ts | 9 +++- .../__tests__/recovery-codes-consume.test.ts | 5 +- apps/web/src/lib/server/functions/admin.ts | 9 ++-- .../server/functions/invitation-magic-link.ts | 40 ++++++++++++--- .../lib/server/functions/portal-invites.ts | 9 ++-- 7 files changed, 111 insertions(+), 17 deletions(-) diff --git a/apps/web/src/lib/server/auth/__tests__/email-signin.test.ts b/apps/web/src/lib/server/auth/__tests__/email-signin.test.ts index e2e1b39ad..2e2aea51e 100644 --- a/apps/web/src/lib/server/auth/__tests__/email-signin.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/email-signin.test.ts @@ -1,7 +1,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' const hoisted = vi.hoisted(() => ({ - mockMintMagicLinkUrl: vi.fn(async () => 'https://example.com/verify-magic-link?token=t'), + mockMintMagicLinkUrl: vi.fn(async () => ({ + url: 'https://example.com/verify-magic-link?token=t', + token: 't', + })), mockSendVerificationOTP: vi.fn(async () => undefined), mockSendMagicLinkEmail: vi.fn(async () => undefined), mockGetOTP: vi.fn(() => '123456'), diff --git a/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts index 4ab0e8a74..5b42d5eeb 100644 --- a/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts @@ -1,12 +1,34 @@ +import type { InviteId } from '@quackback/ids' import { describe, it, expect, vi, beforeEach } from 'vitest' const mockMintMagicLinkUrl = vi.fn() +const mockRevokeMagicLinkToken = vi.fn() vi.mock('@/lib/server/auth/magic-link-mint', () => ({ mintMagicLinkUrl: mockMintMagicLinkUrl, + revokeMagicLinkToken: mockRevokeMagicLinkToken, })) -const { generateInvitationMagicLink } = await import('../invitation-magic-link') +// Minimal db update-chain mock for rotateInviteMagicLinkToken's compare-and-swap. +const mockReturning = vi.fn() +const mockWhere = vi.fn(() => ({ returning: mockReturning })) +const mockSet = vi.fn(() => ({ where: mockWhere })) +const mockUpdate = vi.fn(() => ({ set: mockSet })) + +vi.mock('@/lib/server/db', () => ({ + db: { update: mockUpdate }, + invitation: { + id: 'invitation.id', + status: 'invitation.status', + magicLinkToken: 'invitation.magicLinkToken', + }, + eq: vi.fn((col: unknown, val: unknown) => ({ op: 'eq', col, val })), + and: vi.fn((...parts: unknown[]) => ({ op: 'and', parts })), + isNull: vi.fn((col: unknown) => ({ op: 'isNull', col })), +})) + +const { generateInvitationMagicLink, rotateInviteMagicLinkToken } = + await import('../invitation-magic-link') beforeEach(() => { vi.clearAllMocks() @@ -14,6 +36,9 @@ beforeEach(() => { url: 'https://acme.test/verify-magic-link?token=abc', token: 'tok_team', }) + mockRevokeMagicLinkToken.mockResolvedValue(undefined) + // Default: the compare-and-swap matched one row. + mockReturning.mockResolvedValue([{ id: 'invite_1' }]) }) describe('generateInvitationMagicLink', () => { @@ -45,3 +70,27 @@ describe('generateInvitationMagicLink', () => { }) }) }) + +describe('rotateInviteMagicLinkToken', () => { + it('revokes the prior token and records the new one when the invite still matches', async () => { + mockReturning.mockResolvedValue([{ id: 'invite_1' }]) // CAS matched + + await rotateInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') + + expect(mockRevokeMagicLinkToken).toHaveBeenCalledTimes(1) + expect(mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') + expect(mockSet).toHaveBeenCalledWith({ magicLinkToken: 'tok_new' }) + }) + + it('revokes the just-minted token and throws when the row changed (cancel / double-rotation race)', async () => { + mockReturning.mockResolvedValue([]) // CAS matched nothing — row canceled or re-rotated + + await expect( + rotateInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') + ).rejects.toThrow(/changed during update/) + + // Prior token revoked, AND the orphan we just minted is cleaned up. + expect(mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') + expect(mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_new') + }) +}) diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts index 5719d5620..0ed109d0c 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts @@ -105,6 +105,7 @@ vi.mock('@/lib/server/db', () => { and: vi.fn((...args: unknown[]) => args), or: vi.fn((...args: unknown[]) => args), gt: vi.fn((col, val) => ({ col, val })), + isNull: vi.fn((col) => ({ col, isNull: true })), sql: vi.fn((parts: TemplateStringsArray) => parts.raw[0]), } }) @@ -524,18 +525,22 @@ describe('cancelPortalInviteFn — success', () => { expect(auditCall).toBeDefined() }) - it('revokes the magic-link token so the emailed link can no longer sign anyone in', async () => { + it('revokes the token live at cancel time (from the UPDATE), not a stale earlier read', async () => { hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ id: 'invite_1', kind: 'portal', status: 'pending', email: 'user@example.com', - magicLinkToken: 'tok_live', + magicLinkToken: 'tok_stale', }) + // The cancel UPDATE returns the token actually on the row at that instant — + // a concurrent rotation may have changed it since the read above. + hoisted.mockDbReturning.mockResolvedValue([{ id: 'invite_1', magicLinkToken: 'tok_live' }]) await cancelHandler({ data: { inviteId: 'invite_1' } }) expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_live') + expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalledWith('tok_stale') }) }) diff --git a/apps/web/src/lib/server/functions/__tests__/recovery-codes-consume.test.ts b/apps/web/src/lib/server/functions/__tests__/recovery-codes-consume.test.ts index 33add0446..b6645ae15 100644 --- a/apps/web/src/lib/server/functions/__tests__/recovery-codes-consume.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/recovery-codes-consume.test.ts @@ -132,7 +132,10 @@ beforeEach(() => { hoisted.findUser.mockResolvedValue(null) hoisted.findCodes.mockResolvedValue([]) hoisted.verifyRecoveryCode.mockResolvedValue(false) - hoisted.mintMagicLinkUrl.mockResolvedValue('https://acme.quackback.io/verify-magic-link?token=t') + hoisted.mintMagicLinkUrl.mockResolvedValue({ + url: 'https://acme.quackback.io/verify-magic-link?token=t', + token: 't', + }) }) await import('../recovery-codes-consume') diff --git a/apps/web/src/lib/server/functions/admin.ts b/apps/web/src/lib/server/functions/admin.ts index 7ec650641..bc262e692 100644 --- a/apps/web/src/lib/server/functions/admin.ts +++ b/apps/web/src/lib/server/functions/admin.ts @@ -1044,15 +1044,18 @@ export const cancelInvitationFn = createServerFn({ method: 'POST' }) eq(invitation.status, 'pending') ) ) - .returning({ id: invitation.id }) + .returning({ id: invitation.id, magicLinkToken: invitation.magicLinkToken }) if (cancelled.length === 0) { throw new Error('Invitation is no longer pending — refresh and try again') } - // Invalidate the emailed magic link so a cancelled invite can't sign anyone in. + // Invalidate the emailed magic link so a cancelled invite can't sign + // anyone in. Revoke the token the UPDATE returned (the one live at the + // instant of cancellation) rather than the earlier read, so a rotation + // racing this cancel can't leave a live token behind. const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - await revokeMagicLinkToken(invitationRecord.magicLinkToken) + await revokeMagicLinkToken(cancelled[0].magicLinkToken) console.log(`[fn:admin] cancelInvitationFn: canceled`) return { invitationId } diff --git a/apps/web/src/lib/server/functions/invitation-magic-link.ts b/apps/web/src/lib/server/functions/invitation-magic-link.ts index bb6918ab7..ef4035aba 100644 --- a/apps/web/src/lib/server/functions/invitation-magic-link.ts +++ b/apps/web/src/lib/server/functions/invitation-magic-link.ts @@ -40,18 +40,46 @@ export async function generateInvitationMagicLink( } /** - * Point an invite at a freshly-minted magic-link token: revoke the prior - * token (so the previously-emailed link dies) and record the new one on the - * row. Used by the resend and copy-link paths, which supersede an existing - * link; cancel only revokes, so it calls `revokeMagicLinkToken` directly. + * Point an invite at a freshly-minted magic-link token: revoke the prior token + * (so the previously-emailed link dies) and record the new one on the row. Used + * by the resend and copy-link paths, which supersede an existing link; cancel + * only revokes, so it calls `revokeMagicLinkToken` directly. + * + * The write is a compare-and-swap on `(status='pending', magicLinkToken=prior)` + * rather than a blind last-write-wins, to stay correct under concurrency: + * - a request racing a cancel must not re-arm a live link on a canceled invite + * - a request racing another rotation must not orphan the other token (which + * would leave it live but untracked, so never revoked) + * If the row no longer matches, the just-minted token is revoked (it isn't + * tracked by the invite) and the call throws so the caller doesn't email or + * return a now-dead link as success. */ export async function rotateInviteMagicLinkToken( inviteId: InviteId, priorToken: string | null | undefined, newToken: string ): Promise { - const { db, invitation, eq } = await import('@/lib/server/db') + const { db, invitation, eq, and, isNull } = await import('@/lib/server/db') const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkToken(priorToken) - await db.update(invitation).set({ magicLinkToken: newToken }).where(eq(invitation.id, inviteId)) + + const swapped = await db + .update(invitation) + .set({ magicLinkToken: newToken }) + .where( + and( + eq(invitation.id, inviteId), + eq(invitation.status, 'pending'), + priorToken == null + ? isNull(invitation.magicLinkToken) + : eq(invitation.magicLinkToken, priorToken) + ) + ) + .returning({ id: invitation.id }) + + if (swapped.length === 0) { + await revokeMagicLinkToken(newToken) + throw new Error('Invitation changed during update — refresh and try again') + } } diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index d6314598e..daa3b0d5e 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -283,16 +283,19 @@ export const cancelPortalInviteFn = createServerFn({ method: 'POST' }) eq(invitation.status, 'pending') ) ) - .returning({ id: invitation.id }) + .returning({ id: invitation.id, magicLinkToken: invitation.magicLinkToken }) if (updated.length === 0) { console.log(`[fn:portal-invites] cancelPortalInviteFn: no-op (row concurrently mutated)`) return { inviteId, status: 'no_op_already_accepted' as const } } - // Invalidate the emailed magic link so a cancelled invite can't sign anyone in. + // Invalidate the emailed magic link so a cancelled invite can't sign anyone + // in. Revoke the token the UPDATE returned (live at the instant of + // cancellation) rather than the earlier read, so a rotation racing this + // cancel can't leave a live token behind. const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - await revokeMagicLinkToken(inv.magicLinkToken) + await revokeMagicLinkToken(updated[0].magicLinkToken) await recordAuditEvent({ event: 'portal.invite.revoked', From ef7518b1f34eb0a07adbed282ae0d505e0c38f54 Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 14:15:40 +0100 Subject: [PATCH 3/9] fix(invitations): don't drop the existing link when resend/copy delivery 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 --- .../src/components/admin/users/invite-row.tsx | 148 +++++++++++------- .../__tests__/portal-invites.test.ts | 25 +++ apps/web/src/lib/server/functions/admin.ts | 41 +++-- .../lib/server/functions/portal-invites.ts | 26 +-- 4 files changed, 160 insertions(+), 80 deletions(-) diff --git a/apps/web/src/components/admin/users/invite-row.tsx b/apps/web/src/components/admin/users/invite-row.tsx index 11d1f174c..44327956a 100644 --- a/apps/web/src/components/admin/users/invite-row.tsx +++ b/apps/web/src/components/admin/users/invite-row.tsx @@ -68,6 +68,10 @@ interface InviteRowProps { export function InviteRow({ invite, onRevoke, onResend, revoking, resending }: InviteRowProps) { const [confirmRevoke, setConfirmRevoke] = useState(false) const [copyState, setCopyState] = useState<'idle' | 'copying' | 'copied' | 'error'>('idle') + // When the clipboard write is blocked (denied permission, unfocused doc), the + // link the server already minted (and revoked the prior one for) must still + // reach the admin — surface it here for manual copy rather than losing it. + const [fallbackLink, setFallbackLink] = useState(null) const sentDate = invite.lastSentAt ?? invite.createdAt const handleRevokeClick = () => { @@ -82,73 +86,103 @@ export function InviteRow({ invite, onRevoke, onResend, revoking, resending }: I const handleCopyLink = async () => { if (copyState === 'copying') return setCopyState('copying') + setFallbackLink(null) + + let link: string try { const result = await getPortalInviteLinkFn({ data: { inviteId: invite.id } }) - await navigator.clipboard.writeText(result.inviteLink) - setCopyState('copied') - setTimeout(() => setCopyState('idle'), 3000) + link = result.inviteLink } catch { setCopyState('error') setTimeout(() => setCopyState('idle'), 3000) + return + } + + // The link is already minted server-side (and the prior one revoked), so a + // clipboard failure must not lose it — fall back to showing it for manual copy. + try { + await navigator.clipboard.writeText(link) + setCopyState('copied') + setTimeout(() => setCopyState('idle'), 3000) + } catch { + setFallbackLink(link) + setCopyState('idle') } } return ( -
  • -
    -

    {invite.email}

    -

    Sent {formatInviteDate(sentDate)}

    +
  • +
    +
    +

    {invite.email}

    +

    Sent {formatInviteDate(sentDate)}

    +
    + + {invite.status === 'pending' && ( +
    + + + +
    + )}
    - - {invite.status === 'pending' && ( -
    - - - + {fallbackLink && ( +
    +

    + Couldn't copy automatically. Select and copy this link: +

    + e.currentTarget.select()} + className="w-full rounded border border-border/50 bg-background px-2 py-1 font-mono text-xs" + />
    )}
  • diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts index 0ed109d0c..feaeb06dc 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts @@ -610,6 +610,31 @@ describe('resendPortalInviteFn — success', () => { expect(tokenWrite?.[0]).toMatchObject({ magicLinkToken: 'tok_new' }) }) + it('preserves the existing link when the email send fails (drops the orphan token)', async () => { + const futureDate = new Date(Date.now() + 14 * 24 * 60 * 60 * 1000) + hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ + id: 'invite_1', + kind: 'portal', + status: 'pending', + email: 'user@example.com', + expiresAt: futureDate, + magicLinkToken: 'tok_old', + }) + hoisted.mockSendPortalInviteEmail.mockRejectedValue(new Error('smtp down')) + + await expect(resendHandler({ data: { inviteId: 'invite_1' } })).rejects.toThrow('smtp down') + + // The just-minted token is revoked (orphan cleanup) and the invitee's + // existing token is NOT revoked, so their current link still works. + expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_new') + expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalledWith('tok_old') + // And the new token is never recorded on the row. + const tokenWrite = hoisted.mockDbSet.mock.calls.find( + (c) => (c[0] as Record).magicLinkToken !== undefined + ) + expect(tokenWrite).toBeUndefined() + }) + it('emits portal.invite.resent (not portal.invite.sent) on resend', async () => { const futureDate = new Date(Date.now() + 14 * 24 * 60 * 60 * 1000) hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ diff --git a/apps/web/src/lib/server/functions/admin.ts b/apps/web/src/lib/server/functions/admin.ts index bc262e692..39011c30c 100644 --- a/apps/web/src/lib/server/functions/admin.ts +++ b/apps/web/src/lib/server/functions/admin.ts @@ -1115,9 +1115,10 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) throw new Error('Invitation is no longer pending — refresh and try again') } - // Generate new magic link for one-click authentication. Revoke the - // invite's previous token and record the new one so a later cancel - // invalidates the link that's actually live. + // Generate a new magic link for one-click authentication. The previous + // token is revoked only AFTER the email is sent (or surfaced for manual + // sharing when email isn't configured), so a send failure can't strand + // the invitee by killing the still-valid link they already have. const portalUrl = getBaseUrl() const callbackURL = `/complete-signup/${invitationId}` const { url: inviteLink, token: magicLinkToken } = await generateInvitationMagicLink( @@ -1125,23 +1126,35 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) callbackURL, portalUrl ) + + const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') + const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined + let result: Awaited> + try { + result = await sendInvitationEmail({ + to: invitationRecord.email, + invitedByName: auth.user.name, + inviteeName: invitationRecord.name || undefined, + workspaceName: auth.settings.name, + inviteLink, + logoUrl, + }) + } catch (sendError) { + // The new link won't reach the invitee — drop the orphan token and + // leave their existing link working. + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkToken(magicLinkToken) + throw sendError + } + + // Delivered (or returned for manual sharing): the new link supersedes the + // old one, so revoke the prior token and record the new one. await rotateInviteMagicLinkToken( invitationId, invitationRecord.magicLinkToken, magicLinkToken ) - const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') - const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined - const result = await sendInvitationEmail({ - to: invitationRecord.email, - invitedByName: auth.user.name, - inviteeName: invitationRecord.name || undefined, - workspaceName: auth.settings.name, - inviteLink, - logoUrl, - }) - console.log( `[fn:admin] resendInvitationFn: ${result.sent ? 'resent' : 'regenerated (email not configured)'}` ) diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index daa3b0d5e..a8520ad8d 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -391,18 +391,26 @@ export const resendPortalInviteFn = createServerFn({ method: 'POST' }) inviteId, portalUrl ) - // Revoke the superseded token and record the new one so a later cancel - // invalidates the link that's actually live. - await rotateInviteMagicLinkToken(inviteId, inv.magicLinkToken, magicLinkToken) + // Revoke the superseded token only AFTER the email is sent (or surfaced for + // manual sharing when email isn't configured), so a send failure can't + // strand the invitee by killing the still-valid link they already have. const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined - const result = await sendPortalInviteEmail({ - to: inv.email, - workspaceName: auth.settings.name, - inviteLink, - logoUrl, - }) + let result: Awaited> + try { + result = await sendPortalInviteEmail({ + to: inv.email, + workspaceName: auth.settings.name, + inviteLink, + logoUrl, + }) + } catch (sendError) { + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkToken(magicLinkToken) + throw sendError + } + await rotateInviteMagicLinkToken(inviteId, inv.magicLinkToken, magicLinkToken) await recordAuditEvent({ event: 'portal.invite.resent', From 3ac94d5b352fd9255ee437798e78cbff419fa41b Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 14:29:40 +0100 Subject: [PATCH 4/9] fix(invitations): record invite token before sending, revoke old one 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 --- .../__tests__/invitation-magic-link.test.ts | 26 +++++------ .../__tests__/portal-invites.test.ts | 12 ++--- apps/web/src/lib/server/functions/admin.ts | 27 +++++++---- .../server/functions/invitation-magic-link.ts | 46 ++++++++----------- .../lib/server/functions/portal-invites.ts | 33 ++++++++++--- 5 files changed, 79 insertions(+), 65 deletions(-) diff --git a/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts index 5b42d5eeb..5f0c69ba2 100644 --- a/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts @@ -27,7 +27,7 @@ vi.mock('@/lib/server/db', () => ({ isNull: vi.fn((col: unknown) => ({ op: 'isNull', col })), })) -const { generateInvitationMagicLink, rotateInviteMagicLinkToken } = +const { generateInvitationMagicLink, recordInviteMagicLinkToken } = await import('../invitation-magic-link') beforeEach(() => { @@ -71,26 +71,24 @@ describe('generateInvitationMagicLink', () => { }) }) -describe('rotateInviteMagicLinkToken', () => { - it('revokes the prior token and records the new one when the invite still matches', async () => { +describe('recordInviteMagicLinkToken', () => { + it('compare-and-swaps the token and returns true when the invite still matches', async () => { mockReturning.mockResolvedValue([{ id: 'invite_1' }]) // CAS matched - await rotateInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') + const ok = await recordInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') - expect(mockRevokeMagicLinkToken).toHaveBeenCalledTimes(1) - expect(mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') + expect(ok).toBe(true) expect(mockSet).toHaveBeenCalledWith({ magicLinkToken: 'tok_new' }) + // It is a pure swap — revocation is the caller's responsibility. + expect(mockRevokeMagicLinkToken).not.toHaveBeenCalled() }) - it('revokes the just-minted token and throws when the row changed (cancel / double-rotation race)', async () => { - mockReturning.mockResolvedValue([]) // CAS matched nothing — row canceled or re-rotated + it('returns false without writing when the row changed (cancel / concurrent rotation)', async () => { + mockReturning.mockResolvedValue([]) // CAS matched nothing - await expect( - rotateInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') - ).rejects.toThrow(/changed during update/) + const ok = await recordInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') - // Prior token revoked, AND the orphan we just minted is cleaned up. - expect(mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') - expect(mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_new') + expect(ok).toBe(false) + expect(mockRevokeMagicLinkToken).not.toHaveBeenCalled() }) }) diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts index feaeb06dc..179a211bc 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts @@ -624,15 +624,15 @@ describe('resendPortalInviteFn — success', () => { await expect(resendHandler({ data: { inviteId: 'invite_1' } })).rejects.toThrow('smtp down') - // The just-minted token is revoked (orphan cleanup) and the invitee's + // The undelivered new token is revoked (orphan cleanup) and the invitee's // existing token is NOT revoked, so their current link still works. expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_new') expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalledWith('tok_old') - // And the new token is never recorded on the row. - const tokenWrite = hoisted.mockDbSet.mock.calls.find( - (c) => (c[0] as Record).magicLinkToken !== undefined - ) - expect(tokenWrite).toBeUndefined() + // The pre-send record is rolled back: the final token write restores the prior. + const tokenWrites = hoisted.mockDbSet.mock.calls + .map((c) => (c[0] as Record).magicLinkToken) + .filter((v) => v !== undefined) + expect(tokenWrites.at(-1)).toBe('tok_old') }) it('emits portal.invite.resent (not portal.invite.sent) on resend', async () => { diff --git a/apps/web/src/lib/server/functions/admin.ts b/apps/web/src/lib/server/functions/admin.ts index 39011c30c..62b350c14 100644 --- a/apps/web/src/lib/server/functions/admin.ts +++ b/apps/web/src/lib/server/functions/admin.ts @@ -59,7 +59,7 @@ import { getBaseUrl } from '@/lib/server/config' import { INVITATION_EXPIRY_MS, generateInvitationMagicLink, - rotateInviteMagicLinkToken, + recordInviteMagicLinkToken, } from './invitation-magic-link' /** @@ -1127,6 +1127,17 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) portalUrl ) + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + const priorToken = invitationRecord.magicLinkToken + + // Record the new token BEFORE sending, so the emailed link is always the + // one tracked on the invite and a concurrent cancel/rotation is caught + // before we email it. + if (!(await recordInviteMagicLinkToken(invitationId, priorToken, magicLinkToken))) { + await revokeMagicLinkToken(magicLinkToken) // not emailed yet; safe to drop + throw new Error('Invitation is no longer pending — refresh and try again') + } + const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined let result: Awaited> @@ -1140,20 +1151,16 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) logoUrl, }) } catch (sendError) { - // The new link won't reach the invitee — drop the orphan token and - // leave their existing link working. - const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + // Send failed: undo the record (restore the prior token as current) and + // drop the undelivered new one, so the invitee's existing link still works. + await recordInviteMagicLinkToken(invitationId, magicLinkToken, priorToken) await revokeMagicLinkToken(magicLinkToken) throw sendError } // Delivered (or returned for manual sharing): the new link supersedes the - // old one, so revoke the prior token and record the new one. - await rotateInviteMagicLinkToken( - invitationId, - invitationRecord.magicLinkToken, - magicLinkToken - ) + // old one, so revoke the prior token now. + await revokeMagicLinkToken(priorToken) console.log( `[fn:admin] resendInvitationFn: ${result.sent ? 'resent' : 'regenerated (email not configured)'}` diff --git a/apps/web/src/lib/server/functions/invitation-magic-link.ts b/apps/web/src/lib/server/functions/invitation-magic-link.ts index ef4035aba..0f7fb5cd3 100644 --- a/apps/web/src/lib/server/functions/invitation-magic-link.ts +++ b/apps/web/src/lib/server/functions/invitation-magic-link.ts @@ -40,46 +40,36 @@ export async function generateInvitationMagicLink( } /** - * Point an invite at a freshly-minted magic-link token: revoke the prior token - * (so the previously-emailed link dies) and record the new one on the row. Used - * by the resend and copy-link paths, which supersede an existing link; cancel - * only revokes, so it calls `revokeMagicLinkToken` directly. + * Compare-and-swap the invite's recorded magic-link token from `expected` to + * `next`, only while the invite is still `pending`. Returns true if the row + * matched and was updated, false otherwise (the invite was canceled, or another + * request changed the token first). * - * The write is a compare-and-swap on `(status='pending', magicLinkToken=prior)` - * rather than a blind last-write-wins, to stay correct under concurrency: - * - a request racing a cancel must not re-arm a live link on a canceled invite - * - a request racing another rotation must not orphan the other token (which - * would leave it live but untracked, so never revoked) - * If the row no longer matches, the just-minted token is revoked (it isn't - * tracked by the invite) and the call throws so the caller doesn't email or - * return a now-dead link as success. + * Callers record the new token with this BEFORE delivering its link (emailing, + * or returning it to copy), so the delivered token is always the one tracked on + * the invite and a concurrent cancel/rotation is detected before delivery. The + * old token is only revoked AFTER successful delivery, so a delivery failure + * never strands the recipient. Passing `next: null` restores the unset state + * (used to roll back a record when delivery then fails). */ -export async function rotateInviteMagicLinkToken( +export async function recordInviteMagicLinkToken( inviteId: InviteId, - priorToken: string | null | undefined, - newToken: string -): Promise { + expected: string | null | undefined, + next: string | null +): Promise { const { db, invitation, eq, and, isNull } = await import('@/lib/server/db') - const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - - await revokeMagicLinkToken(priorToken) - const swapped = await db .update(invitation) - .set({ magicLinkToken: newToken }) + .set({ magicLinkToken: next }) .where( and( eq(invitation.id, inviteId), eq(invitation.status, 'pending'), - priorToken == null + expected == null ? isNull(invitation.magicLinkToken) - : eq(invitation.magicLinkToken, priorToken) + : eq(invitation.magicLinkToken, expected) ) ) .returning({ id: invitation.id }) - - if (swapped.length === 0) { - await revokeMagicLinkToken(newToken) - throw new Error('Invitation changed during update — refresh and try again') - } + return swapped.length > 0 } diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index a8520ad8d..cbd66d191 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -16,7 +16,7 @@ import type { InviteId, UserId } from '@quackback/ids' import { generateId } from '@quackback/ids' import { db, invitation, principal, user, eq, and, gt, or, sql } from '@/lib/server/db' import { requireAuth } from './auth-helpers' -import { rotateInviteMagicLinkToken } from './invitation-magic-link' +import { recordInviteMagicLinkToken } from './invitation-magic-link' import { actorFromAuth, recordAuditEvent } from '@/lib/server/audit/log' import { getBaseUrl } from '@/lib/server/config' import { sendPortalInviteEmail } from '@quackback/email' @@ -392,7 +392,17 @@ export const resendPortalInviteFn = createServerFn({ method: 'POST' }) portalUrl ) - // Revoke the superseded token only AFTER the email is sent (or surfaced for + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + const priorToken = inv.magicLinkToken + + // Record the new token BEFORE sending so the emailed link is always tracked + // and a concurrent cancel/rotation is caught before delivery. + if (!(await recordInviteMagicLinkToken(inviteId, priorToken, magicLinkToken))) { + await revokeMagicLinkToken(magicLinkToken) // not emailed yet; safe to drop + throw new Error('Invitation is no longer pending — refresh and try again') + } + + // The prior token is revoked only AFTER the email is sent (or surfaced for // manual sharing when email isn't configured), so a send failure can't // strand the invitee by killing the still-valid link they already have. const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') @@ -406,11 +416,13 @@ export const resendPortalInviteFn = createServerFn({ method: 'POST' }) logoUrl, }) } catch (sendError) { - const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + // Send failed: undo the record (restore the prior token) and drop the + // undelivered new one, leaving the invitee's existing link working. + await recordInviteMagicLinkToken(inviteId, magicLinkToken, priorToken) await revokeMagicLinkToken(magicLinkToken) throw sendError } - await rotateInviteMagicLinkToken(inviteId, inv.magicLinkToken, magicLinkToken) + await revokeMagicLinkToken(priorToken) await recordAuditEvent({ event: 'portal.invite.resent', @@ -508,9 +520,16 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) portalUrl, remainingSeconds ) - // Each copy-link supersedes the prior token; revoke it and record the new - // one so the invite always tracks (and cancel can revoke) the live link. - await rotateInviteMagicLinkToken(inv.id, inv.magicLinkToken, magicLinkToken) + // Record the new token before returning it (so it's tracked and a concurrent + // cancel/rotation is caught), then revoke the prior one it supersedes. The + // link is returned synchronously, so there's no delivery step that can fail + // after the old token is gone. + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + if (!(await recordInviteMagicLinkToken(inv.id, inv.magicLinkToken, magicLinkToken))) { + await revokeMagicLinkToken(magicLinkToken) + throw new Error('Invite is no longer pending — refresh and try again') + } + await revokeMagicLinkToken(inv.magicLinkToken) await recordAuditEvent({ event: 'portal.invite.link_minted', From 99243cca72f86e265c7a1c374a6a56db7764baf2 Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 15:02:17 +0100 Subject: [PATCH 5/9] fix(invitations): copy-link reuses the current live link instead of rotating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../auth/__tests__/magic-link-mint.test.ts | 37 +++++++++++-- .../src/lib/server/auth/magic-link-mint.ts | 16 ++++++ .../__tests__/portal-invites.test.ts | 36 ++++++++++++- .../lib/server/functions/portal-invites.ts | 54 +++++++++++-------- 4 files changed, 116 insertions(+), 27 deletions(-) diff --git a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts index 0b1cdf5d5..e1245675a 100644 --- a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts @@ -28,15 +28,24 @@ vi.mock('../index', async () => { const mockDeleteWhere = vi.fn().mockResolvedValue(undefined) const mockDbDelete = vi.fn(() => ({ where: mockDeleteWhere })) const mockEq = vi.fn((col: unknown, val: unknown) => ({ col, val })) -const mockVerificationTable = { identifier: 'verification.identifier' } +const mockVerificationTable = { + identifier: 'verification.identifier', + expiresAt: 'verification.expiresAt', +} +// select().from().where().limit() chain for isMagicLinkTokenLive +const mockSelectLimit = vi.fn() +const mockDbSelect = vi.fn(() => ({ + from: () => ({ where: () => ({ limit: mockSelectLimit }) }), +})) vi.mock('@/lib/server/db', () => ({ - db: { delete: mockDbDelete }, + db: { delete: mockDbDelete, select: mockDbSelect }, verification: mockVerificationTable, eq: mockEq, })) -const { mintMagicLinkUrl, revokeMagicLinkToken } = await import('../magic-link-mint') +const { mintMagicLinkUrl, revokeMagicLinkToken, isMagicLinkTokenLive } = + await import('../magic-link-mint') beforeEach(() => { vi.clearAllMocks() @@ -115,3 +124,25 @@ describe('revokeMagicLinkToken', () => { expect(mockDbDelete).not.toHaveBeenCalled() }) }) + +describe('isMagicLinkTokenLive', () => { + it('is true when the verification row exists and has not expired', async () => { + mockSelectLimit.mockResolvedValue([{ expiresAt: new Date(Date.now() + 60_000) }]) + expect(await isMagicLinkTokenLive('tok_abc')).toBe(true) + }) + + it('is false when the row is missing (single-use token already consumed)', async () => { + mockSelectLimit.mockResolvedValue([]) + expect(await isMagicLinkTokenLive('tok_abc')).toBe(false) + }) + + it('is false when the row exists but has expired', async () => { + mockSelectLimit.mockResolvedValue([{ expiresAt: new Date(Date.now() - 60_000) }]) + expect(await isMagicLinkTokenLive('tok_abc')).toBe(false) + }) + + it('is false (no query) for a null token', async () => { + expect(await isMagicLinkTokenLive(null)).toBe(false) + expect(mockDbSelect).not.toHaveBeenCalled() + }) +}) diff --git a/apps/web/src/lib/server/auth/magic-link-mint.ts b/apps/web/src/lib/server/auth/magic-link-mint.ts index 188b47955..3a6f2d419 100644 --- a/apps/web/src/lib/server/auth/magic-link-mint.ts +++ b/apps/web/src/lib/server/auth/magic-link-mint.ts @@ -84,3 +84,19 @@ export async function revokeMagicLinkToken(token: string | null | undefined): Pr // mintMagicLinkUrl above, which stores the raw token as the identifier. await db.delete(verification).where(eq(verification.identifier, token)) } + +/** + * Whether a previously-minted magic-link token can still be verified — i.e. its + * verification row still exists (single-use, so it's deleted once consumed) and + * has not expired. Lets the copy-link path reuse the invite's current link + * instead of rotating it. Returns false for null/undefined tokens. + */ +export async function isMagicLinkTokenLive(token: string | null | undefined): Promise { + if (!token) return false + const rows = await db + .select({ expiresAt: verification.expiresAt }) + .from(verification) + .where(eq(verification.identifier, token)) + .limit(1) + return rows.length > 0 && rows[0].expiresAt > new Date() +} diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts index 179a211bc..3554797e6 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts @@ -55,6 +55,8 @@ const hoisted = vi.hoisted(() => ({ mockSendPortalInviteEmail: vi.fn(), mockMintMagicLinkUrl: vi.fn(), mockRevokeMagicLinkToken: vi.fn(), + mockIsMagicLinkTokenLive: vi.fn(), + mockBuildVerifyMagicLinkUrl: vi.fn(), mockGetEmailSafeUrl: vi.fn(), mockGetBaseUrl: vi.fn(), mockGenerateId: vi.fn(), @@ -126,6 +128,8 @@ vi.mock('@quackback/ids', () => ({ vi.mock('@/lib/server/auth/magic-link-mint', () => ({ mintMagicLinkUrl: hoisted.mockMintMagicLinkUrl, revokeMagicLinkToken: hoisted.mockRevokeMagicLinkToken, + isMagicLinkTokenLive: hoisted.mockIsMagicLinkTokenLive, + buildVerifyMagicLinkUrl: hoisted.mockBuildVerifyMagicLinkUrl, })) vi.mock('@/lib/server/storage/s3', () => ({ @@ -177,6 +181,11 @@ beforeEach(async () => { token: 'tok_new', }) hoisted.mockRevokeMagicLinkToken.mockResolvedValue(undefined) + // Default: no live token, so copy-link mints a fresh one (matches most tests). + hoisted.mockIsMagicLinkTokenLive.mockResolvedValue(false) + hoisted.mockBuildVerifyMagicLinkUrl.mockReturnValue( + 'https://acme.example.com/verify-magic-link?token=existing' + ) hoisted.mockGetEmailSafeUrl.mockReturnValue(null) hoisted.mockSendPortalInviteEmail.mockResolvedValue({ sent: false }) hoisted.mockGenerateId.mockReturnValue('invite_test') @@ -405,8 +414,31 @@ describe('getPortalInviteLinkFn', () => { // Allow a few seconds of slack for execution time. expect(opts.expiresInSeconds).toBeGreaterThan(threeDaysSeconds - 60) expect(opts.expiresInSeconds).toBeLessThanOrEqual(threeDaysSeconds) - // And the prior token is revoked. - expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') + }) + + it('reuses the current live link without minting or revoking (copy is non-destructive)', async () => { + hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ + id: 'invite_abc', + kind: 'portal', + status: 'pending', + email: 'user@example.com', + expiresAt: new Date(Date.now() + 10 * 24 * 60 * 60 * 1000), + magicLinkToken: 'tok_live', + }) + hoisted.mockIsMagicLinkTokenLive.mockResolvedValue(true) + hoisted.mockBuildVerifyMagicLinkUrl.mockReturnValue( + 'https://acme.example.com/verify-magic-link?token=tok_live' + ) + + await getLinkHandler({ data: { inviteId: 'invite_abc' } }) + + // The URL is rebuilt from the invite's existing live token... + expect(hoisted.mockBuildVerifyMagicLinkUrl).toHaveBeenCalledWith( + expect.objectContaining({ token: 'tok_live' }) + ) + // ...and copy must not mint a new token nor revoke the existing (emailed) one. + expect(hoisted.mockMintMagicLinkUrl).not.toHaveBeenCalled() + expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalled() }) it('rejects for a non-portal invite kind (returns null from DB)', async () => { diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index cbd66d191..d788f6865 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -32,6 +32,10 @@ const PORTAL_INVITE_EXPIRY_MS = 30 * 24 * 60 * 60 * 1000 * row's own `expiresAt` still governs long-term access. */ const PORTAL_INVITE_MAGIC_LINK_TTL_SECONDS = PORTAL_INVITE_EXPIRY_MS / 1000 +/** The magic-link callback path a portal invitee lands on to accept. Shared by + * the mint and copy-link paths so the two always build the same URL. */ +const portalInviteCallbackPath = (inviteId: string) => `/portal-invite/${inviteId}` + // --------------------------------------------------------------------------- // Schemas // --------------------------------------------------------------------------- @@ -66,7 +70,7 @@ async function mintPortalInviteMagicLink( const { mintMagicLinkUrl } = await import('@/lib/server/auth/magic-link-mint') return mintMagicLinkUrl({ email, - callbackPath: `/portal-invite/${inviteId}`, + callbackPath: portalInviteCallbackPath(inviteId), portalUrl, expiresInSeconds, }) @@ -510,26 +514,34 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) if (inv.expiresAt && inv.expiresAt < new Date()) throw new Error('Invite has expired') const portalUrl = getBaseUrl() - // Copy-link mints a fresh token but does NOT extend the invite row, so cap - // the token at the invite's remaining lifetime — otherwise a link copied - // late could outlive (and sign in past) the invite's own expiry. - const remainingSeconds = Math.floor((inv.expiresAt.getTime() - Date.now()) / 1000) - const { url: inviteLink, token: magicLinkToken } = await mintPortalInviteMagicLink( - inv.email, - inv.id, - portalUrl, - remainingSeconds - ) - // Record the new token before returning it (so it's tracked and a concurrent - // cancel/rotation is caught), then revoke the prior one it supersedes. The - // link is returned synchronously, so there's no delivery step that can fail - // after the old token is gone. - const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - if (!(await recordInviteMagicLinkToken(inv.id, inv.magicLinkToken, magicLinkToken))) { - await revokeMagicLinkToken(magicLinkToken) - throw new Error('Invite is no longer pending — refresh and try again') + const { isMagicLinkTokenLive, buildVerifyMagicLinkUrl, revokeMagicLinkToken } = + await import('@/lib/server/auth/magic-link-mint') + + let inviteLink: string + // Copying returns the invite's CURRENT live link rather than rotating it, so + // a link the invitee may already hold keeps working. Only mint a fresh token + // when there isn't a live one (never sent, or it was already used/expired). + const currentToken = inv.magicLinkToken + if (currentToken && (await isMagicLinkTokenLive(currentToken))) { + inviteLink = buildVerifyMagicLinkUrl({ + origin: portalUrl, + token: currentToken, + callbackPath: portalInviteCallbackPath(inv.id), + }) + } else { + // Cap the fresh token at the invite's remaining lifetime — copy-link does + // not extend the row, so otherwise a late copy could outlive its expiry. + const remainingSeconds = Math.floor((inv.expiresAt.getTime() - Date.now()) / 1000) + const minted = await mintPortalInviteMagicLink(inv.email, inv.id, portalUrl, remainingSeconds) + // Record the new token (catches a concurrent cancel/rotation). The prior + // token, if any, is already dead — that's why we're minting — so there is + // nothing live to revoke. + if (!(await recordInviteMagicLinkToken(inv.id, inv.magicLinkToken, minted.token))) { + await revokeMagicLinkToken(minted.token) + throw new Error('Invite is no longer pending — refresh and try again') + } + inviteLink = minted.url } - await revokeMagicLinkToken(inv.magicLinkToken) await recordAuditEvent({ event: 'portal.invite.link_minted', @@ -540,8 +552,6 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) metadata: { email: inv.email }, }) - // The token is capped to expire with the invite row, so the row's own - // expiry is the accurate cutoff to surface. return { inviteLink, expiresAt: inv.expiresAt } }) From 4e29fd8d7ee3031b64ffd3c9fda6475fc2d78b90 Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 15:34:58 +0100 Subject: [PATCH 6/9] fix(invitations): track the invite's full token set; cancel revokes all 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 --- .../auth/__tests__/magic-link-mint.test.ts | 38 +++++++---- .../src/lib/server/auth/magic-link-mint.ts | 30 ++++++--- .../__tests__/invitation-magic-link.test.ts | 36 ++++++---- .../__tests__/portal-invites.test.ts | 61 ++++++++--------- apps/web/src/lib/server/functions/admin.ts | 53 +++++++-------- .../server/functions/invitation-magic-link.ts | 55 ++++++++-------- .../lib/server/functions/portal-invites.ts | 65 ++++++++----------- .../0112_invitation_magic_link_tokens.sql | 16 +++++ packages/db/drizzle/meta/_journal.json | 7 ++ packages/db/src/schema/auth.ts | 11 ++-- 10 files changed, 206 insertions(+), 166 deletions(-) create mode 100644 packages/db/drizzle/0112_invitation_magic_link_tokens.sql diff --git a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts index e1245675a..b18d8e668 100644 --- a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts @@ -42,9 +42,12 @@ vi.mock('@/lib/server/db', () => ({ db: { delete: mockDbDelete, select: mockDbSelect }, verification: mockVerificationTable, eq: mockEq, + and: vi.fn((...parts: unknown[]) => ({ op: 'and', parts })), + gt: vi.fn((col: unknown, val: unknown) => ({ op: 'gt', col, val })), + inArray: vi.fn((col: unknown, vals: unknown) => ({ op: 'inArray', col, vals })), })) -const { mintMagicLinkUrl, revokeMagicLinkToken, isMagicLinkTokenLive } = +const { mintMagicLinkUrl, revokeMagicLinkToken, revokeMagicLinkTokens, findLiveMagicLinkToken } = await import('../magic-link-mint') beforeEach(() => { @@ -125,24 +128,33 @@ describe('revokeMagicLinkToken', () => { }) }) -describe('isMagicLinkTokenLive', () => { - it('is true when the verification row exists and has not expired', async () => { - mockSelectLimit.mockResolvedValue([{ expiresAt: new Date(Date.now() + 60_000) }]) - expect(await isMagicLinkTokenLive('tok_abc')).toBe(true) +describe('revokeMagicLinkTokens', () => { + it('deletes the verification rows for the whole set', async () => { + await revokeMagicLinkTokens(['tok_a', 'tok_b']) + expect(mockDbDelete).toHaveBeenCalledTimes(1) + expect(mockDbDelete).toHaveBeenCalledWith(mockVerificationTable) + expect(mockDeleteWhere).toHaveBeenCalled() }) - it('is false when the row is missing (single-use token already consumed)', async () => { - mockSelectLimit.mockResolvedValue([]) - expect(await isMagicLinkTokenLive('tok_abc')).toBe(false) + it('is a no-op for an empty set', async () => { + await revokeMagicLinkTokens([]) + expect(mockDbDelete).not.toHaveBeenCalled() + }) +}) + +describe('findLiveMagicLinkToken', () => { + it('returns a token whose verification row exists and has not expired', async () => { + mockSelectLimit.mockResolvedValue([{ identifier: 'tok_b' }]) + expect(await findLiveMagicLinkToken(['tok_a', 'tok_b'])).toBe('tok_b') }) - it('is false when the row exists but has expired', async () => { - mockSelectLimit.mockResolvedValue([{ expiresAt: new Date(Date.now() - 60_000) }]) - expect(await isMagicLinkTokenLive('tok_abc')).toBe(false) + it('returns null when no token in the set is still live', async () => { + mockSelectLimit.mockResolvedValue([]) + expect(await findLiveMagicLinkToken(['tok_a', 'tok_b'])).toBeNull() }) - it('is false (no query) for a null token', async () => { - expect(await isMagicLinkTokenLive(null)).toBe(false) + it('returns null (no query) for an empty set', async () => { + expect(await findLiveMagicLinkToken([])).toBeNull() expect(mockDbSelect).not.toHaveBeenCalled() }) }) diff --git a/apps/web/src/lib/server/auth/magic-link-mint.ts b/apps/web/src/lib/server/auth/magic-link-mint.ts index 3a6f2d419..a3d26e6c0 100644 --- a/apps/web/src/lib/server/auth/magic-link-mint.ts +++ b/apps/web/src/lib/server/auth/magic-link-mint.ts @@ -1,5 +1,5 @@ import { generateRandomString } from 'better-auth/crypto' -import { db, verification, eq } from '@/lib/server/db' +import { db, verification, eq, and, gt, inArray } from '@/lib/server/db' import { getAuth } from './index' interface MintOptions { @@ -86,17 +86,27 @@ export async function revokeMagicLinkToken(token: string | null | undefined): Pr } /** - * Whether a previously-minted magic-link token can still be verified — i.e. its - * verification row still exists (single-use, so it's deleted once consumed) and - * has not expired. Lets the copy-link path reuse the invite's current link - * instead of rotating it. Returns false for null/undefined tokens. + * Bulk-revoke a set of magic-link tokens by deleting their verification rows. + * Used by invite cancellation to invalidate every link the invite ever minted. + * No-op on an empty set; already-consumed/expired tokens simply match no rows. */ -export async function isMagicLinkTokenLive(token: string | null | undefined): Promise { - if (!token) return false +export async function revokeMagicLinkTokens(tokens: string[]): Promise { + if (tokens.length === 0) return + await db.delete(verification).where(inArray(verification.identifier, tokens)) +} + +/** + * Return one still-verifiable token from the given set — its verification row + * exists (single-use, so it's deleted once consumed) and has not expired — or + * null if none are live. Lets the copy-link path reuse the invite's current + * link instead of minting (and thus accumulating) a new one. + */ +export async function findLiveMagicLinkToken(tokens: string[]): Promise { + if (tokens.length === 0) return null const rows = await db - .select({ expiresAt: verification.expiresAt }) + .select({ identifier: verification.identifier }) .from(verification) - .where(eq(verification.identifier, token)) + .where(and(inArray(verification.identifier, tokens), gt(verification.expiresAt, new Date()))) .limit(1) - return rows.length > 0 && rows[0].expiresAt > new Date() + return rows[0]?.identifier ?? null } diff --git a/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts index 5f0c69ba2..11a2cc074 100644 --- a/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/invitation-magic-link.test.ts @@ -20,14 +20,14 @@ vi.mock('@/lib/server/db', () => ({ invitation: { id: 'invitation.id', status: 'invitation.status', - magicLinkToken: 'invitation.magicLinkToken', + magicLinkTokens: 'invitation.magicLinkTokens', }, eq: vi.fn((col: unknown, val: unknown) => ({ op: 'eq', col, val })), and: vi.fn((...parts: unknown[]) => ({ op: 'and', parts })), - isNull: vi.fn((col: unknown) => ({ op: 'isNull', col })), + sql: vi.fn((parts: TemplateStringsArray) => ({ op: 'sql', raw: parts.raw[0] })), })) -const { generateInvitationMagicLink, recordInviteMagicLinkToken } = +const { generateInvitationMagicLink, appendInviteMagicLinkToken, removeInviteMagicLinkToken } = await import('../invitation-magic-link') beforeEach(() => { @@ -37,7 +37,7 @@ beforeEach(() => { token: 'tok_team', }) mockRevokeMagicLinkToken.mockResolvedValue(undefined) - // Default: the compare-and-swap matched one row. + // Default: the status-pinned append matched one row. mockReturning.mockResolvedValue([{ id: 'invite_1' }]) }) @@ -71,24 +71,32 @@ describe('generateInvitationMagicLink', () => { }) }) -describe('recordInviteMagicLinkToken', () => { - it('compare-and-swaps the token and returns true when the invite still matches', async () => { - mockReturning.mockResolvedValue([{ id: 'invite_1' }]) // CAS matched +describe('appendInviteMagicLinkToken', () => { + it('appends and returns true while the invite is pending', async () => { + mockReturning.mockResolvedValue([{ id: 'invite_1' }]) // status-pinned UPDATE matched - const ok = await recordInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') + const ok = await appendInviteMagicLinkToken('invite_1' as InviteId, 'tok_new') expect(ok).toBe(true) - expect(mockSet).toHaveBeenCalledWith({ magicLinkToken: 'tok_new' }) - // It is a pure swap — revocation is the caller's responsibility. + expect(mockSet).toHaveBeenCalledTimes(1) // SET magic_link_tokens = array_append(...) + // Appending is a pure add — revocation is the caller's responsibility. expect(mockRevokeMagicLinkToken).not.toHaveBeenCalled() }) - it('returns false without writing when the row changed (cancel / concurrent rotation)', async () => { - mockReturning.mockResolvedValue([]) // CAS matched nothing + it('returns false without throwing when the invite is no longer pending', async () => { + mockReturning.mockResolvedValue([]) // UPDATE matched nothing (canceled/accepted/expired) - const ok = await recordInviteMagicLinkToken('invite_1' as InviteId, 'tok_old', 'tok_new') + const ok = await appendInviteMagicLinkToken('invite_1' as InviteId, 'tok_new') expect(ok).toBe(false) - expect(mockRevokeMagicLinkToken).not.toHaveBeenCalled() + }) +}) + +describe('removeInviteMagicLinkToken', () => { + it('drops the token from the set and revokes it', async () => { + await removeInviteMagicLinkToken('invite_1' as InviteId, 'tok_x') + + expect(mockSet).toHaveBeenCalledTimes(1) // SET magic_link_tokens = array_remove(...) + expect(mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_x') }) }) diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts index 3554797e6..cd87635fc 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts @@ -55,7 +55,8 @@ const hoisted = vi.hoisted(() => ({ mockSendPortalInviteEmail: vi.fn(), mockMintMagicLinkUrl: vi.fn(), mockRevokeMagicLinkToken: vi.fn(), - mockIsMagicLinkTokenLive: vi.fn(), + mockRevokeMagicLinkTokens: vi.fn(), + mockFindLiveMagicLinkToken: vi.fn(), mockBuildVerifyMagicLinkUrl: vi.fn(), mockGetEmailSafeUrl: vi.fn(), mockGetBaseUrl: vi.fn(), @@ -128,7 +129,8 @@ vi.mock('@quackback/ids', () => ({ vi.mock('@/lib/server/auth/magic-link-mint', () => ({ mintMagicLinkUrl: hoisted.mockMintMagicLinkUrl, revokeMagicLinkToken: hoisted.mockRevokeMagicLinkToken, - isMagicLinkTokenLive: hoisted.mockIsMagicLinkTokenLive, + revokeMagicLinkTokens: hoisted.mockRevokeMagicLinkTokens, + findLiveMagicLinkToken: hoisted.mockFindLiveMagicLinkToken, buildVerifyMagicLinkUrl: hoisted.mockBuildVerifyMagicLinkUrl, })) @@ -181,8 +183,9 @@ beforeEach(async () => { token: 'tok_new', }) hoisted.mockRevokeMagicLinkToken.mockResolvedValue(undefined) - // Default: no live token, so copy-link mints a fresh one (matches most tests). - hoisted.mockIsMagicLinkTokenLive.mockResolvedValue(false) + hoisted.mockRevokeMagicLinkTokens.mockResolvedValue(undefined) + // Default: no live token in the set, so copy-link mints a fresh one. + hoisted.mockFindLiveMagicLinkToken.mockResolvedValue(null) hoisted.mockBuildVerifyMagicLinkUrl.mockReturnValue( 'https://acme.example.com/verify-magic-link?token=existing' ) @@ -301,11 +304,11 @@ describe('sendPortalInviteFn — success (single)', () => { expect(opts.expiresInSeconds).toBe(30 * 24 * 60 * 60) // 30-day invite lifetime, not the 10-min sign-in default }) - it('persists the magic-link token on the invite row so cancel can revoke it', async () => { + it('seeds the invite token set with the minted token so cancel can revoke it', async () => { await sendHandler({ data: { emails: ['invitee@example.com'] } }) const insertCall = hoisted.mockDbInsert.mock.calls[0][0] as Record - expect(insertCall.magicLinkToken).toBe('tok_new') + expect(insertCall.magicLinkTokens).toEqual(['tok_new']) }) it('records a portal.invite.sent audit event', async () => { @@ -423,9 +426,9 @@ describe('getPortalInviteLinkFn', () => { status: 'pending', email: 'user@example.com', expiresAt: new Date(Date.now() + 10 * 24 * 60 * 60 * 1000), - magicLinkToken: 'tok_live', + magicLinkTokens: ['tok_old', 'tok_live'], }) - hoisted.mockIsMagicLinkTokenLive.mockResolvedValue(true) + hoisted.mockFindLiveMagicLinkToken.mockResolvedValue('tok_live') hoisted.mockBuildVerifyMagicLinkUrl.mockReturnValue( 'https://acme.example.com/verify-magic-link?token=tok_live' ) @@ -557,22 +560,22 @@ describe('cancelPortalInviteFn — success', () => { expect(auditCall).toBeDefined() }) - it('revokes the token live at cancel time (from the UPDATE), not a stale earlier read', async () => { + it('revokes the whole token set returned by the cancel UPDATE', async () => { hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ id: 'invite_1', kind: 'portal', status: 'pending', email: 'user@example.com', - magicLinkToken: 'tok_stale', + magicLinkTokens: ['tok_a', 'tok_b'], }) - // The cancel UPDATE returns the token actually on the row at that instant — - // a concurrent rotation may have changed it since the read above. - hoisted.mockDbReturning.mockResolvedValue([{ id: 'invite_1', magicLinkToken: 'tok_live' }]) + // The cancel UPDATE returns the full token set atomically at the flip. + hoisted.mockDbReturning.mockResolvedValue([ + { id: 'invite_1', magicLinkTokens: ['tok_a', 'tok_b', 'tok_c'] }, + ]) await cancelHandler({ data: { inviteId: 'invite_1' } }) - expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_live') - expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalledWith('tok_stale') + expect(hoisted.mockRevokeMagicLinkTokens).toHaveBeenCalledWith(['tok_a', 'tok_b', 'tok_c']) }) }) @@ -622,7 +625,7 @@ describe('resendPortalInviteFn — success', () => { expect((result as { inviteId: string }).inviteId).toBe('invite_1') }) - it('revokes the prior token and records the new one so cancel revokes the live link', async () => { + it('appends the new token without revoking prior links (resend is additive)', async () => { const futureDate = new Date(Date.now() + 14 * 24 * 60 * 60 * 1000) hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ id: 'invite_1', @@ -630,19 +633,22 @@ describe('resendPortalInviteFn — success', () => { status: 'pending', email: 'user@example.com', expiresAt: futureDate, - magicLinkToken: 'tok_old', + magicLinkTokens: ['tok_old'], }) await resendHandler({ data: { inviteId: 'invite_1' } }) - expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_old') - const tokenWrite = hoisted.mockDbSet.mock.calls.find( - (c) => (c[0] as Record).magicLinkToken !== undefined + // The token set is updated (array_append) and the prior token is NOT revoked + // — both the old and new links stay valid until accept/cancel/expiry. + const appended = hoisted.mockDbSet.mock.calls.some( + (c) => 'magicLinkTokens' in (c[0] as Record) ) - expect(tokenWrite?.[0]).toMatchObject({ magicLinkToken: 'tok_new' }) + expect(appended).toBe(true) + expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalled() + expect(hoisted.mockSendPortalInviteEmail).toHaveBeenCalled() }) - it('preserves the existing link when the email send fails (drops the orphan token)', async () => { + it('drops only the new token when the email send fails (prior links untouched)', async () => { const futureDate = new Date(Date.now() + 14 * 24 * 60 * 60 * 1000) hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ id: 'invite_1', @@ -650,21 +656,16 @@ describe('resendPortalInviteFn — success', () => { status: 'pending', email: 'user@example.com', expiresAt: futureDate, - magicLinkToken: 'tok_old', + magicLinkTokens: ['tok_old'], }) hoisted.mockSendPortalInviteEmail.mockRejectedValue(new Error('smtp down')) await expect(resendHandler({ data: { inviteId: 'invite_1' } })).rejects.toThrow('smtp down') - // The undelivered new token is revoked (orphan cleanup) and the invitee's - // existing token is NOT revoked, so their current link still works. + // removeInviteMagicLinkToken revokes the undelivered new token; the prior + // token is never revoked, so the invitee's existing link still works. expect(hoisted.mockRevokeMagicLinkToken).toHaveBeenCalledWith('tok_new') expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalledWith('tok_old') - // The pre-send record is rolled back: the final token write restores the prior. - const tokenWrites = hoisted.mockDbSet.mock.calls - .map((c) => (c[0] as Record).magicLinkToken) - .filter((v) => v !== undefined) - expect(tokenWrites.at(-1)).toBe('tok_old') }) it('emits portal.invite.resent (not portal.invite.sent) on resend', async () => { diff --git a/apps/web/src/lib/server/functions/admin.ts b/apps/web/src/lib/server/functions/admin.ts index 62b350c14..fd2a83e0e 100644 --- a/apps/web/src/lib/server/functions/admin.ts +++ b/apps/web/src/lib/server/functions/admin.ts @@ -59,7 +59,8 @@ import { getBaseUrl } from '@/lib/server/config' import { INVITATION_EXPIRY_MS, generateInvitationMagicLink, - recordInviteMagicLinkToken, + appendInviteMagicLinkToken, + removeInviteMagicLinkToken, } from './invitation-magic-link' /** @@ -955,9 +956,9 @@ export const sendInvitationFn = createServerFn({ method: 'POST' }) const expiresAt = new Date(Date.now() + INVITATION_EXPIRY_MS) const now = new Date() - // Mint the magic link before the insert so the row can record its token - // (lets cancel/resend revoke the link). invitationId is fixed above, so - // the callback path is already known. + // Mint the magic link before the insert so the row records its token in + // its token set (cancel revokes every token in the set). invitationId is + // fixed above, so the callback path is already known. const portalUrl = getBaseUrl() const callbackURL = `/complete-signup/${invitationId}` const { url: inviteLink, token: magicLinkToken } = await generateInvitationMagicLink( @@ -976,7 +977,7 @@ export const sendInvitationFn = createServerFn({ method: 'POST' }) lastSentAt: now, inviterId: auth.user.id, createdAt: now, - magicLinkToken, + magicLinkTokens: [magicLinkToken], }) const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') @@ -1044,18 +1045,18 @@ export const cancelInvitationFn = createServerFn({ method: 'POST' }) eq(invitation.status, 'pending') ) ) - .returning({ id: invitation.id, magicLinkToken: invitation.magicLinkToken }) + .returning({ id: invitation.id, magicLinkTokens: invitation.magicLinkTokens }) if (cancelled.length === 0) { throw new Error('Invitation is no longer pending — refresh and try again') } - // Invalidate the emailed magic link so a cancelled invite can't sign - // anyone in. Revoke the token the UPDATE returned (the one live at the - // instant of cancellation) rather than the earlier read, so a rotation - // racing this cancel can't leave a live token behind. - const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - await revokeMagicLinkToken(cancelled[0].magicLinkToken) + // Invalidate every link this invite ever minted, so a cancelled invite + // can't sign anyone in. Revoking the full set (returned atomically by the + // status flip) closes the resend/copy/worker-restart windows where a + // single rotating pointer could leave a token live but untracked. + const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkTokens(cancelled[0].magicLinkTokens) console.log(`[fn:admin] cancelInvitationFn: canceled`) return { invitationId } @@ -1115,10 +1116,11 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) throw new Error('Invitation is no longer pending — refresh and try again') } - // Generate a new magic link for one-click authentication. The previous - // token is revoked only AFTER the email is sent (or surfaced for manual - // sharing when email isn't configured), so a send failure can't strand - // the invitee by killing the still-valid link they already have. + // Generate a new magic link and add it to the invite's token set. Prior + // tokens are left intact (resend is additive, not destructive) — both the + // old and new links work until the invite is accepted, cancelled, or + // expires. The token is recorded the moment it's minted, so even if the + // send below fails or the worker restarts, cancellation still revokes it. const portalUrl = getBaseUrl() const callbackURL = `/complete-signup/${invitationId}` const { url: inviteLink, token: magicLinkToken } = await generateInvitationMagicLink( @@ -1128,13 +1130,8 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) ) const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - const priorToken = invitationRecord.magicLinkToken - - // Record the new token BEFORE sending, so the emailed link is always the - // one tracked on the invite and a concurrent cancel/rotation is caught - // before we email it. - if (!(await recordInviteMagicLinkToken(invitationId, priorToken, magicLinkToken))) { - await revokeMagicLinkToken(magicLinkToken) // not emailed yet; safe to drop + if (!(await appendInviteMagicLinkToken(invitationId, magicLinkToken))) { + await revokeMagicLinkToken(magicLinkToken) // invite no longer pending; drop it throw new Error('Invitation is no longer pending — refresh and try again') } @@ -1151,17 +1148,11 @@ export const resendInvitationFn = createServerFn({ method: 'POST' }) logoUrl, }) } catch (sendError) { - // Send failed: undo the record (restore the prior token as current) and - // drop the undelivered new one, so the invitee's existing link still works. - await recordInviteMagicLinkToken(invitationId, magicLinkToken, priorToken) - await revokeMagicLinkToken(magicLinkToken) + // The new link never went out — drop it from the set and revoke it. + await removeInviteMagicLinkToken(invitationId, magicLinkToken) throw sendError } - // Delivered (or returned for manual sharing): the new link supersedes the - // old one, so revoke the prior token now. - await revokeMagicLinkToken(priorToken) - console.log( `[fn:admin] resendInvitationFn: ${result.sent ? 'resent' : 'regenerated (email not configured)'}` ) diff --git a/apps/web/src/lib/server/functions/invitation-magic-link.ts b/apps/web/src/lib/server/functions/invitation-magic-link.ts index 0f7fb5cd3..ba62871de 100644 --- a/apps/web/src/lib/server/functions/invitation-magic-link.ts +++ b/apps/web/src/lib/server/functions/invitation-magic-link.ts @@ -40,36 +40,39 @@ export async function generateInvitationMagicLink( } /** - * Compare-and-swap the invite's recorded magic-link token from `expected` to - * `next`, only while the invite is still `pending`. Returns true if the row - * matched and was updated, false otherwise (the invite was canceled, or another - * request changed the token first). + * Append a freshly-minted token to the invite's token set, but only while the + * invite is still `pending`. Returns true if appended, false if the invite is + * no longer pending (canceled / accepted / expired) — in which case the caller + * should revoke the token it just minted rather than leave it live. * - * Callers record the new token with this BEFORE delivering its link (emailing, - * or returning it to copy), so the delivered token is always the one tracked on - * the invite and a concurrent cancel/rotation is detected before delivery. The - * old token is only revoked AFTER successful delivery, so a delivery failure - * never strands the recipient. Passing `next: null` restores the unset state - * (used to roll back a record when delivery then fails). + * Appending (rather than replacing) means a token is recorded the instant it's + * minted, so it can never be live-but-untracked: even if the email send then + * fails or the worker restarts, cancellation still revokes it via the set. */ -export async function recordInviteMagicLinkToken( +export async function appendInviteMagicLinkToken( inviteId: InviteId, - expected: string | null | undefined, - next: string | null + token: string ): Promise { - const { db, invitation, eq, and, isNull } = await import('@/lib/server/db') - const swapped = await db + const { db, invitation, eq, and, sql } = await import('@/lib/server/db') + const updated = await db .update(invitation) - .set({ magicLinkToken: next }) - .where( - and( - eq(invitation.id, inviteId), - eq(invitation.status, 'pending'), - expected == null - ? isNull(invitation.magicLinkToken) - : eq(invitation.magicLinkToken, expected) - ) - ) + .set({ magicLinkTokens: sql`array_append(${invitation.magicLinkTokens}, ${token})` }) + .where(and(eq(invitation.id, inviteId), eq(invitation.status, 'pending'))) .returning({ id: invitation.id }) - return swapped.length > 0 + return updated.length > 0 +} + +/** + * Drop a token from the invite's set and revoke its verification row. Used to + * discard a token whose link was never delivered (e.g. the email send threw), + * keeping the set to links that actually went out. + */ +export async function removeInviteMagicLinkToken(inviteId: InviteId, token: string): Promise { + const { db, invitation, eq, sql } = await import('@/lib/server/db') + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + await db + .update(invitation) + .set({ magicLinkTokens: sql`array_remove(${invitation.magicLinkTokens}, ${token})` }) + .where(eq(invitation.id, inviteId)) + await revokeMagicLinkToken(token) } diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index d788f6865..acb666b15 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -16,7 +16,7 @@ import type { InviteId, UserId } from '@quackback/ids' import { generateId } from '@quackback/ids' import { db, invitation, principal, user, eq, and, gt, or, sql } from '@/lib/server/db' import { requireAuth } from './auth-helpers' -import { recordInviteMagicLinkToken } from './invitation-magic-link' +import { appendInviteMagicLinkToken, removeInviteMagicLinkToken } from './invitation-magic-link' import { actorFromAuth, recordAuditEvent } from '@/lib/server/audit/log' import { getBaseUrl } from '@/lib/server/config' import { sendPortalInviteEmail } from '@quackback/email' @@ -135,8 +135,9 @@ async function sendOnePortalInvite({ const inviteId = generateId('invite') const expiresAt = new Date(now.getTime() + PORTAL_INVITE_EXPIRY_MS) - // Mint before the insert so the row records its token (lets cancel/resend - // revoke the link). inviteId is fixed above, so the callback path is known. + // Mint before the insert so the row records its token in its token set + // (cancel revokes every token in the set). inviteId is fixed above, so the + // callback path is known. const portalUrl = getBaseUrl() const { url: inviteLink, token: magicLinkToken } = await mintPortalInviteMagicLink( email, @@ -155,7 +156,7 @@ async function sendOnePortalInvite({ createdAt: now, lastSentAt: now, inviterId: auth.user.id as UserId, - magicLinkToken, + magicLinkTokens: [magicLinkToken], }) const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') @@ -287,19 +288,19 @@ export const cancelPortalInviteFn = createServerFn({ method: 'POST' }) eq(invitation.status, 'pending') ) ) - .returning({ id: invitation.id, magicLinkToken: invitation.magicLinkToken }) + .returning({ id: invitation.id, magicLinkTokens: invitation.magicLinkTokens }) if (updated.length === 0) { console.log(`[fn:portal-invites] cancelPortalInviteFn: no-op (row concurrently mutated)`) return { inviteId, status: 'no_op_already_accepted' as const } } - // Invalidate the emailed magic link so a cancelled invite can't sign anyone - // in. Revoke the token the UPDATE returned (live at the instant of - // cancellation) rather than the earlier read, so a rotation racing this - // cancel can't leave a live token behind. - const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - await revokeMagicLinkToken(updated[0].magicLinkToken) + // Invalidate every link this invite ever minted so a cancelled invite can't + // sign anyone in. Revoking the full set (returned atomically by the status + // flip) closes the resend/copy/worker-restart windows where a single + // rotating pointer could leave a token live but untracked. + const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkTokens(updated[0].magicLinkTokens) await recordAuditEvent({ event: 'portal.invite.revoked', @@ -396,19 +397,15 @@ export const resendPortalInviteFn = createServerFn({ method: 'POST' }) portalUrl ) + // Add the new token to the invite's set (resend is additive — prior links + // keep working until accept/cancel/expiry). Recorded the moment it's minted, + // so a send failure or worker restart can't leave it live-but-untracked. const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') - const priorToken = inv.magicLinkToken - - // Record the new token BEFORE sending so the emailed link is always tracked - // and a concurrent cancel/rotation is caught before delivery. - if (!(await recordInviteMagicLinkToken(inviteId, priorToken, magicLinkToken))) { - await revokeMagicLinkToken(magicLinkToken) // not emailed yet; safe to drop + if (!(await appendInviteMagicLinkToken(inviteId, magicLinkToken))) { + await revokeMagicLinkToken(magicLinkToken) // invite no longer pending; drop it throw new Error('Invitation is no longer pending — refresh and try again') } - // The prior token is revoked only AFTER the email is sent (or surfaced for - // manual sharing when email isn't configured), so a send failure can't - // strand the invitee by killing the still-valid link they already have. const { getEmailSafeUrl } = await import('@/lib/server/storage/s3') const logoUrl = getEmailSafeUrl(auth.settings.logoKey) ?? undefined let result: Awaited> @@ -420,13 +417,10 @@ export const resendPortalInviteFn = createServerFn({ method: 'POST' }) logoUrl, }) } catch (sendError) { - // Send failed: undo the record (restore the prior token) and drop the - // undelivered new one, leaving the invitee's existing link working. - await recordInviteMagicLinkToken(inviteId, magicLinkToken, priorToken) - await revokeMagicLinkToken(magicLinkToken) + // The new link never went out — drop it from the set and revoke it. + await removeInviteMagicLinkToken(inviteId, magicLinkToken) throw sendError } - await revokeMagicLinkToken(priorToken) await recordAuditEvent({ event: 'portal.invite.resent', @@ -514,18 +508,18 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) if (inv.expiresAt && inv.expiresAt < new Date()) throw new Error('Invite has expired') const portalUrl = getBaseUrl() - const { isMagicLinkTokenLive, buildVerifyMagicLinkUrl, revokeMagicLinkToken } = + const { findLiveMagicLinkToken, buildVerifyMagicLinkUrl, revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') let inviteLink: string - // Copying returns the invite's CURRENT live link rather than rotating it, so - // a link the invitee may already hold keeps working. Only mint a fresh token - // when there isn't a live one (never sent, or it was already used/expired). - const currentToken = inv.magicLinkToken - if (currentToken && (await isMagicLinkTokenLive(currentToken))) { + // Copying returns one of the invite's CURRENT live links rather than minting + // a new one, so a link the invitee may already hold keeps working. Only mint + // when none of the set's tokens are still live (never sent, or all used/expired). + const liveToken = await findLiveMagicLinkToken(inv.magicLinkTokens) + if (liveToken) { inviteLink = buildVerifyMagicLinkUrl({ origin: portalUrl, - token: currentToken, + token: liveToken, callbackPath: portalInviteCallbackPath(inv.id), }) } else { @@ -533,11 +527,8 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) // not extend the row, so otherwise a late copy could outlive its expiry. const remainingSeconds = Math.floor((inv.expiresAt.getTime() - Date.now()) / 1000) const minted = await mintPortalInviteMagicLink(inv.email, inv.id, portalUrl, remainingSeconds) - // Record the new token (catches a concurrent cancel/rotation). The prior - // token, if any, is already dead — that's why we're minting — so there is - // nothing live to revoke. - if (!(await recordInviteMagicLinkToken(inv.id, inv.magicLinkToken, minted.token))) { - await revokeMagicLinkToken(minted.token) + if (!(await appendInviteMagicLinkToken(inv.id, minted.token))) { + await revokeMagicLinkToken(minted.token) // invite no longer pending; drop it throw new Error('Invite is no longer pending — refresh and try again') } inviteLink = minted.url diff --git a/packages/db/drizzle/0112_invitation_magic_link_tokens.sql b/packages/db/drizzle/0112_invitation_magic_link_tokens.sql new file mode 100644 index 000000000..96c039f51 --- /dev/null +++ b/packages/db/drizzle/0112_invitation_magic_link_tokens.sql @@ -0,0 +1,16 @@ +-- Track the FULL set of magic-link tokens minted for an invitation, not just +-- the latest. send/resend/copy each append a token; cancel revokes them all. +-- This makes cancellation robust under concurrency and worker restarts: a +-- token minted during a resend's email-send window is recorded immediately, so +-- it can never end up live-but-untracked (and thus survive a later cancel) the +-- way a single rotating pointer could. +-- +-- Replaces the single magic_link_token column added in 0111. Backfill-safe: +-- existing pending invites carry their one tracked token into the array. +ALTER TABLE "invitation" ADD COLUMN "magic_link_tokens" text[] NOT NULL DEFAULT '{}'; + +UPDATE "invitation" + SET "magic_link_tokens" = ARRAY["magic_link_token"] + WHERE "magic_link_token" IS NOT NULL; + +ALTER TABLE "invitation" DROP COLUMN "magic_link_token"; diff --git a/packages/db/drizzle/meta/_journal.json b/packages/db/drizzle/meta/_journal.json index 64f264cef..e81eafec2 100644 --- a/packages/db/drizzle/meta/_journal.json +++ b/packages/db/drizzle/meta/_journal.json @@ -785,6 +785,13 @@ "when": 1782259200000, "tag": "0111_invitation_magic_link_token", "breakpoints": true + }, + { + "idx": 112, + "version": "7", + "when": 1782345600000, + "tag": "0112_invitation_magic_link_tokens", + "breakpoints": true } ] } diff --git a/packages/db/src/schema/auth.ts b/packages/db/src/schema/auth.ts index 151dbaa95..d64cf4478 100644 --- a/packages/db/src/schema/auth.ts +++ b/packages/db/src/schema/auth.ts @@ -465,12 +465,13 @@ export const invitation = pgTable( createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(), lastSentAt: timestamp('last_sent_at', { withTimezone: true }), /** - * The `verification.identifier` (magic-link token) currently minted for - * this invite. Lets cancel / re-send delete the backing verification row - * so a cancelled or superseded link can't mint a session. Null for invites - * sent before this was tracked (their tokens just self-expire at expiresAt). + * The set of `verification.identifier` magic-link tokens minted for this + * invite (one per send/resend/copy). Cancel revokes every token in the set, + * so no link can outlive the invite — even one minted during a resend's + * send window or after a worker restart. Tokens are single-use and expire + * with the invite, so the set stays small and self-pruning. */ - magicLinkToken: text('magic_link_token'), + magicLinkTokens: text('magic_link_tokens').array().notNull().default([]), inviterId: typeIdColumn('user')('inviter_id') .notNull() .references(() => user.id, { onDelete: 'cascade' }), From 13fe2f0c9a0d26835709eeb691920debbf8bdf69 Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 15:46:27 +0100 Subject: [PATCH 7/9] fix(invitations): revoke the token set on accept; copy reuses the longest-lived token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../server/auth/__tests__/magic-link-mint.test.ts | 5 +++-- apps/web/src/lib/server/auth/magic-link-mint.ts | 13 ++++++++----- .../__tests__/portal-invites-accept.test.ts | 14 ++++++++++++++ apps/web/src/lib/server/functions/invitations.ts | 7 +++++++ .../web/src/lib/server/functions/portal-invites.ts | 7 +++++++ 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts index b18d8e668..e6bb9d4ce 100644 --- a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts @@ -32,10 +32,10 @@ const mockVerificationTable = { identifier: 'verification.identifier', expiresAt: 'verification.expiresAt', } -// select().from().where().limit() chain for isMagicLinkTokenLive +// select().from().where().orderBy().limit() chain for findLiveMagicLinkToken const mockSelectLimit = vi.fn() const mockDbSelect = vi.fn(() => ({ - from: () => ({ where: () => ({ limit: mockSelectLimit }) }), + from: () => ({ where: () => ({ orderBy: () => ({ limit: mockSelectLimit }) }) }), })) vi.mock('@/lib/server/db', () => ({ @@ -45,6 +45,7 @@ vi.mock('@/lib/server/db', () => ({ and: vi.fn((...parts: unknown[]) => ({ op: 'and', parts })), gt: vi.fn((col: unknown, val: unknown) => ({ op: 'gt', col, val })), inArray: vi.fn((col: unknown, vals: unknown) => ({ op: 'inArray', col, vals })), + desc: vi.fn((col: unknown) => ({ op: 'desc', col })), })) const { mintMagicLinkUrl, revokeMagicLinkToken, revokeMagicLinkTokens, findLiveMagicLinkToken } = diff --git a/apps/web/src/lib/server/auth/magic-link-mint.ts b/apps/web/src/lib/server/auth/magic-link-mint.ts index a3d26e6c0..76fb45bcc 100644 --- a/apps/web/src/lib/server/auth/magic-link-mint.ts +++ b/apps/web/src/lib/server/auth/magic-link-mint.ts @@ -1,5 +1,5 @@ import { generateRandomString } from 'better-auth/crypto' -import { db, verification, eq, and, gt, inArray } from '@/lib/server/db' +import { db, verification, eq, and, gt, inArray, desc } from '@/lib/server/db' import { getAuth } from './index' interface MintOptions { @@ -96,10 +96,12 @@ export async function revokeMagicLinkTokens(tokens: string[]): Promise { } /** - * Return one still-verifiable token from the given set — its verification row - * exists (single-use, so it's deleted once consumed) and has not expired — or - * null if none are live. Lets the copy-link path reuse the invite's current - * link instead of minting (and thus accumulating) a new one. + * Return the longest-lived still-verifiable token from the given set — its + * verification row exists (single-use, so it's deleted once consumed) and has + * not expired — or null if none are live. Lets the copy-link path reuse the + * invite's current link instead of minting a new one. Ordered by expiry so a + * resent invite hands out the token that covers the most of its lifetime, not + * an arbitrary older one. */ export async function findLiveMagicLinkToken(tokens: string[]): Promise { if (tokens.length === 0) return null @@ -107,6 +109,7 @@ export async function findLiveMagicLinkToken(tokens: string[]): Promise ({ mockDbInsert: vi.fn(), mockSendPortalInviteEmail: vi.fn(), mockMintMagicLinkUrl: vi.fn(), + mockRevokeMagicLinkTokens: vi.fn(), mockGetEmailSafeUrl: vi.fn(), mockGetBaseUrl: vi.fn(), mockGenerateId: vi.fn(), @@ -126,6 +127,7 @@ vi.mock('@quackback/ids', () => ({ vi.mock('@/lib/server/auth/magic-link-mint', () => ({ mintMagicLinkUrl: hoisted.mockMintMagicLinkUrl, + revokeMagicLinkTokens: hoisted.mockRevokeMagicLinkTokens, })) vi.mock('@/lib/server/storage/s3', () => ({ @@ -403,6 +405,18 @@ describe('acceptPortalInviteFn — happy path', () => { expect(auditCall).toBeDefined() }) + it('revokes the invite token set on accept so sibling links can no longer sign in', async () => { + // An invite resent/copied has more than one live token; accepting via one + // must kill the rest. + hoisted.mockDbUpdateReturning.mockResolvedValue([ + { id: 'invite_1', magicLinkTokens: ['tok_used', 'tok_sibling'] }, + ]) + + await acceptHandler({ data: { inviteId: 'invite_1' } }) + + expect(hoisted.mockRevokeMagicLinkTokens).toHaveBeenCalledWith(['tok_used', 'tok_sibling']) + }) + it('includes the invite email in the audit after-value', async () => { await acceptHandler({ data: { inviteId: 'invite_1' } }) diff --git a/apps/web/src/lib/server/functions/invitations.ts b/apps/web/src/lib/server/functions/invitations.ts index 48e583656..a0be422dd 100644 --- a/apps/web/src/lib/server/functions/invitations.ts +++ b/apps/web/src/lib/server/functions/invitations.ts @@ -237,6 +237,13 @@ export const acceptInvitationFn = createServerFn({ method: 'POST' }) await db.update(user).set({ name: displayName }).where(eq(user.id, userId)) } + // The invite is accepted — revoke every token in its set so no other + // emailed/copied link for this invite can still sign anyone in. (The link + // just used was already consumed by the magic-link verify; siblings from + // resends/copies would otherwise stay live until their 30-day expiry.) + const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkTokens(claimed.magicLinkTokens) + console.log(`[fn:invitations] acceptInvitationFn: accepted`) return { invitationId: invitationId as InviteId } } catch (error) { diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index acb666b15..daad74c23 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -718,6 +718,13 @@ export const acceptPortalInviteFn = createServerFn({ method: 'POST' }) after: { email: inv.email, kind: 'portal' }, }) + // Accepted — revoke every token in the set so no other emailed/copied link + // for this invite can still sign anyone in. The link just used was consumed + // by the magic-link verify; siblings from resends/copies would otherwise + // stay live until their 30-day expiry. + const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkTokens(updated[0].magicLinkTokens) + console.log(`[fn:portal-invites] acceptPortalInviteFn: accepted id=${inviteId}`) return { status: 'accepted', alreadyAccepted: false } }) From 3b62ac7c933ea97e99858594b39efc49ffc74f39 Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 15:56:35 +0100 Subject: [PATCH 8/9] fix(invitations): make accept-time token revoke best-effort; fix test type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../__tests__/portal-invites-accept.test.ts | 5 ++++- apps/web/src/lib/server/functions/invitations.ts | 11 +++++++++-- .../src/lib/server/functions/portal-invites.ts | 15 ++++++++++++--- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites-accept.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites-accept.test.ts index e4baac770..9fe12d233 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites-accept.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites-accept.test.ts @@ -48,7 +48,10 @@ const hoisted = vi.hoisted(() => ({ // The accept handler now uses RETURNING to detect zero-row writes // (concurrent cancel / expiry / accept race). Tests can override this // per-case to simulate the race. - mockDbUpdateReturning: vi.fn(() => Promise.resolve([{ id: 'invite_1' }])), + mockDbUpdateReturning: vi.fn( + (): Promise> => + Promise.resolve([{ id: 'invite_1' }]) + ), mockDbQuery: { invitation: { findFirst: vi.fn() }, principal: { findFirst: vi.fn() }, diff --git a/apps/web/src/lib/server/functions/invitations.ts b/apps/web/src/lib/server/functions/invitations.ts index a0be422dd..ab917f7cd 100644 --- a/apps/web/src/lib/server/functions/invitations.ts +++ b/apps/web/src/lib/server/functions/invitations.ts @@ -241,8 +241,15 @@ export const acceptInvitationFn = createServerFn({ method: 'POST' }) // emailed/copied link for this invite can still sign anyone in. (The link // just used was already consumed by the magic-link verify; siblings from // resends/copies would otherwise stay live until their 30-day expiry.) - const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') - await revokeMagicLinkTokens(claimed.magicLinkTokens) + // Best-effort: the membership is already committed, so a cleanup failure + // here must NOT hit the outer catch and roll the accept back to pending — + // log it and move on (the stray tokens still expire with the invite). + try { + const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkTokens(claimed.magicLinkTokens) + } catch (revokeError) { + console.error(`[fn:invitations] ⚠️ acceptInvitationFn: token revoke failed:`, revokeError) + } console.log(`[fn:invitations] acceptInvitationFn: accepted`) return { invitationId: invitationId as InviteId } diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index daad74c23..b6386dee8 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -721,9 +721,18 @@ export const acceptPortalInviteFn = createServerFn({ method: 'POST' }) // Accepted — revoke every token in the set so no other emailed/copied link // for this invite can still sign anyone in. The link just used was consumed // by the magic-link verify; siblings from resends/copies would otherwise - // stay live until their 30-day expiry. - const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') - await revokeMagicLinkTokens(updated[0].magicLinkTokens) + // stay live until their 30-day expiry. Best-effort: the accept is already + // committed, so a cleanup failure must not fail the request (the stray + // tokens still expire with the invite). + try { + const { revokeMagicLinkTokens } = await import('@/lib/server/auth/magic-link-mint') + await revokeMagicLinkTokens(updated[0].magicLinkTokens) + } catch (revokeError) { + console.error( + `[fn:portal-invites] ⚠️ acceptPortalInviteFn: token revoke failed:`, + revokeError + ) + } console.log(`[fn:portal-invites] acceptPortalInviteFn: accepted id=${inviteId}`) return { status: 'accepted', alreadyAccepted: false } From 164ce0e4c9cea87673b032bd29f1029b49760e14 Mon Sep 17 00:00:00 2001 From: James Morton Date: Sat, 13 Jun 2026 16:13:09 +0100 Subject: [PATCH 9/9] fix(invitations): copy always mints (drop stale-token reuse); SQL array 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 --- .../auth/__tests__/magic-link-mint.test.ts | 34 ++------------- .../src/lib/server/auth/magic-link-mint.ts | 21 +--------- .../__tests__/portal-invites.test.ts | 26 +++--------- .../lib/server/functions/portal-invites.ts | 41 ++++++++----------- packages/db/src/schema/auth.ts | 5 ++- 5 files changed, 30 insertions(+), 97 deletions(-) diff --git a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts index e6bb9d4ce..4d1cc3414 100644 --- a/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/magic-link-mint.test.ts @@ -28,27 +28,16 @@ vi.mock('../index', async () => { const mockDeleteWhere = vi.fn().mockResolvedValue(undefined) const mockDbDelete = vi.fn(() => ({ where: mockDeleteWhere })) const mockEq = vi.fn((col: unknown, val: unknown) => ({ col, val })) -const mockVerificationTable = { - identifier: 'verification.identifier', - expiresAt: 'verification.expiresAt', -} -// select().from().where().orderBy().limit() chain for findLiveMagicLinkToken -const mockSelectLimit = vi.fn() -const mockDbSelect = vi.fn(() => ({ - from: () => ({ where: () => ({ orderBy: () => ({ limit: mockSelectLimit }) }) }), -})) +const mockVerificationTable = { identifier: 'verification.identifier' } vi.mock('@/lib/server/db', () => ({ - db: { delete: mockDbDelete, select: mockDbSelect }, + db: { delete: mockDbDelete }, verification: mockVerificationTable, eq: mockEq, - and: vi.fn((...parts: unknown[]) => ({ op: 'and', parts })), - gt: vi.fn((col: unknown, val: unknown) => ({ op: 'gt', col, val })), inArray: vi.fn((col: unknown, vals: unknown) => ({ op: 'inArray', col, vals })), - desc: vi.fn((col: unknown) => ({ op: 'desc', col })), })) -const { mintMagicLinkUrl, revokeMagicLinkToken, revokeMagicLinkTokens, findLiveMagicLinkToken } = +const { mintMagicLinkUrl, revokeMagicLinkToken, revokeMagicLinkTokens } = await import('../magic-link-mint') beforeEach(() => { @@ -142,20 +131,3 @@ describe('revokeMagicLinkTokens', () => { expect(mockDbDelete).not.toHaveBeenCalled() }) }) - -describe('findLiveMagicLinkToken', () => { - it('returns a token whose verification row exists and has not expired', async () => { - mockSelectLimit.mockResolvedValue([{ identifier: 'tok_b' }]) - expect(await findLiveMagicLinkToken(['tok_a', 'tok_b'])).toBe('tok_b') - }) - - it('returns null when no token in the set is still live', async () => { - mockSelectLimit.mockResolvedValue([]) - expect(await findLiveMagicLinkToken(['tok_a', 'tok_b'])).toBeNull() - }) - - it('returns null (no query) for an empty set', async () => { - expect(await findLiveMagicLinkToken([])).toBeNull() - expect(mockDbSelect).not.toHaveBeenCalled() - }) -}) diff --git a/apps/web/src/lib/server/auth/magic-link-mint.ts b/apps/web/src/lib/server/auth/magic-link-mint.ts index 76fb45bcc..ef7846ba8 100644 --- a/apps/web/src/lib/server/auth/magic-link-mint.ts +++ b/apps/web/src/lib/server/auth/magic-link-mint.ts @@ -1,5 +1,5 @@ import { generateRandomString } from 'better-auth/crypto' -import { db, verification, eq, and, gt, inArray, desc } from '@/lib/server/db' +import { db, verification, eq, inArray } from '@/lib/server/db' import { getAuth } from './index' interface MintOptions { @@ -94,22 +94,3 @@ export async function revokeMagicLinkTokens(tokens: string[]): Promise { if (tokens.length === 0) return await db.delete(verification).where(inArray(verification.identifier, tokens)) } - -/** - * Return the longest-lived still-verifiable token from the given set — its - * verification row exists (single-use, so it's deleted once consumed) and has - * not expired — or null if none are live. Lets the copy-link path reuse the - * invite's current link instead of minting a new one. Ordered by expiry so a - * resent invite hands out the token that covers the most of its lifetime, not - * an arbitrary older one. - */ -export async function findLiveMagicLinkToken(tokens: string[]): Promise { - if (tokens.length === 0) return null - const rows = await db - .select({ identifier: verification.identifier }) - .from(verification) - .where(and(inArray(verification.identifier, tokens), gt(verification.expiresAt, new Date()))) - .orderBy(desc(verification.expiresAt)) - .limit(1) - return rows[0]?.identifier ?? null -} diff --git a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts index cd87635fc..1e12c60b5 100644 --- a/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts +++ b/apps/web/src/lib/server/functions/__tests__/portal-invites.test.ts @@ -56,8 +56,6 @@ const hoisted = vi.hoisted(() => ({ mockMintMagicLinkUrl: vi.fn(), mockRevokeMagicLinkToken: vi.fn(), mockRevokeMagicLinkTokens: vi.fn(), - mockFindLiveMagicLinkToken: vi.fn(), - mockBuildVerifyMagicLinkUrl: vi.fn(), mockGetEmailSafeUrl: vi.fn(), mockGetBaseUrl: vi.fn(), mockGenerateId: vi.fn(), @@ -130,8 +128,6 @@ vi.mock('@/lib/server/auth/magic-link-mint', () => ({ mintMagicLinkUrl: hoisted.mockMintMagicLinkUrl, revokeMagicLinkToken: hoisted.mockRevokeMagicLinkToken, revokeMagicLinkTokens: hoisted.mockRevokeMagicLinkTokens, - findLiveMagicLinkToken: hoisted.mockFindLiveMagicLinkToken, - buildVerifyMagicLinkUrl: hoisted.mockBuildVerifyMagicLinkUrl, })) vi.mock('@/lib/server/storage/s3', () => ({ @@ -184,11 +180,6 @@ beforeEach(async () => { }) hoisted.mockRevokeMagicLinkToken.mockResolvedValue(undefined) hoisted.mockRevokeMagicLinkTokens.mockResolvedValue(undefined) - // Default: no live token in the set, so copy-link mints a fresh one. - hoisted.mockFindLiveMagicLinkToken.mockResolvedValue(null) - hoisted.mockBuildVerifyMagicLinkUrl.mockReturnValue( - 'https://acme.example.com/verify-magic-link?token=existing' - ) hoisted.mockGetEmailSafeUrl.mockReturnValue(null) hoisted.mockSendPortalInviteEmail.mockResolvedValue({ sent: false }) hoisted.mockGenerateId.mockReturnValue('invite_test') @@ -419,29 +410,22 @@ describe('getPortalInviteLinkFn', () => { expect(opts.expiresInSeconds).toBeLessThanOrEqual(threeDaysSeconds) }) - it('reuses the current live link without minting or revoking (copy is non-destructive)', async () => { + it('does not revoke other outstanding links when minting a copy (additive)', async () => { hoisted.mockDbQuery.invitation.findFirst.mockResolvedValue({ id: 'invite_abc', kind: 'portal', status: 'pending', email: 'user@example.com', expiresAt: new Date(Date.now() + 10 * 24 * 60 * 60 * 1000), - magicLinkTokens: ['tok_old', 'tok_live'], + magicLinkTokens: ['tok_old'], }) - hoisted.mockFindLiveMagicLinkToken.mockResolvedValue('tok_live') - hoisted.mockBuildVerifyMagicLinkUrl.mockReturnValue( - 'https://acme.example.com/verify-magic-link?token=tok_live' - ) await getLinkHandler({ data: { inviteId: 'invite_abc' } }) - // The URL is rebuilt from the invite's existing live token... - expect(hoisted.mockBuildVerifyMagicLinkUrl).toHaveBeenCalledWith( - expect.objectContaining({ token: 'tok_live' }) - ) - // ...and copy must not mint a new token nor revoke the existing (emailed) one. - expect(hoisted.mockMintMagicLinkUrl).not.toHaveBeenCalled() + // Copy mints a fresh token (additive) and never revokes existing links. + expect(hoisted.mockMintMagicLinkUrl).toHaveBeenCalled() expect(hoisted.mockRevokeMagicLinkToken).not.toHaveBeenCalled() + expect(hoisted.mockRevokeMagicLinkTokens).not.toHaveBeenCalled() }) it('rejects for a non-portal invite kind (returns null from DB)', async () => { diff --git a/apps/web/src/lib/server/functions/portal-invites.ts b/apps/web/src/lib/server/functions/portal-invites.ts index b6386dee8..72610d4fd 100644 --- a/apps/web/src/lib/server/functions/portal-invites.ts +++ b/apps/web/src/lib/server/functions/portal-invites.ts @@ -508,30 +508,23 @@ export const getPortalInviteLinkFn = createServerFn({ method: 'POST' }) if (inv.expiresAt && inv.expiresAt < new Date()) throw new Error('Invite has expired') const portalUrl = getBaseUrl() - const { findLiveMagicLinkToken, buildVerifyMagicLinkUrl, revokeMagicLinkToken } = - await import('@/lib/server/auth/magic-link-mint') - - let inviteLink: string - // Copying returns one of the invite's CURRENT live links rather than minting - // a new one, so a link the invitee may already hold keeps working. Only mint - // when none of the set's tokens are still live (never sent, or all used/expired). - const liveToken = await findLiveMagicLinkToken(inv.magicLinkTokens) - if (liveToken) { - inviteLink = buildVerifyMagicLinkUrl({ - origin: portalUrl, - token: liveToken, - callbackPath: portalInviteCallbackPath(inv.id), - }) - } else { - // Cap the fresh token at the invite's remaining lifetime — copy-link does - // not extend the row, so otherwise a late copy could outlive its expiry. - const remainingSeconds = Math.floor((inv.expiresAt.getTime() - Date.now()) / 1000) - const minted = await mintPortalInviteMagicLink(inv.email, inv.id, portalUrl, remainingSeconds) - if (!(await appendInviteMagicLinkToken(inv.id, minted.token))) { - await revokeMagicLinkToken(minted.token) // invite no longer pending; drop it - throw new Error('Invite is no longer pending — refresh and try again') - } - inviteLink = minted.url + const { revokeMagicLinkToken } = await import('@/lib/server/auth/magic-link-mint') + + // Mint a fresh link and add it to the set. Minting is additive — it never + // touches the invite's other outstanding links, so copying doesn't + // invalidate a link the invitee may already hold. Capping the token at the + // invite's remaining lifetime keeps it from outliving the invite, and a + // freshly-minted token always covers that full window (no stale reuse). + const remainingSeconds = Math.floor((inv.expiresAt.getTime() - Date.now()) / 1000) + const { url: inviteLink, token } = await mintPortalInviteMagicLink( + inv.email, + inv.id, + portalUrl, + remainingSeconds + ) + if (!(await appendInviteMagicLinkToken(inv.id, token))) { + await revokeMagicLinkToken(token) // invite no longer pending; drop it + throw new Error('Invite is no longer pending — refresh and try again') } await recordAuditEvent({ diff --git a/packages/db/src/schema/auth.ts b/packages/db/src/schema/auth.ts index d64cf4478..2927475e3 100644 --- a/packages/db/src/schema/auth.ts +++ b/packages/db/src/schema/auth.ts @@ -471,7 +471,10 @@ export const invitation = pgTable( * send window or after a worker restart. Tokens are single-use and expire * with the invite, so the set stays small and self-pruning. */ - magicLinkTokens: text('magic_link_tokens').array().notNull().default([]), + magicLinkTokens: text('magic_link_tokens') + .array() + .notNull() + .default(sql`'{}'::text[]`), inviterId: typeIdColumn('user')('inviter_id') .notNull() .references(() => user.id, { onDelete: 'cascade' }),