Uh oh!
There was an error while loading. Please reload this page.
feat: GET /api/accounts/{id} accepts email as identifier - #403
Merged
Conversation
When the path parameter contains '@', resolves it as an email via selectAccountByEmail before running auth checks. Returns 404 if no account is found for the email. UUID paths work unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 19 minutes and 21 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe GET account handler now accepts either a UUID or an email in the path Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant Validator as validateGetAccountParams
participant Resolver as resolveAccountIdByEmail
participant Auth as validateAuthContext
participant DB
participant Service as getAccountWithDetails
Client->>Handler: GET /accounts/{id}
Handler->>Validator: validateGetAccountParams(request, id)
alt id contains "@"
Validator->>Resolver: resolveAccountIdByEmail(request, email)
Resolver->>Auth: validateAuthContext(request)
Auth-->>Resolver: auth OK / NextResponse
Resolver->>DB: selectAccountByEmail(email)
DB-->>Resolver: account_id
Resolver->>Auth: validateAuthContext(request, {accountId})
Auth-->>Resolver: auth OK / NextResponse
Resolver-->>Validator: accountId
else id is UUID
Validator->>Auth: validateAccountParams(id) -> validatedId
Auth-->>Validator: validatedId / NextResponse
Validator->>Auth: validateAuthContext(request, {accountId:validatedId})
Auth-->>Validator: OK / NextResponse
Validator-->>Handler: accountId
end
Handler->>Service: getAccountWithDetails(accountId)
Service-->>Handler: account details
Handler-->>Client: 200 JSON / or earlier NextResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/accounts/getAccountHandler.ts`:
- Around line 28-34: The email lookup branch in getAccountHandler currently
returns a 404 for unknown emails before the handler's authentication check,
enabling account-probing; move or invoke the existing auth gate used later in
getAccountHandler (the same auth check currently at line ~45) before calling
selectAccountByEmail or returning any NextResponse.json about "No account
found", so unauthenticated requests get a 401/unauthorized and only
authenticated requests reach selectAccountByEmail and receive the
404/NextResponse.json with getCorsHeaders.
- Line 36: The code assigns emailAccount.account_id directly to accountId even
though the DB type is string | null; add an explicit null/empty check on
emailAccount.account_id before assigning and return an error NextResponse (using
getCorsHeaders()) if it's missing, then assign accountId and call
getAccountWithDetails(accountId); optionally validate the value as a UUID before
calling getAccountWithDetails to ensure the non-null string contract is
satisfied (refer to symbols emailAccount, accountId, getAccountWithDetails,
getCorsHeaders, NextResponse).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95eace95-ba18-49f3-a822-b111a55d0f89
⛔ Files ignored due to path filters (1)
lib/accounts/__tests__/getAccountHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (1)
lib/accounts/getAccountHandler.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
sweetmantech
commented
Apr 6, 2026
Comment on lines
+29
to
+42
| const emailAccount = await selectAccountByEmail(id); | ||
| if (!emailAccount) { | ||
| return NextResponse.json( | ||
| { status: "error", error: "No account found for the provided email" }, | ||
| { status: 404, headers: getCorsHeaders() }, | ||
| ); | ||
| } | ||
| accountId = emailAccount.account_id; | ||
| } else { | ||
| const validatedParams = validateAccountParams(id); | ||
| if (validatedParams instanceof NextResponse) { | ||
| return validatedParams; | ||
| } | ||
| accountId = validatedParams.id; |
ContributorAuthor
There was a problem hiding this comment.
Open Closed principle
- actual: email check code added directly to existing getAccountHandler.
- required: new lib file for email check code.
- Moved auth check before selectAccountByEmail to prevent unauthenticated account-email probing (CodeRabbit critical) - Added null guard on emailAccount.account_id (CodeRabbit major) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 3/5
- There is a concrete security risk in
lib/accounts/getAccountHandler.ts: returning an email-not-found 404 before authentication can let unauthenticated callers enumerate valid emails via response differences. - Given the issue is medium severity (6/10) with reasonably high confidence (7/10), this introduces real user-impacting regression risk and lowers merge confidence until ordering is fixed.
- This should be straightforward to address by authenticating first and only then returning resource-specific errors, which keeps behavior consistent for unauthorized requests.
- Pay close attention to
lib/accounts/getAccountHandler.ts- response ordering currently leaks account existence to unauthenticated callers.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/accounts/getAccountHandler.ts">
<violation number="1" location="lib/accounts/getAccountHandler.ts:30">
P2: Authenticate before returning the email-not-found 404; otherwise unauthenticated callers can probe which emails exist based on 404 vs 401/403 responses.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant Handler as getAccountHandler
participant DB_Emails as DB: account_emails
participant Validator as validateAccountParams
participant Auth as validateAuthContext
participant DB_Accounts as DB: accounts
Client->>Handler: GET /api/accounts/{id}
Note over Handler,DB_Emails: ID Resolution Logic
alt NEW: id contains "@" (Email lookup)
Handler->>DB_Emails: selectAccountByEmail(email)
DB_Emails-->>Handler: account_id or null
opt Email not found
Handler-->>Client: 404 (No account found for email)
end
else Standard UUID
Handler->>Validator: validateAccountParams(id)
Validator-->>Handler: accountId
end
Note over Handler,DB_Accounts: Authorization & Retrieval
Handler->>Auth: validateAuthContext(request, { accountId })
alt Auth Success
Handler->>DB_Accounts: CHANGED: getAccountWithDetails(resolvedAccountId)
DB_Accounts-->>Handler: account record
alt Account exists
Handler-->>Client: 200 OK (Account Details)
else Account not found
Handler-->>Client: 404 (Account not found)
end
else Auth Failure (401/403)
Auth-->>Handler: Error Response
Handler-->>Client: Error Response
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sweetmantech
commented
Apr 6, 2026
ContributorAuthor
Preview Deployment Testing ResultsPreview URL: GET /api/accounts/sweetmantech@gmail.com
Security
|
sweetmantech
commented
Apr 6, 2026
Comment on lines
26
to
61
| let accountId: string; | ||
| if (id.includes("@")) { | ||
| // Authenticate before email lookup to prevent account-email probing | ||
| const authResult = await validateAuthContext(request); | ||
| if (authResult instanceof NextResponse) { | ||
| return authResult; | ||
| } | ||
| const resolved = await resolveAccountIdFromEmail(id); | ||
| if (resolved instanceof NextResponse) { | ||
| return resolved; | ||
| } | ||
| accountId = resolved; | ||
| // Verify caller can access this account | ||
| const accessResult = await validateAuthContext(request, { | ||
| accountId, | ||
| }); | ||
| if (accessResult instanceof NextResponse) { | ||
| return accessResult; | ||
| } | ||
| } else { | ||
| const validatedParams = validateAccountParams(id); | ||
| if (validatedParams instanceof NextResponse) { | ||
| return validatedParams; | ||
| } | ||
| accountId = validatedParams.id; | ||
| const authResult = await validateAuthContext(request, { | ||
| accountId: validatedParams.id, | ||
| }); | ||
| if (authResult instanceof NextResponse) { | ||
| return authResult; | ||
| const authResult = await validateAuthContext(request, { | ||
| accountId, | ||
| }); | ||
| if (authResult instanceof NextResponse) { | ||
| return authResult; | ||
| } | ||
| } |
ContributorAuthor
There was a problem hiding this comment.
SRP - move all lookup code to a standalone function out of the handler.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/accounts/resolveAccountIdFromEmail.ts">
<violation number="1" location="lib/accounts/resolveAccountIdFromEmail.ts:16">
P2: Do not return 404 for all `selectAccountByEmail` failures; database/query errors are being misreported as "email not found."</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Uh oh!
There was an error while loading. Please reload this page.
Moved auth + email resolve + access check into its own function. Handler now delegates to resolveAccountIdByEmail for email paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/accounts/resolveAccountIdByEmail.ts">
<violation number="1" location="lib/accounts/resolveAccountIdByEmail.ts:32">
P1: Returning the raw 403 from the access check leaks email existence (404 for unknown vs 403 for existing-but-forbidden). For email-based lookup, normalize forbidden access to the same not-found response.</violation>
</file>
<file name="lib/accounts/__tests__/resolveAccountIdByEmail.test.ts">
<violation number="1" location="lib/accounts/__tests__/resolveAccountIdByEmail.test.ts:25">
P3: The second mockResolvedValue overrides the first, so this test doesn’t exercise two different auth results. Use mockResolvedValueOnce for sequential calls so the first auth check and the access check are distinct.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…tions Moved all id resolution logic (email vs UUID detection, auth, access control) into validateGetAccountParams. Handler is now a thin wrapper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sweetmantech
commented
Apr 6, 2026
Uh oh!
There was an error while loading. Please reload this page.
ContributorAuthor
There was a problem hiding this comment.
KISS - is this function required? why not move the call to selectAccountByEmail directly in lib/accounts/resolveAccountIdByEmail.ts and remove this lib?
…FromEmail Removed unnecessary resolveAccountIdFromEmail abstraction. The selectAccountByEmail call is now directly in resolveAccountIdByEmail. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…yEmail
- resolveAccountIdByEmail now only resolves email → accountId (no auth)
- validateGetAccountParams handles auth + access for both paths with one
validateAuthContext(request, { accountId }) call
- Both UUID and email paths now follow the same pattern: resolve, then auth
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>Uh oh!
There was an error while loading. Please reload this page.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /api/accounts/{id}now accepts an email address in addition to a UUID@, resolves it viaselectAccountByEmailbefore running auth/access checksTest plan
GET /api/accounts/customer@example.comreturns account🤖 Generated with Claude Code
Summary by cubic
GET
/api/accounts/{id}now accepts an email or a UUID. Resolution and a single auth/access check are centralized invalidateGetAccountParams; email resolves first, then auth runs (REC-52 chat override).New Features
idcontains@, delegate toresolveAccountIdByEmail(lookup viaselectAccountByEmail; 404 on miss/null), then run onevalidateAuthContext; returns 401/403/404 as needed.validateAccountParamsand the same auth path.Refactors
validateGetAccountParamsto handle UUID/email detection and a singlevalidateAuthContextcall;getAccountHandlernow delegates to it.resolveAccountIdByEmailto only perform email → accountId lookup by inliningselectAccountByEmail; removedresolveAccountIdFromEmail.Written for commit 8c3a49a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes