fix: avoid Clerk user cache collisions

[adityathebe]

Clerk Backend JWT template tokens can omit sid, which caused missing claims to be coerced into shared cache keys.

This could make sid-less direct/OIDC Clerk auth reuse another user's cached identity during MCP login. Validate org/user claims before cache lookup, namespace cache keys by org/session/user, short-cache sid-less tokens, and prefer the relayed Clerk callback token over ambient credentials.

Summary by CodeRabbit

• Bug Fixes
  • Made Clerk sign-in more reliable by validating session claims and correctly deriving identity, role, and organization context across different claim shapes.
  • Improved browser-login callback handling to reliably use the intended session token when multiple token sources are present.
  • Updated session caching to avoid cross-session data reuse, ensuring isolation between different identities.
• Tests
  • Added Ginkgo/Gomega coverage for session-claim parsing, callback token precedence, and cache isolation behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eae76139-bf53-4d67-8ab0-bcdab55d3044

📥 Commits

Reviewing files that changed from the base of the PR and between af54780 and c1e81ae.

📒 Files selected for processing (2)

• auth/clerk_client.go
• auth/clerk_client_test.go

---

Walkthrough

Clerk session authentication now parses multiple JWT claim shapes into user identity, updates RBAC from mapped roles, caches users with a shorter TTL, and prefers clerk_session_token from callback form data. Tests cover claim parsing, cache isolation, and token precedence.

Changes

Clerk session auth flow

Layer / File(s)	Summary
Claim parsing and cache lookup auth/clerk_client.go, auth/clerk_client_test.go	getUserFromSessionToken now delegates to getUserFromSessionClaims, which reads Clerk org and user claims, builds the cache key, and returns cached or newly constructed models.Person values. The tests cover missing claims and nested o claim shapes.
RBAC and missing-sid cache path auth/clerk_client.go, auth/clerk_client_test.go	clerkRole(claims) updates RBAC, and sessions without sid use the Clerk user cache key path with the fixed TTL. The test exercises separate SID-less cache entries across users.
Callback token precedence auth/clerk_client.go, auth/clerk_client_test.go	CallbackSubject reads clerk_session_token from the callback form before falling back to extractSessionToken, and the tests cover form-field precedence and query-parameter fallback behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main fix: preventing Clerk user cache collisions.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/clerk-session-cache

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/clerk-session-cache

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@auth/clerk_client.go`:
- Around line 192-193: The RBAC update path in clerk auth is returning the raw
error from updateRole, which drops Clerk/user request context. In the auth flow
around updateRole and clerkRole, wrap the failure with ctx.Oops().Wrapf(err,
"failed to update role for Clerk user %s", dbUser.ID.String()) (or equivalent
request context) before returning, so AuthResult failures carry actionable
context while preserving the original error.
- Around line 160-170: The Clerk claims validation in the auth flow is returning
plain fmt.Errorf values, which loses the EINVALID contract for malformed input.
Update the error returns in the validation path around clerkOrgID(claims),
clerkUserID(claims), and the orgID checks to use
dutyAPI.Errorf(dutyAPI.EINVALID, "...") instead of fmt.Errorf so callers receive
typed validation errors consistently.
- Around line 461-465: The callbackSessionToken path is reading
clerk_session_token too broadly via ec.FormValue, which allows query-string
values to be accepted outside the POST body. Update
ClerkCredentialChecker.callbackSessionToken to only read the token from the POST
body using the echo context’s POST-form accessor, then fall back to
handler.extractSessionToken(ec) when absent. Add a regression test covering a
query-string clerk_session_token to ensure it is ignored.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 36f5c52d-c9e3-407e-8672-52813b88c7f3

📥 Commits

Reviewing files that changed from the base of the PR and between 2e103d7 and af54780.

📒 Files selected for processing (2)

• auth/clerk_client.go
• auth/clerk_client_test.go