Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds project-level config for requiring publishable client keys, a DB migration, backend and dashboard support to toggle/enforce it, SDK and client changes to make publishableClientKey optional, sentinel-based OAuth handling, and expanded E2E tests covering keyless flows. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin as Admin/User
participant Dashboard as Dashboard UI
participant Backend as Backend API
participant DB as Database
participant AuthHandler as SmartRequest
participant Tenancy as Tenancy Config
Admin->>Dashboard: Toggle requirePublishableClientKey
Dashboard->>Backend: PUT /api/latest/internal/config/override/project
Backend->>DB: Update Project.projectConfigOverride (jsonb_set)
DB-->>Backend: Success
Backend-->>Dashboard: Config updated
rect rgba(100, 150, 200, 0.5)
Note over AuthHandler,Tenancy: Later: OAuth/token request flow
AuthHandler->>Tenancy: Load project config
Tenancy-->>AuthHandler: project.requirePublishableClientKey
alt requirePublishableClientKey == true
AuthHandler->>AuthHandler: Validate publishableClientKey present and valid
alt valid
AuthHandler-->>Client: 200 OK
else
AuthHandler-->>Client: 401 PublishableClientKeyRequiredForProject
end
else
AuthHandler->>AuthHandler: Accept sentinel or missing key
AuthHandler-->>Client: 200 OK
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
Greptile OverviewGreptile SummaryThis PR implements a configurable Key Changes
TestingComprehensive test coverage added for:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Backend
participant OAuthProvider
participant Database
Note over Client,Database: Standard API Request Flow
Client->>Backend: Send API request
Backend->>Database: Query project configuration
Database-->>Backend: requirePublishableClientKey setting
alt Key not required
Backend-->>Client: Process request
else Key required
Backend-->>Client: 401 Unauthorized
end
Note over Client,Database: OAuth Authentication Flow
Client->>Backend: Initiate OAuth authorize
Backend->>Database: Query project configuration
alt Keyless allowed
Backend->>Database: Store flow information
Backend->>OAuthProvider: Redirect user
OAuthProvider-->>Backend: Callback with code
Backend->>Backend: Exchange code
Backend->>Database: Fetch flow information
Backend->>Database: Persist session
Backend-->>Client: Authentication complete
else Key mandatory
Backend-->>Client: 401 Unauthorized
end
|
There was a problem hiding this comment.
Pull request overview
This pull request adds a configurable "Require publishable client key" toggle that allows projects to optionally require publishable client keys for client access. The feature defaults to false for new projects but is set to true for existing projects via a database migration to maintain backwards compatibility.
Changes:
- Adds
project.requirePublishableClientKeyconfig field at the project level that controls whether client requests must include a valid publishable client key - Introduces a
publicOAuthClientSecretSentinelconstant ("stack_public_client") to represent keyless OAuth clients - Updates OAuth flows (authorize, callback, token) to support optional publishable client keys
- Adds comprehensive E2E test coverage for the new authentication scenarios
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/stack-shared/src/config/schema.ts | Adds project.requirePublishableClientKey boolean field to project config schema with default value false |
| packages/stack-shared/src/known-errors.tsx | Adds new PublishableClientKeyRequiredForProject error for when keys are required but missing |
| packages/stack-shared/src/interface/client-interface.ts | Updates to support optional publishable client keys in OAuth flows |
| packages/stack-shared/src/interface/admin-interface.ts | Adds "project" level support to config override endpoints |
| packages/stack-shared/src/utils/oauth.tsx | Exports publicOAuthClientSecretSentinel constant |
| packages/template/src/lib/stack-app/projects/index.ts | Adds requirePublishableClientKey field to AdminProjectUpdateOptions |
| packages/template/src/lib/stack-app/apps/implementations/*.ts | Updates app constructors to handle optional publishable client keys |
| apps/backend/src/route-handlers/smart-request.tsx | Adds validation logic to check requirePublishableClientKey config before allowing keyless client access |
| apps/backend/src/app/api/latest/auth/oauth/*.tsx | Updates OAuth endpoints to support keyless clients when not required |
| apps/backend/src/oauth/model.tsx | Updates OAuth model to accept public client secret sentinel |
| apps/backend/src/lib/tokens.tsx | Makes publishableClientKey optional in oauthCookieSchema |
| apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx | Adds project-level config override support |
| apps/backend/prisma/migrations/*.sql | Migration that sets requirePublishableClientKey to true for existing projects |
| apps/dashboard/src/components/data-table/api-key-table.tsx | Updates API key table to conditionally show publishable client key column and handle keys without client keys |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/*.tsx | Updates dashboard pages to support the new toggle and conditionally show publishable keys |
| apps/e2e/tests/backend/*.ts | Adds comprehensive E2E tests for all scenarios with optional/required publishable keys |
| examples/demo/.env.development | Removes hardcoded publishable client key to demonstrate optional behavior |
| claude/CLAUDE-KNOWLEDGE.md | Documents the new feature and related implementation details |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/backend/src/app/api/latest/auth/oauth/authorize/[provider_id]/route.tsx
Outdated
Show resolved
Hide resolved
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx
Outdated
Show resolved
Hide resolved
...ackend/prisma/migrations/20260203120000_project_require_publishable_client_key/migration.sql
Show resolved
Hide resolved
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stack-shared/src/interface/admin-interface.ts (1)
549-575:⚠️ Potential issue | 🟠 MajorUse discriminated union typing for level-specific config overrides.
The methods accept
anybut should be strongly typed toProjectConfigOverride,BranchConfigOverride, orEnvironmentConfigOverridebased on thelevelparameter. Since schema.ts already defines these types and callers pass typed values, use overloads or a discriminated union to enforce type safety at the call site.- async setConfigOverride(level: "project" | "branch" | "environment", configOverride: any, source?: BranchConfigSourceApi): Promise<void> { + async setConfigOverride(level: "project", configOverride: ProjectConfigOverride, source?: BranchConfigSourceApi): Promise<void>; + async setConfigOverride(level: "branch", configOverride: BranchConfigOverride, source?: BranchConfigSourceApi): Promise<void>; + async setConfigOverride(level: "environment", configOverride: EnvironmentConfigOverride, source?: BranchConfigSourceApi): Promise<void>; + async setConfigOverride(level: "project" | "branch" | "environment", configOverride: ProjectConfigOverride | BranchConfigOverride | EnvironmentConfigOverride, source?: BranchConfigSourceApi): Promise<void> {Apply the same pattern to
updateConfigOverrideusing the corresponding*ConfigOverrideOverridetypes.
🤖 Fix all issues with AI agents
In `@apps/dashboard/src/components/data-table/api-key-table.tsx`:
- Around line 96-102: serverKeyColumn currently renders "*******" even when
secretServerKey is null; update serverKeyColumn so accessorFn returns the
lastFour or null (e.g., row.secretServerKey?.lastFour ?? null) and change the
cell renderer (serverKeyColumn.cell) to conditionally render the masked value
plus lastFour when row.original.secretServerKey exists, otherwise render a clear
placeholder like "No key" or "—" inside the same TextCell component; keep
enableSorting false and header as-is.
In `@apps/e2e/tests/backend/backend-helpers.ts`:
- Around line 657-671: The includeClientSecret flag in OAuth.getAuthorizeQuery
is misleading because when includeClientSecret is false the code still sets
client_secret to publicOAuthClientSecretSentinel; change the logic so that when
options.includeClientSecret is false you set client_secret to undefined (instead
of publicOAuthClientSecretSentinel) so the parameter is truly omitted in the
generated query; update the same pattern in the other occurrence of
OAuth.getAuthorizeQuery (the later block around the similar client_secret
construction) and keep references to includeClientSecret and
publicOAuthClientSecretSentinel to locate and modify the assignments.
In `@packages/stack-shared/src/known-errors.tsx`:
- Around line 422-433: Narrow the `json` parameter in the
`constructorArgsFromJson` callback for PublishableClientKeyRequiredForProject
instead of using `any`: update the callback signature in the call to
createKnownErrorConstructor so `json` is typed (e.g. `{ project_id?: string |
null }`) and cast/return `[json.project_id ?? undefined] as const`; also add the
short explanatory comment used elsewhere in this file describing why we narrow
the shape locally for type safety. Reference:
PublishableClientKeyRequiredForProject and the constructorArgsFromJson callback
passed to createKnownErrorConstructor.
🧹 Nitpick comments (5)
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx (1)
18-19: Centralize the public OAuth sentinel constant.To avoid drift across routes/models, import the shared sentinel from stack-shared instead of redefining it locally.
♻️ Suggested change
-import { KnownError, KnownErrors } from "@stackframe/stack-shared"; +import { KnownError, KnownErrors } from "@stackframe/stack-shared"; +import { publicOAuthClientSecretSentinel } from "@stackframe/stack-shared/dist/utils/oauth"; @@ -const publicOAuthClientSecretSentinel = "__stack_public_client__";apps/backend/src/oauth/model.tsx (1)
45-56: Reuse the shared public-client sentinel constant.This avoids string duplication across OAuth flows and keeps the sentinel consistent everywhere.
♻️ Suggested change
-import { KnownErrors } from "@stackframe/stack-shared"; +import { KnownErrors } from "@stackframe/stack-shared"; +import { publicOAuthClientSecretSentinel } from "@stackframe/stack-shared/dist/utils/oauth"; @@ - const publicOAuthClientSecretSentinel = "__stack_public_client__";apps/backend/src/app/api/latest/auth/oauth/token/route.tsx (1)
30-30: Import the sentinel constant instead of redefining it.The
publicOAuthClientSecretSentinelis already defined inpackages/stack-shared/src/utils/oauth.tsx. Importing it ensures consistency and avoids potential drift if the value changes.♻️ Proposed fix
import { getSoleTenancyFromProjectBranch } from "@/lib/tenancies"; import { getProjectBranchFromClientId, oauthServer } from "@/oauth"; +import { publicOAuthClientSecretSentinel } from "@stackframe/stack-shared/dist/utils/oauth"; import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler"; ... async handler(req, fullReq) { - const publicOAuthClientSecretSentinel = "__stack_public_client__"; const clientId = req.body.client_id;apps/backend/src/app/api/latest/auth/oauth/authorize/[provider_id]/route.tsx (1)
17-17: Import the sentinel constant from shared utils.Same as the token route - this constant is already defined in
packages/stack-shared/src/utils/oauth.tsx. Import it for consistency.♻️ Proposed fix
+import { publicOAuthClientSecretSentinel } from "@stackframe/stack-shared/dist/utils/oauth"; ... -const publicOAuthClientSecretSentinel = "__stack_public_client__";apps/e2e/tests/backend/backend-helpers.ts (1)
1238-1246: Please justify the newanyparameter (or tighten it).Consider adding a short comment explaining why
anyis needed, or replace it with a concrete type/unknown.📝 Minimal guideline-compliant tweak
-export async function updateProjectConfig(config: any) { +export async function updateProjectConfig( + // Config shape varies per test; allow free-form overrides here. + config: any, +) {As per coding guidelines, Try to avoid the
anytype; whenever you need to useany, leave a comment explaining why you're using it.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdks/spec/src/apps/client-app.spec.md (1)
770-781:⚠️ Potential issue | 🟡 MinorFix list indentation for the client_secret bullet.
Line 780 is flagged by MD005 (inconsistent list indentation). Align it with the other Body list items.🛠️ Suggested fix
- - client_secret=<publishableClientKey | publishableClientKeyNotNecessarySentinel> + - client_secret=<publishableClientKey | publishableClientKeyNotNecessarySentinel>packages/stack-shared/src/interface/client-interface.ts (1)
158-170:⚠️ Potential issue | 🟡 MinorNormalize empty publishableClientKey to avoid empty client_secret.
Line 158 uses??, so an empty string becomesclient_secret="", while Line 328 treats empty as “missing”. This can cause OAuth failures when the key is optional. Consider normalizing to a truthy check (and reusing it in getOAuthUrl/callOAuthCallback).🛠️ Suggested fix (apply similarly in other occurrences)
- const clientSecret = this.options.publishableClientKey ?? publishableClientKeyNotNecessarySentinel; + const publishableClientKey = + "publishableClientKey" in this.options ? this.options.publishableClientKey : undefined; + const clientSecret = + publishableClientKey && publishableClientKey.length > 0 + ? publishableClientKey + : publishableClientKeyNotNecessarySentinel;Also applies to: 995-999, 1036-1048
🤖 Fix all issues with AI agents
In `@apps/e2e/tests/backend/backend-helpers.ts`:
- Around line 1237-1245: Change the updateProjectConfig parameter from any to
the concrete ProjectConfigOverride type: update the function signature of
updateProjectConfig to accept config: ProjectConfigOverride, import
ProjectConfigOverride from packages/stack-shared/src/config/schema (where it’s
defined), and keep the existing JSON.stringify usage and call to
niceBackendFetch unchanged; ensure the import is a type import if your TS
settings require it.
In `@sdks/spec/src/_utilities.spec.md`:
- Around line 34-37: Update the "Required Headers" doc to mark the
x-stack-publishable-client-key header as conditional based on the
requirePublishableClientKey flag: note that when requirePublishableClientKey is
false, callers may use the sentinel publishableClientKeyNotNecessarySentinel
("__stack_public_client__") as the OAuth client_secret and the
x-stack-publishable-client-key header is optional; keep the header described as
required only when requirePublishableClientKey is true and include a short
example or parenthetical indicating the sentinel usage.
🧹 Nitpick comments (1)
sdks/implementations/swift/Sources/StackAuth/APIClient.swift (1)
314-323: Consider reusinggetOAuthClientSecret()for consistency.Line 318 duplicates the sentinel fallback logic that already exists in
getOAuthClientSecret(). While functionally correct, reusing the helper would improve maintainability.♻️ Suggested refactor
if let publishableClientKey = publishableClientKey { request.setValue(publishableClientKey, forHTTPHeaderField: "x-stack-publishable-client-key") } - let oauthClientSecret = publishableClientKey ?? publishableClientKeyNotNecessarySentinel + let oauthClientSecret = getOAuthClientSecret() let body = [ "grant_type=refresh_token", "refresh_token=\(formURLEncode(refreshToken))", "client_id=\(formURLEncode(projectId))", "client_secret=\(formURLEncode(oauthClientSecret))" ].joined(separator: "&")
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Note
Medium Risk
Changes request authentication and OAuth client-secret handling, which can impact access control if misconfigured. Includes DB backfill and broad SDK/dashboard behavior changes, but is covered by new e2e tests.
Overview
Adds a project-level config flag
project.requirePublishableClientKeywith end-to-end support: a DB migration backfills existing projects totrue, config schema/defaults and fuzzer coverage are updated, and internal config override endpoints now support theprojectlevel.Updates authentication so client requests may omit
x-stack-publishable-client-keywhen the flag is disabled (or a public__stack_public_client__sentinel is used), but returns a newPUBLISHABLE_CLIENT_KEY_REQUIRED_FOR_PROJECTerror when the flag is enabled. Dashboard onboarding/keys pages now hide/generate publishable keys conditionally and expose an advanced toggle; SDK/template constructors treatpublishableClientKeyas optional and adjust OAuth flows accordingly, with e2e tests covering required vs optional behavior.Written by Cursor Bugbot for commit 2d5b12d. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Behavioral Changes