feat: add domain-based access control for API keys#224
Conversation
Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds a nullable ApiKey.domainId column and a Prisma relation to Domain (ON DELETE SET NULL, ON UPDATE CASCADE). Schema, migration, and service layers updated to accept, persist, and validate domainId when creating and resolving API keys. Auth now exposes team.apiKey.domainId. Public API endpoints for domains and emails conditionally restrict queries by the API key’s domainId when present. UI: “Domain access” selector in Create API key and a “Domain Access” column in the API keys list. Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
Deploying usesend with
|
| Latest commit: |
df48a7c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://32d920e3.usesend.pages.dev |
| Branch Preview URL: | https://api-key-with-domain-access-c.usesend.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/src/server/public-api/api/emails/list-emails.ts (1)
115-124: Date range filter bug: endDate overwrites startDate.Setting
createdAttwice drops the first constraint. Merge both bounds into a single filter.- if (startDate) { - whereClause.createdAt = { - gte: new Date(startDate), - }; - } - if (endDate) { - whereClause.createdAt = { - lte: new Date(endDate), - }; - } + const createdAt: Prisma.DateTimeFilter = {}; + if (startDate) createdAt.gte = new Date(startDate); + if (endDate) createdAt.lte = new Date(endDate); + if (createdAt.gte || createdAt.lte) { + whereClause.createdAt = createdAt; + }apps/web/src/server/service/email-service.ts (4)
10-34: Service-level scope: include teamId in validation to prevent cross-team updates
checkIfValidEmailqueries byidonly. Tighten to includeteamId, and threadteamIdthrough callers to enforce scoping at the service layer (not just the route).-async function checkIfValidEmail(emailId: string) { - const email = await db.email.findUnique({ - where: { id: emailId }, - }); +async function checkIfValidEmail(emailId: string, teamId: number) { + const email = await db.email.findFirst({ + where: { id: emailId, teamId }, + });Propagate
teamIdto callers:-export async function updateEmail( - emailId: string, - { - scheduledAt, - }: { - scheduledAt?: string; - } -) { - const { email, domain } = await checkIfValidEmail(emailId); +export async function updateEmail( + emailId: string, + teamId: number, + { + scheduledAt, + }: { + scheduledAt?: string; + } +) { + const { email, domain } = await checkIfValidEmail(emailId, teamId);-export async function cancelEmail(emailId: string) { - const { email, domain } = await checkIfValidEmail(emailId); +export async function cancelEmail(emailId: string, teamId: number) { + const { email, domain } = await checkIfValidEmail(emailId, teamId);
490-522: Bulk path bypass: enforce API-key domain restriction for suppressed-email recordsSuppressed-record creation validates domain without considering API-key restrictions. Enforce
validateApiKeyDomainAccesswhenapiKeyIdis provided.- // Validate domain for suppressed email too - const domain = await validateDomainFromEmail(from, teamId); + // Validate domain for suppressed email too (respect API key restriction) + let domain; + if (apiKeyId) { + const apiKey = await db.apiKey.findUnique({ + where: { id: apiKeyId }, + include: { domain: true }, + }); + if (!apiKey) { + throw new UnsendApiError({ + code: "BAD_REQUEST", + message: "Invalid API key", + }); + } + if (apiKey.teamId !== teamId) { + throw new UnsendApiError({ + code: "FORBIDDEN", + message: "API key does not belong to this team", + }); + } + domain = await validateApiKeyDomainAccess(from, teamId, apiKey); + } else { + domain = await validateDomainFromEmail(from, teamId); + }
593-601: Bulk path bypass: validate per-content API-key access before groupingFor bulk sends, domain grouping validates by
fromonly; requests with restricted API keys could slip through. Validate API-key access per content (cache API keys).- // First pass: validate domains and group emails - for (const content of filteredEmailContents) { - const { from } = content; - if (!emailsByDomain.has(from)) { - const domain = await validateDomainFromEmail(from, content.teamId); - emailsByDomain.set(from, { domain, emails: [] }); - } - emailsByDomain.get(from)?.emails.push(content); - } + // First pass: validate domains (respect API key restriction) and group emails + const apiKeyCache = new Map<number, NonNullable<Awaited<ReturnType<typeof db.apiKey.findUnique>>>>(); + for (const content of filteredEmailContents) { + const { from, teamId, apiKeyId } = content; + + // Ensure domain access for this content + let domainForContent; + if (apiKeyId) { + let apiKey = apiKeyCache.get(apiKeyId); + if (!apiKey) { + apiKey = await db.apiKey.findUnique({ + where: { id: apiKeyId }, + include: { domain: true }, + }) as NonNullable<Awaited<ReturnType<typeof db.apiKey.findUnique>>>; + if (!apiKey) { + throw new UnsendApiError({ code: "BAD_REQUEST", message: "Invalid API key" }); + } + if (apiKey.teamId !== teamId) { + throw new UnsendApiError({ code: "FORBIDDEN", message: "API key does not belong to this team" }); + } + apiKeyCache.set(apiKeyId, apiKey); + } + domainForContent = await validateApiKeyDomainAccess(from, teamId, apiKey); + } else { + domainForContent = await validateDomainFromEmail(from, teamId); + } + + // Group by from-domain (already validated) + if (!emailsByDomain.has(from)) { + emailsByDomain.set(from, { domain: domainForContent, emails: [] }); + } else { + // Sanity: if an API key restricts to a different domain than the group, block it + const group = emailsByDomain.get(from)!; + if (group.domain.id !== domainForContent.id) { + throw new UnsendApiError({ + code: "FORBIDDEN", + message: `Mixed domain access detected for from: ${from}`, + }); + } + } + emailsByDomain.get(from)!.emails.push(content); + }
297-304: Update cancelEmail/updateEmail call sites to pass team.id
After tightening the service signatures to require ateamId, update all call sites in:
apps/web/src/server/api/routers/email.ts(calls at lines ~92 and ~98)apps/web/src/server/public-api/api/emails/cancel-email.ts(around line 42)apps/web/src/server/public-api/api/emails/update-email.ts(around line 53)
🧹 Nitpick comments (18)
apps/web/src/server/public-api/auth.ts (1)
1-7: Use ~/ alias in apps/web and drop unused import.
- Switch local imports to the
~/alias per repo guidelines.isSelfHostedis unused here—remove it.Apply:
-import { db } from "../db"; -import { UnsendApiError } from "./api-error"; -import { getTeamAndApiKey } from "../service/api-service"; -import { isSelfHosted } from "~/utils/common"; -import { logger } from "../logger/log"; +import { db } from "~/server/db"; +import { UnsendApiError } from "./api-error"; +import { getTeamAndApiKey } from "~/server/service/api-service"; +import { logger } from "~/server/logger/log";apps/web/src/server/public-api/hono.ts (3)
10-10: Remove unused ApiKey import.Not referenced in this file.
-import { Team, ApiKey } from "@prisma/client"; +import { Team } from "@prisma/client";
11-11: Use ~/ alias for logger import in apps/web.-import { logger } from "../logger/log"; +import { logger } from "~/server/logger/log";
36-37: Avoidas anyin middleware.Consider typing
getTeamFromTokento acceptContext<AppEnv>(or a narrower interface) so this cast is unnecessary.apps/web/src/server/public-api/api/emails/get-email.ts (2)
1-7: Remove unused getTeamFromToken import.-import { getTeamFromToken } from "~/server/public-api/auth";
72-93: Optional: enforce event ordering.If consumers expect latest-first events, add
orderBy: { createdAt: "desc" }underemailEvents.- emailEvents: { - select: { + emailEvents: { + orderBy: { createdAt: "desc" }, + select: {apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsx (1)
1-1: Rename file to PascalCase.Per guidelines, React component files should be PascalCase:
ApiList.tsx.apps/web/src/server/public-api/api/emails/update-email.ts (1)
1-6: Remove unused import.
getTeamFromTokenisn't used in this handler.-import { getTeamFromToken } from "~/server/public-api/auth";apps/web/src/server/public-api/api-utils.ts (1)
29-35: Return the email for consistency and reuse; align with new helper
checkIsValidEmailIdcurrently throws or returns void. Returning the found email mirrors the new helper and avoids follow-up re-queries at call sites.-export const checkIsValidEmailId = async (emailId: string, teamId: number) => { - const email = await db.email.findUnique({ where: { id: emailId, teamId } }); +export const checkIsValidEmailId = async (emailId: string, teamId: number) => { + const email = await db.email.findFirst({ where: { id: emailId, teamId } }); if (!email) { throw new UnsendApiError({ code: "NOT_FOUND", message: "Email not found" }); } -}; + return email; +};apps/web/src/server/public-api/api/emails/cancel-email.ts (2)
5-5: Use the ~/ alias for src imports in apps/webFollow repo guidelines for module resolution.
-import { checkIsValidEmailIdWithDomainRestriction } from "../../api-utils"; +import { checkIsValidEmailIdWithDomainRestriction } from "~/server/public-api/api-utils";
25-33: Fix response descriptionThe route cancels an email, not “Retrieve the user”.
- description: "Retrieve the user", + description: "Cancel a scheduled email",apps/web/src/server/public-api/api/domains/get-domains.ts (1)
25-30: Guard for undefined apiKey and add stable orderingOptional hardening + deterministic output ordering.
- const domains = team.apiKey.domainId + const domains = team.apiKey?.domainId ? await db.domain.findMany({ - where: { teamId: team.id, id: team.apiKey.domainId }, + where: { teamId: team.id, id: team.apiKey.domainId }, }) - : await db.domain.findMany({ where: { teamId: team.id } }); + : await db.domain.findMany({ + where: { teamId: team.id }, + orderBy: { name: "asc" }, + });apps/web/src/server/service/email-service.ts (1)
329-355: Minor: Typo persists in queue service callIf not intentional,
EmailQueueService.chancelEmaillikely meantcancelEmail. If the method is truly named with the typo, ignore; else fix both definition and call.apps/web/src/server/service/domain-service.ts (2)
38-40: Switch to composite-unique or index for correctness and performanceGood move using findFirst with teamId. If (teamId, name) is intended to be unique, consider adding a composite unique in Prisma and using findUnique with the compound key to enforce integrity at the DB layer.
Example once compound unique exists:
- const domain = await db.domain.findFirst({ - where: { name: fromDomain, teamId }, - }); + const domain = await db.domain.findUnique({ + where: { name_teamId: { name: fromDomain, teamId } }, + });
59-81: Harden null/undefined check for unrestricted keysUse a nullish check to avoid treating a hypothetical 0 as falsy. Also keep message consistent with other routes (“doesn't” vs “does not”).
- if (!apiKey.domainId) { + if (apiKey.domainId == null) { return domain; }apps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsx (3)
196-222: Use controlled Select with RHFPrefer value={field.value} over defaultValue for consistency with Controller patterns.
- <Select onValueChange={field.onChange} defaultValue={field.value}> + <Select onValueChange={field.onChange} value={field.value}>
39-45: Schema: encode “all” sentinel explicitlyMinor: make domainId accept "all" or a numeric string to reflect actual values and reduce accidental inputs.
-const apiKeySchema = z.object({ +const apiKeySchema = z.object({ name: z.string({ required_error: "Name is required" }).min(1, { message: "Name is required", }), - domainId: z.string().optional(), + domainId: z.union([z.literal("all"), z.string().regex(/^\d+$/)]), });Also applies to: 61-62, 70-71
46-46: File naming conventionPer guidelines, name React component files in PascalCase. Consider renaming to AddApiKey.tsx.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/web/prisma/migrations/20250822125136_add_domain_access_to_api_keys/migration.sql(1 hunks)apps/web/prisma/schema.prisma(2 hunks)apps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsx(4 hunks)apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsx(4 hunks)apps/web/src/server/api/routers/api.ts(3 hunks)apps/web/src/server/public-api/api-utils.ts(1 hunks)apps/web/src/server/public-api/api/domains/get-domains.ts(2 hunks)apps/web/src/server/public-api/api/domains/verify-domain.ts(1 hunks)apps/web/src/server/public-api/api/emails/cancel-email.ts(2 hunks)apps/web/src/server/public-api/api/emails/get-email.ts(1 hunks)apps/web/src/server/public-api/api/emails/list-emails.ts(1 hunks)apps/web/src/server/public-api/api/emails/update-email.ts(2 hunks)apps/web/src/server/public-api/auth.ts(1 hunks)apps/web/src/server/public-api/hono.ts(1 hunks)apps/web/src/server/service/api-service.ts(3 hunks)apps/web/src/server/service/domain-service.ts(3 hunks)apps/web/src/server/service/email-service.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsxapps/web/src/server/public-api/auth.tsapps/web/src/server/public-api/api/emails/list-emails.tsapps/web/src/server/public-api/api/emails/update-email.tsapps/web/src/server/service/api-service.tsapps/web/src/server/public-api/api/domains/verify-domain.tsapps/web/src/server/public-api/api/emails/cancel-email.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/public-api/api/domains/get-domains.tsapps/web/src/server/api/routers/api.tsapps/web/src/server/public-api/api-utils.tsapps/web/src/server/public-api/api/emails/get-email.tsapps/web/src/server/public-api/hono.tsapps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsxapps/web/src/server/service/email-service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use 2-space indentation in TypeScript code (enforced by Prettier)
Use semicolons in TypeScript code (enforced by Prettier)
Do not use dynamic imports
Files:
apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsxapps/web/src/server/public-api/auth.tsapps/web/src/server/public-api/api/emails/list-emails.tsapps/web/src/server/public-api/api/emails/update-email.tsapps/web/src/server/service/api-service.tsapps/web/src/server/public-api/api/domains/verify-domain.tsapps/web/src/server/public-api/api/emails/cancel-email.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/public-api/api/domains/get-domains.tsapps/web/src/server/api/routers/api.tsapps/web/src/server/public-api/api-utils.tsapps/web/src/server/public-api/api/emails/get-email.tsapps/web/src/server/public-api/hono.tsapps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsxapps/web/src/server/service/email-service.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 via pnpm format for TypeScript and Markdown files
Files:
apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsxapps/web/src/server/public-api/auth.tsapps/web/src/server/public-api/api/emails/list-emails.tsapps/web/src/server/public-api/api/emails/update-email.tsapps/web/src/server/service/api-service.tsapps/web/src/server/public-api/api/domains/verify-domain.tsapps/web/src/server/public-api/api/emails/cancel-email.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/public-api/api/domains/get-domains.tsapps/web/src/server/api/routers/api.tsapps/web/src/server/public-api/api-utils.tsapps/web/src/server/public-api/api/emails/get-email.tsapps/web/src/server/public-api/hono.tsapps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsxapps/web/src/server/service/email-service.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsxapps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
In apps/web, use the
/ alias for src imports (e.g., import { x } from "/utils/x")
Files:
apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsxapps/web/src/server/public-api/auth.tsapps/web/src/server/public-api/api/emails/list-emails.tsapps/web/src/server/public-api/api/emails/update-email.tsapps/web/src/server/service/api-service.tsapps/web/src/server/public-api/api/domains/verify-domain.tsapps/web/src/server/public-api/api/emails/cancel-email.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/public-api/api/domains/get-domains.tsapps/web/src/server/api/routers/api.tsapps/web/src/server/public-api/api-utils.tsapps/web/src/server/public-api/api/emails/get-email.tsapps/web/src/server/public-api/hono.tsapps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsxapps/web/src/server/service/email-service.ts
🧬 Code graph analysis (12)
apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsx (1)
packages/ui/src/table.tsx (2)
TableHead(113-113)TableCell(115-115)
apps/web/src/server/public-api/api/emails/update-email.ts (1)
apps/web/src/server/public-api/api-utils.ts (1)
checkIsValidEmailIdWithDomainRestriction(37-58)
apps/web/src/server/service/api-service.ts (1)
apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/server/public-api/api/domains/verify-domain.ts (2)
apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/server/public-api/api/emails/cancel-email.ts (1)
apps/web/src/server/public-api/api-utils.ts (1)
checkIsValidEmailIdWithDomainRestriction(37-58)
apps/web/src/server/service/domain-service.ts (2)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)
apps/web/src/server/public-api/api/domains/get-domains.ts (1)
apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/server/api/routers/api.ts (1)
apps/web/src/server/service/api-service.ts (1)
addApiKey(8-57)
apps/web/src/server/public-api/api-utils.ts (2)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)
apps/web/src/server/public-api/api/emails/get-email.ts (1)
apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/app/(dashboard)/dev-settings/api-keys/add-api-key.tsx (2)
packages/ui/src/form.tsx (5)
FormField(176-176)FormItem(171-171)FormLabel(172-172)FormControl(173-173)FormDescription(174-174)packages/ui/src/select.tsx (5)
Select(150-150)SelectTrigger(153-153)SelectValue(152-152)SelectContent(154-154)SelectItem(156-156)
apps/web/src/server/service/email-service.ts (3)
apps/web/src/server/service/domain-service.ts (2)
validateDomainFromEmail(14-57)validateApiKeyDomainAccess(59-81)apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)
🔇 Additional comments (13)
apps/web/src/server/public-api/auth.ts (1)
62-63: Domain scoping propagated correctly.Returning
apiKey: { domainId }on the team object matches the new AppEnv typing and enables downstream domain-scoped checks.apps/web/src/server/public-api/hono.ts (1)
16-17: AppEnv typing looks good.Matches
getTeamFromTokenreturn shape (apiKeyId+apiKey.domainId).apps/web/src/server/public-api/api/emails/get-email.ts (1)
63-74: Scan for similarfindUniquemisuse across public API.To catch other instances querying by
id+ extra fields, run:rg -nP -g 'apps/web/src/server/**/*.{ts,tsx}' -C2 '\.findUnique\(\s*\{\s*where:\s*\{(?s:.*?)\}\s*\}\)' \ | rg -nP 'teamId|domainId|apiKeyId'apps/web/prisma/migrations/20250822125136_add_domain_access_to_api_keys/migration.sql (1)
1-5: LGTM: adds domainId with FK as SET NULL.Matches intended semantics for “All domains if null”. Ensure schema uses
onDelete: SetNull(see schema comment).apps/web/src/app/(dashboard)/dev-settings/api-keys/api-list.tsx (4)
28-31: Domain Access column addition looks good.Header placement and ordering are clear.
37-38: colSpan updated correctly for loading/empty states.Also applies to: 46-47
56-60: Row: domain display is robust.Handles specific domain, removed domain, and “All domains”.
63-64: Nice UX touch on lastUsed.Readable relative time with a good “Never” fallback.
apps/web/src/server/public-api/api/emails/list-emails.ts (1)
126-129: Correct domain scoping precedence.API key–scoped domain overrides query filter as expected.
apps/web/src/server/public-api/api/domains/get-domains.ts (1)
16-16: LGTM: updated description matches domain-scoped behaviorapps/web/src/server/service/email-service.ts (1)
5-5: LGTM: importing validateApiKeyDomainAccessapps/web/src/server/api/routers/api.ts (2)
15-19: Input schema update looks goodUsing z.nativeEnum(ApiPermission) and optional domainId is appropriate.
42-47: Good: exposing domain linkage in listingIncluding domainId and domain.name will unblock the UI.
| return await addApiKey({ | ||
| name: input.name, | ||
| permission: input.permission, | ||
| teamId: ctx.team.id, | ||
| domainId: input.domainId, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Map service error to typed TRPC error
addApiKey throws "DOMAIN_NOT_FOUND". Convert to TRPC NOT_FOUND so clients get a proper 404-ish signal instead of 500.
- .mutation(async ({ ctx, input }) => {
- return await addApiKey({
- name: input.name,
- permission: input.permission,
- teamId: ctx.team.id,
- domainId: input.domainId,
- });
- }),
+ .mutation(async ({ ctx, input }) => {
+ try {
+ return await addApiKey({
+ name: input.name,
+ permission: input.permission,
+ teamId: ctx.team.id,
+ domainId: input.domainId,
+ });
+ } catch (err) {
+ if (err instanceof Error && err.message === "DOMAIN_NOT_FOUND") {
+ throw new TRPCError({ code: "NOT_FOUND", message: "Domain not found" });
+ }
+ throw err;
+ }
+ }),📝 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.
| return await addApiKey({ | |
| name: input.name, | |
| permission: input.permission, | |
| teamId: ctx.team.id, | |
| domainId: input.domainId, | |
| }); | |
| .mutation(async ({ ctx, input }) => { | |
| try { | |
| return await addApiKey({ | |
| name: input.name, | |
| permission: input.permission, | |
| teamId: ctx.team.id, | |
| domainId: input.domainId, | |
| }); | |
| } catch (err) { | |
| if (err instanceof Error && err.message === "DOMAIN_NOT_FOUND") { | |
| throw new TRPCError({ code: "NOT_FOUND", message: "Domain not found" }); | |
| } | |
| throw err; | |
| } | |
| }), |
🤖 Prompt for AI Agents
In apps/web/src/server/api/routers/api.ts around lines 22 to 27, add error
mapping when calling addApiKey: wrap the await addApiKey(...) call in a
try/catch, import TRPCError from '@trpc/server', and if the caught error has
code === 'DOMAIN_NOT_FOUND' throw new TRPCError({ code: 'NOT_FOUND', message:
error.message || 'Domain not found' }); otherwise rethrow the original error so
other failures surface as before.
| if (apiKeyDomainId !== undefined) { | ||
| whereClause.domainId = apiKeyDomainId; | ||
| } | ||
|
|
||
| const email = await db.email.findUnique({ where: whereClause }); | ||
|
|
||
| if (!email) { | ||
| throw new UnsendApiError({ code: "NOT_FOUND", message: "Email not found" }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Critical: null domainId narrows the query unexpectedly (unrestricted keys break); also use findFirst for composite filters
When team.apiKey.domainId is null (unrestricted key), this sets whereClause.domainId = null, filtering to emails with domainId = null and causing false NOT_FOUND. Additionally, findUnique with non-unique combinations is fragile; prefer findFirst with a compound where.
Apply:
-export const checkIsValidEmailIdWithDomainRestriction = async (
- emailId: string,
- teamId: number,
- apiKeyDomainId?: number
-) => {
- const whereClause: { id: string; teamId: number; domainId?: number } = {
- id: emailId,
- teamId,
- };
-
- if (apiKeyDomainId !== undefined) {
- whereClause.domainId = apiKeyDomainId;
- }
-
- const email = await db.email.findUnique({ where: whereClause });
+export const checkIsValidEmailIdWithDomainRestriction = async (
+ emailId: string,
+ teamId: number,
+ apiKeyDomainId?: number | null
+) => {
+ const email = await db.email.findFirst({
+ where: {
+ id: emailId,
+ teamId,
+ ...(apiKeyDomainId != null ? { domainId: apiKeyDomainId } : {}),
+ },
+ });
if (!email) {
throw new UnsendApiError({ code: "NOT_FOUND", message: "Email not found" });
}
return email;
};📝 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 (apiKeyDomainId !== undefined) { | |
| whereClause.domainId = apiKeyDomainId; | |
| } | |
| const email = await db.email.findUnique({ where: whereClause }); | |
| if (!email) { | |
| throw new UnsendApiError({ code: "NOT_FOUND", message: "Email not found" }); | |
| } | |
| export const checkIsValidEmailIdWithDomainRestriction = async ( | |
| emailId: string, | |
| teamId: number, | |
| apiKeyDomainId?: number | null | |
| ) => { | |
| const email = await db.email.findFirst({ | |
| where: { | |
| id: emailId, | |
| teamId, | |
| ...(apiKeyDomainId != null ? { domainId: apiKeyDomainId } : {}), | |
| }, | |
| }); | |
| if (!email) { | |
| throw new UnsendApiError({ code: "NOT_FOUND", message: "Email not found" }); | |
| } | |
| return email; | |
| }; |
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api-utils.ts around lines 47 to 55, the code
currently sets whereClause.domainId to apiKeyDomainId which, when apiKeyDomainId
is null (an unrestricted key), narrows the query to domainId = null and causes
false NOT_FOUNDs, and it uses findUnique which is fragile for composite filters;
fix by only adding domainId to the whereClause when apiKeyDomainId is neither
undefined nor null (i.e., check apiKeyDomainId !== undefined && apiKeyDomainId
!== null), and replace db.email.findUnique(...) with db.email.findFirst({ where:
whereClause }) so the query is not incorrectly restricted and supports
non-unique/composite filters.
| description: "Verify domain", | ||
| }, | ||
| 403: { | ||
| content: { | ||
| "application/json": { | ||
| schema: z.object({ | ||
| error: z.string(), | ||
| }), | ||
| }, | ||
| }, | ||
| description: "Forbidden - API key doesn't have access to this domain", | ||
| }, | ||
| 404: { | ||
| content: { | ||
| "application/json": { | ||
| schema: z.object({ | ||
| error: z.string(), | ||
| }), | ||
| }, | ||
| }, | ||
| description: "Domain not found", | ||
| }, |
There was a problem hiding this comment.
Docs/code mismatch for 403 vs 404
OpenAPI declares a 403, but the handler always returns 404 on denied access. Return 403 when the API key is restricted to another domain.
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/domains/verify-domain.ts around lines
28-49 the OpenAPI spec declares a 403 for "API key doesn't have access to this
domain" but the handler currently always returns 404 on denied access; update
the request handling so that when an API key is valid but restricted to a
different domain the handler responds with HTTP 403 (with a JSON body matching
the existing { error: string } schema), and only return 404 when the domain
genuinely does not exist; ensure the condition checks the API key's domain
ownership/permissions before the not-found branch and return the appropriate 403
response there.
| await checkIsValidEmailIdWithDomainRestriction(emailId, team.id, team.apiKey.domainId); | ||
|
|
||
| await cancelEmail(emailId); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden domainId read and pass teamId to service (see email-service change)
- Use optional chaining to avoid runtime if
apiKeyis absent. - After tightening service checks, pass
team.idintocancelEmail(emailId, team.id).
- await checkIsValidEmailIdWithDomainRestriction(emailId, team.id, team.apiKey.domainId);
+ await checkIsValidEmailIdWithDomainRestriction(
+ emailId,
+ team.id,
+ team.apiKey?.domainId
+ );
-
- await cancelEmail(emailId);
+ await cancelEmail(emailId, team.id);📝 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.
| await checkIsValidEmailIdWithDomainRestriction(emailId, team.id, team.apiKey.domainId); | |
| await cancelEmail(emailId); | |
| await checkIsValidEmailIdWithDomainRestriction( | |
| emailId, | |
| team.id, | |
| team.apiKey?.domainId | |
| ); | |
| await cancelEmail(emailId, team.id); |
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/emails/cancel-email.ts around lines 40 to
42, harden the read of domainId and pass teamId into the cancelEmail call: use
optional chaining to read team.apiKey?.domainId when calling
checkIsValidEmailIdWithDomainRestriction to avoid runtime errors if apiKey is
missing, and update the subsequent service call to cancelEmail(emailId, team.id)
so the service receives the team context required by the tightened checks.
| const whereClause: { id: string; teamId: number; domainId?: number } = { | ||
| id: emailId, | ||
| teamId: team.id, | ||
| }; | ||
|
|
||
| if (team.apiKey.domainId !== null) { | ||
| whereClause.domainId = team.apiKey.domainId; | ||
| } | ||
|
|
||
| const email = await db.email.findUnique({ | ||
| where: { | ||
| id: emailId, | ||
| teamId: team.id, | ||
| }, | ||
| where: whereClause, | ||
| select: { |
There was a problem hiding this comment.
findUnique used with non-unique composite filter — will not work.
findUnique only accepts a unique selector (e.g., id). Use findFirst with a where that includes teamId/domainId, or query by id and validate ownership afterwards.
-import { getTeamFromToken } from "~/server/public-api/auth";
+// (no import)
-import { EmailStatus } from "@prisma/client";
+import { EmailStatus, Prisma } from "@prisma/client";
@@
- const whereClause: { id: string; teamId: number; domainId?: number } = {
- id: emailId,
- teamId: team.id,
- };
+ const whereClause: Prisma.EmailWhereInput = {
+ id: emailId,
+ teamId: team.id,
+ };
@@
- if (team.apiKey.domainId !== null) {
+ if (team.apiKey.domainId !== null) {
whereClause.domainId = team.apiKey.domainId;
}
@@
- const email = await db.email.findUnique({
- where: whereClause,
+ const email = await db.email.findFirst({
+ where: whereClause,
select: {
id: true,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/emails/get-email.ts around lines 63 to 74,
the code uses db.email.findUnique with a non-unique composite where object (id,
teamId, optional domainId), which is invalid; change the query to
db.email.findFirst with the same where clause (id, teamId, domainId when
present) so Prisma accepts the composite filter, or alternatively fetch by id
with findUnique and then verify teamId/domainId on the returned record —
implement the findFirst approach for minimal change and keep the existing
ownership checks.
There was a problem hiding this comment.
🧩 Analysis chain
I understand you're saying it's working. Let me check the current Prisma documentation to understand if findUnique with composite where clauses is supported in the version you're using.
@KMKoushik, thanks for the clarification! I've noted that this pattern is working in your setup. The behavior might vary depending on the Prisma version or specific schema configuration being used.
🏁 Script executed:
#!/bin/bash
# Check the Prisma version being used in the project
cat package.json | jq '.dependencies.prisma // .devDependencies.prisma // .dependencies."@prisma/client" // .devDependencies."@prisma/client"'
# Check if there are any lock files that might give us version info
find . -name "package-lock.json" -o -name "yarn.lock" -o -name "pnpm-lock.yaml" | head -1 | xargs cat | grep -A5 -B5 prisma | head -20
Length of output: 518
🌐 Web query:
Prisma findUnique composite where clause support 2024 2025
💡 Result:
Short answer: Prisma has supported using composite (compound) IDs and compound unique constraints in findUnique/findUniqueOrThrow since Prisma Client introduced composite keys (examples in docs) and this remained supported through 2024–2025. The official docs show the findUnique where syntax for composite keys (e.g. { likeId: { userId, postId } }). [1]
Notes / caveats:
- MongoDB: composite IDs/@@id are not supported for MongoDB; Prisma’s MongoDB handling and findUnique have had multiple MongoDB-specific issues reported in 2024 (performance and behaviour), so expect caveats and check current Prisma release notes if you use MongoDB. [1][2]
- There have been related client/typing issues (e.g. UniqueInput with more-than-one-unique-field) and other GitHub issues impacting findUnique/compound-unique behaviour across 2023–2024; consult the Prisma GitHub issues for details if you hit problems. [3][4]
Sources:
- Prisma docs — Working with composite IDs and constraints (shows findUnique with compound keys). [Prisma Docs]. [2]
- GitHub: MongoDB findUnique() / performance issues (2024). [3]
- GitHub issue: UniqueInput with more than one unique field (Prisma 5 / 2023–2024). [4]
If you want, I can fetch the exact doc snippet and current release notes for your Prisma version (specify the version).
@KMKoushik, thanks for confirming! I can see you're using Prisma Client 6.6.0. While findUnique typically works with composite keys defined via @@unique or @@id directives in the schema, it appears to be working in your setup.
This might be working because:
- The
idfield is the primary unique identifier, and Prisma is using that for the unique lookup - The additional fields (
teamId,domainId) might be getting filtered appropriately within the query execution - Your specific Prisma schema configuration supports this pattern
Since you've confirmed it's working as expected, I'll note this for future reference. Thanks for the clarification!
✏️ Learnings added
Learnt from: KMKoushik
PR: usesend/useSend#224
File: apps/web/src/server/public-api/api/emails/get-email.ts:63-74
Timestamp: 2025-09-10T12:33:42.627Z
Learning: In the useSend project, Prisma's findUnique method works with composite where clauses including id, teamId, and optional domainId fields in apps/web/src/server/public-api/api/emails/get-email.ts
Learnt from: KMKoushik
PR: usesend/useSend#224
File: apps/web/src/server/public-api/api/emails/get-email.ts:63-74
Timestamp: 2025-09-10T12:33:42.627Z
Learning: In the useSend project using Prisma Client 6.6.0, findUnique works with composite where clauses containing id, teamId, and optional domainId fields in apps/web/src/server/public-api/api/emails/get-email.ts, as confirmed by the project maintainer KMKoushik.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| await checkIsValidEmailIdWithDomainRestriction(emailId, team.id, team.apiKey.domainId); | ||
|
|
There was a problem hiding this comment.
Passing null narrows results to domainId IS NULL. Use ?? undefined.
When the key has “All domains” (null), passing it through restricts the query incorrectly. Send undefined to skip domain filtering.
- await checkIsValidEmailIdWithDomainRestriction(emailId, team.id, team.apiKey.domainId);
+ await checkIsValidEmailIdWithDomainRestriction(
+ emailId,
+ team.id,
+ team.apiKey.domainId ?? undefined
+ );📝 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.
| await checkIsValidEmailIdWithDomainRestriction(emailId, team.id, team.apiKey.domainId); | |
| await checkIsValidEmailIdWithDomainRestriction( | |
| emailId, | |
| team.id, | |
| team.apiKey.domainId ?? undefined | |
| ); |
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/emails/update-email.ts around lines 51 to
52, the call passes team.apiKey.domainId directly which can be null and will
narrow DB queries to "domainId IS NULL"; change the argument to use the nullish
coalescing result (team.apiKey.domainId ?? undefined) so null becomes undefined
and the domain filter is skipped as intended.
💡 Verification agent
🧩 Analysis chain
Audit other callers for the same null/undefined domainId issue.
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts --type=tsx 'checkIsValidEmailIdWithDomainRestriction\s*\(' apps/web/src \
-A1 | rg -nP 'team\.apiKey\.domainId\)?\s*\)?$'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for all calls to checkIsValidEmailIdWithDomainRestriction and filter those using team.apiKey.domainId
rg -nP 'checkIsValidEmailIdWithDomainRestriction\s*\(' -g '*.ts' -g '*.tsx' apps/web/src -A1 | rg -nP 'team\.apiKey\.domainId'Length of output: 309
Guard null/undefined team.apiKey.domainId in cancel-email.ts
In apps/web/src/server/public-api/api/emails/cancel-email.ts (line 40), you’re calling
await checkIsValidEmailIdWithDomainRestriction(emailId, team.id, team.apiKey.domainId);just as in update-email.ts—ensure you apply the same null-check or fallback logic here to prevent passing an undefined domainId.
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/emails/update-email.ts around lines 51-52,
the call passes team.apiKey.domainId directly; ensure you mirror the same
null-check/fallback in cancel-email.ts by guarding team.apiKey and domainId
before calling checkIsValidEmailIdWithDomainRestriction — e.g. derive a safe
domainId via optional chaining and a nullish fallback (team.apiKey?.domainId ??
undefined) and pass that safe value into
checkIsValidEmailIdWithDomainRestriction so undefined/null is never accessed on
a missing apiKey.
| // Validate domain ownership if domainId is provided | ||
| if (domainId !== undefined) { | ||
| const domain = await db.domain.findUnique({ | ||
| where: { | ||
| id: domainId, | ||
| teamId: teamId | ||
| }, | ||
| select: { id: true }, | ||
| }); | ||
|
|
||
| if (!domain) { | ||
| throw new Error("DOMAIN_NOT_FOUND"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: invalid Prisma findUnique where-clause
Prisma’s findUnique only accepts unique fields. Passing both id and teamId will not compile. Use findFirst with both predicates, or fetch by id and verify teamId.
- const domain = await db.domain.findUnique({
- where: {
- id: domainId,
- teamId: teamId
- },
- select: { id: true },
- });
+ const domain = await db.domain.findFirst({
+ where: { id: domainId, teamId },
+ select: { id: true },
+ });📝 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.
| // Validate domain ownership if domainId is provided | |
| if (domainId !== undefined) { | |
| const domain = await db.domain.findUnique({ | |
| where: { | |
| id: domainId, | |
| teamId: teamId | |
| }, | |
| select: { id: true }, | |
| }); | |
| if (!domain) { | |
| throw new Error("DOMAIN_NOT_FOUND"); | |
| } | |
| } | |
| // Validate domain ownership if domainId is provided | |
| if (domainId !== undefined) { | |
| const domain = await db.domain.findFirst({ | |
| where: { id: domainId, teamId }, | |
| select: { id: true }, | |
| }); | |
| if (!domain) { | |
| throw new Error("DOMAIN_NOT_FOUND"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/server/service/api-service.ts around lines 20 to 33, the Prisma
call uses findUnique with a composite where (id and teamId) which is invalid;
change db.domain.findUnique(...) to db.domain.findFirst({ where: { id: domainId,
teamId }, select: { id: true } }) so both predicates are allowed, or
alternatively fetch by id with findUnique({ where: { id: domainId }, select: {
id: true, teamId: true } }) and then verify domain.teamId === teamId and throw
DOMAIN_NOT_FOUND if it doesn't match.
| let domain: Awaited<ReturnType<typeof validateDomainFromEmail>>; | ||
|
|
||
| // If this is an API call with an API key, validate domain access | ||
| if (apiKeyId) { | ||
| const apiKey = await db.apiKey.findUnique({ | ||
| where: { id: apiKeyId }, | ||
| include: { domain: true }, | ||
| }); | ||
|
|
||
| if (!apiKey) { | ||
| throw new UnsendApiError({ | ||
| code: "BAD_REQUEST", | ||
| message: "Invalid API key", | ||
| }); | ||
| } | ||
|
|
||
| domain = await validateApiKeyDomainAccess(from, teamId, apiKey); | ||
| } else { | ||
| // For non-API calls (dashboard, etc.), use regular domain validation | ||
| domain = await validateDomainFromEmail(from, teamId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good domain-path split; also verify apiKey.teamId matches the request’s teamId
The split correctly enforces API-key domain restrictions. Add a team ownership check to prevent cross-team apiKey usage.
if (apiKeyId) {
const apiKey = await db.apiKey.findUnique({
where: { id: apiKeyId },
include: { domain: true },
});
if (!apiKey) {
throw new UnsendApiError({
code: "BAD_REQUEST",
message: "Invalid API key",
});
}
+ if (apiKey.teamId !== teamId) {
+ throw new UnsendApiError({
+ code: "FORBIDDEN",
+ message: "API key does not belong to this team",
+ });
+ }
domain = await validateApiKeyDomainAccess(from, teamId, apiKey);
} else {📝 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.
| let domain: Awaited<ReturnType<typeof validateDomainFromEmail>>; | |
| // If this is an API call with an API key, validate domain access | |
| if (apiKeyId) { | |
| const apiKey = await db.apiKey.findUnique({ | |
| where: { id: apiKeyId }, | |
| include: { domain: true }, | |
| }); | |
| if (!apiKey) { | |
| throw new UnsendApiError({ | |
| code: "BAD_REQUEST", | |
| message: "Invalid API key", | |
| }); | |
| } | |
| domain = await validateApiKeyDomainAccess(from, teamId, apiKey); | |
| } else { | |
| // For non-API calls (dashboard, etc.), use regular domain validation | |
| domain = await validateDomainFromEmail(from, teamId); | |
| } | |
| let domain: Awaited<ReturnType<typeof validateDomainFromEmail>>; | |
| // If this is an API call with an API key, validate domain access | |
| if (apiKeyId) { | |
| const apiKey = await db.apiKey.findUnique({ | |
| where: { id: apiKeyId }, | |
| include: { domain: true }, | |
| }); | |
| if (!apiKey) { | |
| throw new UnsendApiError({ | |
| code: "BAD_REQUEST", | |
| message: "Invalid API key", | |
| }); | |
| } | |
| if (apiKey.teamId !== teamId) { | |
| throw new UnsendApiError({ | |
| code: "FORBIDDEN", | |
| message: "API key does not belong to this team", | |
| }); | |
| } | |
| domain = await validateApiKeyDomainAccess(from, teamId, apiKey); | |
| } else { | |
| // For non-API calls (dashboard, etc.), use regular domain validation | |
| domain = await validateDomainFromEmail(from, teamId); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/server/service/email-service.ts around lines 73 to 93, add a
team ownership check after loading the apiKey: verify apiKey.teamId === teamId
and if it does not match throw an UnsendApiError (BAD_REQUEST or FORBIDDEN)
before calling validateApiKeyDomainAccess; this prevents cross-team API key
usage while keeping the existing domain validation flow.
Summary by CodeRabbit