Harden account API routes with CSRF origin checks#14
Merged
schartrand77 merged 1 commit intomainfrom Feb 24, 2026
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds CSRF hardening to sensitive account- and profile-related API routes by enforcing same-origin checks using a shared helper before executing authenticated PATCH/POST handlers. Sequence diagram for CSRF-hardened protected API routesequenceDiagram
actor Client
participant NextAPI as NextAPI_Route
participant CSRF as CSRF_Helper
participant Auth as Auth_Helper
participant DB as Database
Client->>NextAPI: HTTP PATCH/POST request with cookies and headers
NextAPI->>CSRF: isSameOriginRequest(req)
CSRF-->>NextAPI: true or false
alt invalid_origin
NextAPI-->>Client: 403 JSON { error: Invalid CSRF origin }
else valid_origin
NextAPI->>Auth: getUserIdFromCookie()
Auth-->>NextAPI: userId or null
alt no_user
NextAPI-->>Client: 401 JSON { error: Unauthorized }
else authenticated
NextAPI->>DB: Perform protected update
DB-->>NextAPI: Update result
NextAPI-->>Client: 200 JSON success response
end
end
Flow diagram for CSRF and auth checks in PATCH_profile_handlerflowchart TD
A[PATCH /api/profile handler invoked] --> B[Call isSameOriginRequest req]
B -->|false| C[Return 403 - error: Invalid CSRF origin]
B -->|true| D[Call getUserIdFromCookie]
D -->|no userId| E[Return 401 - error: Unauthorized]
D -->|userId present| F[Proceed with profile update logic]
F --> G[Update database and filesystem]
G --> H[Return 200 success response]
Flow diagram for CSRF and auth checks in account_email_handlersflowchart TD
A[POST or PATCH /api/account/email handler invoked] --> B[Call isSameOriginRequest req]
B -->|false| C["Return 403 { error: Invalid CSRF origin }"]
B -->|true| D[Call getUserIdFromCookie]
D -->|no userId| E["Return 401 { error: Unauthorized }"]
D -->|userId present| F[Execute email-specific logic]
F --> G[For POST: create token and send verification email]
F --> H[For PATCH: update email in database]
G --> I[Return 200 verification requested]
H --> J[Return 200 email updated]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider using a shared helper or constant for the CSRF failure response so the error shape and message are consistent across routes and easier to update in one place.
- You may want to align the error semantics of the CSRF failure (403 with a specific message) with your existing auth errors (e.g., 401/403 messaging) to avoid leaking whether a request failed due to origin vs. authentication.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using a shared helper or constant for the CSRF failure response so the error shape and message are consistent across routes and easier to update in one place.
- You may want to align the error semantics of the CSRF failure (403 with a specific message) with your existing auth errors (e.g., 401/403 messaging) to avoid leaking whether a request failed due to origin vs. authentication.
## Individual Comments
### Comment 1
<location path="app/api/account/email/request/route.ts" line_range="11-13" />
<code_context>
const schema = z.object({ email: z.string().email() })
export async function POST(req: NextRequest) {
+ if (!isSameOriginRequest(req)) return NextResponse.json({ error: 'Invalid CSRF origin' }, { status: 403 })
const userId = await getUserIdFromCookie()
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider using a more generic error message for CSRF failures to avoid leaking implementation details.
A specific message like `'Invalid CSRF origin'` reveals which validation step failed, which can help an attacker probe your CSRF protections. Prefer a generic body (e.g. `{ error: 'Forbidden' }` with status 403) and, if needed, log the detailed reason server-side instead.
```suggestion
export async function POST(req: NextRequest) {
if (!isSameOriginRequest(req)) {
console.warn('CSRF validation failed for email verification request', {
origin: req.headers.get('origin'),
referer: req.headers.get('referer'),
})
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
}
const userId = await getUserIdFromCookie()
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
11
to
13
| export async function POST(req: NextRequest) { | ||
| if (!isSameOriginRequest(req)) return NextResponse.json({ error: 'Invalid CSRF origin' }, { status: 403 }) | ||
| const userId = await getUserIdFromCookie() |
There was a problem hiding this comment.
🚨 suggestion (security): Consider using a more generic error message for CSRF failures to avoid leaking implementation details.
A specific message like 'Invalid CSRF origin' reveals which validation step failed, which can help an attacker probe your CSRF protections. Prefer a generic body (e.g. { error: 'Forbidden' } with status 403) and, if needed, log the detailed reason server-side instead.
Suggested change
| export async function POST(req: NextRequest) { | |
| if (!isSameOriginRequest(req)) return NextResponse.json({ error: 'Invalid CSRF origin' }, { status: 403 }) | |
| const userId = await getUserIdFromCookie() | |
| export async function POST(req: NextRequest) { | |
| if (!isSameOriginRequest(req)) { | |
| console.warn('CSRF validation failed for email verification request', { | |
| origin: req.headers.get('origin'), | |
| referer: req.headers.get('referer'), | |
| }) | |
| return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) | |
| } | |
| const userId = await getUserIdFromCookie() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Description
isSameOriginRequestfromlib/csrfand return403when the origin is invalid forPATCH /api/account/email.isSameOriginRequestfromlib/csrfand return403when the origin is invalid forPOST /api/account/email/request.isSameOriginRequestfromlib/csrfand return403when the origin is invalid forPATCH /api/profile.Testing
npm run lint -- app/api/account/email/route.ts app/api/account/email/request/route.ts app/api/profile/route.tsand it completed (repo contains unrelated lint warnings but no new errors for these changes).npm run typecheckand it succeeded.Codex Task
Summary by Sourcery
Bug Fixes: