feat: OIDC federation (exchange API, dashboard, audit)#1360
Draft
mantrakp04 wants to merge 19 commits intodevfrom
Draft
feat: OIDC federation (exchange API, dashboard, audit)#1360mantrakp04 wants to merge 19 commits intodevfrom
mantrakp04 wants to merge 19 commits intodevfrom
Conversation
- Add exchange route, JWT verification, and server access token helpers - Prisma migration and schema for OIDC federation audit events - Dashboard OIDC federation policy UI and sidebar link - stack-shared config/schema and oidc-federation utilities - Template StackServerApp integration for federation token exchange - E2E and unit tests; Mintlify guide for OIDC federation Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OIDC federation: discovery/JWKS caching and validation, RFC‑8693 token-exchange endpoint, server access token mint/verify and auth-path integration, audit DB migration and writes, dashboard policy UI and validation, SDK token-store integration, mock local IdP and demo, tests, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/SDK
participant Backend as Stack Backend
participant IdP as OIDC IdP
participant DB as Postgres
participant Cache as Discovery/JWKS Cache
rect rgba(0, 150, 200, 0.5)
Note over Client,IdP: OIDC Federation Exchange Flow
Client->>IdP: Obtain subject JWT
Client->>Backend: POST /api/v1/auth/oidc-federation/exchange (subject_token)
Backend->>IdP: GET /.well-known/openid-configuration
IdP-->>Backend: { issuer, jwks_uri }
Backend->>Cache: Check discovery + JWKS cache
alt cache miss or invalidated
Backend->>IdP: GET jwks_uri
IdP-->>Backend: { keys }
Backend->>Cache: Store JWKS
end
Backend->>Backend: Verify JWT signature & claims
Backend->>Backend: Match claim conditions
alt policy matches
Backend->>Backend: Mint server access token
Backend->>DB: Insert audit (outcome=success)
Backend-->>Client: 200 { access_token, expires_in }
else no match
Backend->>DB: Insert audit (outcome=failure)
Backend-->>Client: 400 invalid_request
end
end
rect rgba(100, 200, 100, 0.5)
Note over Client,Backend: Subsequent server requests
Client->>Backend: Request with x-stack-server-access-token
Backend->>Backend: Verify server access token (scope/project/branch)
Backend-->>Client: Resource response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
Greptile SummaryThis PR adds OIDC federation — an RFC 8693 token-exchange endpoint that lets workloads swap IdP-issued JWTs (Vercel, GitHub Actions, GCP, etc.) for short-lived Stack server access tokens, replacing long-lived secret server keys. The implementation covers the full stack: backend exchange + probe-discovery APIs, SSRF-safe outbound fetching, DB-backed discovery/JWKS caching with key-rotation retry, dashboard policy wizard, SDK token store with proactive refresh and in-flight deduplication, audit logging, and a mock IdP for local development.
Confidence Score: 4/5Safe to merge after fixing the empty-audience filter in the exchange handler. One P1 finding: empty audience strings are not filtered before being passed to jose, allowing a blank audience row to effectively disable the audience check for matching tokens. All other components (SSRF protection, JWT verification, caching, branch enforcement, audit logging) are implemented correctly. Previously raised P1s appear addressed in the current diff. apps/backend/src/app/api/latest/auth/oidc-federation/exchange/route.tsx — empty audience filter on line 125
|
| Filename | Overview |
|---|---|
| apps/backend/src/app/api/latest/auth/oidc-federation/exchange/route.tsx | New RFC 8693 token-exchange endpoint; solid overall, but empty audience strings are not filtered — could allow policy misconfiguration to accept tokens with an empty aud claim |
| apps/backend/src/lib/safe-fetch.ts | New SSRF-safe fetch utility with DNS pre-resolution, IP blocklist, IPv4-mapped IPv6 normalization, and pinned undici dispatcher; implementation is thorough and correct |
| apps/backend/src/lib/oidc-jwt.tsx | New OIDC JWT validation library with DB-backed discovery/JWKS caching, key-rotation retry, issuer mismatch guard, and SSRF-safe URL validation; logic is sound |
| apps/backend/src/lib/server-access-token.tsx | New server access token mint/verify helpers; issuer, audience, and project ID are all bound to the specific project, preventing cross-project reuse |
| apps/backend/src/route-handlers/smart-request.tsx | Adds x-stack-server-access-token support; branch derivation from token, mismatch enforcement, and error types are all handled correctly |
| apps/backend/src/app/api/latest/internal/oidc-federation/probe-discovery/route.tsx | Admin-only discovery probe; skips the DB cache so every call triggers a live outbound HTTP request — could generate significant external traffic under heavy use |
| packages/stack-shared/src/config/schema.ts | Adds OIDC federation config schema; issuer URL HTTPS validation is thorough, but claim-name keys lack a max-length bound |
| packages/template/src/lib/stack-app/oidc-federation/index.ts | Token store with proactive refresh, negative caching, and JS single-threaded in-flight deduplication; concurrency logic is correct |
| apps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/migration.sql | Clean migration adding OidcFederationExchangeAudit table with composite index and NOT VALID outcome check constraint |
Sequence Diagram
sequenceDiagram
participant W as Workload
participant BE as Backend /exchange
participant IdP as OIDC IdP
participant DB as Postgres (CacheEntry)
participant SDK as Template SDK
W->>SDK: getAccessToken()
SDK->>SDK: cached & fresh? return early
SDK->>IdP: getOidcToken()
IdP-->>SDK: subject_token (OIDC JWT)
SDK->>BE: POST /auth/oidc-federation/exchange
BE->>DB: loadDiscovery(issuerUrl)
DB-->>BE: discovery doc (cache hit or miss)
BE->>IdP: GET /.well-known/openid-configuration
IdP-->>BE: issuer + jwks_uri
BE->>DB: loadJwks(jwks_uri)
BE->>IdP: GET /jwks (on cache miss)
IdP-->>BE: keys
BE->>BE: jwtVerify + matchClaims
BE->>BE: mintServerAccessToken
BE->>DB: writeAudit
BE-->>SDK: access_token + expires_in
SDK->>SDK: cache token (refresh at 80% TTL)
SDK-->>W: access_token
W->>BE: API call with x-stack-server-access-token
BE->>BE: verifyServerAccessToken
BE-->>W: response
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/backend/src/app/api/latest/auth/oidc-federation/exchange/route.tsx
Line: 125-129
Comment:
**Empty audience strings are not rejected**
`Object.values(policy.audiences ?? {}).filter((v): v is string => typeof v === "string")` keeps empty-string values (`""`). When `audiences = [""]`, jose's `jwtVerify` will accept any token whose `aud` claim is also `""`. An admin who accidentally leaves an audience row blank ends up with a policy that can be satisfied by a zero-length audience, making the audience check vacuous for those matching tokens.
```suggestion
const audiences = Object.values(policy.audiences ?? {}).filter((v): v is string => typeof v === "string" && v.length > 0);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/backend/src/app/api/latest/internal/oidc-federation/probe-discovery/route.tsx
Line: 41-43
Comment:
**Probe bypasses DB cache on every call**
`fetchOidcDiscoveryDocument` performs a fresh DNS lookup and HTTP request each invocation — it does not read or write the Prisma `CacheEntry` table that the exchange path uses. A dashboard user who rapidly clicks "Test Discovery" (or a script iterating policies) will fire an unbounded sequence of outbound OIDC requests from the backend. Consider calling the cached `loadDiscovery` variant, or rate-limiting this endpoint, to avoid inadvertently load-testing third-party OIDC providers.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/stack-shared/src/config/schema.ts
Line: 117-120
Comment:
**Claim-name keys have no length constraint**
The outer key schema for `stringEquals` and `stringLike` is a bare `yupString()` with no `.max()`. Claim names are stored in the project config and iterated on every exchange request. A very long claim name won't be caught at save time. A short max (e.g. 256 characters, matching typical JWT claim name conventions) would give admins an early validation error and bound the per-policy scanning cost.
How can I resolve this? If you propose a fix, please make it concise.Reviews (8): Last reviewed commit: "fix oidc federation review feedback" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds OIDC federation across backend + SDK + dashboard so workloads can exchange IdP-issued OIDC JWTs for short-lived Stack server access tokens (RFC 8693), with audit/event logging, config schema, docs, and some operational/dev tooling improvements.
Changes:
- Backend: introduce
/api/v1/auth/oidc-federation/exchange, OIDC JWT verification + Stack server-access-token mint/verify, and audit/event logging. - SDK/dashboard: add OIDC federation config schema + dashboard policy editor + template SDK helpers for token exchange/caching.
- Ops/docs: Next.js dev-runtime postinstall patch for async debug info, plus substantial documentation additions/updates.
Reviewed changes
Copilot reviewed 72 out of 73 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdks/spec/src/_utilities.spec.md | Clarifies x-stack-override-error-status semantics (4xx only). |
| scripts/postinstall-patch-next-async-debug-info.mjs | Install-time patching of Next dev runtimes to gate async debug info hook. |
| pnpm-lock.yaml | Lockfile updates (notably @types/node changes, dependency graph adjustments). |
| packages/template/src/lib/stack-app/oidc-federation/index.ts | Adds token exchange client + token source helpers (Vercel/GHA/GCP/custom). |
| packages/template/src/lib/stack-app/oidc-federation/index.test.ts | Tests server-app branch header passthrough during exchange. |
| packages/template/src/lib/stack-app/index.ts | Re-exports OIDC federation helpers/types from template SDK entrypoint. |
| packages/template/src/lib/stack-app/apps/interfaces/server-app.ts | Adds auth.oidcFederation option to StackServerApp constructor types. |
| packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts | Wires OIDC federation token store into StackServerInterface construction. |
| packages/template/src/lib/env.ts | Adds env accessors for Vercel/GitHub Actions OIDC env vars. |
| packages/stack-shared/src/utils/oidc-federation.ts | Implements claim matching (StringEquals/StringLike) utilities. |
| packages/stack-shared/src/utils/oidc-federation.test.ts | Unit tests for claim matching + wildcard behavior. |
| packages/stack-shared/src/interface/server-interface.ts | Adds oidcFederation auth mode for server requests. |
| packages/stack-shared/src/helpers/init-prompt.ts | Moves init prompt template to shared package for reuse. |
| packages/stack-shared/src/config/schema.ts | Adds OIDC federation config schema + defaults. |
| packages/stack-shared/src/config/schema-fuzzer.test.ts | Extends config fuzzer to cover OIDC federation config shapes. |
| packages/stack-cli/src/lib/init-prompt.ts | Re-exports shared init prompt for CLI usage. |
| package.json | Adds root postinstall hook to run Next runtime patch script. |
| docs-mintlify/guides/other/tutorials/ship-production-ready-auth.mdx | Replaces stub with full production hardening guide. |
| docs-mintlify/guides/other/tutorials/build-a-team-based-app.mdx | Replaces stub with full teams/RBAC tutorial. |
| docs-mintlify/guides/other/tutorials/build-a-saas-with-stack-auth.mdx | Replaces stub with full SaaS onboarding/auth tutorial. |
| docs-mintlify/guides/going-further/oidc-federation.mdx | Adds OIDC federation guide + SDK usage snippets. |
| docs-mintlify/docs.json | Adds OIDC federation guide to navigation. |
| docker/server/Dockerfile | Ensures postinstall patch script exists during pruned docker installs. |
| docker/local-emulator/Dockerfile | Ensures postinstall patch script exists during pruned docker installs. |
| docker/backend/Dockerfile | Ensures postinstall patch script exists during pruned docker installs. |
| claude/CLAUDE-KNOWLEDGE.md | Documents new behaviors (override status semantics, Next runtime patch rationale, docker prune fix). |
| apps/internal-tool/scripts/pre-dev.mjs | Makes Spacetime publish best-effort so dev server still boots. |
| apps/e2e/tests/backend/endpoints/api/v1/auth/oidc-federation/exchange.test.ts | Adds E2E coverage for exchange flow, policy matching, branch binding, no admin escalation. |
| apps/e2e/tests/backend/endpoints/api/v1/snapshots/internal-metrics.test.ts.snap | Updates metrics snapshot to match new split logic. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx | Adds sidebar entry + expansion matching for OIDC federation settings. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/oidc-federation/policy-form.ts | Implements draft<->policy transforms, validation, and issuer discovery probe. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/oidc-federation/policy-form.test.ts | Unit tests for policy form transforms/validation/discovery probe. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/oidc-federation/page.tsx | Adds OIDC federation settings page shell. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/oidc-federation/page-client.tsx | Adds OIDC federation UI (presets, policy editor, discovery, snippet). |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/page-client.tsx | Responsive layout tweak (grid columns). |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsx | Responsive layout tweaks (truncate/shrink behavior). |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx | Adds human-readable managed domain status labels. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-sent/page-client.tsx | Responsive layout tweak (stack on small screens). |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-sent/domain-reputation-card.tsx | Responsive width + improved boosted-capacity label/tooltips. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/draft-progress-bar.tsx | Responsive spacing tweaks. |
| apps/backend/src/route-handlers/smart-response.tsx | Changes override status behavior to only coerce 4xx to 200. |
| apps/backend/src/route-handlers/smart-request.tsx | Adds x-stack-server-access-token auth path and branch binding enforcement. |
| apps/backend/src/private/index.ts | Exposes preprocessProxyBody from private implementation. |
| apps/backend/src/private/implementation-fallback/index.ts | Adds fallback no-op preprocessProxyBody. |
| apps/backend/src/polyfills.tsx | Improves Sentry sink behavior and flushes via waitUntil tracking. |
| apps/backend/src/lib/tokens.tsx | Downgrades expired token logging from warn to log. |
| apps/backend/src/lib/server-access-token.tsx | Implements mint/verify for Stack-issued server access tokens. |
| apps/backend/src/lib/server-access-token.test.tsx | Unit tests for server access token TTL clamp + verify behavior. |
| apps/backend/src/lib/oidc-jwt.tsx | Implements OIDC discovery + JWKS verification with caching + errors. |
| apps/backend/src/lib/oidc-jwt.test.tsx | Unit tests for OIDC JWT validation (aud, exp, signature, discovery mismatch). |
| apps/backend/src/lib/events.tsx | Adds system event type for OIDC federation exchange. |
| apps/backend/src/lib/email-queue-step.tsx | Improves stuck-in-sending diagnostics payload. |
| apps/backend/src/lib/clickhouse.tsx | Increases ClickHouse request timeout. |
| apps/backend/src/lib/ai/proxy-preprocessing.ts | Defines public type for AI proxy preprocessing hook. |
| apps/backend/src/lib/ai/models.ts | Adds ability to instantiate OpenRouter provider with a direct API key. |
| apps/backend/src/app/api/latest/internal/metrics/route.tsx | Refactors activity split logic into ClickHouse-only computation + MAU improvements. |
| apps/backend/src/app/api/latest/integrations/ai-proxy/[[...path]]/route.ts | Adds preprocessProxyBody hook to sanitize pipeline. |
| apps/backend/src/app/api/latest/auth/oidc-federation/exchange/route.tsx | Adds OIDC federation exchange endpoint with audit + event logging. |
| apps/backend/src/app/api/latest/ai/query/[mode]/route.ts | Uses an authenticated OpenRouter API key when available. |
| apps/backend/prisma/schema.prisma | Adds OidcFederationExchangeAudit model. |
| apps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/tests/shape-and-index.ts | Migration-level test for audit table/index/query shape. |
| apps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/migration.sql | Creates audit table + index. |
| apps/backend/package.json | Sets STACK_DISABLE_REACT_ASYNC_DEBUG_INFO default true for backend dev. |
| .agents/skills/pr-visual-writeup/scripts/upload_gist.sh | Adds helper script for gist-based asset upload. |
| .agents/skills/pr-visual-writeup/scripts/detect_dev_server.sh | Adds helper script to detect dev server ports/titles. |
| .agents/skills/pr-visual-writeup/scripts/convert_clips.sh | Adds helper to convert WebM clips to GIFs. |
| .agents/skills/pr-visual-writeup/references/pr-body-template.md | Adds reference PR body template for visual writeups. |
| .agents/skills/pr-visual-writeup/references/gist-upload.md | Adds reference doc for gist upload mechanism. |
| .agents/skills/pr-visual-writeup/references/capture-patterns.md | Adds reference doc for screenshot capture patterns. |
| .agents/skills/pr-visual-writeup/SKILL.md | Adds skill definition + workflow docs for visual PR writeups. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Introduced a new mock OIDC Identity Provider in `apps/mock-oidc-idp` for OIDC federation testing. - Updated `package.json` scripts to include the new mock server. - Enhanced backend seed script to integrate with the mock IdP for trust policy setup. - Added demo pages and API routes for minting and exchanging tokens with the mock IdP. - Updated environment configurations to support the new mock IdP. This setup facilitates local testing of OIDC federation features without external dependencies.
- Eliminated the DesignBadge component and associated icons (CheckCircleIcon, XCircleIcon) from the PolicyCard in the OIDC federation dashboard. - This cleanup reduces unnecessary imports and streamlines the component's code.
…ndling - Added a new endpoint for probing OIDC discovery, allowing clients to validate issuer URLs and retrieve JWKS URIs. - Refactored claim conditions handling to use JSON format instead of individual claim rows, simplifying the policy draft structure. - Updated related functions and tests to accommodate the new claim conditions format and ensure compatibility with existing features. - Enhanced caching mechanism for discovery responses to improve performance and reduce redundant network calls.
- Removed the OIDC Federation section from the project settings sidebar. - Updated sidebar logic to reflect the removal, ensuring the project settings state is correctly initialized without the OIDC Federation match. - Introduced a new OIDC policy dialog component for managing trust policies, enhancing the user interface for OIDC federation management. - Added new policy form and validation logic to support the creation and editing of trust policies. - Implemented tests for the new policy form functionality to ensure reliability and correctness.
…ents - Adjusted layout of the OIDC policy dialog to improve spacing and alignment. - Removed the SectionLabel component from various sections to streamline the UI. - Updated the audience section to align content to the right for better visual consistency. - Enhanced overall readability and maintainability of the code by eliminating unnecessary elements.
- Updated error handling in the OIDC exchange route to provide clearer error messages for missing project or branch. - Refactored the JWT validation process to include Prisma client transactions for improved caching and error management. - Introduced new functions for managing discovery and JWKS caching, enhancing performance and reliability. - Improved overall code structure and readability by consolidating related logic and removing unnecessary components.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (7)
apps/backend/src/lib/seed-dummy-data.ts (1)
1915-1937: Use path notation for the seeded OIDC policy.This nested override can replace sibling
oidcFederationconfig/trust policies on seed reruns. Set only the intended policy path instead.♻️ Proposed refactor
- oidcFederation: { - // Default trust policy pointing at the local mock OIDC IdP - // (apps/mock-oidc-idp). The demo at `examples/demo/oidc-federation-demo` - // mints tokens from this IdP and exchanges them here. - trustPolicies: { - "mock-idp-demo": { - displayName: "Mock IdP (local dev)", - enabled: true, - issuerUrl: process.env.STACK_MOCK_OIDC_ISSUER_URL - ?? `http://localhost:${process.env.NEXT_PUBLIC_STACK_PORT_PREFIX ?? "81"}15`, - audiences: { - default: "stack-demo", - }, - claimConditions: { - stringLike: { - sub: { demo: "workload:*" }, - }, - stringEquals: {}, - }, - tokenTtlSeconds: 900, - }, - }, + // Default trust policy pointing at the local mock OIDC IdP + // (apps/mock-oidc-idp). The demo at `examples/demo/oidc-federation-demo` + // mints tokens from this IdP and exchanges them here. + "oidcFederation.trustPolicies.mock-idp-demo": { + displayName: "Mock IdP (local dev)", + enabled: true, + issuerUrl: process.env.STACK_MOCK_OIDC_ISSUER_URL + ?? `http://localhost:${process.env.NEXT_PUBLIC_STACK_PORT_PREFIX ?? "81"}15`, + audiences: { + default: "stack-demo", + }, + claimConditions: { + stringLike: { + sub: { demo: "workload:*" }, + }, + stringEquals: {}, + }, + tokenTtlSeconds: 900, },As per coding guidelines, when making config updates, use path notation (
{ "path.to.field": my-value }) to avoid overwriting sibling properties.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/seed-dummy-data.ts` around lines 1915 - 1937, The seed currently replaces the entire oidcFederation tree; instead update only the intended policy using path notation so siblings are preserved—set the key "oidcFederation.trustPolicies.mock-idp-demo" to the policy object (preserving displayName, enabled, issuerUrl expression, audiences, claimConditions, tokenTtlSeconds) rather than assigning a full oidcFederation object; locate the code that creates the seed object (refs: oidcFederation, trustPolicies, "mock-idp-demo") and change it to use the single-path update format { "oidcFederation.trustPolicies.mock-idp-demo": { ...policy... } } for seeding.apps/mock-oidc-idp/package.json (1)
14-21: Move@types/expresstodevDependencies; declarerimrafexplicitly.
@types/expressis a type-only package; it belongs indevDependencies. Keeping it independenciespollutes runtime dep graphs and SBOMs for anything that happens to depend on this package.- The
cleanscript invokesrimraf, but it isn't listed in this package's deps — it only works because of workspace hoisting. Adding it explicitly avoids breakage if hoisting changes.Proposed fix
"dependencies": { - "@types/express": "^5.0.0", "express": "^4.21.2", "jose": "^6.1.3" }, "devDependencies": { + "@types/express": "^5.0.0", + "rimraf": "^5.0.0", "tsx": "^4.16.2" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mock-oidc-idp/package.json` around lines 14 - 21, Update package.json so that the type-only package "@types/express" is removed from "dependencies" and added to "devDependencies", and add "rimraf" to "devDependencies" (since the "clean" script invokes rimraf) to avoid relying on workspace hoisting; ensure you update the "dependencies" and "devDependencies" sections accordingly and run npm/yarn/pnpm install to refresh the lockfile.packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)
440-449: Minor:apiBaseUrlcaptured eagerly whilegetBaseUrlis lazy.
apiBaseUrl: apiUrls()[0]snapshots the base URL at construction time, but the rest of the interface usesgetBaseUrl: () => apiUrls()[0]lazily. IfresolveApiUrlsever produces values that change over the app's lifetime (e.g., dynamic env-based resolution), the federation store would exchange tokens against a stale URL. Consider passing a getter (e.g.,getApiBaseUrl: () => apiUrls()[0]) intocreateOidcFederationTokenStoreForServerAppfor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts` around lines 440 - 449, The federation store is being given a snapshot apiBaseUrl via apiUrls()[0] while the rest of the interface uses lazy getBaseUrl; change the call to createOidcFederationTokenStoreForServerApp to accept a getter instead (e.g., pass getApiBaseUrl: () => apiUrls()[0] or rename to match existing param expected by createOidcFederationTokenStoreForServerApp) so the store resolves the base URL lazily; update the StackServerInterface construction to pass the matching lazy field (oidcFederation created with the getter) and ensure the parameter name aligns with createOidcFederationTokenStoreForServerApp's signature.apps/backend/src/route-handlers/smart-request.tsx (1)
248-258: Control flow LGTM; one minor consistency note.The verify + branch-consistency +
effectiveBranchIdlogic is correct:
- Verification is only attempted when all three (
projectId,serverAccessToken,requestType === "server") are present.- The
branchIdHeader != nullguard means the assertion only fires when the client explicitly set the header (not theDEFAULT_BRANCH_IDfallback), so clients that omit the branch header don't get spurious 401s.- If verification errored,
effectiveBranchIdfalls back torequestedBranchId; bundled queries run against that, and the verification error is thrown at line 318 before any mismatched-branch tenancy leaks to the caller.One nit: the 401 at line 254 and line 187 is a
StatusError, whereas the surrounding auth rejections (e.g.,KnownErrors.UnparsableAccessToken,KnownErrors.InvalidSecretServerKey) areKnownErrors. Consider aKnownErrorvariant (or reuseUnparsableAccessTokenwith a distinct reason) so SDK consumers can programmatically differentiate "branch mismatch" from a generic 401.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/route-handlers/smart-request.tsx` around lines 248 - 258, The branch-mismatch currently throws a StatusError (at the branch check using verifyServerAccessToken, branchIdHeader, and effectiveBranchId), but surrounding auth failures use KnownErrors (e.g., KnownErrors.UnparsableAccessToken, KnownErrors.InvalidSecretServerKey); change the branch-mismatch to emit a KnownError variant so SDKs can detect it programmatically — either add a new KnownErrors.BranchMismatch (preferred) or reuse KnownErrors.UnparsableAccessToken with a clear distinct reason string, and update the throw site that currently constructs a StatusError to throw the appropriate KnownError instead while keeping the same message text for logs.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-keys/page-client.tsx (1)
135-179: Theeslint-disable-next-line react-hooks/exhaustive-depsis fragile — consider memoizing the policy callbacks.The
columnsmemo capturessavePolicy,deletePolicy,togglePolicy, andsetPolicyDialogin cell closures but only lists[requirePublishableClientKey]as a dep.savePolicy/deletePolicy/togglePolicyare re-created on every render, so the closures here reference the first render's versions. This happens to work today because those functions only close overstackAdminApp/updateConfig(both stable refs), but the disable hides that invariant from future maintainers and TypeScript won't catch a regression if someone adds another captured value.Wrap the three mutators in
useCallback(deps:[stackAdminApp, updateConfig]) and add them to the memo's deps; then the disable comment goes away.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-keys/page-client.tsx around lines 135 - 179, The columns memo captures mutable callbacks (savePolicy, deletePolicy, togglePolicy, setPolicyDialog) but only depends on requirePublishableClientKey; wrap savePolicy, deletePolicy and togglePolicy in useCallback with deps [stackAdminApp, updateConfig] (or whatever stable refs they use), keep setPolicyDialog stable (or also memoize via useCallback if needed), then include these callbacks in the dependency array for the columns useMemo (so columns depends on [requirePublishableClientKey, savePolicy, deletePolicy, togglePolicy, setPolicyDialog]); remove the eslint-disable-next-line react-hooks/exhaustive-deps comment after ensuring all captured functions are memoized.apps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/migration.sql (1)
1-16: Optional: consider FK ontenancyIdand CHECK onoutcome.Two lightweight hardening opportunities, both deferrable:
tenancyId UUIDhas no FK. If the intent is for audit rows to survive tenancy deletion (common for compliance audit trails), leaving it unconstrained is fine — but please document that decision. Otherwise anREFERENCES "Tenancy"("id") ON DELETE CASCADE(orSET NULLif the column were nullable) would prevent orphaned rows.outcome TEXTaccepts any string, yet the event schema inapps/backend/src/lib/events.tsx(OidcFederationExchangeEventType.dataSchema) restricts it to'success' | 'failure'. ACHECK ("outcome" IN ('success','failure'))would prevent schema drift. Per coding guidelines, addCHECK ... NOT VALIDin this migration and a separate follow-up migration withVALIDATE CONSTRAINT(not strictly required here since the table is new and empty, but worth being consistent with the project's migration pattern).As per coding guidelines: "When adding CHECK constraints, use
NOT VALIDin one migration, thenVALIDATE CONSTRAINTin a separate migration file".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/migration.sql` around lines 1 - 16, Add a foreign key on OidcFederationExchangeAudit.tenancyId referencing Tenancy(id) and add a CHECK constraint on OidcFederationExchangeAudit.outcome limiting values to 'success' or 'failure'; name the constraints clearly (e.g., OidcFederationExchangeAudit_tenancy_fkey and OidcFederationExchangeAudit_outcome_check), add the CHECK with NOT VALID in this migration per project policy, and plan a follow-up migration that issues VALIDATE CONSTRAINT for the outcome check; if you intentionally want audit rows to survive tenancy deletion, document that decision instead of adding the FK.apps/mock-oidc-idp/src/index.ts (1)
110-119: Avoidvoiding the startup promise.Line 110 silences the startup promise with
void. Please route this through the project’s approved async handling while preserving fatal startup logging/exit behavior for this standalone process.As per coding guidelines, “NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error) (or similar). Use
runAsynchronouslyorrunAsynchronouslyWithAlertinstead as it deals with error logging.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mock-oidc-idp/src/index.ts` around lines 110 - 119, The startup promise is being silenced via "void main().then(...)" — replace this pattern by calling the approved helper that handles async startup errors (use runAsynchronouslyWithAlert(main) to preserve fatal logging/exit semantics; alternatively runAsynchronously(main) if alerts aren’t needed). Remove the manual .then error handler and eslint-disable, and rely on runAsynchronouslyWithAlert to log the error and exit; locate the current call site of main() in index.ts and swap it to the helper invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/tests/shape-and-index.ts`:
- Around line 16-41: The test currently only checks column metadata (columnRows,
byName) but never exercises the createdAt column default; add a test that
inserts a row into OidcFederationExchangeAudit omitting the createdAt field
(e.g., use the same helper DB/seed logic or a direct INSERT in
shape-and-index.ts), then query that row and assert createdAt is populated (not
null) and is approximately now (or within a small delta), thereby failing if the
DEFAULT now() was removed; apply the same change to the similar check referenced
at lines 59-66 so the migration test actually verifies the default behavior
rather than just the schema.
In `@apps/backend/src/app/api/latest/auth/oidc-federation/exchange/route.tsx`:
- Around line 58-61: The request schema currently allows requested_token_type,
audience, resource, and scope but the handler always returns a project server
access token; either reject unsupported options or remove them from the accepted
shape. Update the validation for the schema (the yup schema that defines
requested_token_type, audience, resource, scope) so that requested_token_type is
either absent or equals "urn:ietf:params:oauth:token-type:access_token" and that
audience/resource/scope are either absent or match the exact project-server
values you issue (or otherwise return a 400 with a clear error). Alternatively,
remove those optional fields entirely from the schema used by the POST handler
so clients cannot pass them; apply the same change to the duplicate schema block
around the other occurrence (lines 74-77 in the diff).
In
`@apps/backend/src/app/api/latest/internal/oidc-federation/probe-discovery/route.tsx`:
- Around line 38-69: The probe currently constructs discoveryUrl, validates only
syntax with new URL and then fetches it (discoveryUrl, new URL(...), fetch(...))
which enables SSRF; change it to require only https: scheme (reject if
url.protocol !== "https:"), perform a DNS lookup for the hostname (use
dns.resolve/lookup for the host from the URL) and reject any resolved IP that is
loopback, RFC1918/private, link-local, CGNAT or IPv4-mapped-IPv6 ranges, then
pin the fetch to the resolved safe IP (use an Agent that connects to that IP
while preserving the original Host header) to avoid DNS rebinding, and finally
do not leak raw errors or HTTP status — replace error responses from the fetch
block and catch block with a generic { error: "discovery failed" } while still
logging full details server-side.
In `@apps/backend/src/lib/oidc-jwt.tsx`:
- Around line 46-63: Validate and constrain network targets before performing
server-side fetches in loadDiscovery: before calling fetch with
`${cacheKey}/.well-known/openid-configuration` (and similarly before fetching
the returned jwks_uri later), ensure the URL derived from issuerUrl is
normalized and uses https in production, resolve its hostname to IPs and reject
any localhost, loopback, link‑local, private RFC1918 ranges or cloud metadata IP
ranges; if DNS resolution fails or any resolved address is disallowed, throw and
do not call fetch. Implement the same validation routine (e.g.,
validateIssuerUrl/validateJwksUrl) and call it for issuerUrl and for the
discovery doc's jwks_uri after parsing, and keep using DISCOVERY_NAMESPACE,
PrismaClientTransaction, FETCH_TIMEOUT_MS, and loadDiscovery as the locations to
add these checks.
In `@apps/backend/src/lib/server-access-token.tsx`:
- Line 3: Replace throws of StackAssertionError in the access token verification
logic with returning an UnparsableAccessToken result so malformed signed
payloads produce a 401-style failure rather than bubbling as a 500; locate the
verification function(s) in server-access-token.tsx (the code paths that
currently throw StackAssertionError and the blocks around lines 88-98), catch or
detect malformed signed payloads there and return an instance/value of
UnparsableAccessToken instead of throwing StackAssertionError, ensuring callers
(e.g., smart-request.tsx) receive the intended auth-failure result.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-keys/oidc-policy-dialog.tsx:
- Line 474: The onChange currently masks invalid TTLs by using
Number(e.target.value) || 900; change it so props.setDraft stores the
raw/parsing-aware value instead of silently falling back to 900 — e.g. parse the
input with parseInt or Number, treat an empty string or non-numeric as
undefined/NaN (so validateDraft can detect and show an error), and preserve
legitimate 0 values; update the handler referenced (props.setDraft and
tokenTtlSeconds) to set tokenTtlSeconds to the parsed value or undefined rather
than using the || 900 fallback so validateDraft can surface TTL validation
errors.
- Around line 179-187: The runDiscovery function can leave discoveryState stuck
at { kind: "loading" } if props.adminApp.probeOidcDiscovery throws; wrap the
asynchronous probe call in a try/catch (or use .catch) inside runDiscovery so
any thrown error clears or sets discoveryState to an error/idle state, and
ensure the catch block also rethrows or forwards the error to
runAsynchronouslyWithAlert if needed; update runDiscovery (referencing
runDiscovery, setDiscoveryState, and props.adminApp.probeOidcDiscovery) to
always set discoveryState out of "loading" in both success, controlled error
result, and exception paths.
- Around line 152-156: The component initializes
draft/preset/step/discoveryState only once with useState(props.initial), so
reopening the dialog for a different policy can leave stale state; add a
useEffect that runs when props.initial (or the dialog "open" prop if present)
changes and inside call setDraft(props.initial), setPreset("custom"),
setStep("identity"), setDiscoveryState({ kind: "idle" }), and setSaving(false)
to reset those values; reference the existing state setters (setDraft,
setPreset, setStep, setDiscoveryState, setSaving) and props.initial in the
effect dependency array.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-keys/oidc-policy-form.ts:
- Around line 123-124: The current code silently falls back to empty claim
conditions when parseClaimConditionsJson(draft.claimConditionsJson) fails, which
broadens trust; instead, detect parseResult.kind !== "ok" and do not produce a
permissive policy: propagate the parse error back to the caller (e.g., make
draftToPolicy return a failure/throw or mark the draft invalid) so callers must
call validateDraft or handle the error; update the logic around parseResult /
parsed (and any callers of draftToPolicy) to fail closed on invalid JSON rather
than using the empty { stringEquals: {}, stringLike: {} } fallback.
- Around line 57-65: In parseClaimSection, values are checked for length before
trimming and filtering, which allows [""] or [" "] to persist and retains
surrounding whitespace; change the logic to (after verifying
isStringArray(values)) map each value to v.trim(), filter out empty strings,
then only set out[claim] if the resulting array has length > 0. Refer to
parseClaimSection and isStringArray to locate and modify the code accordingly.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-keys/page-client.tsx:
- Around line 87-109: The code mutates InternalApiKey instances by doing
Object.assign(apiKey, { status }) inside the useMemo that builds rows from
apiKeySets; instead, avoid mutating the React state object — keep status only on
the KeyRow (already present) and pass the original apiKey reference unchanged.
Change the mapping that creates KeyRow (in the useMemo over apiKeySets) to
construct the KeyRow with apiKey: apiKey (no Object.assign) and compute status
locally from apiKey.whyInvalid() using the explicit null/undefined fallback
(e.g., ?? "valid") rather than || so you don’t overwrite or leak a status
property onto InternalApiKey and maintain correct typing for KeyRow.apiKey and
consumers like RevokeKeyDialog/KeyDetails.
In `@docs-mintlify/guides/going-further/oidc-federation.mdx`:
- Around line 139-143: Update the Troubleshooting section to list the exact
error messages emitted by validateOidcJwt() and matchClaims(): replace the
simplified "claim validation failed" entry with the full message format "claim
validation failed: {claimName}", and add entries for "stringEquals failed on
claim \"{claimKey}\"", "stringLike failed on claim \"{claimKey}\"", and "missing
claim \"{claimKey}\"" which are thrown by matchClaims(); ensure wording ties
each message back to the relevant validation path (validateOidcJwt() and
matchClaims()) so users can grep logs and map messages to the correct failure
cause.
In `@examples/demo/src/app/oidc-federation-demo/api/exchange/route.ts`:
- Around line 12-23: The fetch in the exchange route is calling the wrong
backend path and has no timeout; update the request URL from
`${BACKEND_URL}/api/v1/auth/oidc-federation/exchange` to
`${BACKEND_URL}/api/latest/auth/oidc-federation/exchange` and add a 5s abort
timeout to the fetch options by including `signal: AbortSignal.timeout(5000)` in
the options object (the same place as method/headers/body) so the request using
BACKEND_URL will hit the correct handler and will not hang indefinitely.
In `@examples/demo/src/app/oidc-federation-demo/api/mint/route.ts`:
- Around line 18-22: The current code calls await res.json() before checking
res.ok which can throw on non-JSON error bodies; change the logic in route.ts to
read the raw response first (e.g., await res.text()), then if !res.ok return
NextResponse.json({ error: <use text or a fallback like `mock IdP returned
${res.status}`> }, { status: 502 }); otherwise parse the text into JSON
(JSON.parse or try/catch to handle unexpected content) and return via
NextResponse.json(data); ensure you reference the existing res, data variable
and NextResponse.json calls when making the change.
In `@packages/stack-shared/src/config/schema.ts`:
- Around line 112-135: The issuer URL scheme must be enforced to prevent HTTP
(MITM) issuers — update the validateOidcJwt function to parse the provided
issuerUrl and reject it unless the URL.protocol is "https:" or it's an explicit
local exception (allow "http:" only when the hostname is "localhost" or
"127.0.0.1" and not in production); if the check fails, return/throw the same
validation error used elsewhere for invalid issuers. Locate validateOidcJwt and
add this protocol/hostname check before fetching discovery/jwks (and apply the
same guard to any discovered jwks_uri if present), ensuring the enforcement is
consistent with branchOidcFederationSchema's usage of issuerUrl.
In `@packages/stack-shared/src/utils/oidc-federation.ts`:
- Around line 8-16: The current stringLikeToRegExp produces backtracking-prone
regexes (e.g., ".*:.*:.*") which can degrade performance; replace the
regex-based approach by implementing a linear glob matcher (e.g., a new
stringLikeMatch or globMatch) that scans the pattern and target string
iteratively honoring '*' and '?' and escapes, then update matchClaims to use
that matcher instead of new RegExp(out); additionally add simple sanity checks
(maxPatternLength and maxValueLength) in matchClaims to bail out/return false
for overly long inputs before any matching to avoid pathological inputs.
In `@packages/template/src/lib/stack-app/oidc-federation/index.test.ts`:
- Around line 10-19: The mock uses Jest-style two-generic syntax
vi.fn<Parameters<typeof fetch>, ReturnType<typeof fetch>> which Vitest 1.0+
doesn't accept; change the mock to use the single generic form vi.fn<typeof
fetch>(...) so the mocked function type matches fetch; update the declaration of
fetchMock (the vi.fn call) to use vi.fn<typeof fetch> and keep the async (input,
_init) => { ... } implementation and the same URL branch for
"https://api.example.com/api/v1/auth/oidc-federation/exchange" returning the
Response.
In `@packages/template/src/lib/stack-app/oidc-federation/index.ts`:
- Around line 55-65: The POST fetch that exchanges the subject_token (the block
creating `response` by calling `fetch(url.toString(), { method: "POST", headers,
body: JSON.stringify({...}) })`) and the other provider/fetch calls noted
(around lines 159-161 and 178-180) need request timeouts to avoid hanging
server-auth requests; update each fetch to use an AbortController with a
configurable timeout constant (e.g., DEFAULT_FETCH_TIMEOUT_MS), pass
controller.signal into the fetch options, set a timer to call controller.abort()
after the timeout, and clear the timer on success/failure so the request is
aborted safely and resources are cleaned up. Ensure you reference the same fetch
invocations (the POST exchange using `subjectToken`, and the OIDC provider/fetch
calls) and propagate/handle AbortError appropriately.
- Around line 15-35: The cache refresh logic uses Date.now() which is subject to
wall-clock jumps; update the timing to use monotonic time via performance.now()
instead: change any uses of Date.now() in shouldRefresh and where refreshAtMs is
set (e.g., in the token exchange/assignment logic inside
createOidcFederationTokenStore and the code referenced around lines 81-87) to
performance.now(), and compute refreshAtMs by adding the token lifetime (ms) to
performance.now(); keep the semantics (refresh 5_000ms before expiry) but rely
on performance.now() for all elapsed-time comparisons and assignments.
---
Nitpick comments:
In
`@apps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/migration.sql`:
- Around line 1-16: Add a foreign key on OidcFederationExchangeAudit.tenancyId
referencing Tenancy(id) and add a CHECK constraint on
OidcFederationExchangeAudit.outcome limiting values to 'success' or 'failure';
name the constraints clearly (e.g., OidcFederationExchangeAudit_tenancy_fkey and
OidcFederationExchangeAudit_outcome_check), add the CHECK with NOT VALID in this
migration per project policy, and plan a follow-up migration that issues
VALIDATE CONSTRAINT for the outcome check; if you intentionally want audit rows
to survive tenancy deletion, document that decision instead of adding the FK.
In `@apps/backend/src/lib/seed-dummy-data.ts`:
- Around line 1915-1937: The seed currently replaces the entire oidcFederation
tree; instead update only the intended policy using path notation so siblings
are preserved—set the key "oidcFederation.trustPolicies.mock-idp-demo" to the
policy object (preserving displayName, enabled, issuerUrl expression, audiences,
claimConditions, tokenTtlSeconds) rather than assigning a full oidcFederation
object; locate the code that creates the seed object (refs: oidcFederation,
trustPolicies, "mock-idp-demo") and change it to use the single-path update
format { "oidcFederation.trustPolicies.mock-idp-demo": { ...policy... } } for
seeding.
In `@apps/backend/src/route-handlers/smart-request.tsx`:
- Around line 248-258: The branch-mismatch currently throws a StatusError (at
the branch check using verifyServerAccessToken, branchIdHeader, and
effectiveBranchId), but surrounding auth failures use KnownErrors (e.g.,
KnownErrors.UnparsableAccessToken, KnownErrors.InvalidSecretServerKey); change
the branch-mismatch to emit a KnownError variant so SDKs can detect it
programmatically — either add a new KnownErrors.BranchMismatch (preferred) or
reuse KnownErrors.UnparsableAccessToken with a clear distinct reason string, and
update the throw site that currently constructs a StatusError to throw the
appropriate KnownError instead while keeping the same message text for logs.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-keys/page-client.tsx:
- Around line 135-179: The columns memo captures mutable callbacks (savePolicy,
deletePolicy, togglePolicy, setPolicyDialog) but only depends on
requirePublishableClientKey; wrap savePolicy, deletePolicy and togglePolicy in
useCallback with deps [stackAdminApp, updateConfig] (or whatever stable refs
they use), keep setPolicyDialog stable (or also memoize via useCallback if
needed), then include these callbacks in the dependency array for the columns
useMemo (so columns depends on [requirePublishableClientKey, savePolicy,
deletePolicy, togglePolicy, setPolicyDialog]); remove the
eslint-disable-next-line react-hooks/exhaustive-deps comment after ensuring all
captured functions are memoized.
In `@apps/mock-oidc-idp/package.json`:
- Around line 14-21: Update package.json so that the type-only package
"@types/express" is removed from "dependencies" and added to "devDependencies",
and add "rimraf" to "devDependencies" (since the "clean" script invokes rimraf)
to avoid relying on workspace hoisting; ensure you update the "dependencies" and
"devDependencies" sections accordingly and run npm/yarn/pnpm install to refresh
the lockfile.
In `@apps/mock-oidc-idp/src/index.ts`:
- Around line 110-119: The startup promise is being silenced via "void
main().then(...)" — replace this pattern by calling the approved helper that
handles async startup errors (use runAsynchronouslyWithAlert(main) to preserve
fatal logging/exit semantics; alternatively runAsynchronously(main) if alerts
aren’t needed). Remove the manual .then error handler and eslint-disable, and
rely on runAsynchronouslyWithAlert to log the error and exit; locate the current
call site of main() in index.ts and swap it to the helper invocation.
In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts`:
- Around line 440-449: The federation store is being given a snapshot apiBaseUrl
via apiUrls()[0] while the rest of the interface uses lazy getBaseUrl; change
the call to createOidcFederationTokenStoreForServerApp to accept a getter
instead (e.g., pass getApiBaseUrl: () => apiUrls()[0] or rename to match
existing param expected by createOidcFederationTokenStoreForServerApp) so the
store resolves the base URL lazily; update the StackServerInterface construction
to pass the matching lazy field (oidcFederation created with the getter) and
ensure the parameter name aligns with
createOidcFederationTokenStoreForServerApp's signature.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4bbd5d8-77bc-4153-9b07-279cdedece6e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (46)
apps/backend/.env.developmentapps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/migration.sqlapps/backend/prisma/migrations/20260420000000_add_oidc_federation_audit/tests/shape-and-index.tsapps/backend/prisma/schema.prismaapps/backend/src/app/api/latest/auth/oidc-federation/exchange/route.tsxapps/backend/src/app/api/latest/internal/oidc-federation/probe-discovery/route.tsxapps/backend/src/lib/events.tsxapps/backend/src/lib/oidc-jwt.test.tsxapps/backend/src/lib/oidc-jwt.tsxapps/backend/src/lib/seed-dummy-data.tsapps/backend/src/lib/server-access-token.test.tsxapps/backend/src/lib/server-access-token.tsxapps/backend/src/route-handlers/smart-request.tsxapps/backend/src/route-handlers/smart-response.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-keys/oidc-policy-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-keys/oidc-policy-form.test.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-keys/oidc-policy-form.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-keys/page-client.tsxapps/e2e/tests/backend/endpoints/api/v1/auth/oidc-federation/exchange.test.tsapps/internal-tool/scripts/pre-dev.mjsapps/mock-oidc-idp/.eslintrc.jsapps/mock-oidc-idp/package.jsonapps/mock-oidc-idp/src/index.tsapps/mock-oidc-idp/tsconfig.jsondocs-mintlify/docs.jsondocs-mintlify/guides/going-further/oidc-federation.mdxexamples/demo/.envexamples/demo/.env.developmentexamples/demo/src/app/oidc-federation-demo/api/exchange/route.tsexamples/demo/src/app/oidc-federation-demo/api/mint/route.tsexamples/demo/src/app/oidc-federation-demo/page.tsxpackage.jsonpackages/stack-shared/src/config/schema-fuzzer.test.tspackages/stack-shared/src/config/schema.tspackages/stack-shared/src/interface/admin-interface.tspackages/stack-shared/src/interface/server-interface.tspackages/stack-shared/src/utils/oidc-federation.test.tspackages/stack-shared/src/utils/oidc-federation.tspackages/template/src/lib/env.tspackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/admin-app.tspackages/template/src/lib/stack-app/apps/interfaces/server-app.tspackages/template/src/lib/stack-app/index.tspackages/template/src/lib/stack-app/oidc-federation/index.test.tspackages/template/src/lib/stack-app/oidc-federation/index.ts
mantrakp04
requested review from
BilalG1,
N2D4 and
nams1570
and removed request for
nams1570
BilalG1
reviewed
Apr 28, 2026
BilalG1
reviewed
Apr 28, 2026
BilalG1
reviewed
Apr 28, 2026
BilalG1
requested changes
Apr 28, 2026
Comment on lines
+125
to
+129
| let validated: Awaited<ReturnType<typeof validateOidcJwt>>; | ||
| try { | ||
| validated = await validateOidcJwt({ issuerUrl, audiences, token: req.body.subject_token, prisma: globalPrismaClient }); | ||
| } catch (error) { | ||
| attemptReasons.push({ policyId, reason: error instanceof Error ? error.message : String(error) }); |
Contributor
There was a problem hiding this comment.
Object.values(policy.audiences ?? {}).filter((v): v is string => typeof v === "string") keeps empty-string values (""). When audiences = [""], jose's jwtVerify will accept any token whose aud claim is also "". An admin who accidentally leaves an audience row blank ends up with a policy that can be satisfied by a zero-length audience, making the audience check vacuous for those matching tokens.
Suggested change
| let validated: Awaited<ReturnType<typeof validateOidcJwt>>; | |
| try { | |
| validated = await validateOidcJwt({ issuerUrl, audiences, token: req.body.subject_token, prisma: globalPrismaClient }); | |
| } catch (error) { | |
| attemptReasons.push({ policyId, reason: error instanceof Error ? error.message : String(error) }); | |
| const audiences = Object.values(policy.audiences ?? {}).filter((v): v is string => typeof v === "string" && v.length > 0); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/backend/src/app/api/latest/auth/oidc-federation/exchange/route.tsx
Line: 125-129
Comment:
**Empty audience strings are not rejected**
`Object.values(policy.audiences ?? {}).filter((v): v is string => typeof v === "string")` keeps empty-string values (`""`). When `audiences = [""]`, jose's `jwtVerify` will accept any token whose `aud` claim is also `""`. An admin who accidentally leaves an audience row blank ends up with a policy that can be satisfied by a zero-length audience, making the audience check vacuous for those matching tokens.
```suggestion
const audiences = Object.values(policy.audiences ?? {}).filter((v): v is string => typeof v === "string" && v.length > 0);
```
How can I resolve this? If you propose a fix, please make it concise.
BilalG1
approved these changes
Apr 29, 2026
mantrakp04
force-pushed
the
feat/oidc-federation
branch
from
69cb208 to
8f0f9ff
Compare
- Adjusted the `tenancyId` field in the `OidcFederationExchangeAudit` model to remove unnecessary whitespace. - Updated migration SQL to remove redundant foreign key constraint for `tenancyId`. - Modified test cases to reflect changes in the `tenancyId` handling, ensuring audit writes do not trigger delete/update overhead on the Tenancy table. - Changed event types in transaction tests to align with the new invoice processing logic. This commit addresses feedback from previous reviews and improves the clarity and efficiency of the OIDC federation audit implementation.
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.
Summary
Adds OIDC federation so projects can exchange IdP-issued JWTs for Stack sessions, with audit logging, dashboard configuration, SDK support, and documentation.
Backend
auth/oidc-federation/exchangeDashboard
Packages
stack-shared: config schema and OIDC federation utilitiestemplate:StackServerAppintegration for federation exchangeTests & docs
Made with Cursor
Summary by CodeRabbit