Repository navigation
feat(invites): GDPR + zero-trust drive invites - #1267
Merged
Merged
Conversation
Restart of #1266. New epic file scoped to the approved plan: hard-cutover deletions of post-login broad-sweep + 9 auth callers + resend route, magic-link service body untouched, wipe-not-port migration, /invite/[token]/accept gateway as its own task. Supersedes #1266. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add pending_invites schema with token_hash, email, drive_id, role, invited_by, expires_at, consumed_at. Both FKs cascade on delete. Partial unique index on (drive_id, email) WHERE consumed_at IS NULL prevents duplicate active invites at the DB level. Wired into schema barrel + namespace + package exports. Migration 0122 generated via pnpm db:generate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
isInviteExpired, isInviteConsumed, isEmailMatchingInvite — pure functions with destructured object args, now injected. Email match is case + whitespace insensitive (matches the trim+lowercase normalization the invite endpoint already applies). 15 colocated tests cover the boundary case (now equals expiresAt → expired) plus 1ms-before / 1ms-after. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
createInviteToken({ now, expiryMinutes? }) mints ps_invite_* tokens with default 48h expiry. verifyInviteToken({ token, tokenHash }) compares via secureCompare (timing-safe). Reuses generateToken + hashToken from token-utils — no new hashing primitive. 9 colocated tests cover expiry math, prefix shape, hash distinctness, mint-twice uniqueness, and rejection of empty/tampered tokens.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add createPendingInvite (sweeps expired-unconsumed for the (driveId, email) pair before insert so the partial unique index never blocks legit re-invites), findPendingInviteByTokenHash (joined drive name + inviter name), findActivePendingInviteByDriveAndEmail (filters consumedAt IS NULL AND expiresAt > now), markInviteConsumed (atomic conditional UPDATE), deletePendingInvite, findUserToSStatusByEmail, and consumeInviteAndCreateMembership — a single Drizzle transaction that conditionally consumes the token then inserts driveMembers; throws a sentinel on (driveId, userId) unique violation so the transaction rolls back and the caller receives ALREADY_MEMBER without burning the token. 15 new tests cover happy/race/duplicate/connection-error paths. Legacy methods (findPendingMembersForUser, acceptPendingMember, bumpInvitedAt) remain in place; T9 deletes them once their callers are removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
handleEmailPath swaps createMagicLinkToken + createDriveMember(acceptedAt:null) for createInviteToken + createPendingInvite. URL becomes /invite/<rawToken>. Active-pending pre-check uses findActivePendingInviteByDriveAndEmail. Concurrent re-invite race surfaces as 409 via partial unique index. Email-send failure rolls back via deletePendingInvite. logMemberActivity is intentionally not called on the pending path — there is no targetUserId, the audit event captures the email-keyed invite. handleUserIdPath is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
resolveInviteContext({ token, now }) hashes the token (SHA3, never plaintext) and returns a discriminated result: data with drive name, inviter name, role, invited email, and isExistingUser (tosAcceptedAt IS NOT NULL); or NOT_FOUND/EXPIRED/CONSUMED. The server-component page awaits Next.js 15 async params and renders the consent card with ToS/Privacy CTAs — or an opaque 'no longer valid' card on any failure (NEVER redirects, which would leak token existence). 7 colocated resolver tests cover the happy paths + each error variant + the SHA-not-plaintext lookup contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
acceptInviteForExistingUser + acceptInviteForNewUser pipes share validateAndLoadInvite + consumeAndShape so the discriminated TOKEN_NOT_FOUND/EXPIRED/CONSUMED/EMAIL_MISMATCH/ALREADY_MEMBER ladder is identical across signup and existing-user paths. The existing-user path runs findExistingMember pre-check so already-accepted users surface ALREADY_MEMBER without burning the token. The GET handler authenticates via session, redirects unauth'd users to /auth/signin?invite=&next=, redirects success to /dashboard/<driveId>?invited=1, redirects failure to /dashboard?inviteError=<code>. 12 pipe tests + 5 gateway tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Server signup page resolves ?invite= via resolveInviteContext and passes invite context + token to a new SignUpClient (extracted from CloudSignUp). PasskeySignupButton replaces the hardcoded acceptedTos:true with a real checkbox above submit, accepts a lockedEmail prop (disabled+prefilled email), and forwards inviteToken in the POST body. signup-passkey/route.ts accepts an optional inviteToken in zod and runs acceptInviteForNewUser after session creation NON-FATALLY — signup still succeeds; the dashboard surfaces ?inviteError=<code>. A successful invite acceptance overrides the getting-started provisioning redirect to /dashboard/<driveId>?welcome=true. The duplicate 'By signing up...' footer paragraph is removed (the checkbox replaces it). 4 new tests for the inviteToken plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Delete the broad-sweep acceptUserPendingInvitations helper, its 9 auth-route call-sites (apple/native, apple/callback, magic-link/verify, passkey/authenticate, google/native, google/one-tap, google/callback, signup-passkey, mobile/oauth/google/exchange), the post-login-acceptance-coverage gate test, and the now-orphan findPendingMembersForUser/acceptPendingMember/bumpInvitedAt repo methods. Delete the resend route + its tests + handleResendInvitation/onResend plumbing in DriveMembers/MemberRow. Remove the orphaned INVITATION_LINK_EXPIRY_MINUTES constant from magic-link-service.ts (its body is otherwise untouched). The magic-link verify route's matchedInviteDriveId hint is also gone — drive-invite acceptance now lives entirely on the new pendingInvites flow. No no-op shims, no dead UI gated by always-false flags, no orphaned routes — per feedback_no_backwards_compat_for_unreleased.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
migrate-pending-invites: idempotent transactional script that wipes legacy drive_members rows where acceptedAt IS NULL plus the orphan email-only users they reference (provider='email' AND tosAcceptedAt IS NULL AND emailVerified IS NULL AND no passkeys AND no remaining drive_members). The original raw invite token was never persisted, so the legacy rows cannot be ported into pending_invites; the script emits the (driveId, email) pairs to stdout instead so admins can re-invite via the new flow. Run BEFORE deploying the new code. --dry-run flag prints the wipe set without mutating. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 of 13 tasks
Contributor
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR migrates from auto-accepted pending drive invitations to a token-based explicit consent model. It introduces a new ChangesInvite Token & Consent Flow Refactoring
Sequence DiagramsequenceDiagram
actor User as User (new)
participant Web as Web App
participant SharePage as Share/Invite Page
participant API as Auth API
participant DB as Database
participant Email as Email Service
User->>Web: Receives invite link<br/>/invite/[token]
Web->>API: GET /invite/[token]<br/>(resolveInviteContext)
API->>DB: SELECT pending_invites<br/>WHERE tokenHash = hash(token)
DB-->>API: pending_invite + drive + inviter
API-->>Web: { driveId, inviterName,<br/>driveName, email,<br/>isExistingUser }
Web->>SharePage: Render disclosure page<br/>+ CTA button
SharePage->>User: Display invite details<br/>& route choice
alt Existing User
User->>SharePage: Click accept
SharePage->>Web: Navigate to<br/>/invite/[token]/accept
Web->>API: GET /invite/[token]/accept<br/>(with session)
API->>DB: SELECT pending_invites<br/>WHERE tokenHash = hash(token)
API->>DB: UPDATE pending_invites<br/>SET consumedAt = now<br/>WHERE id = ?
API->>DB: INSERT INTO drive_members
DB-->>API: membership created
API-->>Web: 303 redirect<br/>/dashboard/[driveId]
else New User
User->>SharePage: Click create account
SharePage->>Web: Navigate to<br/>/auth/signup?invite=[token]
Web->>API: POST /api/auth/signup-passkey<br/>{ inviteToken, ... }
API->>DB: SELECT pending_invites<br/>WHERE tokenHash = hash(token)
API->>DB: UPDATE pending_invites<br/>SET consumedAt = now
API->>DB: INSERT INTO drive_members
DB-->>API: membership created
API-->>Web: { sessionToken, driveId }
end
Web-->>User: Signed in & invited<br/>to drive
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2428e5a7e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/members/DriveMembers.tsx (1)
146-202:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftVerify and address pending-invite visibility gap in DriveMembers post-migration.
After the migration,
drive_membersrows withacceptedAt IS NULLare deleted. ThependingMemberssection currently filters fromdriveMembers(not from the newpending_invitestable), so it will always render empty. Since/api/drives/${driveId}/memberscallslistDriveMembers()which queries onlydriveMembers, admins lose UI visibility into sent-but-not-yet-accepted invitations.Two options:
- Update the members API to merge pending invites from
pending_invitestable into the response, then updateDriveMembertype to handle rows withoutuserId(pending rows only have email/role/invitedBy), or create a separatePendingDriveMembertype in the response union.- Leave the API unchanged and document that the pending-invites section intentionally renders empty post-cutover (as noted in the task as future work), or add a placeholder note directing admins to resend invites as needed.
Currently, option 2 is implicit. Confirm the intended approach and document it in a comment or task.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/members/DriveMembers.tsx` around lines 146 - 202, DriveMembers currently computes pendingMembers by filtering members (members.filter(m => m.acceptedAt === null)) but the backend listDriveMembers() no longer returns pending_invites rows after migration, so the pending-invites UI will always be empty; fix by either (A) updating the members API (listDriveMembers) to merge pending_invites into its response and extend the DriveMember type (or add a PendingDriveMember union) so DriveMembers can render invites, or (B) explicitly document/annotate in the DriveMembers component (and/or server handler) that pending invites are fetched from a separate table and the UI is intentionally empty post-cutover and add a TODO/task comment to implement option A later; choose one approach, update DriveMembers, DriveMember type and API usage accordingly (references: pendingMembers, members.filter, DriveMembers component, listDriveMembers, DriveMember type).apps/web/src/components/auth/PasskeySignupButton.tsx (1)
187-203:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winToS gating currently blocks opening the signup form (flow deadlock).
The collapsed button is disabled by
!acceptedTos, but the checkbox is only visible after expanding. Users cannot proceed at all.Proposed patch
- const isButtonDisabled = - disabled || isRegistering || isSupported === null || !acceptedTos; + const isExpandDisabled = + disabled || isRegistering || isSupported === null; + const isSubmitDisabled = + isExpandDisabled || !acceptedTos || !name.trim() || !email.trim(); ... - disabled={isButtonDisabled} + disabled={isExpandDisabled} ... - disabled={isButtonDisabled || !name.trim() || !email.trim()} + disabled={isSubmitDisabled}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/auth/PasskeySignupButton.tsx` around lines 187 - 203, The signup flow is deadlocked because the collapsed Button uses isButtonDisabled (which includes !acceptedTos) so users cannot expand the form to see the ToS checkbox; update the logic in PasskeySignupButton so the collapsed "open" Button uses a lighter guard (e.g., isCollapsedButtonDisabled = disabled || isRegistering || isSupported === null) and call setIsExpanded(true) even if acceptedTos is false, while keeping the existing isButtonDisabled (or the acceptedTos check) on the final submit/register Button inside the expanded form so users must still accept the ToS before completing registration.
🧹 Nitpick comments (4)
apps/web/src/app/invite/[token]/accept/__tests__/route.test.ts (1)
47-54: ⚡ Quick winMissing test coverage for suspended user path.
beforeEachalways setssuspendedAt: null. If the acceptance route blocks invite consumption for suspended accounts (a reasonable security check), that branch has no test. Given the verification status query explicitly fetchessuspendedAt, the route likely uses it.🧪 Suggested additional test case
+ it('given a suspended account, redirects to /dashboard?inviteError=ACCOUNT_SUSPENDED (or analogous)', async () => { + vi.mocked(isAuthError).mockReturnValue(false); + vi.mocked(authenticateRequestWithOptions).mockResolvedValue(session('user_suspended')); + vi.mocked(driveInviteRepository.findUserVerificationStatusById).mockResolvedValue({ + email: 'invitee@example.com', + emailVerified: new Date('2025-01-01'), + suspendedAt: new Date('2026-01-01'), + } as never); + + const response = await GET(buildGet('tok'), ctx('tok')); + + expect(acceptInviteForExistingUser).not.toHaveBeenCalled(); + expect(response.status).toBe(303); + // Adjust expected error code to match route implementation + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/invite/`[token]/accept/__tests__/route.test.ts around lines 47 - 54, The current beforeEach always mocks driveInviteRepository.findUserVerificationStatusById with suspendedAt: null which omits the suspended-user branch; add a new test case that mocks vi.mocked(driveInviteRepository.findUserVerificationStatusById) to return suspendedAt: new Date(...) (non-null) and then exercise the invite accept route (the same handler under test in route.test.ts) asserting the route rejects consumption (e.g., returns the expected error status/body) and that driveInviteRepository.consumeInvite (or whichever method consumes the invite) is not called; you can also isolate this by overriding the beforeEach mock inside that single test.packages/db/drizzle/0122_wonderful_the_hood.sql (1)
11-11: ⚡ Quick winRedundant explicit index on
token_hash— theUNIQUEconstraint already creates one.In PostgreSQL,
CONSTRAINT "pending_invites_token_hash_unique" UNIQUE("token_hash")implicitly creates a unique B-tree index ontoken_hash. The explicitpending_invites_token_hash_idxbtree index on line 26 is therefore redundant: it adds write overhead and extra storage without providing any read-path benefit that the unique index doesn't already cover.💡 Proposed fix
-CREATE INDEX IF NOT EXISTS "pending_invites_token_hash_idx" ON "pending_invites" USING btree ("token_hash");--> statement-breakpoint CREATE INDEX IF NOT EXISTS "pending_invites_drive_id_idx" ON "pending_invites" USING btree ("drive_id");--> statement-breakpointIf this migration was generated by
pnpm db:generate, the source-of-truth fix should be applied inpackages/db/src/schema/pending-invites.ts(remove the explicitindex()call ontokenHash) and a new migration regenerated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/drizzle/0122_wonderful_the_hood.sql` at line 11, The migration creates both a UNIQUE constraint pending_invites_token_hash_unique on token_hash and an explicit btree index pending_invites_token_hash_idx which is redundant; remove the explicit index creation from the migration (or remove the index() call on tokenHash in the pending-invites schema and regenerate the migration with pnpm db:generate) so only the UNIQUE constraint remains, or if editing the SQL directly, delete the CREATE INDEX for pending_invites_token_hash_idx and re-run/record the corrected migration.apps/web/src/app/api/auth/signup-passkey/route.ts (1)
298-300: ⚡ Quick winEncode
inviteErrorbefore appending it to the redirect query.
inviteAcceptErroris currently interpolated raw into the URL. Encoding it prevents malformed query strings if future error codes include non-URL-safe characters.Proposed patch
- } else if (inviteAcceptError) { - redirectUrl = `/dashboard?welcome=true&inviteError=${inviteAcceptError}`; + } else if (inviteAcceptError) { + redirectUrl = `/dashboard?welcome=true&inviteError=${encodeURIComponent(inviteAcceptError)}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/auth/signup-passkey/route.ts` around lines 298 - 300, The redirect builds a query with inviteAcceptError unescaped; update the branch that sets redirectUrl to encode the error value using encodeURIComponent before interpolation (i.e., replace the raw inviteAcceptError in the redirectUrl assignment with an encoded version) so redirectUrl and any downstream callers handle URL-safe error strings.apps/web/src/lib/repositories/drive-invite-repository.ts (1)
368-415: ⚡ Quick winThrowing string literal / Symbol violates
no-throw-literal.
throw 'TOKEN_CONSUMED'(line 385) andthrow ALREADY_MEMBER(line 401) throw non-Error values. This loses stack traces, breaks Sentry/observability symbolication, and is flagged by@typescript-eslint/no-throw-literal. The pattern works because===on string and symbol references is identity-stable, but it conflicts with idiomatic TS error handling.A cleaner alternative is to return a discriminated value from the transaction for the no-rollback-needed path and reserve
throw(with a real Error subclass) for the rollback-required ALREADY_MEMBER case.♻️ Proposed refactor
- const ALREADY_MEMBER = Symbol('ALREADY_MEMBER'); + class AlreadyMemberRollback extends Error { + constructor() { super('ALREADY_MEMBER'); } + } try { - const memberId = await db.transaction(async (tx) => { + const txResult = await db.transaction(async (tx): Promise< + { state: 'OK'; memberId: string } | { state: 'TOKEN_CONSUMED' } + > => { const consumed = await tx .update(pendingInvites) .set({ consumedAt: acceptedAt }) .where( and( eq(pendingInvites.id, inviteId), isNull(pendingInvites.consumedAt), ) ) .returning({ id: pendingInvites.id }); if (consumed.length === 0) { - // No rollback needed — nothing was written. Sentinel-throw so the - // caller's result type can carry the discriminated reason. - throw 'TOKEN_CONSUMED'; + // No rollback needed — nothing was written. Return so the tx + // commits a no-op rather than throwing a literal. + return { state: 'TOKEN_CONSUMED' }; } try { const [member] = await tx .insert(driveMembers) .values({ driveId, userId, role, invitedBy, acceptedAt }) .returning({ id: driveMembers.id }); - return member.id; + return { state: 'OK', memberId: member.id }; } catch (error) { // ... unique-violation detection ... if (isUniqueViolation) { - throw ALREADY_MEMBER; + // Throw to roll back the consume. + throw new AlreadyMemberRollback(); } throw error; } }); - return { ok: true, memberId }; + if (txResult.state === 'TOKEN_CONSUMED') { + return { ok: false, reason: 'TOKEN_CONSUMED' }; + } + return { ok: true, memberId: txResult.memberId }; } catch (error) { - if (error === 'TOKEN_CONSUMED') { - return { ok: false, reason: 'TOKEN_CONSUMED' }; - } - if (error === ALREADY_MEMBER) { + if (error instanceof AlreadyMemberRollback) { return { ok: false, reason: 'ALREADY_MEMBER' }; } throw error; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/repositories/drive-invite-repository.ts` around lines 368 - 415, The code currently throws a string ('TOKEN_CONSUMED') and a Symbol (ALREADY_MEMBER) inside db.transaction which violates no-throw-literal; instead, change the transaction to return a discriminated result for the "no-op" case (e.g., return { status: 'TOKEN_CONSUMED' } from the async tx block when consumed.length === 0) and only throw a real Error subclass for the unique-violation path (replace throw ALREADY_MEMBER with throw new AlreadyMemberError() or similar). Update the outer try/catch to check the transaction return shape (inspect the returned object for status === 'TOKEN_CONSUMED' or memberId) and to detect the AlreadyMemberError via instanceof to return { ok: false, reason: 'ALREADY_MEMBER' } while rethrowing other errors; keep references to db.transaction, pendingInvites.consume branch, driveMembers insert, ALREADY_MEMBER -> AlreadyMemberError, and the outer catch handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/auth/signup/SignUpClient.tsx`:
- Around line 30-41: The OAuth and magic-link flows are not propagating the
invite token so invited users aren't attached to drives; wire the invite through
by passing the current invite token into useOAuthSignIn (so
handleGoogleSignIn/handleAppleSignIn include it in the OAuth state) and into
OAuthButtons props, and append the invite token to the email-link launch URL
used by the "Or sign up with email link" path (the route that currently
hard-codes "/auth/magic-link") and include it in the payload to
/api/auth/magic-link/send; update useOAuthSignIn.ts to accept an
inviteToken/inviteContext parameter and ensure the handlers call
/api/auth/google/signin and /api/auth/apple/signin with the inviteToken encoded
in the signed state, and modify those API routes (/api/auth/google/signin,
/api/auth/apple/signin) to accept and forward the inviteToken so the OAuth
callback can consume the pending_invite just like PasskeySignupButton does.
In `@apps/web/src/lib/auth/invite-resolver.ts`:
- Around line 46-47: The current check sets isExistingUser using
tosStatus?.tosAcceptedAt != null which misclassifies users who exist but have
null tosAcceptedAt; update the logic in invite-resolver.ts (the call to
driveInviteRepository.findUserToSStatusByEmail and the variable isExistingUser)
to use tosStatus !== null as the proxy for “user exists” so existing accounts
route to the accept/ signin flow while ToS re-prompting remains handled
downstream.
---
Outside diff comments:
In `@apps/web/src/components/auth/PasskeySignupButton.tsx`:
- Around line 187-203: The signup flow is deadlocked because the collapsed
Button uses isButtonDisabled (which includes !acceptedTos) so users cannot
expand the form to see the ToS checkbox; update the logic in PasskeySignupButton
so the collapsed "open" Button uses a lighter guard (e.g.,
isCollapsedButtonDisabled = disabled || isRegistering || isSupported === null)
and call setIsExpanded(true) even if acceptedTos is false, while keeping the
existing isButtonDisabled (or the acceptedTos check) on the final
submit/register Button inside the expanded form so users must still accept the
ToS before completing registration.
In `@apps/web/src/components/members/DriveMembers.tsx`:
- Around line 146-202: DriveMembers currently computes pendingMembers by
filtering members (members.filter(m => m.acceptedAt === null)) but the backend
listDriveMembers() no longer returns pending_invites rows after migration, so
the pending-invites UI will always be empty; fix by either (A) updating the
members API (listDriveMembers) to merge pending_invites into its response and
extend the DriveMember type (or add a PendingDriveMember union) so DriveMembers
can render invites, or (B) explicitly document/annotate in the DriveMembers
component (and/or server handler) that pending invites are fetched from a
separate table and the UI is intentionally empty post-cutover and add a
TODO/task comment to implement option A later; choose one approach, update
DriveMembers, DriveMember type and API usage accordingly (references:
pendingMembers, members.filter, DriveMembers component, listDriveMembers,
DriveMember type).
---
Nitpick comments:
In `@apps/web/src/app/api/auth/signup-passkey/route.ts`:
- Around line 298-300: The redirect builds a query with inviteAcceptError
unescaped; update the branch that sets redirectUrl to encode the error value
using encodeURIComponent before interpolation (i.e., replace the raw
inviteAcceptError in the redirectUrl assignment with an encoded version) so
redirectUrl and any downstream callers handle URL-safe error strings.
In `@apps/web/src/app/invite/`[token]/accept/__tests__/route.test.ts:
- Around line 47-54: The current beforeEach always mocks
driveInviteRepository.findUserVerificationStatusById with suspendedAt: null
which omits the suspended-user branch; add a new test case that mocks
vi.mocked(driveInviteRepository.findUserVerificationStatusById) to return
suspendedAt: new Date(...) (non-null) and then exercise the invite accept route
(the same handler under test in route.test.ts) asserting the route rejects
consumption (e.g., returns the expected error status/body) and that
driveInviteRepository.consumeInvite (or whichever method consumes the invite) is
not called; you can also isolate this by overriding the beforeEach mock inside
that single test.
In `@apps/web/src/lib/repositories/drive-invite-repository.ts`:
- Around line 368-415: The code currently throws a string ('TOKEN_CONSUMED') and
a Symbol (ALREADY_MEMBER) inside db.transaction which violates no-throw-literal;
instead, change the transaction to return a discriminated result for the "no-op"
case (e.g., return { status: 'TOKEN_CONSUMED' } from the async tx block when
consumed.length === 0) and only throw a real Error subclass for the
unique-violation path (replace throw ALREADY_MEMBER with throw new
AlreadyMemberError() or similar). Update the outer try/catch to check the
transaction return shape (inspect the returned object for status ===
'TOKEN_CONSUMED' or memberId) and to detect the AlreadyMemberError via
instanceof to return { ok: false, reason: 'ALREADY_MEMBER' } while rethrowing
other errors; keep references to db.transaction, pendingInvites.consume branch,
driveMembers insert, ALREADY_MEMBER -> AlreadyMemberError, and the outer catch
handling.
In `@packages/db/drizzle/0122_wonderful_the_hood.sql`:
- Line 11: The migration creates both a UNIQUE constraint
pending_invites_token_hash_unique on token_hash and an explicit btree index
pending_invites_token_hash_idx which is redundant; remove the explicit index
creation from the migration (or remove the index() call on tokenHash in the
pending-invites schema and regenerate the migration with pnpm db:generate) so
only the UNIQUE constraint remains, or if editing the SQL directly, delete the
CREATE INDEX for pending_invites_token_hash_idx and re-run/record the corrected
migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b0e9c0d-9b41-4558-a772-c700f0210a88
📒 Files selected for processing (62)
apps/web/src/app/api/auth/__tests__/google-callback-redirect.test.tsapps/web/src/app/api/auth/__tests__/mobile-oauth-google-exchange.test.tsapps/web/src/app/api/auth/__tests__/post-login-acceptance-coverage.test.tsapps/web/src/app/api/auth/apple/callback/__tests__/route.test.tsapps/web/src/app/api/auth/apple/callback/route.tsapps/web/src/app/api/auth/apple/native/__tests__/route.test.tsapps/web/src/app/api/auth/apple/native/route.tsapps/web/src/app/api/auth/google/__tests__/google-callback-redirect.test.tsapps/web/src/app/api/auth/google/__tests__/one-tap.test.tsapps/web/src/app/api/auth/google/__tests__/open-redirect-protection.test.tsapps/web/src/app/api/auth/google/callback/__tests__/route.test.tsapps/web/src/app/api/auth/google/callback/route.tsapps/web/src/app/api/auth/google/native/__tests__/route.test.tsapps/web/src/app/api/auth/google/native/route.tsapps/web/src/app/api/auth/google/one-tap/__tests__/route.test.tsapps/web/src/app/api/auth/google/one-tap/route.tsapps/web/src/app/api/auth/magic-link/verify/__tests__/desktop-verify.test.tsapps/web/src/app/api/auth/magic-link/verify/__tests__/route.test.tsapps/web/src/app/api/auth/magic-link/verify/route.tsapps/web/src/app/api/auth/mobile/oauth/google/exchange/__tests__/route.test.tsapps/web/src/app/api/auth/mobile/oauth/google/exchange/route.tsapps/web/src/app/api/auth/passkey/authenticate/__tests__/route.test.tsapps/web/src/app/api/auth/passkey/authenticate/route.tsapps/web/src/app/api/auth/signup-passkey/__tests__/route.test.tsapps/web/src/app/api/auth/signup-passkey/route.tsapps/web/src/app/api/drives/[driveId]/members/[userId]/resend/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/members/[userId]/resend/route.tsapps/web/src/app/api/drives/[driveId]/members/invite/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/members/invite/route.tsapps/web/src/app/auth/signup/SignUpClient.tsxapps/web/src/app/auth/signup/page.tsxapps/web/src/app/invite/[token]/accept/__tests__/route.test.tsapps/web/src/app/invite/[token]/accept/route.tsapps/web/src/app/invite/[token]/page.tsxapps/web/src/components/auth/PasskeySignupButton.tsxapps/web/src/components/members/DriveMembers.tsxapps/web/src/components/members/MemberRow.tsxapps/web/src/components/members/__tests__/DriveMembers.test.tsxapps/web/src/components/members/__tests__/MemberRow.test.tsxapps/web/src/lib/auth/__tests__/invite-acceptance.test.tsapps/web/src/lib/auth/__tests__/invite-resolver.test.tsapps/web/src/lib/auth/__tests__/post-login-pending-acceptance.test.tsapps/web/src/lib/auth/invite-acceptance.tsapps/web/src/lib/auth/invite-resolver.tsapps/web/src/lib/auth/post-login-pending-acceptance.tsapps/web/src/lib/repositories/__tests__/drive-invite-repository.test.tsapps/web/src/lib/repositories/drive-invite-repository.tspackages/db/drizzle/0122_wonderful_the_hood.sqlpackages/db/drizzle/meta/0122_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/package.jsonpackages/db/src/migrate-pending-invites.tspackages/db/src/schema.tspackages/db/src/schema/pending-invites.tspackages/lib/package.jsonpackages/lib/src/auth/__tests__/invite-token.test.tspackages/lib/src/auth/invite-token.tspackages/lib/src/auth/magic-link-service.test.tspackages/lib/src/auth/magic-link-service.tspackages/lib/src/services/__tests__/invite-predicates.test.tspackages/lib/src/services/invite-predicates.tstasks/drive-invite-gdpr-zero-trust.md
💤 Files with no reviewable changes (28)
- apps/web/src/app/api/auth/magic-link/verify/tests/desktop-verify.test.ts
- apps/web/src/lib/auth/tests/post-login-pending-acceptance.test.ts
- apps/web/src/app/api/auth/google/tests/open-redirect-protection.test.ts
- apps/web/src/app/api/auth/google/callback/route.ts
- apps/web/src/app/api/auth/passkey/authenticate/route.ts
- apps/web/src/app/api/auth/google/native/route.ts
- apps/web/src/app/api/auth/tests/post-login-acceptance-coverage.test.ts
- apps/web/src/app/api/auth/apple/native/tests/route.test.ts
- apps/web/src/app/api/auth/google/tests/google-callback-redirect.test.ts
- apps/web/src/app/api/auth/tests/mobile-oauth-google-exchange.test.ts
- apps/web/src/app/api/auth/apple/callback/route.ts
- apps/web/src/lib/auth/post-login-pending-acceptance.ts
- apps/web/src/app/api/auth/google/callback/tests/route.test.ts
- apps/web/src/app/api/auth/google/tests/one-tap.test.ts
- apps/web/src/app/api/auth/apple/callback/tests/route.test.ts
- apps/web/src/app/api/auth/mobile/oauth/google/exchange/route.ts
- apps/web/src/app/api/auth/magic-link/verify/tests/route.test.ts
- apps/web/src/app/api/auth/mobile/oauth/google/exchange/tests/route.test.ts
- apps/web/src/app/api/auth/tests/google-callback-redirect.test.ts
- apps/web/src/app/api/auth/google/one-tap/tests/route.test.ts
- packages/lib/src/auth/magic-link-service.ts
- apps/web/src/app/api/auth/google/native/tests/route.test.ts
- apps/web/src/app/api/auth/apple/native/route.ts
- packages/lib/src/auth/magic-link-service.test.ts
- apps/web/src/app/api/auth/passkey/authenticate/tests/route.test.ts
- apps/web/src/app/api/drives/[driveId]/members/[userId]/resend/route.ts
- apps/web/src/app/api/auth/magic-link/verify/route.ts
- apps/web/src/app/api/drives/[driveId]/members/[userId]/resend/tests/route.test.ts
P0 (CodeRabbit): PasskeySignupButton's collapsed 'Create with Passkey' trigger was disabled by !acceptedTos, but the ToS checkbox only renders inside the expanded form — making signup unreachable for everyone. Split the gate: collapsed expand-trigger requires only the base disabled state, the in-form submit button additionally requires acceptedTos. P1 (CodeRabbit): isExistingUser was gated on tosAcceptedAt != null, which misroutes OAuth/magic-link users (and accounts predating the ToS column) to /auth/signup where signup-passkey returns EMAIL_EXISTS — invites become unclaimable. Gate on account presence (tosStatus !== null) instead; the accept gateway handles ToS re-prompting separately. Allow-list invite-acceptance.ts in the lib drive-member gate sweep — the lone driveMembers reference is documentation prose, the actual write goes through the already-allow-listed repository seam. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit nits + defense-in-depth: - Drop redundant explicit B-tree index on pending_invites.token_hash — the UNIQUE constraint already creates an implicit index. Migration regenerated as 0122_easy_carlie_cooper.sql; net change is 1 fewer CREATE INDEX statement and saved write overhead. - Document the intentionally-empty pendingMembers section in DriveMembers.tsx — pending state lives in pending_invites and surfacing it through the members API is explicit follow-up scope (epic 'Out of scope'). The legacy filter remains as a safety net for any straggler acceptedAt=null row that might survive cutover. - Defense-in-depth: explicitly reject suspended sessions in /invite/[token]/accept (suspended users are already rejected at the session layer, but reading suspendedAt here makes the gate survive any future refactor of the auth helpers). New test asserts ACCOUNT_SUSPENDED redirect + no consume. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
Addressed remaining CodeRabbit feedback in 58dae1d: DriveMembers post-migration visibility (outside-diff Heavy lift) — Added an explicit code comment in DriveMembers.tsx documenting that the Suspended-user nit on accept gateway tests — Added a defense-in-depth Redundant token_hash index nit — Removed the explicit CI on commit 58dae1d is running. |
5 of 6 tasks
2witstudios
added a commit
that referenced
this pull request
May 7, 2026
* docs(invites): add follow-ups epic spec Documents the 7-item follow-up scope deferred from PR #1267: magic-link auto-create kill, post-acceptance side effects on all entry points, pending-invites surfacing in members API + UI, OAuth (Google/Apple, web/native/one-tap) inviteToken plumbing, next= honoring on signin, revoke endpoint, naming polish. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): scaffold pure-core module Adds packages/lib/src/services/invites/{types,ports,index}.ts as the type-only foundation for the from-scratch invite logic rebuild. types.ts: Result<T,E>, Invite, AcceptedInviteData (carries inviteId, inviteEmail, memberId, drive/role/user ids — everything the four side-effect ports need), AcceptInviteResult, RevokePendingInviteResult, RequestMagicLinkResult (= Result<void, MagicLinkErrorCode>), PendingInviteSummary, UserAccount. ports.ts: AcceptancePorts (loadInvite, findExistingMembership, consumeInviteAndCreateMember, broadcastMemberAdded, notifyMemberAdded, trackInviteMember, auditPermissionGranted), MagicLinkPorts, RevokePorts. ConsumeMembershipReason narrows from InviteAcceptanceErrorCode rather than redeclaring sentinels. Pipes/adapters land in T2-T10. Codex pre-T1 + adversarial reviewers feedback incorporated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): relocate predicates + add suspended Move invite-domain predicates into the new services/invites/ module and rename isEmailMatchingInvite -> isEmailMatch (verb-style, scoped by argument names). Adds isAccountSuspended for the upcoming validateInviteForUser pipe (route-layer suspendedAt check moves into the validator boundary). Hard cutover: deletes packages/lib/src/services/invite-predicates.ts and updates the two consumers (invite-acceptance.ts, invite-resolver.ts) to import from @pagespace/lib/services/invites in the same commit. No compat shim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auth): add isSafeNextPath allowlist predicate Composes isSafeReturnUrl (existing protocol/backslash defenses) with a caller-supplied prefix allowlist and URL normalisation, so .. segments cannot escape the allowed surface (/dashboard/../etc resolves to /etc and is rejected). Lives in auth-helpers.ts beside the existing isSafeReturnUrl rather than duplicating its protocol checks elsewhere. Empty/undefined input returns false (unlike isSafeReturnUrl which returns true to default to /dashboard). T27 will use this for the signin next= flow; no other callers yet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add validators (compose predicates) validateInviteForUser composes the four invite predicates into a single discriminated-union check ordered ACCOUNT_SUSPENDED -> TOKEN_CONSUMED -> TOKEN_EXPIRED -> EMAIL_MISMATCH so a suspended user cannot probe invite state through error responses. validateMagicLinkRequest collapses 'unknown email' and 'suspended account' into NO_ACCOUNT_FOUND / ACCOUNT_SUSPENDED. The unknown-email branch is what kills the auto-create dance in T13. validateRevokeRequest enforces drive scope (mismatched driveId -> NOT_FOUND, never FORBIDDEN — never disclose invite existence to the wrong drive's admins) and gates strict on accepted-OWNER/ADMIN to stay aligned with drive-member-gate-coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add asyncPipe with short-circuit guard Variadic-typed asyncPipe (overloads up to 5 steps) that short-circuits the chain on { ok: false } at any position. The guard is load-bearing: without it, a validation failure flows into the consume step and writes bad data. Steps may be sync or async, return their data directly (or wrapped as { ok, data }). The first step receives the initial input as-is; subsequent steps receive the previous step's output. Exceptions propagate (we do not swallow them inside the pipe). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): acceptInviteForExistingUser pipe Curried factory shape: (ports) => async (input) => Result. Routes will call acceptInviteForExistingUser(buildAcceptancePorts(...))(input). Sequence: load -> validate -> existing-membership check (ALREADY_MEMBER short-circuits without burning the token) -> consume -> shared emitAcceptanceSideEffects -> shape. emitAcceptanceSideEffects fires broadcast + notify + track + audit in one place; T6's acceptForNew reuses it. No port omission possible across pipes by construction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): acceptInviteForNewUser pipe Same factory shape as existing-user variant. Skips findExistingMembership (a freshly minted user has none); consume's ALREADY_MEMBER reason still propagates if a concurrent create races. Reuses emitAcceptanceSideEffects so signup-passkey + OAuth-new-user callbacks fire identical broadcast/notify/track/audit ports as the existing-user path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): revokePendingInvite pipe Loads invite + actor membership in parallel, validates via validateRevokeRequest, deletes the row scoped to (inviteId, driveId), emits authz.permission.revoked audit with targetEmail. Mismatched driveId returns NOT_FOUND (never FORBIDDEN — never disclose invite existence to the wrong drive's admins). Strict OWNER/ADMIN-with-acceptedAt gate matches drive-member-gate-coverage requirements. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): requestMagicLink pipe (kills auto-create) validateAccountExists -> createTokenAndPersist -> sendMagicLinkEmail. The validator returns NO_ACCOUNT_FOUND for unknown emails; the pipe never reaches the token-persist port for those, so no users row is ever minted from the magic-link flow. T13 will gut the auto-create else-branch in magic-link-service.ts and route the send route through this pipe + the magic-link adapter built in T10. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): incorporate codex post-T8 feedback Defense-in-depth try/catch in emitAcceptanceSideEffects: a flaky websocket fan-out or audit-DB blip cannot reverse a successful membership write (consume already committed). Adapter contract documented on AcceptancePorts: ports MUST NOT throw; adapter owns logging at the IO boundary. AcceptInviteInput.suspendedAt is now Date | null (was Date | null | undefined) so callers must explicitly pass null — uniform with every other temporal field in the module. Drop unused now field from RevokePendingInviteInput. Audit timestamping is the audit infrastructure's concern, not the pipe's. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add invite-acceptance adapter (drizzle/ws/audit) buildAcceptancePorts(request) wires AcceptancePorts to drizzle (loadInvite + findExistingMember + consumeInviteAndCreateMember via driveInviteRepository), websocket (broadcastDriveMemberEvent + recipients fan-out), notifications (createDriveNotification), monitoring (trackDriveOperation + logMemberActivity, with the actor-info pre-fetch the legacy emitJoinSideEffects also did), and audit (auditRequest with the original Request closed in via the factory). Each side-effect adapter swallows + warns its own errors per the port contract: a flaky websocket fan-out cannot reverse a successful membership write. Make trackInviteMember port async to compose activity-tracker (sync) with activity-logger (needs await getActorInfo). Pipe already wraps the call in async swallow so no pipe-layer change needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add magic-link + revoke adapters buildMagicLinkPorts(): drizzle user lookup (id + suspendedAt only) + inline token persist (mirrors createMagicLinkToken's mint pattern but without the auto-create dance — pipe's validateMagicLinkRequest pre-validates account existence) + email send via sendEmail + MagicLinkEmail template. buildRevokePorts(request): drizzle scoped pending-invite load (filters consumedAt IS NULL), drive_members lookup for actor role + acceptedAt, scoped delete WHERE id AND drive_id (defense-in-depth even though validator already checked the scope), audit authz.permission.revoked with the originating Request closed in via the factory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(repo): rename + extend drive-invite repository surface Rename findUserToSStatusByEmail -> loadUserAccountByEmail. Drop the unused tosAcceptedAt field and replace it with suspendedAt — every consumer (invite-resolver, magic-link-adapter) only needs to know whether the account exists and whether it's suspended; ToS state is the consent screen's concern, not this lookup's. Add findActivePendingInvitesByDrive(driveId): joins pending_invites with users for inviter display name, filters consumedAt IS NULL, used by T23's members API extension. Add findActivePendingInviteForDrive({inviteId, driveId}) + deletePendingInviteForDrive: scoped lookup + scoped delete used by the revoke pipe's adapter (defense-in-depth — pipe validator already gates the scope, but the SQL also enforces it). Update revoke + magic-link adapters to delegate to the repo (no inline drizzle). Update invite-resolver + repo tests for the renamed surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(email): rename magicLinkUrl -> inviteUrl sendPendingDriveInvitationEmail's prop is renamed because the URL it carries points at /invite/[token] (the consent screen + accept gateway), not at /api/auth/magic-link/verify. The DriveInvitationEmail template body never used the phrase 'magic link'; only the helper's prop name was leaky. Updates the route caller, the helper's JSDoc, and both test suites that asserted the old prop name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auth): kill magic-link auto-create dance createMagicLinkToken's else-branch (db.insert(users) for unknown emails + the unique-constraint race-handler around it) is gone. Unknown emails now return { ok: false, error: { code: 'NO_ACCOUNT_FOUND' } }. The route's catch-all already maps unknown errors to a generic 'If an account exists' response, so user-facing behaviour is unchanged at this hop; T14 surfaces NO_ACCOUNT_FOUND specifically with a /auth/signup CTA. isNewUser is dropped from CreateMagicLinkResult.data — it was always true on the auto-create branch and false on the existing-user branch; with auto-create gone it's always false, so it's dead. send route stops logging it; verifyMagicLinkToken still computes its own isNewUser from emailVerified for new-account hint UX (separate concept). Tests: replace 'creates pending signup token for non-existent user' with NO_ACCOUNT_FOUND assertion that also verifies no users row was inserted; drop the auto-create unique-violation race test (no longer reachable); keep the multi-token preservation test but drop the auto-create framing — the no-blind-cleanup invariant still matters for sign-in vs invite token coexistence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auth): surface NO_ACCOUNT_FOUND with signup CTA Send route returns 404 + { code: 'no_account', email } for unknown emails. Trades enumeration-resistance for a clearer onboarding path (the prior generic 'If an account exists' response masked unknown emails as success and dead-ended the user). MagicLinkForm gains a 'no-account' state that renders 'No PageSpace account for that email' + a 'Sign up' button linking to /auth/signup?email=<encoded> with the email pre-filled, plus a 'Try a different email' fallback. Route audit fires 'auth.login.failure' with reason magic_link_no_account_found at riskScore 0.2. Update existing send-route tests for the dropped isNewUser field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): kill emitJoinSideEffects, route via adapter Delete the inline 67-line helper in members/invite/route.ts. The userId path's fresh-join branch now builds AcceptancePorts via buildAcceptancePorts(request) and calls broadcast/notify/track/audit ports directly with an AcceptedInviteData payload — the same adapter the new acceptance pipes will use in T16/T17. AcceptedInviteData.inviteId/inviteEmail are now optional: present on invite-driven acceptances (set by the pipes), absent on direct-grant flows. Audit + activity-log adapters spread the field conditionally so direct grants don't carry empty 'inviteId' strings. Audit field rename: details.sourceEmail -> details.targetEmail. Same field, more accurate name (it's the invitee's email, not a 'source'). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): wire accept gateway to new pipe + adapter /invite/[token]/accept/route.ts: replace the call into apps/web's local invite-acceptance.ts with acceptInviteForExistingUser(buildAcceptancePorts(request))(input). The route shrinks to thin HTTP wiring; the pipe owns load + validate + consume + side effects + shape. Suspended-account check moves into the pipe via the validator (suspendedAt is forwarded as part of the input). The route now lets the pipe surface ACCOUNT_SUSPENDED, removing the duplicate route-layer guard. Tests rewritten with vi.hoisted() to mock the curried factory shape: each scenario asserts on the pipe inner function being called with the right input or returning the relevant error code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): retarget signup-passkey + delete old invite-acceptance.ts /api/auth/signup-passkey: replace the call into apps/web's local invite-acceptance.ts with acceptInviteForNewUser(buildAcceptancePorts(req))(input). Side-effect ports (broadcast/notify/track/audit) now fire on the passkey-signup path identically to the email-acceptance path — no more silent gap on this entry point. Delete apps/web/src/lib/auth/invite-acceptance.ts and its test (logic relocated to packages/lib/src/services/invites/pipes.ts in T5+T6). Both former consumers (T16 accept gateway, T17 signup-passkey) now go through the new pipe + adapter. Hard cutover, no compat shim. Tests: vi.hoisted() + curried-factory mock pattern matches T16's setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(invites): swap gate-coverage allow-list entry Drop the stale auth/invite-acceptance.ts entry (file deleted in T17) and add auth/revoke-adapters.ts in its place. The revoke adapter's findActorMembership returns the raw row so validateRevokeRequest in the pure core can enforce the strict OWNER/ADMIN-with-acceptedAt gate at the discriminated-union boundary; the SQL is keyed by (driveId, userId) and does not need its own isNotNull filter. Also mark the epic file as PARTIAL — this PR covers items 1/2/7 + the architecture; items 3/4/5/6 ship in a follow-up branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auth): rewire magic-link send route through requestMagicLink pipe Eliminates the parallel-implementation H1 the reviewer flagged: createMagicLinkToken in magic-link-service.ts and the requestMagicLink pipe + buildMagicLinkPorts adapter were both shipped, but the send route only called the legacy primitive. Now the route calls the pipe; the legacy createMagicLinkToken is deleted. magic-link-service.ts shrinks to verify-side only: verifyMagicLinkToken, MAGIC_LINK_EXPIRY_MINUTES, the verify-scoped MagicLinkError union (drops NO_ACCOUNT_FOUND — that's pipe-side now). The integration test gets a local mintMagicLinkToken helper that mirrors the adapter's mint-and-persist pattern, so verify tests no longer depend on the deleted issuance function. Unit test drops the entire createMagicLinkToken describe block; verify cases stay. Send route: pipe-throw is caught at the route boundary and returns generic 200 to preserve enumeration resistance (a 5xx would leak existence-of-email by triggering only on accounts that hit the email-send adapter). NO_ACCOUNT_FOUND -> 404 + signup CTA payload (existing behaviour preserved). ACCOUNT_SUSPENDED -> generic 200 + audit (USER_SUSPENDED was the legacy code; pipe semantics are equivalent). Closes review item H1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(invites): delete unused asyncPipe utility Codex post-T8 affirmed imperative pipes were correct for these flows (branch conditions in the middle make context-threading worse than direct discriminated-union early returns). asyncPipe shipped + tested but no pipe ever called it. Subtractive cleanup of -106 LOC + the corresponding index.ts re-export. Closes review item H2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(invites): drop unused revoke adapter revoke-adapters.ts shipped without the DELETE route that consumes it (route is in PR 2 scope per the deferred items). The reviewer rightly flagged this as scaffolding that would rot before its consumer lands. Removed for this PR; PR 2 re-adds the adapter alongside the new route in one cohesive change. The pipe (revokePendingInvite at packages/lib/src/services/invites/pipes.ts), validator (validateRevokeRequest), and repo methods (findActivePendingInviteForDrive, deletePendingInviteForDrive) all stay — they're the architecture and are tested via stub ports. Only the concrete IO wiring goes. Drops the now-stale auth/revoke-adapters.ts entry from drive-member-gate-coverage's LIB_ACCEPTED_AT_GATE_EXEMPT — gate test stays green (8/8). Closes review items H3 + M4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): export emitAcceptanceSideEffects + reuse The members/invite userId-direct-add path was hand-fanning broadcast/notify/track/audit, duplicating the pipe-internal emitAcceptanceSideEffects helper. A future fifth side-effect port (analytics, etc.) added to the helper would silently drop on the direct-add path. Promote the helper to a public export of the invites module and have the route call it. permissionsGranted moves from a hardcoded 0 inside the helper to an optional parameter (defaults to 0 for invite-driven acceptances; the direct-add path forwards its real count). All four pipes + the route now share one fanout sequence. Closes review item M6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(invites): drop unused now port field + tighten contract L7: createTokenAndPersist accepted now but the adapter never read it (verificationTokens.createdAt defaults to now() in SQL). The pipe constructed expiresAt from input.now BEFORE calling the port, so the field on the port boundary was dead. Removed from the port input shape; pipe and tests updated. L8: Port-contract comment in ports.ts said 'side-effect ports MUST NOT throw' but conflated acceptance side-effect ports (which fire after the membership commit and rightly cannot fail visibly) with pre-commit ports like sendMagicLinkEmail (which DO throw — the route catches and returns enumeration-resistant generic success). Tightened: split the contract into acceptance-side-effect (no-throw) and pre-commit (may-throw) sections, with the full port roster of each enumerated. Closes review items L7 + L8. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): wrap revokePendingInvite audit in swallow Brings the revoke pipe in line with the acceptance pipes' defense-in-depth posture. After deletePendingInviteForDrive commits, an exception from a buggy auditPermissionRevoked adapter would have surfaced as a 500 even though the revoke itself succeeded. The acceptance pipes already wrap each post-write side effect in `swallow`; revoke now matches. Adds the symmetric "audit throws after commit, still returns ok" test that the acceptance pipe already had. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): address coderabbit review feedback Two real bugs + four quality nitpicks from the coderabbit review: Real bugs: - isSafeNextPath: a `/` element in allowedPrefixes normalised to `''`, so `startsWith(`${p}/`)` matched every same-origin path and silently degraded the allowlist to a no-op. Short-circuit when `p === ''` so the prefix only matches the root path. Adds three regression tests. - packages/lib/package.json typesVersions: the `services/invite-predicates` entry was a stale leftover from the rename, and `services/invites` was missing — TS consumers on `moduleResolution: node` (older toolchains) could not resolve the new subpath. Mirror the exports map. Quality: - magic-link-adapters: encodeURIComponent the magic-link token so it stays safe if generateToken's alphabet ever grows (matches the invite-token encoding posture in members/invite/route.ts). - types.ts: unify RequestMagicLinkResult with the generic Result<T, E> (Result<void, MagicLinkErrorCode>) so consumers can share helpers that narrow on .ok across all three pipe results. Pipe + test updated. - pipes.ts: drop the duplicate DEFAULT_MAGIC_LINK_EXPIRY_MINUTES = 5 in favour of the canonical MAGIC_LINK_EXPIRY_MINUTES re-exported from magic-link-service. Single source of truth. - magic-link/send/route.ts: pass an Error instance (not a metadata bag) to loggers.auth.error for the unexpected-pipe-error branch so the entry populates entry.error consistently with the other error log on this handler. - drive-invite-repository: rename findActivePendingInvitesByDrive + findActivePendingInviteForDrive to findUnconsumedInvitesByDrive + findUnconsumedInviteForDrive. The "Active" prefix implied `expiresAt > now` (matching findActivePendingInviteByDriveAndEmail) but these methods need to surface expired-unconsumed rows so the pending- invites UI (PR 2) can show stuck invites and the revoke route can clean them up. Comment documents the contract. No callers changed for the rename — both methods are scaffolding for PR 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auth): split magic-link constants into pure leaf pipes.ts was importing MAGIC_LINK_EXPIRY_MINUTES from magic-link-service.ts, which itself imports from @pagespace/db/db. That made every consumer of @pagespace/lib/services/invites transitively load drizzle config + the DB pool at module-load time, violating the pure-core architecture's 'zero IO imports' invariant for a single 1-character constant. Move the constant to packages/lib/src/auth/magic-link-constants.ts (zero-import leaf). magic-link-service.ts re-exports it for back-compat — verify-side tests and any future consumer of the verify primitive keep their existing import paths. pipes.ts imports from the leaf module so the pure core stays IO-free. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 of 9 tasks
2witstudios
added a commit
that referenced
this pull request
May 15, 2026
* docs(invites): GDPR zero-trust epic spec Restart of #1266. New epic file scoped to the approved plan: hard-cutover deletions of post-login broad-sweep + 9 auth callers + resend route, magic-link service body untouched, wipe-not-port migration, /invite/[token]/accept gateway as its own task. Supersedes #1266. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): scaffold pending_invites table Add pending_invites schema with token_hash, email, drive_id, role, invited_by, expires_at, consumed_at. Both FKs cascade on delete. Partial unique index on (drive_id, email) WHERE consumed_at IS NULL prevents duplicate active invites at the DB level. Wired into schema barrel + namespace + package exports. Migration 0122 generated via pnpm db:generate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add pure invite predicates isInviteExpired, isInviteConsumed, isEmailMatchingInvite — pure functions with destructured object args, now injected. Email match is case + whitespace insensitive (matches the trim+lowercase normalization the invite endpoint already applies). 15 colocated tests cover the boundary case (now equals expiresAt → expired) plus 1ms-before / 1ms-after. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add invite token primitive createInviteToken({ now, expiryMinutes? }) mints ps_invite_* tokens with default 48h expiry. verifyInviteToken({ token, tokenHash }) compares via secureCompare (timing-safe). Reuses generateToken + hashToken from token-utils — no new hashing primitive. 9 colocated tests cover expiry math, prefix shape, hash distinctness, mint-twice uniqueness, and rejection of empty/tampered tokens. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): pendingInvites repository CRUD Add createPendingInvite (sweeps expired-unconsumed for the (driveId, email) pair before insert so the partial unique index never blocks legit re-invites), findPendingInviteByTokenHash (joined drive name + inviter name), findActivePendingInviteByDriveAndEmail (filters consumedAt IS NULL AND expiresAt > now), markInviteConsumed (atomic conditional UPDATE), deletePendingInvite, findUserToSStatusByEmail, and consumeInviteAndCreateMembership — a single Drizzle transaction that conditionally consumes the token then inserts driveMembers; throws a sentinel on (driveId, userId) unique violation so the transaction rolls back and the caller receives ALREADY_MEMBER without burning the token. 15 new tests cover happy/race/duplicate/connection-error paths. Legacy methods (findPendingMembersForUser, acceptPendingMember, bumpInvitedAt) remain in place; T9 deletes them once their callers are removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): rewrite invite endpoint handleEmailPath swaps createMagicLinkToken + createDriveMember(acceptedAt:null) for createInviteToken + createPendingInvite. URL becomes /invite/<rawToken>. Active-pending pre-check uses findActivePendingInviteByDriveAndEmail. Concurrent re-invite race surfaces as 409 via partial unique index. Email-send failure rolls back via deletePendingInvite. logMemberActivity is intentionally not called on the pending path — there is no targetUserId, the audit event captures the email-keyed invite. handleUserIdPath is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): /invite/[token] consent page resolveInviteContext({ token, now }) hashes the token (SHA3, never plaintext) and returns a discriminated result: data with drive name, inviter name, role, invited email, and isExistingUser (tosAcceptedAt IS NOT NULL); or NOT_FOUND/EXPIRED/CONSUMED. The server-component page awaits Next.js 15 async params and renders the consent card with ToS/Privacy CTAs — or an opaque 'no longer valid' card on any failure (NEVER redirects, which would leak token existence). 7 colocated resolver tests cover the happy paths + each error variant + the SHA-not-plaintext lookup contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): /invite/[token]/accept gateway acceptInviteForExistingUser + acceptInviteForNewUser pipes share validateAndLoadInvite + consumeAndShape so the discriminated TOKEN_NOT_FOUND/EXPIRED/CONSUMED/EMAIL_MISMATCH/ALREADY_MEMBER ladder is identical across signup and existing-user paths. The existing-user path runs findExistingMember pre-check so already-accepted users surface ALREADY_MEMBER without burning the token. The GET handler authenticates via session, redirects unauth'd users to /auth/signin?invite=&next=, redirects success to /dashboard/<driveId>?invited=1, redirects failure to /dashboard?inviteError=<code>. 12 pipe tests + 5 gateway tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auth): wire ?invite= into signup flow + ToS checkbox Server signup page resolves ?invite= via resolveInviteContext and passes invite context + token to a new SignUpClient (extracted from CloudSignUp). PasskeySignupButton replaces the hardcoded acceptedTos:true with a real checkbox above submit, accepts a lockedEmail prop (disabled+prefilled email), and forwards inviteToken in the POST body. signup-passkey/route.ts accepts an optional inviteToken in zod and runs acceptInviteForNewUser after session creation NON-FATALLY — signup still succeeds; the dashboard surfaces ?inviteError=<code>. A successful invite acceptance overrides the getting-started provisioning redirect to /dashboard/<driveId>?welcome=true. The duplicate 'By signing up...' footer paragraph is removed (the checkbox replaces it). 4 new tests for the inviteToken plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(invites): hard-cutover deletions Delete the broad-sweep acceptUserPendingInvitations helper, its 9 auth-route call-sites (apple/native, apple/callback, magic-link/verify, passkey/authenticate, google/native, google/one-tap, google/callback, signup-passkey, mobile/oauth/google/exchange), the post-login-acceptance-coverage gate test, and the now-orphan findPendingMembersForUser/acceptPendingMember/bumpInvitedAt repo methods. Delete the resend route + its tests + handleResendInvitation/onResend plumbing in DriveMembers/MemberRow. Remove the orphaned INVITATION_LINK_EXPIRY_MINUTES constant from magic-link-service.ts (its body is otherwise untouched). The magic-link verify route's matchedInviteDriveId hint is also gone — drive-invite acceptance now lives entirely on the new pendingInvites flow. No no-op shims, no dead UI gated by always-false flags, no orphaned routes — per feedback_no_backwards_compat_for_unreleased.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(db): one-shot pending_invites data migration migrate-pending-invites: idempotent transactional script that wipes legacy drive_members rows where acceptedAt IS NULL plus the orphan email-only users they reference (provider='email' AND tosAcceptedAt IS NULL AND emailVerified IS NULL AND no passkeys AND no remaining drive_members). The original raw invite token was never persisted, so the legacy rows cannot be ported into pending_invites; the script emits the (driveId, email) pairs to stdout instead so admins can re-invite via the new flow. Run BEFORE deploying the new code. --dry-run flag prints the wipe set without mutating. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): unblock collapsed CTA + classify by account presence P0 (CodeRabbit): PasskeySignupButton's collapsed 'Create with Passkey' trigger was disabled by !acceptedTos, but the ToS checkbox only renders inside the expanded form — making signup unreachable for everyone. Split the gate: collapsed expand-trigger requires only the base disabled state, the in-form submit button additionally requires acceptedTos. P1 (CodeRabbit): isExistingUser was gated on tosAcceptedAt != null, which misroutes OAuth/magic-link users (and accounts predating the ToS column) to /auth/signup where signup-passkey returns EMAIL_EXISTS — invites become unclaimable. Gate on account presence (tosStatus !== null) instead; the accept gateway handles ToS re-prompting separately. Allow-list invite-acceptance.ts in the lib drive-member gate sweep — the lone driveMembers reference is documentation prose, the actual write goes through the already-allow-listed repository seam. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): drop redundant idx, document UI scope, gate suspended CodeRabbit nits + defense-in-depth: - Drop redundant explicit B-tree index on pending_invites.token_hash — the UNIQUE constraint already creates an implicit index. Migration regenerated as 0122_easy_carlie_cooper.sql; net change is 1 fewer CREATE INDEX statement and saved write overhead. - Document the intentionally-empty pendingMembers section in DriveMembers.tsx — pending state lives in pending_invites and surfacing it through the members API is explicit follow-up scope (epic 'Out of scope'). The legacy filter remains as a safety net for any straggler acceptedAt=null row that might survive cutover. - Defense-in-depth: explicitly reject suspended sessions in /invite/[token]/accept (suspended users are already rejected at the session layer, but reading suspendedAt here makes the gate survive any future refactor of the auth helpers). New test asserts ACCOUNT_SUSPENDED redirect + no consume. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2witstudios
added a commit
that referenced
this pull request
May 15, 2026
* docs(invites): add follow-ups epic spec Documents the 7-item follow-up scope deferred from PR #1267: magic-link auto-create kill, post-acceptance side effects on all entry points, pending-invites surfacing in members API + UI, OAuth (Google/Apple, web/native/one-tap) inviteToken plumbing, next= honoring on signin, revoke endpoint, naming polish. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): scaffold pure-core module Adds packages/lib/src/services/invites/{types,ports,index}.ts as the type-only foundation for the from-scratch invite logic rebuild. types.ts: Result<T,E>, Invite, AcceptedInviteData (carries inviteId, inviteEmail, memberId, drive/role/user ids — everything the four side-effect ports need), AcceptInviteResult, RevokePendingInviteResult, RequestMagicLinkResult (= Result<void, MagicLinkErrorCode>), PendingInviteSummary, UserAccount. ports.ts: AcceptancePorts (loadInvite, findExistingMembership, consumeInviteAndCreateMember, broadcastMemberAdded, notifyMemberAdded, trackInviteMember, auditPermissionGranted), MagicLinkPorts, RevokePorts. ConsumeMembershipReason narrows from InviteAcceptanceErrorCode rather than redeclaring sentinels. Pipes/adapters land in T2-T10. Codex pre-T1 + adversarial reviewers feedback incorporated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): relocate predicates + add suspended Move invite-domain predicates into the new services/invites/ module and rename isEmailMatchingInvite -> isEmailMatch (verb-style, scoped by argument names). Adds isAccountSuspended for the upcoming validateInviteForUser pipe (route-layer suspendedAt check moves into the validator boundary). Hard cutover: deletes packages/lib/src/services/invite-predicates.ts and updates the two consumers (invite-acceptance.ts, invite-resolver.ts) to import from @pagespace/lib/services/invites in the same commit. No compat shim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auth): add isSafeNextPath allowlist predicate Composes isSafeReturnUrl (existing protocol/backslash defenses) with a caller-supplied prefix allowlist and URL normalisation, so .. segments cannot escape the allowed surface (/dashboard/../etc resolves to /etc and is rejected). Lives in auth-helpers.ts beside the existing isSafeReturnUrl rather than duplicating its protocol checks elsewhere. Empty/undefined input returns false (unlike isSafeReturnUrl which returns true to default to /dashboard). T27 will use this for the signin next= flow; no other callers yet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add validators (compose predicates) validateInviteForUser composes the four invite predicates into a single discriminated-union check ordered ACCOUNT_SUSPENDED -> TOKEN_CONSUMED -> TOKEN_EXPIRED -> EMAIL_MISMATCH so a suspended user cannot probe invite state through error responses. validateMagicLinkRequest collapses 'unknown email' and 'suspended account' into NO_ACCOUNT_FOUND / ACCOUNT_SUSPENDED. The unknown-email branch is what kills the auto-create dance in T13. validateRevokeRequest enforces drive scope (mismatched driveId -> NOT_FOUND, never FORBIDDEN — never disclose invite existence to the wrong drive's admins) and gates strict on accepted-OWNER/ADMIN to stay aligned with drive-member-gate-coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add asyncPipe with short-circuit guard Variadic-typed asyncPipe (overloads up to 5 steps) that short-circuits the chain on { ok: false } at any position. The guard is load-bearing: without it, a validation failure flows into the consume step and writes bad data. Steps may be sync or async, return their data directly (or wrapped as { ok, data }). The first step receives the initial input as-is; subsequent steps receive the previous step's output. Exceptions propagate (we do not swallow them inside the pipe). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): acceptInviteForExistingUser pipe Curried factory shape: (ports) => async (input) => Result. Routes will call acceptInviteForExistingUser(buildAcceptancePorts(...))(input). Sequence: load -> validate -> existing-membership check (ALREADY_MEMBER short-circuits without burning the token) -> consume -> shared emitAcceptanceSideEffects -> shape. emitAcceptanceSideEffects fires broadcast + notify + track + audit in one place; T6's acceptForNew reuses it. No port omission possible across pipes by construction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): acceptInviteForNewUser pipe Same factory shape as existing-user variant. Skips findExistingMembership (a freshly minted user has none); consume's ALREADY_MEMBER reason still propagates if a concurrent create races. Reuses emitAcceptanceSideEffects so signup-passkey + OAuth-new-user callbacks fire identical broadcast/notify/track/audit ports as the existing-user path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): revokePendingInvite pipe Loads invite + actor membership in parallel, validates via validateRevokeRequest, deletes the row scoped to (inviteId, driveId), emits authz.permission.revoked audit with targetEmail. Mismatched driveId returns NOT_FOUND (never FORBIDDEN — never disclose invite existence to the wrong drive's admins). Strict OWNER/ADMIN-with-acceptedAt gate matches drive-member-gate-coverage requirements. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): requestMagicLink pipe (kills auto-create) validateAccountExists -> createTokenAndPersist -> sendMagicLinkEmail. The validator returns NO_ACCOUNT_FOUND for unknown emails; the pipe never reaches the token-persist port for those, so no users row is ever minted from the magic-link flow. T13 will gut the auto-create else-branch in magic-link-service.ts and route the send route through this pipe + the magic-link adapter built in T10. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): incorporate codex post-T8 feedback Defense-in-depth try/catch in emitAcceptanceSideEffects: a flaky websocket fan-out or audit-DB blip cannot reverse a successful membership write (consume already committed). Adapter contract documented on AcceptancePorts: ports MUST NOT throw; adapter owns logging at the IO boundary. AcceptInviteInput.suspendedAt is now Date | null (was Date | null | undefined) so callers must explicitly pass null — uniform with every other temporal field in the module. Drop unused now field from RevokePendingInviteInput. Audit timestamping is the audit infrastructure's concern, not the pipe's. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add invite-acceptance adapter (drizzle/ws/audit) buildAcceptancePorts(request) wires AcceptancePorts to drizzle (loadInvite + findExistingMember + consumeInviteAndCreateMember via driveInviteRepository), websocket (broadcastDriveMemberEvent + recipients fan-out), notifications (createDriveNotification), monitoring (trackDriveOperation + logMemberActivity, with the actor-info pre-fetch the legacy emitJoinSideEffects also did), and audit (auditRequest with the original Request closed in via the factory). Each side-effect adapter swallows + warns its own errors per the port contract: a flaky websocket fan-out cannot reverse a successful membership write. Make trackInviteMember port async to compose activity-tracker (sync) with activity-logger (needs await getActorInfo). Pipe already wraps the call in async swallow so no pipe-layer change needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): add magic-link + revoke adapters buildMagicLinkPorts(): drizzle user lookup (id + suspendedAt only) + inline token persist (mirrors createMagicLinkToken's mint pattern but without the auto-create dance — pipe's validateMagicLinkRequest pre-validates account existence) + email send via sendEmail + MagicLinkEmail template. buildRevokePorts(request): drizzle scoped pending-invite load (filters consumedAt IS NULL), drive_members lookup for actor role + acceptedAt, scoped delete WHERE id AND drive_id (defense-in-depth even though validator already checked the scope), audit authz.permission.revoked with the originating Request closed in via the factory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(repo): rename + extend drive-invite repository surface Rename findUserToSStatusByEmail -> loadUserAccountByEmail. Drop the unused tosAcceptedAt field and replace it with suspendedAt — every consumer (invite-resolver, magic-link-adapter) only needs to know whether the account exists and whether it's suspended; ToS state is the consent screen's concern, not this lookup's. Add findActivePendingInvitesByDrive(driveId): joins pending_invites with users for inviter display name, filters consumedAt IS NULL, used by T23's members API extension. Add findActivePendingInviteForDrive({inviteId, driveId}) + deletePendingInviteForDrive: scoped lookup + scoped delete used by the revoke pipe's adapter (defense-in-depth — pipe validator already gates the scope, but the SQL also enforces it). Update revoke + magic-link adapters to delegate to the repo (no inline drizzle). Update invite-resolver + repo tests for the renamed surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(email): rename magicLinkUrl -> inviteUrl sendPendingDriveInvitationEmail's prop is renamed because the URL it carries points at /invite/[token] (the consent screen + accept gateway), not at /api/auth/magic-link/verify. The DriveInvitationEmail template body never used the phrase 'magic link'; only the helper's prop name was leaky. Updates the route caller, the helper's JSDoc, and both test suites that asserted the old prop name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auth): kill magic-link auto-create dance createMagicLinkToken's else-branch (db.insert(users) for unknown emails + the unique-constraint race-handler around it) is gone. Unknown emails now return { ok: false, error: { code: 'NO_ACCOUNT_FOUND' } }. The route's catch-all already maps unknown errors to a generic 'If an account exists' response, so user-facing behaviour is unchanged at this hop; T14 surfaces NO_ACCOUNT_FOUND specifically with a /auth/signup CTA. isNewUser is dropped from CreateMagicLinkResult.data — it was always true on the auto-create branch and false on the existing-user branch; with auto-create gone it's always false, so it's dead. send route stops logging it; verifyMagicLinkToken still computes its own isNewUser from emailVerified for new-account hint UX (separate concept). Tests: replace 'creates pending signup token for non-existent user' with NO_ACCOUNT_FOUND assertion that also verifies no users row was inserted; drop the auto-create unique-violation race test (no longer reachable); keep the multi-token preservation test but drop the auto-create framing — the no-blind-cleanup invariant still matters for sign-in vs invite token coexistence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auth): surface NO_ACCOUNT_FOUND with signup CTA Send route returns 404 + { code: 'no_account', email } for unknown emails. Trades enumeration-resistance for a clearer onboarding path (the prior generic 'If an account exists' response masked unknown emails as success and dead-ended the user). MagicLinkForm gains a 'no-account' state that renders 'No PageSpace account for that email' + a 'Sign up' button linking to /auth/signup?email=<encoded> with the email pre-filled, plus a 'Try a different email' fallback. Route audit fires 'auth.login.failure' with reason magic_link_no_account_found at riskScore 0.2. Update existing send-route tests for the dropped isNewUser field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): kill emitJoinSideEffects, route via adapter Delete the inline 67-line helper in members/invite/route.ts. The userId path's fresh-join branch now builds AcceptancePorts via buildAcceptancePorts(request) and calls broadcast/notify/track/audit ports directly with an AcceptedInviteData payload — the same adapter the new acceptance pipes will use in T16/T17. AcceptedInviteData.inviteId/inviteEmail are now optional: present on invite-driven acceptances (set by the pipes), absent on direct-grant flows. Audit + activity-log adapters spread the field conditionally so direct grants don't carry empty 'inviteId' strings. Audit field rename: details.sourceEmail -> details.targetEmail. Same field, more accurate name (it's the invitee's email, not a 'source'). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): wire accept gateway to new pipe + adapter /invite/[token]/accept/route.ts: replace the call into apps/web's local invite-acceptance.ts with acceptInviteForExistingUser(buildAcceptancePorts(request))(input). The route shrinks to thin HTTP wiring; the pipe owns load + validate + consume + side effects + shape. Suspended-account check moves into the pipe via the validator (suspendedAt is forwarded as part of the input). The route now lets the pipe surface ACCOUNT_SUSPENDED, removing the duplicate route-layer guard. Tests rewritten with vi.hoisted() to mock the curried factory shape: each scenario asserts on the pipe inner function being called with the right input or returning the relevant error code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): retarget signup-passkey + delete old invite-acceptance.ts /api/auth/signup-passkey: replace the call into apps/web's local invite-acceptance.ts with acceptInviteForNewUser(buildAcceptancePorts(req))(input). Side-effect ports (broadcast/notify/track/audit) now fire on the passkey-signup path identically to the email-acceptance path — no more silent gap on this entry point. Delete apps/web/src/lib/auth/invite-acceptance.ts and its test (logic relocated to packages/lib/src/services/invites/pipes.ts in T5+T6). Both former consumers (T16 accept gateway, T17 signup-passkey) now go through the new pipe + adapter. Hard cutover, no compat shim. Tests: vi.hoisted() + curried-factory mock pattern matches T16's setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(invites): swap gate-coverage allow-list entry Drop the stale auth/invite-acceptance.ts entry (file deleted in T17) and add auth/revoke-adapters.ts in its place. The revoke adapter's findActorMembership returns the raw row so validateRevokeRequest in the pure core can enforce the strict OWNER/ADMIN-with-acceptedAt gate at the discriminated-union boundary; the SQL is keyed by (driveId, userId) and does not need its own isNotNull filter. Also mark the epic file as PARTIAL — this PR covers items 1/2/7 + the architecture; items 3/4/5/6 ship in a follow-up branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auth): rewire magic-link send route through requestMagicLink pipe Eliminates the parallel-implementation H1 the reviewer flagged: createMagicLinkToken in magic-link-service.ts and the requestMagicLink pipe + buildMagicLinkPorts adapter were both shipped, but the send route only called the legacy primitive. Now the route calls the pipe; the legacy createMagicLinkToken is deleted. magic-link-service.ts shrinks to verify-side only: verifyMagicLinkToken, MAGIC_LINK_EXPIRY_MINUTES, the verify-scoped MagicLinkError union (drops NO_ACCOUNT_FOUND — that's pipe-side now). The integration test gets a local mintMagicLinkToken helper that mirrors the adapter's mint-and-persist pattern, so verify tests no longer depend on the deleted issuance function. Unit test drops the entire createMagicLinkToken describe block; verify cases stay. Send route: pipe-throw is caught at the route boundary and returns generic 200 to preserve enumeration resistance (a 5xx would leak existence-of-email by triggering only on accounts that hit the email-send adapter). NO_ACCOUNT_FOUND -> 404 + signup CTA payload (existing behaviour preserved). ACCOUNT_SUSPENDED -> generic 200 + audit (USER_SUSPENDED was the legacy code; pipe semantics are equivalent). Closes review item H1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(invites): delete unused asyncPipe utility Codex post-T8 affirmed imperative pipes were correct for these flows (branch conditions in the middle make context-threading worse than direct discriminated-union early returns). asyncPipe shipped + tested but no pipe ever called it. Subtractive cleanup of -106 LOC + the corresponding index.ts re-export. Closes review item H2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(invites): drop unused revoke adapter revoke-adapters.ts shipped without the DELETE route that consumes it (route is in PR 2 scope per the deferred items). The reviewer rightly flagged this as scaffolding that would rot before its consumer lands. Removed for this PR; PR 2 re-adds the adapter alongside the new route in one cohesive change. The pipe (revokePendingInvite at packages/lib/src/services/invites/pipes.ts), validator (validateRevokeRequest), and repo methods (findActivePendingInviteForDrive, deletePendingInviteForDrive) all stay — they're the architecture and are tested via stub ports. Only the concrete IO wiring goes. Drops the now-stale auth/revoke-adapters.ts entry from drive-member-gate-coverage's LIB_ACCEPTED_AT_GATE_EXEMPT — gate test stays green (8/8). Closes review items H3 + M4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(invites): export emitAcceptanceSideEffects + reuse The members/invite userId-direct-add path was hand-fanning broadcast/notify/track/audit, duplicating the pipe-internal emitAcceptanceSideEffects helper. A future fifth side-effect port (analytics, etc.) added to the helper would silently drop on the direct-add path. Promote the helper to a public export of the invites module and have the route call it. permissionsGranted moves from a hardcoded 0 inside the helper to an optional parameter (defaults to 0 for invite-driven acceptances; the direct-add path forwards its real count). All four pipes + the route now share one fanout sequence. Closes review item M6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(invites): drop unused now port field + tighten contract L7: createTokenAndPersist accepted now but the adapter never read it (verificationTokens.createdAt defaults to now() in SQL). The pipe constructed expiresAt from input.now BEFORE calling the port, so the field on the port boundary was dead. Removed from the port input shape; pipe and tests updated. L8: Port-contract comment in ports.ts said 'side-effect ports MUST NOT throw' but conflated acceptance side-effect ports (which fire after the membership commit and rightly cannot fail visibly) with pre-commit ports like sendMagicLinkEmail (which DO throw — the route catches and returns enumeration-resistant generic success). Tightened: split the contract into acceptance-side-effect (no-throw) and pre-commit (may-throw) sections, with the full port roster of each enumerated. Closes review items L7 + L8. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): wrap revokePendingInvite audit in swallow Brings the revoke pipe in line with the acceptance pipes' defense-in-depth posture. After deletePendingInviteForDrive commits, an exception from a buggy auditPermissionRevoked adapter would have surfaced as a 500 even though the revoke itself succeeded. The acceptance pipes already wrap each post-write side effect in `swallow`; revoke now matches. Adds the symmetric "audit throws after commit, still returns ok" test that the acceptance pipe already had. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invites): address coderabbit review feedback Two real bugs + four quality nitpicks from the coderabbit review: Real bugs: - isSafeNextPath: a `/` element in allowedPrefixes normalised to `''`, so `startsWith(`${p}/`)` matched every same-origin path and silently degraded the allowlist to a no-op. Short-circuit when `p === ''` so the prefix only matches the root path. Adds three regression tests. - packages/lib/package.json typesVersions: the `services/invite-predicates` entry was a stale leftover from the rename, and `services/invites` was missing — TS consumers on `moduleResolution: node` (older toolchains) could not resolve the new subpath. Mirror the exports map. Quality: - magic-link-adapters: encodeURIComponent the magic-link token so it stays safe if generateToken's alphabet ever grows (matches the invite-token encoding posture in members/invite/route.ts). - types.ts: unify RequestMagicLinkResult with the generic Result<T, E> (Result<void, MagicLinkErrorCode>) so consumers can share helpers that narrow on .ok across all three pipe results. Pipe + test updated. - pipes.ts: drop the duplicate DEFAULT_MAGIC_LINK_EXPIRY_MINUTES = 5 in favour of the canonical MAGIC_LINK_EXPIRY_MINUTES re-exported from magic-link-service. Single source of truth. - magic-link/send/route.ts: pass an Error instance (not a metadata bag) to loggers.auth.error for the unexpected-pipe-error branch so the entry populates entry.error consistently with the other error log on this handler. - drive-invite-repository: rename findActivePendingInvitesByDrive + findActivePendingInviteForDrive to findUnconsumedInvitesByDrive + findUnconsumedInviteForDrive. The "Active" prefix implied `expiresAt > now` (matching findActivePendingInviteByDriveAndEmail) but these methods need to surface expired-unconsumed rows so the pending- invites UI (PR 2) can show stuck invites and the revoke route can clean them up. Comment documents the contract. No callers changed for the rename — both methods are scaffolding for PR 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auth): split magic-link constants into pure leaf pipes.ts was importing MAGIC_LINK_EXPIRY_MINUTES from magic-link-service.ts, which itself imports from @pagespace/db/db. That made every consumer of @pagespace/lib/services/invites transitively load drizzle config + the DB pool at module-load time, violating the pure-core architecture's 'zero IO imports' invariant for a single 1-character constant. Move the constant to packages/lib/src/auth/magic-link-constants.ts (zero-import leaf). magic-link-service.ts re-exports it for back-compat — verify-side tests and any future consumer of the verify primitive keep their existing import paths. pipes.ts imports from the leaf module so the pure core stays IO-free. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drive invites currently auto-create a
usersrow at invite-send time (before consent), and the 7-day magic-link is the sole credential to claim that pre-baked account — silently bypassing passkeys for users who already have one. That violates GDPR Art. 6 lawful basis + Art. 13 transparency, violates zero-trust, and breaks the UX expectation set by Slack/Notion/Linear/Google Drive.This PR moves pending state into a dedicated
pending_invitestable, makes the invite token a page-load credential with no auth power, sends invitees through/invite/[token](consent screen) →/auth/signup?invite=<token>(real ToS checkbox, locked email) for new users or/invite/[token]/accept(session check) for existing users, and consumes the invite viaasyncPipe(validate → consume → grant)only after a real session is minted.This PR supersedes #1266. Same architecture, clean scope and history.
magic-link-service.tsitself is untouched (only the orphanedINVITATION_LINK_EXPIRY_MINUTESconstant comes out).User stories covered
/auth/signup?invite=with required ToS checkbox → joined as drive member, lands on/dashboard/<driveId>?welcome=true./invite/<token>/accept→ if no session, two-click via/auth/signin?invite=&next=→ joined, lands on/dashboard/<driveId>?invited=1.handleUserIdPath) → unchanged. Nopending_invitesrow written; member added directly.pending_invitesinteraction (regression check).Hard-cutover deletions (per
feedback_no_backwards_compat_for_unreleased.md)acceptUserPendingInvitationshelper + colocated testacceptUserPendingInvitationsimport + try/catch in 9 auth routes (apple/native, apple/callback, magic-link/verify, passkey/authenticate, google/native, google/one-tap, google/callback, signup-passkey, mobile/oauth/google/exchange)post-login-acceptance-coverage.test.tsgate testhandleResendInvitation/onResendUI plumbingfindPendingMembersForUser,acceptPendingMember,bumpInvitedAtINVITATION_LINK_EXPIRY_MINUTESfrommagic-link-service.ts(now orphaned)matchedInviteDriveIdredirect-hint logic in the magic-link verify route (drive-invite acceptance no longer rides on magic-link verification)Pre-deploy migration
pnpm --filter @pagespace/db migrate-pending-inviteswipes legacydrive_membersrows whereacceptedAt IS NULLplus the orphan email-only users they reference. The original raw invite token was never persisted (legacy magic-link stored only the hash), so legacy rows can't be ported intopending_inviteswith a usable token. The script emits(driveId, email)pairs to stdout so admins can re-invite via the new flow. Idempotent. Run BEFORE deploying.Out of scope (follow-ups)
next=plumbing on/auth/signin?invite=— two-click fallback is acceptable for this PR.?invite=plumbing through Google/Apple flows).pendingMemberssection will render empty post-cutover; refactor later).Test plan
pnpm typecheckclean across the monorepopnpm lintclean (only pre-existing warning in QuickCreatePalette.tsx unrelated to this PR)@pagespace/lib4003/4003 unit tests passing (incl. 15 new invite-predicates, 9 new invite-token tests)Architecture frozen
The architecture matches the approved plan at
/Users/jono/.claude/plans/flickering-floating-crane.mdand the epic spec attasks/drive-invite-gdpr-zero-trust.md. Eric Elliott style: pure predicates with destructured object args,nowinjected, discriminated result objects,asyncPipe-shaped composition, atomic single-use UPDATE-WHERE-NULL, transactional consume + insert with sentinel-rollback forALREADY_MEMBER.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Changes