Repository navigation
feat: implement magic link invitation acceptance with Better Auth - #844
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds an end-to-end organization invitation flow: DB schema and types, server endpoints and token workflow, Resend email integration, Upstash rate limiting, client/trpc changes, desktop UI for creating/managing invitations, and a web accept-invitation page with session creation and redirect. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Desktop User
participant UI as Invite Dialog
participant Client as Auth Client
participant RateLimit as Upstash Ratelimit
participant Server as Auth Server
participant DB as Database
participant Resend as Resend Email Service
User->>UI: submit email + role
UI->>Client: inviteMember({organizationId,email,role})
Client->>RateLimit: check limit(inviterId)
alt limit exceeded
RateLimit-->>Client: blocked
Client-->>UI: error
else allowed
RateLimit-->>Client: ok
Client->>Server: create invitation request
Server->>DB: insert invitation
Server->>Server: generate magic token & store verification
Server->>Resend: send OrganizationInvitationEmail(inviteLink,...)
Resend-->>Server: sent
Server-->>Client: success
Client-->>UI: show success
end
sequenceDiagram
participant Invitee as Invitee (Web)
participant Page as Accept Page
participant TRPC as TRPC Client
participant Server as Auth Server
participant DB as Database
participant Session as Session Manager
Invitee->>Page: GET /accept-invitation/[id]?token=xxx
Page->>TRPC: getInvitation(id)
TRPC->>Server: fetch invitation with org & inviter
Server->>DB: query & return invitation
alt valid (pending & not expired) and token present
Page->>Invitee: render details + Accept button
Invitee->>Page: click Accept
Page->>Server: POST /api/auth/accept-invitation {id,token}
Server->>DB: validate token matches email
Server->>DB: create/find user, create session, mark invitation accepted, create membership, delete token
Server-->>Page: success
Page->>Invitee: redirect to "/"
else invalid
Page-->>Invitee: show error + dashboard link
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~105 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/src/schema/auth.ts (1)
130-154: Schema and database migrations are out of sync — add migration fornamecolumnThe
namefield was added to the invitations table schema (line 138), but no database migration exists for this column. The latest migration snapshot (0010) does not include thenamecolumn, and no migration files (0007-0010) create it.Generate a new migration to add the nullable
namecolumn to theinvitationstable in the database before merging this change.
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 72-76: Add the missing Upstash/QStash env vars to .env.example:
declare KV_REST_API_URL and KV_REST_API_TOKEN (used by
packages/auth/src/lib/rate-limit.ts), QSTASH_TOKEN (used by the QStash client in
packages/trpc and apps/api), and QSTASH_CURRENT_SIGNING_KEY plus
QSTASH_NEXT_SIGNING_KEY (used for webhook signature verification); ensure each
variable is included with a short comment describing its purpose so developers
can set them according to the environment schemas.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/InviteMemberButton.tsx`:
- Around line 48-66: In handleInvite, normalize the email before validation and
sending: trim (and lowercase if desired) the email into a normalizedEmail
variable, check that normalizedEmail is non-empty and use it for permission
checks, the invite payload passed to authClient.organization.inviteMember, and
for the success toast; after sending, clear the original email state via
setEmail("") and reset role via setRole("member") as before. Ensure any
early-return checks (like canInvite) and error messages reference
normalizedEmail where appropriate so whitespace/case differences don't cause
duplicate or invalid invites.
- Around line 101-104: The DialogDescription in InviteMemberButton currently
hardcodes "Expires in 48 hours"; update InviteMemberButton (and specifically the
DialogDescription usage) to read the invitation expiry from a single source of
truth instead: either import a shared constant (e.g., INVITATION_EXPIRY_HOURS)
exposed to the frontend or compute the human-readable expiry from the backend
response (e.g., invitationExpiresIn seconds) when available, and render that
value instead of the hardcoded string; add the new constant to the shared config
module or adjust the API hook that fetches org/invite metadata so
InviteMemberButton uses that value for display.
In
`@apps/web/src/app/accept-invitation/`[invitationId]/AcceptInvitationButton.tsx:
- Around line 37-40: In AcceptInvitationButton (the code that handles the fetch
response), wrap the call to response.json() in a try/catch (or inspect
response.headers.get('content-type')) so non-JSON error bodies don’t throw and
hide the real HTTP error; attempt to parse JSON and use data.error if present,
but on parse failure fall back to await response.text() or response.statusText
to construct the thrown Error (e.g., "Failed to accept invitation: <status or
body>") so network/HTML/error pages are surfaced.
- Around line 24-25: The code uses process.env.NEXT_PUBLIC_API_URL (assigned to
apiUrl) directly when calling fetch inside the AcceptInvitationButton component,
which leads to requests like "undefined/..." if the env var is missing; add a
validation guard before calling fetch: check that apiUrl is truthy (e.g., if
(!apiUrl) ...) and handle the case by either throwing/logging an explicit error,
showing a user-facing error state, or returning early to avoid calling fetch
with an invalid URL; update the code around the apiUrl variable and the
fetch(`${apiUrl}/api/auth/accept-invitation`, ...) call to use this guard so the
request is only made when NEXT_PUBLIC_API_URL is defined.
In `@apps/web/src/app/accept-invitation/`[invitationId]/page.tsx:
- Around line 81-84: The displayed inviter name can be null/undefined; update
the JSX that uses invitation.inviter.name to use a safe fallback (e.g.,
invitation.inviter?.name ?? invitation.inviter?.email ?? 'Someone') so it
doesn't render "null" or "undefined" — change the string in the <p> that
currently reads "{invitation.inviter.name} invited you to join as a
{invitation.role}." to use the safe accessor and fallback while keeping
invitation.role as-is.
In `@packages/auth/src/lib/accept-invitation-endpoint.ts`:
- Around line 28-33: The log in accept-invitation-endpoint.ts exposes a token
fragment; remove any logging of token or its substrings (the console.log call
that prints invitationId and token.substring(0, 8)). Update the log to only
include non-sensitive identifiers (e.g., invitationId) and if a token presence
must be recorded use a constant placeholder like "[REDACTED]" or omit it
entirely; also search for other occurrences of token or token.substring in
functions in this file (e.g., the accept-invitation handler) and replace them
similarly to ensure no partial tokens are ever logged.
- Around line 45-71: The code currently skips checking invitation expiry after
loading the invitation in accept-invitation-endpoint (after the
invitations.findFirst call), so add an expiration check: after retrieving
invitation and before allowing acceptance (i.e., before checking
invitation.status), compare invitation.expiresAt (or the appropriate expiry
field on the invitation record) to the current time (Date.now()/new Date()) and
if expired log a clear error and throw (e.g., "Invitation expired"); ensure the
new check runs alongside the existing token and status validations so expired
invitations cannot be accepted even with a valid token.
- Around line 97-150: Wrap the invitation acceptance DB operations in a single
transaction (use db.transaction) that updates invitations.status, conditionally
creates the members record (avoid duplicates by checking within the same
transaction rather than using db.query.members.findFirst outside it), and
deletes the verifications token; only after the transaction successfully
commits, call ctx.context.internalAdapter.createSession / updateSession, then
set the cookie via ctx.setSignedCookie and call ctx.context.setNewSession so the
user is only logged in when DB changes are atomic and committed. Ensure you move
the existing createSession/updateSession/setSignedCookie/setNewSession sequence
(currently around createSession, updateSession, setSignedCookie, setNewSession)
to after the transactional block and perform the members existence check and
insert inside that transaction to prevent concurrent duplicate members.
In `@packages/auth/src/lib/generate-magic-token.ts`:
- Around line 5-16: generateMagicTokenForInvite currently sets token expiry to 7
days; change the expiresAt calculation in generateMagicTokenForInvite to 48
hours (e.g. Date.now() + 48 * 60 * 60 * 1000) and update the invitation TTL in
the auth plugin config (the invitationExpiresIn setting in
packages/auth/src/server.ts) to the same 48-hour value so the magic-link and
invitation lifetimes match the UI "Expires in 48 hours" message.
In `@packages/auth/src/lib/rate-limit.ts`:
- Around line 4-7: The code uses non-null assertions when constructing Redis
(const redis = new Redis({...})) which will give cryptic runtime errors if
KV_REST_API_URL or KV_REST_API_TOKEN are missing; add an explicit guard before
creating the Redis instance that checks process.env.KV_REST_API_URL and
process.env.KV_REST_API_TOKEN and throws a clear Error (e.g., "Missing Upstash
env var KV_REST_API_URL or KV_REST_API_TOKEN") so the failure is fast and
descriptive; update the Redis construction to use the validated values.
In `@packages/email/src/emails/organization-invitation.tsx`:
- Around line 37-39: Update the expiration copy in the organization-invitation
email component so it matches the actual 48-hour TTL: locate the Text element in
packages/email/src/emails/organization-invitation.tsx (the paragraph currently
saying "This invitation will expire in 1 week...") and change the copy to
reference "48 hours" (or "48 hours (2 days)") instead of "1 week";
alternatively, make the expiration phrase a prop (e.g., expirationText or
ttlHours) on the Organization Invitation component so the TTL can be configured
and rendered dynamically.
In `@packages/trpc/tsconfig.json`:
- Around line 3-5: Remove the unnecessary JSX compiler option from the package
tsconfig by deleting the "jsx": "react-jsx" entry in the "compilerOptions" block
of packages/trpc/tsconfig.json; since the package contains only .ts files and no
React/.tsx usage, simply remove that key so the "compilerOptions" no longer
includes "jsx".
In `@REBASE_PLAN.md`:
- Around line 76-86: The markdown fenced code blocks in REBASE_PLAN.md (the
blocks showing the directory tree for settings/team and the "New Structure
(main)" snippet and the block at line ~220) are missing language identifiers;
update each triple-backtick fence to include an appropriate language (e.g., use
"text" for plain directory trees, "bash" if showing shell commands, or
"typescript" for code snippets) so the blocks around the settings/team directory
(referencing page.tsx, InvitationActions/InvitationActions.tsx, index.ts) and
the block near line 220 all have explicit language tags.
🧹 Nitpick comments (8)
packages/auth/src/lib/rate-limit.ts (1)
9-13: Extract rate-limit settings into named constants.
This improves readability and makes tuning easier. As per coding guidelines, extract magic numbers to module-level constants.♻️ Suggested refactor
+const INVITE_RATE_LIMIT = 10; +const INVITE_RATE_WINDOW = "1 h"; +const INVITE_RATE_PREFIX = "ratelimit:invitation"; + // 10 invitations per hour per user export const invitationRateLimit = new Ratelimit({ redis, - limiter: Ratelimit.slidingWindow(10, "1 h"), - prefix: "ratelimit:invitation", + limiter: Ratelimit.slidingWindow(INVITE_RATE_LIMIT, INVITE_RATE_WINDOW), + prefix: INVITE_RATE_PREFIX, });packages/shared/src/auth/authorization/authorization.ts (1)
126-130: Prefer a params object forcanInviteto avoid positional args.
This aligns with the project’s function-parameter convention; update call sites accordingly. As per coding guidelines, use object params for functions with 2+ parameters.♻️ Suggested refactor
-export function canInvite( - actorRole: OrganizationRole, - inviteRole: OrganizationRole, -): boolean { - return getInvitableRoles(actorRole).includes(inviteRole); -} +export function canInvite({ + actorRole, + inviteRole, +}: { + actorRole: OrganizationRole; + inviteRole: OrganizationRole; +}): boolean { + return getInvitableRoles(actorRole).includes(inviteRole); +}apps/desktop/src/renderer/routes/_authenticated/settings/team/components/InvitationActions/InvitationActions.tsx (1)
35-38: Add prefixed error logging in the catch block.Right now the error is only surfaced via toast. Please log with a prefixed message for diagnostics. As per coding guidelines, errors shouldn’t be swallowed and logs should be prefixed.
🧭 Suggested logging addition
} catch (error) { + console.error( + "[invitations/cancel] Failed to cancel invitation", + error, + ); toast.error( error instanceof Error ? error.message : "Failed to cancel invitation", );As per coding guidelines, log errors with a prefixed context message.
apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/MembersSettings.tsx (1)
86-92: Consider extracting duplicatedformatDateutility.This
formatDatefunction is duplicated inPendingInvitations.tsx(with minor differences in format options). Consider extracting it to a shared utility.Example shared utility
// In a shared utils file, e.g., renderer/utils/date.ts export function formatDate(date: Date | string, options?: Intl.DateTimeFormatOptions) { const d = date instanceof Date ? date : new Date(date); return d.toLocaleDateString("en-US", options ?? { month: "short", day: "numeric" }); }apps/web/src/app/accept-invitation/[invitationId]/page.tsx (2)
2-2: Remove unused import.
ArrowLeftis imported but not used in this file.Suggested fix
-import { ArrowLeft, Users } from "lucide-react"; +import { Users } from "lucide-react";
23-27: Log the error before falling back.Per coding guidelines, errors should not be swallowed silently. While the fallback to
nullis appropriate UX, the error should be logged for debugging.Suggested fix
try { invitation = await trpc.organization.getInvitation.query(invitationId); } catch (error) { + console.error("[accept-invitation] Failed to fetch invitation:", error); invitation = null; }apps/desktop/src/renderer/routes/_authenticated/settings/members/components/PendingInvitations/components/InvitationActions/InvitationActions.tsx (1)
26-40: Consider adding confirmation for destructive action.Canceling an invitation is irreversible. Consider adding a confirmation dialog or using a confirmation pattern to prevent accidental cancellation.
This could be implemented using an AlertDialog or by changing the dropdown item to trigger a confirmation step first.
packages/auth/src/server.ts (1)
90-91: Extract invitation timing constants.
The 7‑day expiry and 10/hour limit are hardcoded; promote them to named constants to avoid drift and keep error text aligned. As per coding guidelines, avoid magic numbers.♻️ Suggested refactor
+const INVITATION_EXPIRES_IN_SECONDS = 60 * 60 * 24 * 7; +const INVITATION_RATE_LIMIT_PER_HOUR = 10; + organization({ creatorRole: "owner", - invitationExpiresIn: 60 * 60 * 24 * 7, // 1 week + invitationExpiresIn: INVITATION_EXPIRES_IN_SECONDS, sendInvitationEmail: async (data) => { @@ - if (!success) { - throw new Error("Rate limit exceeded. Max 10 invitations per hour."); - } + if (!success) { + throw new Error( + `Rate limit exceeded. Max ${INVITATION_RATE_LIMIT_PER_HOUR} invitations per hour.`, + ); + }Also applies to: 117-121
| const handleInvite = async () => { | ||
| if (!organizationId || !currentUserRole) return; | ||
|
|
||
| if (!canInvite(currentUserRole, role)) { | ||
| toast.error(`Cannot invite users as ${ORGANIZATION_ROLES[role].name}`); | ||
| return; | ||
| } | ||
|
|
||
| setIsInviting(true); | ||
| try { | ||
| await authClient.organization.inviteMember({ | ||
| organizationId, | ||
| email, | ||
| role, | ||
| }); | ||
|
|
||
| toast.success(`Invitation sent to ${email}`); | ||
| setEmail(""); | ||
| setRole("member"); |
There was a problem hiding this comment.
Normalize email before sending invites.
Trimming (and optionally lowercasing) prevents whitespace-only input and duplicate emails with case variance.
💡 Proposed fix
- setIsInviting(true);
- try {
- await authClient.organization.inviteMember({
- organizationId,
- email,
- role,
- });
-
- toast.success(`Invitation sent to ${email}`);
+ setIsInviting(true);
+ try {
+ const normalizedEmail = email.trim().toLowerCase();
+ if (!normalizedEmail) {
+ toast.error("Please enter a valid email address");
+ return;
+ }
+ await authClient.organization.inviteMember({
+ organizationId,
+ email: normalizedEmail,
+ role,
+ });
+
+ toast.success(`Invitation sent to ${normalizedEmail}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleInvite = async () => { | |
| if (!organizationId || !currentUserRole) return; | |
| if (!canInvite(currentUserRole, role)) { | |
| toast.error(`Cannot invite users as ${ORGANIZATION_ROLES[role].name}`); | |
| return; | |
| } | |
| setIsInviting(true); | |
| try { | |
| await authClient.organization.inviteMember({ | |
| organizationId, | |
| email, | |
| role, | |
| }); | |
| toast.success(`Invitation sent to ${email}`); | |
| setEmail(""); | |
| setRole("member"); | |
| const handleInvite = async () => { | |
| if (!organizationId || !currentUserRole) return; | |
| if (!canInvite(currentUserRole, role)) { | |
| toast.error(`Cannot invite users as ${ORGANIZATION_ROLES[role].name}`); | |
| return; | |
| } | |
| setIsInviting(true); | |
| try { | |
| const normalizedEmail = email.trim().toLowerCase(); | |
| if (!normalizedEmail) { | |
| toast.error("Please enter a valid email address"); | |
| return; | |
| } | |
| await authClient.organization.inviteMember({ | |
| organizationId, | |
| email: normalizedEmail, | |
| role, | |
| }); | |
| toast.success(`Invitation sent to ${normalizedEmail}`); | |
| setEmail(""); | |
| setRole("member"); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/InviteMemberButton.tsx`
around lines 48 - 66, In handleInvite, normalize the email before validation and
sending: trim (and lowercase if desired) the email into a normalizedEmail
variable, check that normalizedEmail is non-empty and use it for permission
checks, the invite payload passed to authClient.organization.inviteMember, and
for the success toast; after sending, clear the original email state via
setEmail("") and reset role via setRole("member") as before. Ensure any
early-return checks (like canInvite) and error messages reference
normalizedEmail where appropriate so whitespace/case differences don't cause
duplicate or invalid invites.
| const apiUrl = process.env.NEXT_PUBLIC_API_URL; | ||
| const response = await fetch(`${apiUrl}/api/auth/accept-invitation`, { |
There was a problem hiding this comment.
Validate NEXT_PUBLIC_API_URL before use.
If NEXT_PUBLIC_API_URL is undefined, the fetch URL becomes undefined/api/auth/accept-invitation, causing a confusing error. Consider adding validation.
Suggested fix
const handleContinue = async () => {
setIsProcessing(true);
+ const apiUrl = process.env.NEXT_PUBLIC_API_URL;
+ if (!apiUrl) {
+ setError("Application configuration error");
+ setIsProcessing(false);
+ return;
+ }
try {
- // Call the Better Auth endpoint that handles auth and cookies properly
- const apiUrl = process.env.NEXT_PUBLIC_API_URL;
const response = await fetch(`${apiUrl}/api/auth/accept-invitation`, {🤖 Prompt for AI Agents
In `@apps/web/src/app/accept-invitation/`[invitationId]/AcceptInvitationButton.tsx
around lines 24 - 25, The code uses process.env.NEXT_PUBLIC_API_URL (assigned to
apiUrl) directly when calling fetch inside the AcceptInvitationButton component,
which leads to requests like "undefined/..." if the env var is missing; add a
validation guard before calling fetch: check that apiUrl is truthy (e.g., if
(!apiUrl) ...) and handle the case by either throwing/logging an explicit error,
showing a user-facing error state, or returning early to avoid calling fetch
with an invalid URL; update the code around the apiUrl variable and the
fetch(`${apiUrl}/api/auth/accept-invitation`, ...) call to use this guard so the
request is only made when NEXT_PUBLIC_API_URL is defined.
| if (!response.ok) { | ||
| const data = await response.json(); | ||
| throw new Error(data.error || "Failed to accept invitation"); | ||
| } |
There was a problem hiding this comment.
Guard against non-JSON error responses.
If the server returns a non-JSON error (e.g., HTML error page, network timeout), response.json() will throw, and the actual HTTP error will be masked.
Suggested fix
if (!response.ok) {
- const data = await response.json();
- throw new Error(data.error || "Failed to accept invitation");
+ let errorMessage = "Failed to accept invitation";
+ try {
+ const data = await response.json();
+ errorMessage = data.error || errorMessage;
+ } catch {
+ // Response wasn't JSON, use default message
+ }
+ throw new Error(errorMessage);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!response.ok) { | |
| const data = await response.json(); | |
| throw new Error(data.error || "Failed to accept invitation"); | |
| } | |
| if (!response.ok) { | |
| let errorMessage = "Failed to accept invitation"; | |
| try { | |
| const data = await response.json(); | |
| errorMessage = data.error || errorMessage; | |
| } catch { | |
| // Response wasn't JSON, use default message | |
| } | |
| throw new Error(errorMessage); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/src/app/accept-invitation/`[invitationId]/AcceptInvitationButton.tsx
around lines 37 - 40, In AcceptInvitationButton (the code that handles the fetch
response), wrap the call to response.json() in a try/catch (or inspect
response.headers.get('content-type')) so non-JSON error bodies don’t throw and
hide the real HTTP error; attempt to parse JSON and use data.error if present,
but on parse failure fall back to await response.text() or response.statusText
to construct the thrown Error (e.g., "Failed to accept invitation: <status or
body>") so network/HTML/error pages are surfaced.
| export async function generateMagicTokenForInvite({ | ||
| email, | ||
| invitationId, | ||
| }: { | ||
| email: string; | ||
| invitationId: string; | ||
| }): Promise<string> { | ||
| // Generate cryptographically secure token (64 hex characters) | ||
| const token = crypto.randomBytes(32).toString("hex"); | ||
|
|
||
| // 1 week expiry (matches invitation expiry) | ||
| const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for invitation TTL, expiry, and related constants
rg -n "invitation|invite|expiresAt|expiry|ttl|TOKEN.*TTL|48.*hour|7.*day|7\s*\*\s*24" --type ts --type tsx --type js packages apps -A 2 -B 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Look for PR objectives, requirements, or related documentation
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "REQUIREMENTS*" -o -name "PR*" -o -name "OBJECTIVES*" \) | head -20 | xargs grep -l "48.*hour\|magic.*link\|invitation.*expir" 2>/dev/nullRepository: superset-sh/superset
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check the specific file for context around the token generation
head -50 packages/auth/src/lib/generate-magic-token.tsRepository: superset-sh/superset
Length of output: 761
🏁 Script executed:
#!/bin/bash
# Search for invitations schema or model definition
find packages apps -type f -name "*.ts" -o -name "*.js" | xargs grep -l "invitations\|invitation" | head -20Repository: superset-sh/superset
Length of output: 664
🏁 Script executed:
#!/bin/bash
# Search for 48-hour references in the codebase
rg "48\s*(hour|.*hour)" --type ts --type js packages apps 2>/dev/null || rg "48\s*(hour|.*hour)" packages appsRepository: superset-sh/superset
Length of output: 306
🏁 Script executed:
#!/bin/bash
# Search for test files related to magic token or invitations
find packages apps -type f \( -name "*test*" -o -name "*spec*" \) | xargs grep -l "generateMagicToken\|magic.*token" 2>/dev/nullRepository: superset-sh/superset
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Look for the schema definition of invitations/verifications
find packages apps -type f \( -name "*schema*" -o -name "*model*" \) | xargs grep -l "invitations\|verifications" 2>/dev/nullRepository: superset-sh/superset
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check the database schema for invitations/verifications
cat packages/db/src/schema/auth.tsRepository: superset-sh/superset
Length of output: 4904
🏁 Script executed:
#!/bin/bash
# Check invitation creation endpoint
cat packages/auth/src/lib/accept-invitation-endpoint.tsRepository: superset-sh/superset
Length of output: 4401
🏁 Script executed:
#!/bin/bash
# Search for where invitations are created (to see if it also uses 7-day TTL)
rg -n "insert.*invitations|db\.insert\(invitations" packages apps -A 5 -B 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for where invitations are created more broadly
rg -n "invitations\)" packages apps | grep -i "insert\|create\|new"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/an/bin/bash
# Look for files that might create invitations
rg -l "invitations" packages apps | xargs grep -l "insert\|create" | head -10Repository: superset-sh/superset
Length of output: 484
🏁 Script executed:
#!/bin/bash
# Check the organization router for invitation creation
cat packages/trpc/src/router/organization/organization.tsRepository: superset-sh/superset
Length of output: 12954
🏁 Script executed:
#!/bin/bash
# Check the auth server for invitation creation
cat packages/auth/src/server.tsRepository: superset-sh/superset
Length of output: 5326
Align magic-link and invitation TTL with the 48‑hour requirement.
The UI promises "Expires in 48 hours," but both the magic token and invitation are set to 7 days. Update both generateMagicTokenForInvite and the invitationExpiresIn plugin config to use 48 hours instead.
🐛 Proposed fix
+const INVITE_TOKEN_BYTES = 32;
+const INVITE_TOKEN_TTL_MS = 48 * 60 * 60 * 1000; // 48 hours
+
export async function generateMagicTokenForInvite({
email,
invitationId,
}: {
email: string;
invitationId: string;
}): Promise<string> {
// Generate cryptographically secure token (64 hex characters)
- const token = crypto.randomBytes(32).toString("hex");
+ const token = crypto.randomBytes(INVITE_TOKEN_BYTES).toString("hex");
- // 1 week expiry (matches invitation expiry)
- const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000);
+ // 48h expiry
+ const expiresAt = new Date(Date.now() + INVITE_TOKEN_TTL_MS);Also update packages/auth/src/server.ts:
- invitationExpiresIn: 60 * 60 * 24 * 7, // 1 week
+ invitationExpiresIn: 48 * 60 * 60, // 48 hours🤖 Prompt for AI Agents
In `@packages/auth/src/lib/generate-magic-token.ts` around lines 5 - 16,
generateMagicTokenForInvite currently sets token expiry to 7 days; change the
expiresAt calculation in generateMagicTokenForInvite to 48 hours (e.g.
Date.now() + 48 * 60 * 60 * 1000) and update the invitation TTL in the auth
plugin config (the invitationExpiresIn setting in packages/auth/src/server.ts)
to the same 48-hour value so the magic-link and invitation lifetimes match the
UI "Expires in 48 hours" message.
| "compilerOptions": { | ||
| "jsx": "react-jsx" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List any TSX files under the TRPC package
fd -a -e tsx packages/trpc/srcRepository: superset-sh/superset
Length of output: 437
🏁 Script executed:
# Also check the overall structure of the trpc package
find packages/trpc -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: superset-sh/superset
Length of output: 956
🏁 Script executed:
# Check the package.json to understand what this package does
cat packages/trpc/package.jsonRepository: superset-sh/superset
Length of output: 973
Remove the unnecessary jsx compiler option from this package's tsconfig.
The packages/trpc package contains only .ts files (no .tsx files) and is a pure server-side tRPC package with no React dependencies. The "jsx": "react-jsx" setting is unnecessary config churn.
🤖 Prompt for AI Agents
In `@packages/trpc/tsconfig.json` around lines 3 - 5, Remove the unnecessary JSX
compiler option from the package tsconfig by deleting the "jsx": "react-jsx"
entry in the "compilerOptions" block of packages/trpc/tsconfig.json; since the
package contains only .ts files and no React/.tsx usage, simply remove that key
so the "compilerOptions" no longer includes "jsx".
| ``` | ||
| settings/team/ | ||
| ├── page.tsx # Shows members + invitations mixed | ||
| ├── components/ | ||
| └── InvitationActions/ # Actions for invitations | ||
| ├── InvitationActions.tsx | ||
| └── index.ts | ||
| ``` | ||
|
|
||
| ### New Structure (main) | ||
| ``` |
There was a problem hiding this comment.
Add languages to fenced code blocks (MD040).
Markdownlint flags missing language identifiers at Line 76, 86, and 220. Add bash, text, or typescript as appropriate.
Also applies to: 220-220
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@REBASE_PLAN.md` around lines 76 - 86, The markdown fenced code blocks in
REBASE_PLAN.md (the blocks showing the directory tree for settings/team and the
"New Structure (main)" snippet and the block at line ~220) are missing language
identifiers; update each triple-backtick fence to include an appropriate
language (e.g., use "text" for plain directory trees, "bash" if showing shell
commands, or "typescript" for code snippets) so the blocks around the
settings/team directory (referencing page.tsx,
InvitationActions/InvitationActions.tsx, index.ts) and the block near line 220
all have explicit language tags.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/components/InviteMemberDialog/InviteMemberDialog.tsx`:
- Around line 81-83: The DialogDescription in InviteMemberDialog currently
hardcodes "Expires in 48 hours"; update it to reflect the backend config by
changing that copy to "Expires in 7 days" (or, preferably, read the actual
invitation expiry from the backend API/config and render that value) so the text
shown in InviteMemberDialog (DialogDescription) matches the server's
invitationExpiresIn setting used in packages/auth (server.ts).
🧹 Nitpick comments (5)
apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/components/InviteMemberDialog/InviteMemberDialog.tsx (2)
49-63: Normalize email before sending invite.The email should be trimmed (and optionally lowercased) to prevent whitespace-only submissions and duplicate invites caused by case variance.
Proposed fix
const handleInvite = async () => { + const normalizedEmail = email.trim().toLowerCase(); + if (!normalizedEmail) { + toast.error("Please enter a valid email address"); + return; + } + - if (!canInvite(currentUserRole, role)) { + if (!canInvite(currentUserRole, role)) { toast.error(`Cannot invite users as ${ORGANIZATION_ROLES[role].name}`); return; } setIsInviting(true); try { await authClient.organization.inviteMember({ organizationId, - email, + email: normalizedEmail, role, }); - toast.success(`Invitation sent to ${email}`); + toast.success(`Invitation sent to ${normalizedEmail}`);
127-127: Consider validating email format before enabling send.The button is disabled when
Proposed fix
+const isValidEmail = (email: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email.trim()); + <Button onClick={handleInvite} disabled={isInviting || !email}> +<Button onClick={handleInvite} disabled={isInviting || !isValidEmail(email)}>apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/MembersSettings.tsx (1)
86-92: Consider extractingformatDateto a shared utility.This
formatDatefunction is duplicated inPendingInvitations.tsx(lines 70-77) with slightly different formatting options. Extracting to a shared utility would reduce duplication and ensure consistent date formatting across the members settings UI.apps/desktop/src/renderer/routes/_authenticated/settings/members/components/PendingInvitations/PendingInvitations.tsx (2)
70-77: DuplicateformatDateutility.This helper is duplicated in
MembersSettings.tsxwith slightly different options (this one includesyear, the other doesn't). Consider extracting to a shared date formatting utility for consistency.
142-162: Consider handling potential nullinvitation.role.According to the schema in
packages/db/src/schema/auth.ts, therolecolumn is defined astext("role")without.notNull(), meaning it could benull. Consider adding a fallback for display.Proposed fix
<Badge variant="outline" className="text-xs capitalize"> - {invitation.role} + {invitation.role ?? "member"} </Badge>
| <DialogDescription> | ||
| Send an invitation to join {organizationName}. Expires in 48 hours. | ||
| </DialogDescription> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "invitationExpiresIn|expiresIn.*invit" --type ts -A 2 -B 2Repository: superset-sh/superset
Length of output: 402
Update invitation expiry text to match backend configuration.
The frontend shows "Expires in 48 hours" but the backend is configured for 1 week (invitationExpiresIn: 60 * 60 * 24 * 7 in packages/auth/src/server.ts). Update the text to "Expires in 7 days" or extract the actual expiry duration from backend configuration to prevent misleading users.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/components/InviteMemberDialog/InviteMemberDialog.tsx`
around lines 81 - 83, The DialogDescription in InviteMemberDialog currently
hardcodes "Expires in 48 hours"; update it to reflect the backend config by
changing that copy to "Expires in 7 days" (or, preferably, read the actual
invitation expiry from the backend API/config and render that value) so the text
shown in InviteMemberDialog (DialogDescription) matches the server's
invitationExpiresIn setting used in packages/auth (server.ts).
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/InviteMemberButton.tsx`:
- Around line 30-46: Update the invite flow to normalize the email before
sending: inside the invite handler (handleInvite) compute a normalizedEmail =
email.trim().toLowerCase() and use that in the call to
authClient.organization.inviteMember(...) instead of the raw email; keep the
existing permission check (canInvite(currentUserRole, role)) and any validations
but reference normalizedEmail for validation and the API call so invited
addresses are consistently trimmed and lowercased.
In `@packages/db/src/schema/auth.ts`:
- Line 138: Add a new SQL migration that alters the auth.invitations table to
include the missing nullable name column so the Drizzle schema in
packages/db/src/schema/auth.ts (the invitations table with name: text("name"))
matches the DB; create a new migration file that runs: ALTER TABLE
"auth"."invitations" ADD COLUMN "name" text; and ensure the migration
filename/order follows the project's existing migration convention and is
committed alongside the schema change.
In `@packages/email/src/emails/organization-invitation.tsx`:
- Around line 26-29: The logic using differenceInDays(expiresAt, new Date())
(stored in daysUntilExpiration) can produce 0 or negative values leading to "0
days" or "-1 days"; update the logic around daysUntilExpiration and
expirationText to handle three cases: if daysUntilExpiration <= 0 set
expirationText to "today" (or "expired" if you prefer separate messaging for
past dates), if daysUntilExpiration === 1 set "1 day", otherwise set
`${daysUntilExpiration} days`; reference the variables daysUntilExpiration,
expirationText and the input expiresAt/differenceInDays when implementing this
conditional fix.
In `@packages/email/tsconfig.json`:
- Around line 7-9: The tsconfig's "include" globs currently only cover
"src/**/*.ts" and "src/**/*.tsx", which excludes
packages/email/tailwind.config.ts; update the tsconfig.json include array to
also include "tailwind.config.ts" (or a broader pattern like "*.ts" or
"tailwind.config.ts") so the TypeScript tailwind config file is typechecked, or
alternatively move tailwind.config.ts into the src/ directory; locate the
include array in tsconfig.json and add the tailwind config file pattern
accordingly.
♻️ Duplicate comments (5)
packages/auth/src/lib/generate-magic-token.ts (1)
13-14: Token TTL mismatch with UI and extract magic number to constant.The PR objectives state magic links expire in 48 hours, but this sets a 7-day expiry. Additionally, per coding guidelines, extract magic numbers to named constants.
🐛 Proposed fix
import crypto from "node:crypto"; import { db } from "@superset/db/client"; import { verifications } from "@superset/db/schema/auth"; +const TOKEN_BYTES = 32; +const TOKEN_TTL_MS = 48 * 60 * 60 * 1000; // 48 hours - matches UI display + export async function generateMagicTokenForInvite({ email, }: { email: string; }): Promise<string> { - // Generate cryptographically secure token (64 hex characters) - const token = crypto.randomBytes(32).toString("hex"); + const token = crypto.randomBytes(TOKEN_BYTES).toString("hex"); - // 1 week expiry (matches invitation expiry) - const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); + const expiresAt = new Date(Date.now() + TOKEN_TTL_MS);packages/auth/src/lib/accept-invitation-endpoint.ts (3)
48-66: Invitation expiry isn't enforced.Only the token expiry is checked (line 35), but an expired invitation could still be accepted if the token remains valid. This was flagged in a previous review but appears unaddressed.
✅ Proposed fix
if (!invitation) { console.log("[invitation/accept] ERROR - Invitation not found"); throw new Error("Invitation not found"); } + if (new Date() > new Date(invitation.expiresAt)) { + console.log("[invitation/accept] ERROR - Invitation expired"); + throw new Error("Invitation expired"); + } + if (invitation.email !== verification.identifier) {
30-38: Bind magic tokens to the invitation, not just the email.Currently any valid token for the same email can be used with a different
invitationId. The verification lookup should also match the invitation ID to prevent token reuse across different invitations. This was flagged in a previous review but appears unaddressed.🛠️ Directional fix
- const verification = await db.query.verifications.findFirst({ - where: eq(verifications.value, token), - }); + // Validate token against invitation-scoped identifier + const verification = await db.query.verifications.findFirst({ + where: and( + eq(verifications.value, token), + eq(verifications.identifier, invitationId), + ), + });Also update
generateMagicTokenForInviteto storeidentifier: invitationIdwhen creating the verification.
106-145: Wrap DB updates in a transaction and move session cookie after completion.The session cookie is set (lines 107–115) before the invitation update, member creation, and token deletion (lines 123–145). If any DB operation fails, the user remains logged in without membership and the verification token stays in the database. This was flagged in a previous review but appears unaddressed.
🛠️ Suggested fix
- // Set session cookie (follows Better Auth's setSessionCookie pattern) - await ctx.setSignedCookie( - ctx.context.authCookies.sessionToken.name, - session.token, - ctx.context.secret, - { - ...ctx.context.authCookies.sessionToken.attributes, - maxAge: ctx.context.sessionConfig.expiresIn, - }, - ); - - ctx.context.setNewSession({ - session: session, - user: user, - }); - - // 5. Accept invitation by updating status and creating member - await db - .update(invitations) - .set({ status: "accepted" }) - .where(eq(invitations.id, invitationId)); - - // Create member record (check if not already a member) - const existingMember = await db.query.members.findFirst({ - where: and( - eq(members.organizationId, invitation.organization.id), - eq(members.userId, user.id), - ), - }); - - if (!existingMember) { - await db.insert(members).values({ - organizationId: invitation.organization.id, - userId: user.id, - role: invitation.role ?? "member", - }); - } - - // 6. Delete verification token (one-time use) - await db.delete(verifications).where(eq(verifications.value, token)); + // 5. Accept invitation atomically + await db.transaction(async (tx) => { + await tx + .update(invitations) + .set({ status: "accepted" }) + .where(eq(invitations.id, invitationId)); + + const existingMember = await tx.query.members.findFirst({ + where: and( + eq(members.organizationId, invitation.organization.id), + eq(members.userId, user.id), + ), + }); + if (!existingMember) { + await tx.insert(members).values({ + organizationId: invitation.organization.id, + userId: user.id, + role: invitation.role ?? "member", + }); + } + + await tx.delete(verifications).where(eq(verifications.value, token)); + }); + + // Set session cookie only after DB changes succeed + await ctx.setSignedCookie( + ctx.context.authCookies.sessionToken.name, + session.token, + ctx.context.secret, + { + ...ctx.context.authCookies.sessionToken.attributes, + maxAge: ctx.context.sessionConfig.expiresIn, + }, + ); + ctx.context.setNewSession({ session, user });apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/components/InviteMemberDialog/InviteMemberDialog.tsx (1)
81-83: Update invitation expiry text to match backend configuration.The dialog shows "Expires in 48 hours" but the backend
invitationExpiresInis configured for 7 days (60 * 60 * 24 * 7inpackages/auth/src/server.ts). This will mislead users about when their invitation actually expires.✏️ Proposed fix
<DialogDescription> - Send an invitation to join {organizationName}. Expires in 48 hours. + Send an invitation to join {organizationName}. Expires in 7 days. </DialogDescription>Alternatively, consider fetching this value from a shared config to keep it in sync with the backend.
🧹 Nitpick comments (7)
.github/workflows/deploy-preview.yml (1)
303-325: Confirm non‑API deployments truly require Resend/KV secrets.
RESEND_API_KEYandKV_REST_API_*are now injected into Web/Marketing/Admin preview builds. If those targets don’t send emails or need KV at runtime, consider removing these to reduce secret blast radius and accidental client bundling risk.Also applies to: 386-405, 481-504
.github/workflows/deploy-production.yml (1)
179-201: Validate least‑privilege for production secrets.Web/Marketing/Admin production deploys now receive
RESEND_API_KEYandKV_REST_API_*. If these apps don’t need them server-side, drop the envs to reduce secret exposure risk.Also applies to: 246-265, 314-337
packages/email/src/lib/resend.ts (1)
1-3: Consider using validated environment variables.The Resend client is initialized with a potentially undefined
process.env.RESEND_API_KEY. If the key is missing, this will silently create an invalid client that fails later at runtime when attempting to send emails.Consider either:
- Importing from the validated environment (if this package has access to
@superset/auth/env)- Adding an early validation check
♻️ Option: Add initialization validation
import { Resend } from "resend"; +const RESEND_API_KEY = process.env.RESEND_API_KEY; + +if (!RESEND_API_KEY) { + console.warn("[email/resend] RESEND_API_KEY is not configured - email sending will fail"); +} + -export const resend = new Resend(process.env.RESEND_API_KEY); +export const resend = new Resend(RESEND_API_KEY);packages/shared/src/auth/authorization/authorization.test.ts (1)
61-90: Good test coverage for new authorization functions.The tests properly validate
getInvitableRolesandcanInvitebehavior for all three roles. Consider adding explicit tests for:
canInvite("admin", "member")— to verify admin can invite memberscanInvite("owner", "member")— to verify owner can invite membersThese are implied by
getInvitableRolestests but explicitcanInvitecoverage would strengthen the suite.apps/desktop/src/renderer/routes/_authenticated/settings/members/components/PendingInvitations/components/InvitationActions/InvitationActions.tsx (1)
21-35: Consider adding confirmation before canceling invitation.Canceling an invitation is a destructive action that cannot be undone. Users might accidentally click "Cancel" from the dropdown menu. Consider adding a confirmation dialog or using a two-step action pattern.
💡 Example approach
const handleCancel = async () => { const confirmed = window.confirm( `Cancel invitation to ${invitation.email}?` ); if (!confirmed) return; setIsCanceling(true); // ... rest of the logic };Or use a dedicated confirmation dialog component for a more polished UX.
apps/web/src/trpc/client.ts (1)
7-17: Clean tRPC client configuration.The setup correctly uses
httpBatchLinkwith SuperJSON transformer and includes credentials for cookie-based authentication.Consider adding an error handling link for better observability:
💡 Optional: Add error logging
+import { loggerLink } from "@trpc/client"; + export const trpcClient = createTRPCClient<AppRouter>({ links: [ + loggerLink({ + enabled: (opts) => + process.env.NODE_ENV === "development" || + (opts.direction === "down" && opts.result instanceof Error), + }), httpBatchLink({ url: `${env.NEXT_PUBLIC_API_URL}/api/trpc`,apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/components/InviteMemberDialog/InviteMemberDialog.tsx (1)
95-99: Consider basic email validation before submission.The Enter key handler only checks if
♻️ Optional improvement
+const isValidEmail = (email: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email); + onKeyDown={(e) => { - if (e.key === "Enter" && email && !isInviting) { + if (e.key === "Enter" && isValidEmail(email) && !isInviting) { handleInvite(); } }}Also apply to the submit button's
disabledprop for consistency.
| return ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button disabled className="gap-2"> | ||
| <HiOutlinePlus className="h-4 w-4" /> | ||
| Invite Member | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p>Coming soon - invitation system in development</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| <> | ||
| <Button onClick={() => setOpen(true)} className="gap-2"> | ||
| <HiOutlinePlus className="h-4 w-4" /> | ||
| Invite Member | ||
| </Button> | ||
|
|
||
| <InviteMemberDialog | ||
| open={open} | ||
| onOpenChange={setOpen} | ||
| organizationId={organizationId} | ||
| organizationName={organizationName} | ||
| invitableRoles={invitableRoles} | ||
| currentUserRole={currentUserRole} | ||
| /> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify email normalization is implemented in InviteMemberDialog
rg -n "trim|toLowerCase|normalizedEmail" apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/components/InviteMemberDialog/Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Find the InviteMemberDialog component
fd -type f "InviteMemberDialog" apps/desktop/src/renderer/routes/_authenticated/settings/members/Repository: superset-sh/superset
Length of output: 234
🏁 Script executed:
#!/bin/bash
# Find the InviteMemberDialog component file
find apps/desktop/src/renderer/routes/_authenticated/settings/members -name "*InviteMemberDialog*"Repository: superset-sh/superset
Length of output: 390
🏁 Script executed:
#!/bin/bash
# Examine InviteMemberDialog component
cat -n "apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/components/InviteMemberDialog/InviteMemberDialog.tsx"Repository: superset-sh/superset
Length of output: 4374
Email input normalization not implemented.
The email is passed to authClient.organization.inviteMember() without trim() or toLowerCase() normalization. Add email normalization before the API call:
const handleInvite = async () => {
const normalizedEmail = email.trim().toLowerCase();
if (!canInvite(currentUserRole, role)) {
Then update the invitation call to use normalizedEmail instead of email.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/InviteMemberButton.tsx`
around lines 30 - 46, Update the invite flow to normalize the email before
sending: inside the invite handler (handleInvite) compute a normalizedEmail =
email.trim().toLowerCase() and use that in the call to
authClient.organization.inviteMember(...) instead of the raw email; keep the
existing permission check (canInvite(currentUserRole, role)) and any validations
but reference normalizedEmail for validation and the API call so invited
addresses are consistently trimmed and lowercased.
| .notNull() | ||
| .references(() => organizations.id, { onDelete: "cascade" }), | ||
| email: text("email").notNull(), | ||
| name: text("name"), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, get an overview of the auth.ts file
wc -l packages/db/src/schema/auth.tsRepository: superset-sh/superset
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Check the specific lines mentioned in the review (around line 138)
sed -n '130,160p' packages/db/src/schema/auth.tsRepository: superset-sh/superset
Length of output: 913
🏁 Script executed:
#!/bin/bash
# Search for invitations table definition
rg -n "invitations\s*=" packages/db/src/schema/auth.ts -A 20Repository: superset-sh/superset
Length of output: 865
🏁 Script executed:
#!/bin/bash
# Search for migration files related to invitations
find . -type f -name "*migration*" -o -name "*migrate*" | head -20Repository: superset-sh/superset
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Search for SQL migration files
find . -type f \( -name "*.sql" -o -name "*migration*.ts" \) | grep -E "(migration|migrate)" | head -20Repository: superset-sh/superset
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Read the end of the auth.ts file to see where the exports actually are
tail -20 packages/db/src/schema/auth.tsRepository: superset-sh/superset
Length of output: 760
🏁 Script executed:
#!/bin/bash
# Check the migration file for invitations table and name column
cat packages/db/drizzle/0000_initial_migration.sqlRepository: superset-sh/superset
Length of output: 5073
🏁 Script executed:
#!/bin/bash
# Check if there are other migration files or if invitations is defined elsewhere
ls -la packages/db/drizzle/Repository: superset-sh/superset
Length of output: 1142
🏁 Script executed:
#!/bin/bash
# Search the entire codebase for invitations table definition in migrations
rg -i "invitations" packages/db/ --type sqlRepository: superset-sh/superset
Length of output: 1035
🏁 Script executed:
#!/bin/bash
# Check if this is a new table being added - search git for invitations
git log --oneline --all -- "**/*invitation*" | head -10Repository: superset-sh/superset
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Check the full invitations table definition in the migration
cat packages/db/drizzle/0006_add_better_auth_tables.sql | grep -A 30 "CREATE TABLE.*invitations"Repository: superset-sh/superset
Length of output: 1047
🏁 Script executed:
#!/bin/bash
# Check if there's a newer migration file that adds the name column
grep -r "name" packages/db/drizzle/*.sql | grep -i invitationsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# List all migration files in order to see if there's one after 0006
ls -1 packages/db/drizzle/*.sql | sortRepository: superset-sh/superset
Length of output: 657
🏁 Script executed:
#!/bin/bash
# Check if there are any recent migration generation files or meta files
ls -la packages/db/drizzle/meta/Repository: superset-sh/superset
Length of output: 1001
🏁 Script executed:
#!/bin/bash
# Check the meta files to see if they reference the name column
cat packages/db/drizzle/meta/_journal.json | head -50Repository: superset-sh/superset
Length of output: 1087
🏁 Script executed:
#!/bin/bash
# Check the latest snapshot to see if name column is in the schema
grep -A 50 '"invitations"' packages/db/drizzle/meta/0010_snapshot.json | head -60Repository: superset-sh/superset
Length of output: 1568
🏁 Script executed:
#!/bin/bash
# Check if name column exists in the latest migration snapshot - search more carefully
python3 << 'EOF'
import json
with open('packages/db/drizzle/meta/0010_snapshot.json', 'r') as f:
data = json.load(f)
# Find invitations table
for table in data.get('tables', []):
if table.get('name') == 'invitations':
print("Invitations table columns:")
for col_name in table.get('columns', {}).keys():
print(f" - {col_name}")
break
EOFRepository: superset-sh/superset
Length of output: 188
🏁 Script executed:
#!/bin/bash
# Check if there are any untracked or staged migration files
git status packages/db/drizzle/ 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Search the entire codebase to see if name was recently added to the schema
git diff --name-only HEAD~5..HEAD 2>/dev/null | grep -E "(auth\.ts|migration)" | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Let's check the schema file one more time to be absolutely sure about the name column
head -n 160 packages/db/src/schema/auth.ts | tail -n 40Repository: superset-sh/superset
Length of output: 1374
🏁 Script executed:
#!/bin/bash
# Check the 0006 migration file to see invitations table creation in full detail
cat packages/db/drizzle/0006_add_better_auth_tables.sql | grep -A 20 "CREATE TABLE.*invitations"Repository: superset-sh/superset
Length of output: 760
Create a migration to add the name column to the invitations table.
The Drizzle schema in packages/db/src/schema/auth.ts defines a nullable name column for invitations, but this column does not exist in any migration file. The database schema in 0006_add_better_auth_tables.sql lacks this column, creating a mismatch between your TypeScript schema and the actual database. This will cause runtime errors when code attempts to insert or select the name field.
Generate a new migration file to add the name column:
ALTER TABLE "auth"."invitations" ADD COLUMN "name" text;🤖 Prompt for AI Agents
In `@packages/db/src/schema/auth.ts` at line 138, Add a new SQL migration that
alters the auth.invitations table to include the missing nullable name column so
the Drizzle schema in packages/db/src/schema/auth.ts (the invitations table with
name: text("name")) matches the DB; create a new migration file that runs: ALTER
TABLE "auth"."invitations" ADD COLUMN "name" text; and ensure the migration
filename/order follows the project's existing migration convention and is
committed alongside the schema change.
| // Calculate days until expiration | ||
| const daysUntilExpiration = differenceInDays(expiresAt, new Date()); | ||
| const expirationText = | ||
| daysUntilExpiration === 1 ? "1 day" : `${daysUntilExpiration} days`; |
There was a problem hiding this comment.
Handle edge case when invitation expires today or is already expired.
If expiresAt is today or in the past, daysUntilExpiration will be 0 or negative, resulting in misleading text like "0 days" or "-1 days".
🐛 Proposed fix
// Calculate days until expiration
const daysUntilExpiration = differenceInDays(expiresAt, new Date());
- const expirationText =
- daysUntilExpiration === 1 ? "1 day" : `${daysUntilExpiration} days`;
+ const expirationText =
+ daysUntilExpiration <= 0
+ ? "soon"
+ : daysUntilExpiration === 1
+ ? "1 day"
+ : `${daysUntilExpiration} days`;🤖 Prompt for AI Agents
In `@packages/email/src/emails/organization-invitation.tsx` around lines 26 - 29,
The logic using differenceInDays(expiresAt, new Date()) (stored in
daysUntilExpiration) can produce 0 or negative values leading to "0 days" or "-1
days"; update the logic around daysUntilExpiration and expirationText to handle
three cases: if daysUntilExpiration <= 0 set expirationText to "today" (or
"expired" if you prefer separate messaging for past dates), if
daysUntilExpiration === 1 set "1 day", otherwise set `${daysUntilExpiration}
days`; reference the variables daysUntilExpiration, expirationText and the input
expiresAt/differenceInDays when implementing this conditional fix.
| "baseUrl": "." | ||
| }, | ||
| "include": [ | ||
| "src/**/*.ts", | ||
| "src/**/*.tsx", | ||
| "components/**/*.ts", | ||
| "components/**/*.tsx", | ||
| "emails/**/*.tsx" | ||
| ], | ||
| "include": ["src/**/*.ts", "src/**/*.tsx"], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f \( -e ts -e tsx \) packages/email | grep -v '^packages/email/src/'Repository: superset-sh/superset
Length of output: 368
🏁 Script executed:
fd -t f -e ts -e tsx packages/email | grep -v '/src/'Repository: superset-sh/superset
Length of output: 425
🏁 Script executed:
fd -t f -e ts -e tsx . packages/email | grep -v '/src/'Repository: superset-sh/superset
Length of output: 97
🏁 Script executed:
cat packages/email/tailwind.config.tsRepository: superset-sh/superset
Length of output: 959
Include tailwind.config.ts in tsconfig—it is a TypeScript file outside src/ and will be excluded from typechecking.
packages/email/tailwind.config.ts exists at the package root and uses TypeScript syntax (satisfies Config), but the narrowed include globs ["src/**/*.ts", "src/**/*.tsx"] exclude it. Either add the file to the include list or move it into src/.
🤖 Prompt for AI Agents
In `@packages/email/tsconfig.json` around lines 7 - 9, The tsconfig's "include"
globs currently only cover "src/**/*.ts" and "src/**/*.tsx", which excludes
packages/email/tailwind.config.ts; update the tsconfig.json include array to
also include "tailwind.config.ts" (or a broader pattern like "*.ts" or
"tailwind.config.ts") so the TypeScript tailwind config file is typechecked, or
alternatively move tailwind.config.ts into the src/ directory; locate the
include array in tsconfig.json and add the tailwind config file pattern
accordingly.
- Move resend instantiation from email package to auth package - Use validated env.RESEND_API_KEY from auth/env.ts instead of process.env - Remove resend dependency from email package (only contains React templates) - Delete packages/email/src/lib/resend.ts - Add resend dependency to auth package
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/auth/src/server.ts`:
- Around line 102-130: The sendInvitationEmail handler does not handle failures
from resend.emails.send; wrap the resend.emails.send(...) call inside a
try/catch in sendInvitationEmail (after generateMagicTokenForInvite and building
inviteLink) and on error log a prefixed message including context (e.g.,
"sendInvitationEmail: failed to send invitation", invitation id data.id, email
data.email) along with the actual error object; decide whether to rethrow or
return a failure result so the caller can react (do not silently swallow the
exception). Ensure you reference sendInvitationEmail,
generateMagicTokenForInvite, and resend.emails.send when locating the change.
♻️ Duplicate comments (1)
apps/web/src/app/accept-invitation/[invitationId]/AcceptInvitationButton.tsx (1)
40-43: Guard against non‑JSON error bodies.
Line 40-42 callsresponse.json()unguarded; a non‑JSON error response will throw and mask the real HTTP error.🛠️ Suggested fix
if (!response.ok) { - const data = await response.json(); - throw new Error(data.error || "Failed to accept invitation"); + let errorMessage = "Failed to accept invitation"; + const contentType = response.headers.get("content-type") ?? ""; + if (contentType.includes("application/json")) { + const data = await response.json(); + errorMessage = data.error || errorMessage; + } else { + const text = await response.text(); + if (text) errorMessage = text; + } + throw new Error(errorMessage); }
🧹 Nitpick comments (2)
packages/auth/src/server.ts (1)
101-101: Extract magic number to a named constant.Per coding guidelines, hardcoded values should be extracted to named constants at module top for clarity and maintainability.
♻️ Suggested refactor
+const INVITATION_EXPIRY_SECONDS = 60 * 60 * 24 * 7; // 1 week + export const auth = betterAuth({ // ... plugins: [ organization({ creatorRole: "owner", - invitationExpiresIn: 60 * 60 * 24 * 7, // 1 week + invitationExpiresIn: INVITATION_EXPIRY_SECONDS,apps/web/src/app/accept-invitation/[invitationId]/AcceptInvitationButton.tsx (1)
26-27: Extract hardcoded endpoint/redirect strings into constants.
Lines 26-27 and Line 47 use hardcoded values; please move them to module‑level constants for maintainability. As per coding guidelines, extract hardcoded values to named constants.♻️ Suggested refactor
import { env } from "@/env"; +const ACCEPT_INVITATION_PATH = "/api/auth/accept-invitation"; +const POST_ACCEPT_REDIRECT = "/"; + interface AcceptInvitationButtonProps { invitationId: string; token: string; email: string; } @@ const response = await fetch( - `${env.NEXT_PUBLIC_API_URL}/api/auth/accept-invitation`, + `${env.NEXT_PUBLIC_API_URL}${ACCEPT_INVITATION_PATH}`, { @@ - window.location.href = "/"; + window.location.href = POST_ACCEPT_REDIRECT;Also applies to: 47-47
| sendInvitationEmail: async (data) => { | ||
| // Generate magic token for this invitation | ||
| const token = await generateMagicTokenForInvite({ | ||
| email: data.email, | ||
| }); | ||
|
|
||
| // Construct invitation link with magic token | ||
| const inviteLink = `${env.NEXT_PUBLIC_WEB_URL}/accept-invitation/${data.id}?token=${token}`; | ||
|
|
||
| // Check if user already exists to personalize greeting | ||
| const existingUser = await db.query.users.findFirst({ | ||
| where: eq(authSchema.users.email, data.email), | ||
| }); | ||
|
|
||
| await resend.emails.send({ | ||
| from: "Superset <noreply@superset.sh>", | ||
| to: data.email, | ||
| subject: `${data.inviter.user.name} invited you to join ${data.organization.name}`, | ||
| react: OrganizationInvitationEmail({ | ||
| organizationName: data.organization.name, | ||
| inviterName: data.inviter.user.name, | ||
| inviteLink, | ||
| role: data.role, | ||
| inviteeName: existingUser?.name ?? null, | ||
| inviterEmail: data.inviter.user.email, | ||
| expiresAt: data.invitation.expiresAt, | ||
| }), | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Add error handling and logging for email sending.
The resend.emails.send() call can fail (network issues, invalid API key, rate limits, etc.), but errors are not caught or logged. Per coding guidelines: never swallow errors silently and use prefixed console logging with context.
If email sending fails, the invitation is still created in Better Auth's flow, but the user never receives it—a silent failure that's hard to diagnose.
🔧 Suggested fix with error handling
sendInvitationEmail: async (data) => {
+ console.log("[invitation/email] Sending invitation email to:", data.email);
+
// Generate magic token for this invitation
const token = await generateMagicTokenForInvite({
email: data.email,
});
// Construct invitation link with magic token
const inviteLink = `${env.NEXT_PUBLIC_WEB_URL}/accept-invitation/${data.id}?token=${token}`;
// Check if user already exists to personalize greeting
const existingUser = await db.query.users.findFirst({
where: eq(authSchema.users.email, data.email),
});
- await resend.emails.send({
- from: "Superset <noreply@superset.sh>",
- to: data.email,
- subject: `${data.inviter.user.name} invited you to join ${data.organization.name}`,
- react: OrganizationInvitationEmail({
- organizationName: data.organization.name,
- inviterName: data.inviter.user.name,
- inviteLink,
- role: data.role,
- inviteeName: existingUser?.name ?? null,
- inviterEmail: data.inviter.user.email,
- expiresAt: data.invitation.expiresAt,
- }),
- });
+ try {
+ await resend.emails.send({
+ from: "Superset <noreply@superset.sh>",
+ to: data.email,
+ subject: `${data.inviter.user.name} invited you to join ${data.organization.name}`,
+ react: OrganizationInvitationEmail({
+ organizationName: data.organization.name,
+ inviterName: data.inviter.user.name,
+ inviteLink,
+ role: data.role,
+ inviteeName: existingUser?.name ?? null,
+ inviterEmail: data.inviter.user.email,
+ expiresAt: data.invitation.expiresAt,
+ }),
+ });
+ console.log("[invitation/email] Successfully sent invitation to:", data.email);
+ } catch (error) {
+ console.error("[invitation/email] Failed to send invitation email:", {
+ email: data.email,
+ invitationId: data.id,
+ error,
+ });
+ throw error; // Re-throw to prevent silent failure
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sendInvitationEmail: async (data) => { | |
| // Generate magic token for this invitation | |
| const token = await generateMagicTokenForInvite({ | |
| email: data.email, | |
| }); | |
| // Construct invitation link with magic token | |
| const inviteLink = `${env.NEXT_PUBLIC_WEB_URL}/accept-invitation/${data.id}?token=${token}`; | |
| // Check if user already exists to personalize greeting | |
| const existingUser = await db.query.users.findFirst({ | |
| where: eq(authSchema.users.email, data.email), | |
| }); | |
| await resend.emails.send({ | |
| from: "Superset <noreply@superset.sh>", | |
| to: data.email, | |
| subject: `${data.inviter.user.name} invited you to join ${data.organization.name}`, | |
| react: OrganizationInvitationEmail({ | |
| organizationName: data.organization.name, | |
| inviterName: data.inviter.user.name, | |
| inviteLink, | |
| role: data.role, | |
| inviteeName: existingUser?.name ?? null, | |
| inviterEmail: data.inviter.user.email, | |
| expiresAt: data.invitation.expiresAt, | |
| }), | |
| }); | |
| }, | |
| sendInvitationEmail: async (data) => { | |
| console.log("[invitation/email] Sending invitation email to:", data.email); | |
| // Generate magic token for this invitation | |
| const token = await generateMagicTokenForInvite({ | |
| email: data.email, | |
| }); | |
| // Construct invitation link with magic token | |
| const inviteLink = `${env.NEXT_PUBLIC_WEB_URL}/accept-invitation/${data.id}?token=${token}`; | |
| // Check if user already exists to personalize greeting | |
| const existingUser = await db.query.users.findFirst({ | |
| where: eq(authSchema.users.email, data.email), | |
| }); | |
| try { | |
| await resend.emails.send({ | |
| from: "Superset <noreply@superset.sh>", | |
| to: data.email, | |
| subject: `${data.inviter.user.name} invited you to join ${data.organization.name}`, | |
| react: OrganizationInvitationEmail({ | |
| organizationName: data.organization.name, | |
| inviterName: data.inviter.user.name, | |
| inviteLink, | |
| role: data.role, | |
| inviteeName: existingUser?.name ?? null, | |
| inviterEmail: data.inviter.user.email, | |
| expiresAt: data.invitation.expiresAt, | |
| }), | |
| }); | |
| console.log("[invitation/email] Successfully sent invitation to:", data.email); | |
| } catch (error) { | |
| console.error("[invitation/email] Failed to send invitation email:", { | |
| email: data.email, | |
| invitationId: data.id, | |
| error, | |
| }); | |
| throw error; // Re-throw to prevent silent failure | |
| } | |
| }, |
🤖 Prompt for AI Agents
In `@packages/auth/src/server.ts` around lines 102 - 130, The sendInvitationEmail
handler does not handle failures from resend.emails.send; wrap the
resend.emails.send(...) call inside a try/catch in sendInvitationEmail (after
generateMagicTokenForInvite and building inviteLink) and on error log a prefixed
message including context (e.g., "sendInvitationEmail: failed to send
invitation", invitation id data.id, email data.email) along with the actual
error object; decide whether to rethrow or return a failure result so the caller
can react (do not silently swallow the exception). Ensure you reference
sendInvitationEmail, generateMagicTokenForInvite, and resend.emails.send when
locating the change.
- Add RESEND_API_KEY, KV_REST_API_URL, KV_REST_API_TOKEN to api, web, admin, and marketing env.ts - Required for build-time env validation with tree-shaking - Each app needs these registered even if not directly used
- Add KV_REST_API_URL and KV_REST_API_TOKEN for rate limiting - Add QSTASH_TOKEN for background job scheduling - Add QSTASH_CURRENT_SIGNING_KEY and QSTASH_NEXT_SIGNING_KEY for webhook verification - Include descriptive comments for each variable
Summary
Implements magic link-based invitation acceptance using Better Auth, replacing the old tRPC-based flow. Invitations can now be accepted by clicking a magic link in the email, which automatically creates/signs in the user and adds them to the organization.
What Changed
Authentication Flow
Better Auth Plugin (
packages/auth/src/lib/accept-invitation-endpoint.ts)acceptInvitationendpoint using Better Auth's plugin systemAccept Invitation Page (
apps/web/src/app/accept-invitation/[invitationId]/page.tsx)Public Route Configuration (
apps/web/src/proxy.ts)/accept-invitation/:path*to public routesDesktop UI
Pending Invitations Section (
apps/desktop/.../PendingInvitations/)Layout Updates (
apps/desktop/.../settings/members/)Code Quality
acceptInvitationendpoint (243 lines)Technical Details
Session Cookie Handling
Uses Better Auth's recommended pattern:
ctx.context.internalAdapter.createSession()ctx.setSignedCookie()ctx.context.setNewSession()This ensures the session cookie is properly set for cross-domain auth.
Magic Link Token Generation
Desktop Sync
Test Plan
Summary by CodeRabbit
New Features
Configuration
Email
✏️ Tip: You can customize this high-level summary in your review settings.