Skip to content

fix(identity): send the user's token to /user, not the operator token - #814

Open
zmingxie wants to merge 1 commit into
mainfrom
mxie/identity-getuser-cookie
Open

zmingxie wants to merge 1 commit into
mainfrom
mxie/identity-getuser-cookie

Conversation

@zmingxie

@zmingxie zmingxie commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • On the server, getUser() sent netlifyIdentityContext.token to GoTrue's GET /user. That token is the operator token that admin.ts and getIdentityConfig use for admin calls, not the user's JWT. GoTrue's /user looks the user up by the token's sub (UserGet in netlify/gotrue), so the request failed and getUser() always fell back to the context's claims instead of returning the full user.
  • It now sends the nf_jwt cookie. When there is no cookie, or the cookie belongs to a different user than the context's verified claims, it skips the request and returns the claims, as it did before in effect.

The existing tests used a user JWT as the context token, so two of them now set an nf_jwt cookie and an operator token instead.

Test plan

  • npx vitest run in packages/identity/prod: 185 passed. Four of the new tests fail against main.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 38 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 839d846d-59cb-4db5-9870-099cdf1c1729
📥 Commits

Reviewing files that changed from the base of the PR and between 4e487aa and a196830.

📒 Files selected for processing: 2
  • packages/identity/prod/src/user.ts
  • packages/identity/prod/test/user.test.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 57d013c1-b1fd-4b2e-947d-34f92741653b
📥 Commits

Reviewing files that changed from the base of the PR and between 38377e8 and 4e487aa.

📒 Files selected for processing: 2
  • packages/identity/prod/src/user.ts
  • packages/identity/prod/test/user.test.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Improved server-side user identification by using the request’s user session when it matches the verified identity.
    • When the session is missing, does not match, or cannot be validated, the app now falls back to the verified identity instead of using a different session.

The server-side getUser path now uses the nf_jwt cookie for GoTrue requests. It fetches a full user when the cookie is present and its subject is eligible relative to the validated subject. Otherwise, or if the fetch fails, it returns the verified identity-context user. Tests cover the cookie token, successful and failed requests, missing cookies, and subject mismatches.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: khendrikse

Merge Risk: ⚪ Minimal · up to 4e487

The cookie-based lookup and verified-claims fallback have no identified merge-blocking issue; the change is ready for normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: using the user's token instead of the operator token for the GoTrue /user request.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The pull request description directly explains the change from the operator token to the nf_jwt cookie in server-side getUser(), the fallback behavior, and the related tests.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Linked repositories: Some configured repositories could not be included in this review. Cross-repository analysis may be incomplete. Please check linked-repository access and settings.

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@netlify/ai

npm i https://pkg.pr.new/@netlify/ai@814

@netlify/aws-lambda-compat

npm i https://pkg.pr.new/@netlify/aws-lambda-compat@814

@netlify/blobs

npm i https://pkg.pr.new/@netlify/blobs@814

@netlify/cache

npm i https://pkg.pr.new/@netlify/cache@814

@netlify/dev

npm i https://pkg.pr.new/@netlify/dev@814

@netlify/dev-utils

npm i https://pkg.pr.new/@netlify/dev-utils@814

@netlify/headers

npm i https://pkg.pr.new/@netlify/headers@814

@netlify/images

npm i https://pkg.pr.new/@netlify/images@814

@netlify/otel

npm i https://pkg.pr.new/@netlify/otel@814

@netlify/redirects

npm i https://pkg.pr.new/@netlify/redirects@814

@netlify/runtime

npm i https://pkg.pr.new/@netlify/runtime@814

@netlify/runtime-utils

npm i https://pkg.pr.new/@netlify/runtime-utils@814

@netlify/static

npm i https://pkg.pr.new/@netlify/static@814

@netlify/types

npm i https://pkg.pr.new/@netlify/types@814

@netlify/database-dev

npm i https://pkg.pr.new/@netlify/database-dev@814

@netlify/database

npm i https://pkg.pr.new/@netlify/database@814

@netlify/database-proxy

npm i https://pkg.pr.new/@netlify/database-proxy@814

@netlify/edge-functions-dev

npm i https://pkg.pr.new/@netlify/edge-functions-dev@814

@netlify/edge-functions

npm i https://pkg.pr.new/@netlify/edge-functions@814

@netlify/functions-dev

npm i https://pkg.pr.new/@netlify/functions-dev@814

@netlify/functions

npm i https://pkg.pr.new/@netlify/functions@814

@netlify/identity

npm i https://pkg.pr.new/@netlify/identity@814

@netlify/server-dev

npm i https://pkg.pr.new/@netlify/server-dev@814

@netlify/server

npm i https://pkg.pr.new/@netlify/server@814

commit: a196830

@zmingxie
zmingxie force-pushed the mxie/identity-getuser-cookie branch from 7befa0d to a196830 Compare October 22, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant