feat(workspace): multi-user workspaces with invites and RBAC#70
Conversation
Enable multiple users per workspace with email + link invites, domain-based auto-join suggestions, workspace switching, and role-based access control (owner/admin/member). - Drop UNIQUE(user_id) constraint on workspace_members - Add workspace_invites and workspace_domains tables with RLS - Refactor session layer for multi-workspace (active workspace cookie) - Add 13 new API routes for invites, members, domains, switching - Add workspace switcher in header and member management settings page - Add invite accept page and domain-based join page - Add RBAC permissions module applied to workspace and integration routes - Add seqd email client for transactional invite emails - Add 7 PostHog/GTM analytics events for workspace lifecycle Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
nadyyym
left a comment
There was a problem hiding this comment.
Code Review
Critical (3 runtime bugs)
C1. Column name mismatch: created_at vs joined_at
workspace_members has joined_at, not created_at. Three routes will crash at runtime with PostgREST 400:
src/app/api/workspace/members/route.ts--.select('user_id, role, created_at'), returnsm.created_atasjoinedAtsrc/app/api/workspace/members/[userId]/route.ts--.select('user_id, role, created_at')in PATCH responsesrc/app/api/workspace/list/route.ts--.order('created_at', ...),.select('workspace_id, role, created_at, ...')
Fix: Replace created_at with joined_at in all three files.
C2. Missing added_by in domain claim INSERT
workspace_domains.added_by is UUID NOT NULL in migration, but the POST handler omits it:
// src/app/api/workspace/domains/route.ts
.insert({ workspace_id: workspaceId, domain: normalizedDomain } as never)Will crash with NOT NULL constraint violation. Fix: Add added_by: user.id.
C3. verified column not defined in migration
workspace_domains has no verified column in the migration, but domains/route.ts selects it and the UI renders it. Fix: Add verified BOOLEAN DEFAULT false to the CREATE TABLE, or remove from select/UI.
Important
I4. Invite token entropy -- crypto.randomUUID() gives 122 bits. Consider crypto.randomBytes(32).toString('hex') (256 bits) for URL-exposed security tokens.
I5. Race condition on use_count increment
use_count: (invite.use_count ?? 0) + 1 // read-then-write, not atomicTwo concurrent acceptances can both read use_count=0 and both pass the limit check. Fix: Use a Postgres expression for atomic increment, or a WHERE use_count < max_uses guard.
I6. Email invite accepted by wrong user -- No check that user.email === invite.email for email-type invites. Any user with the token can accept. If intentional (token-as-proof), document it. If not, add email match check.
I7. adminClient.auth.admin.listUsers() scalability -- Fetches ALL users in the Supabase project, filters client-side. Default pagination is 50 per page. With more than 50 total users, some workspace members will silently disappear from the list. Fix: Use getUserById() per member, or paginate with filtering.
I9. No DELETE endpoint for workspace domains -- RLS allows DELETE, but no API route or UI button exists. Once claimed, a domain cannot be unclaimed without direct DB access.
Minor
- 16x
as anycasts (expected until types regenerated, but track cleanup) - No email format validation on invite creation
InvitesSectionuseskey={inviteKey}for refresh instead of callback pattern
Positive
- Clean RBAC architecture -- static permission map,
requireRole()helper, owner-only for role changes and domain management - Idempotent invite acceptance -- composite PK prevents duplicate membership
- Well-designed domain auto-join -- redirects to choice page, public email domains blocked
- Secure workspace switching -- httpOnly cookie, verified on every request via
getUserWorkspace() - Backward compatibility preserved -- single-workspace users unaffected, auth callback still creates workspaces
Security Assessment
- RBAC prevents escalation (members cannot self-promote)
- Cookie is httpOnly with sameSite: lax
- No stale cache risk (role queries DB on every request)
- IDOR protection via
requireWorkspace()on all routes - Domain claims have no DNS/email verification (acceptable for MVP, document as limitation)
Verdict
Not ready to merge. The 3 critical bugs (C1-C3) are P0 -- they will crash the members list, domain claiming, and workspace list for all users. All are simple fixes (column rename, add field, add column). After fixing, this PR is solid for staging.
Review by Claude Code
Browser Testing — Vercel PreviewTested the preview deployment at Findings1. Workspace settings page: infinite loading spinner Navigating to 2. Invite accept page: stuck on "Loading invite..." Navigating to 3. ALL API routes hang indefinitely (not just workspace routes) Tested from the browser console using
The homepage HTML renders fine (200 in 0.7s from curl). Only API route serverless functions hang. 4. Cannot confirm if PR-specific or Vercel environment issue Other PR preview deployments (PR #66, PR #69) have expired (DEPLOYMENT_NOT_FOUND), so a direct comparison was not possible. The middleware code looks correct — it calls
5. UI observations (from what rendered)
RecommendationTo properly test the multi-user workspace features, either:
Browser test by Claude Code |
…handlers Rename invites/[token]/info to invites/[id]/info to match the sibling [id] dynamic segment. Next.js requires consistent param names at the same path level — the mismatch caused a runtime router crash that silently broke every route.ts handler on Vercel (pages unaffected). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
C1: workspace_members uses `joined_at` not `created_at` — fixes 500 on
/members, /members/[userId] PATCH, and /list endpoints
C2: add missing `added_by: user.id` to workspace_domains INSERT — fixes
NOT NULL constraint violation on domain claim
C3: remove references to non-existent `verified` column on
workspace_domains — fixes 500 on /domains endpoint
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@Sashmark97 TL;DR — 3 critical runtime bugs fixed, all verified:
Tested via direct Supabase queries (correct columns return rows, old columns return error 42703) and Vercel preview deploy (all routes return clean 401 instead of 500). Browser confirmed the old code shows "Failed to fetch members" / "Failed to fetch domains" on the settings page. Ready to merge to staging. |
Summary
UNIQUE(user_id)constraint onworkspace_members./api/workspace/switchendpoint.New files (17)
supabase/migrations/20260406112258_multi_user_workspaces.sql— schema migrationsrc/lib/auth/permissions.ts— RBAC permission map and route guardssrc/lib/email/client.ts— seqd transactional email HTTP clientsrc/lib/utils/email-domains.ts— sharedPUBLIC_EMAIL_DOMAINSset/api/workspace/(invites, members, domains, switch, list, join-by-domain, create-personal)/invite/[token](accept page),/join(domain suggestion page)Modified files (16)
constants.ts,session.ts) — multi-workspaceSessionUser, active workspace cookieserver.ts,helpers.ts) — removed.single(), active workspace resolutionx-workspace-idheader, exempts invite pagesgtm.ts) — 7 new workspace lifecycle eventsDatabase changes (staging applied)
workspace_members_user_id_uniqueconstraintworkspace_invitestable with RLSworkspace_domainstable with RLSTest plan
npm run build)🤖 Generated with Claude Code