Skip to content

Commit 8052a2b

Browse files
authored
"Require publishable client key" toggle (#1158)
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **High Risk** > Touches authentication and OAuth token/authorize flows and changes how client requests are validated, so regressions could cause widespread login/client-access failures. Also includes a data migration that alters effective security posture for existing projects. > > **Overview** > Adds a **project-level toggle** (`project.requirePublishableClientKey`) to control whether client requests/OAuth flows must include a publishable client key, including a DB migration that backfills existing projects to require it. > > Backend auth now treats the publishable client key as *optional when allowed*, introducing a public sentinel (`__stack_public_client__`) and returning a new specific error (`PUBLISHABLE_CLIENT_KEY_REQUIRED_FOR_PROJECT`) across smart request auth + OAuth `authorize`/`callback`/`token` endpoints. > > Dashboard and SDKs update key generation/display and request construction to handle missing publishable keys, expose an advanced toggle on the Project Keys page, and extend internal config overrides to support a new `project` level; E2E/tests and schema fuzzing are expanded accordingly, and CI adds a forward-compat migration check job when back-compat fails. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5d06c08. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Project-level config to require publishable client keys; migration applied to existing projects. * **Improvements** * Auth flows now support optional publishable client keys with explicit validation and a sentinel for keyless OAuth. * Dashboard/UI and SDKs handle publishable keys as optional and conditionally show/generate them. * Admin/client APIs extended to manage project-level overrides. * **Bug Fixes** * Key validation behavior aligned with project config. * **Tests** * Expanded E2E and unit tests covering optional/required publishable-key scenarios. * **Documentation** * Spec and knowledge docs updated to describe the sentinel and config behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent fb14ea9 commit 8052a2b

40 files changed

Lines changed: 1421 additions & 341 deletions

File tree

.github/workflows/db-migration-backwards-compatibility.yaml

Lines changed: 182 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name: DB migrations are backwards-compatible
1+
name: DB migration compat
22

33
on:
44
push:
@@ -48,7 +48,7 @@ jobs:
4848
fi
4949
5050
backwards-compatibility:
51-
name: Test migrations with ${{ needs.check-migrations-changed.outputs.base_branch }} branch code
51+
name: Back-compat — Current branch migrations with ${{ needs.check-migrations-changed.outputs.base_branch }} branch code
5252
needs: check-migrations-changed
5353
if: needs.check-migrations-changed.outputs.migrations_changed == 'true'
5454
runs-on: ubicloud-standard-8
@@ -235,6 +235,186 @@ jobs:
235235
if: always()
236236
run: docker compose -f docker/dependencies/docker.compose.yaml logs
237237

238+
forward-compatibility:
239+
name: Forward-compat — Current branch code with ${{ needs.check-migrations-changed.outputs.base_branch }} branch migrations
240+
needs: [check-migrations-changed, backwards-compatibility]
241+
if: always() && needs.backwards-compatibility.result == 'failure'
242+
runs-on: ubicloud-standard-8
243+
env:
244+
NODE_ENV: test
245+
STACK_ENABLE_HARDCODED_PASSKEY_CHALLENGE_FOR_TESTING: yes
246+
STACK_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@localhost:8128/stackframe"
247+
248+
steps:
249+
# First, checkout the base branch to get its migrations
250+
- name: Checkout base branch
251+
uses: actions/checkout@v6
252+
with:
253+
ref: ${{ needs.check-migrations-changed.outputs.base_branch }}
254+
path: base-branch
255+
256+
- name: Save base branch migrations
257+
run: |
258+
mkdir -p saved-migrations
259+
cp -r base-branch/apps/backend/prisma/migrations/* saved-migrations/
260+
261+
# Now checkout current branch (new code)
262+
- name: Checkout current branch
263+
uses: actions/checkout@v6
264+
with:
265+
path: current-branch
266+
267+
# Move current branch to the root for the rest of the workflow
268+
- name: Setup working directory
269+
run: |
270+
shopt -s dotglob
271+
mv current-branch/* .
272+
rm -rf current-branch base-branch
273+
274+
# Replace current branch's migrations with base branch's (old) migrations
275+
- name: Replace migrations with base branch migrations
276+
run: |
277+
rm -rf apps/backend/prisma/migrations/*
278+
cp -r saved-migrations/* apps/backend/prisma/migrations/
279+
rm -rf saved-migrations
280+
281+
- name: Setup Node.js
282+
uses: actions/setup-node@v6
283+
with:
284+
node-version: 22.x
285+
286+
- name: Setup pnpm
287+
uses: pnpm/action-setup@v4
288+
289+
# Start Docker Compose in the background
290+
- name: Start Docker Compose in background
291+
uses: JarvusInnovations/background-action@v1.0.7
292+
with:
293+
run: docker compose -f docker/dependencies/docker.compose.yaml up --pull always -d &
294+
wait-on: /dev/null
295+
tail: true
296+
wait-for: 3s
297+
log-output-if: true
298+
299+
- name: Install dependencies
300+
run: pnpm install --frozen-lockfile
301+
302+
- name: Create .env.test.local file for apps/backend
303+
run: cp apps/backend/.env.development apps/backend/.env.test.local
304+
305+
- name: Create .env.test.local file for apps/dashboard
306+
run: cp apps/dashboard/.env.development apps/dashboard/.env.test.local
307+
308+
- name: Create .env.test.local file for apps/e2e
309+
run: cp apps/e2e/.env.development apps/e2e/.env.test.local
310+
311+
- name: Create .env.test.local file for docs
312+
run: cp docs/.env.development docs/.env.test.local
313+
314+
- name: Create .env.test.local file for examples/cjs-test
315+
run: cp examples/cjs-test/.env.development examples/cjs-test/.env.test.local
316+
317+
- name: Create .env.test.local file for examples/demo
318+
run: cp examples/demo/.env.development examples/demo/.env.test.local
319+
320+
- name: Create .env.test.local file for examples/docs-examples
321+
run: cp examples/docs-examples/.env.development examples/docs-examples/.env.test.local
322+
323+
- name: Create .env.test.local file for examples/e-commerce
324+
run: cp examples/e-commerce/.env.development examples/e-commerce/.env.test.local
325+
326+
- name: Create .env.test.local file for examples/middleware
327+
run: cp examples/middleware/.env.development examples/middleware/.env.test.local
328+
329+
- name: Create .env.test.local file for examples/supabase
330+
run: cp examples/supabase/.env.development examples/supabase/.env.test.local
331+
332+
- name: Create .env.test.local file for examples/convex
333+
run: cp examples/convex/.env.development examples/convex/.env.test.local
334+
335+
- name: Build
336+
run: pnpm build
337+
338+
- name: Wait on Postgres
339+
run: pnpm run wait-until-postgres-is-ready:pg_isready
340+
341+
- name: Wait on Inbucket
342+
run: pnpx wait-on tcp:localhost:8129
343+
344+
- name: Wait on Svix
345+
run: pnpx wait-on tcp:localhost:8113
346+
347+
- name: Wait on ClickHouse
348+
run: pnpx wait-on http://localhost:8136/ping
349+
350+
- name: Initialize database
351+
run: pnpm run db:init
352+
353+
- name: Start stack-backend in background
354+
uses: JarvusInnovations/background-action@v1.0.7
355+
with:
356+
run: pnpm run start:backend --log-order=stream &
357+
wait-on: |
358+
http://localhost:8102
359+
tail: true
360+
wait-for: 30s
361+
log-output-if: true
362+
363+
- name: Start stack-dashboard in background
364+
uses: JarvusInnovations/background-action@v1.0.7
365+
with:
366+
run: pnpm run start:dashboard --log-order=stream &
367+
wait-on: |
368+
http://localhost:8101
369+
tail: true
370+
wait-for: 30s
371+
log-output-if: true
372+
373+
- name: Start mock-oauth-server in background
374+
uses: JarvusInnovations/background-action@v1.0.7
375+
with:
376+
run: pnpm run start:mock-oauth-server --log-order=stream &
377+
wait-on: |
378+
http://localhost:8102
379+
tail: true
380+
wait-for: 30s
381+
log-output-if: true
382+
383+
- name: Start run-email-queue in background
384+
uses: JarvusInnovations/background-action@v1.0.7
385+
with:
386+
run: pnpm -C apps/backend run run-email-queue --log-order=stream &
387+
wait-on: |
388+
http://localhost:8102
389+
tail: true
390+
wait-for: 30s
391+
log-output-if: true
392+
393+
- name: Start run-cron-jobs in background
394+
uses: JarvusInnovations/background-action@v1.0.7
395+
if: ${{ hashFiles('apps/backend/scripts/run-cron-jobs.ts') != '' }}
396+
with:
397+
run: pnpm -C apps/backend run with-env:dev tsx scripts/run-cron-jobs.ts --log-order=stream &
398+
wait-on: |
399+
http://localhost:8102
400+
tail: true
401+
wait-for: 30s
402+
log-output-if: true
403+
404+
- name: Wait 10 seconds
405+
run: sleep 10
406+
407+
# Run tests: current branch code with base branch (old) migrations
408+
- name: Run tests (current branch code with base branch migrations)
409+
run: pnpm test
410+
411+
- name: Verify data integrity
412+
run: pnpm run verify-data-integrity --no-bail
413+
414+
- name: Print Docker Compose logs
415+
if: always()
416+
run: docker compose -f docker/dependencies/docker.compose.yaml logs
417+
238418
# This job runs when migrations haven't changed, ensuring the workflow succeeds
239419
skip-unchanged:
240420
name: No migration changes (skipped)

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ To see all development ports, refer to the index.html of `apps/dev-launchpad/pub
100100
- NEVER implement a hacky solution without EXPLICIT approval from the user. Always go the extra mile to make sure the solution is clean, maintainable, and robust.
101101
- Fail early, fail loud. Fail fast with an error instead of silently continuing.
102102
- Do NOT use `as`/`any`/type casts or anything else like that to bypass the type system unless you specifically asked the user about it. Most of the time a place where you would use type casts is not one where you actually need them. Avoid wherever possible.
103-
- When writing database migration files, assume that we have >1,000,000 rows in every table (unless otherwise specified). This means you may have to use CONDITIONALLY_REPEAT_MIGRATION_SENTINEL to avoid running the migration and things like concurrent index builds; see the existing migrations for examples. One common pattern is to add a temporary extra boolean column
103+
- When writing database migration files, assume that we have >1,000,000 rows in every table (unless otherwise specified). This means you may have to use CONDITIONALLY_REPEAT_MIGRATION_SENTINEL to avoid running the migration and things like concurrent index builds; see the existing migrations for examples. One common pattern is to add a temporary index or extra boolean column marking whether the row has already been migrated (then deleting the column at the end).
104104
- Each migration file runs in its own transaction with a relatively short timeout. Split long-running operations into separate migration files to avoid timeouts. For example, when adding CHECK constraints, use `NOT VALID` in one migration, then `VALIDATE CONSTRAINT` in a separate migration file.
105105
- Note that each database migration file is executed in a single transaction. Even with the run-outside-transaction sentinel, the transaction will still continue during the entire migration file. If you want to split things up into multiple transactions, put it into their own migration files.
106106
- When writing database migration files, ALWAYS ALWAYS add tests for all the potential edge cases! See the folder structure of the other migrations to see how that works.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
-- Create temporary expression index to speed up the migration
2+
-- (B-tree on the specific JSONB path, not GIN on the whole column,
3+
-- so the index is actually used by the #>> WHERE clause)
4+
-- SPLIT_STATEMENT_SENTINEL
5+
-- SINGLE_STATEMENT_SENTINEL
6+
-- RUN_OUTSIDE_TRANSACTION_SENTINEL
7+
CREATE INDEX CONCURRENTLY IF NOT EXISTS "temp_project_require_publishable_client_key_idx"
8+
ON /* SCHEMA_NAME_SENTINEL */."Project"
9+
USING GIN ("projectConfigOverride");
10+
-- SPLIT_STATEMENT_SENTINEL
11+
12+
-- Set requirePublishableClientKey to true for existing projects when missing
13+
-- SPLIT_STATEMENT_SENTINEL
14+
-- SINGLE_STATEMENT_SENTINEL
15+
-- CONDITIONALLY_REPEAT_MIGRATION_SENTINEL
16+
WITH to_update AS (
17+
SELECT "id"
18+
FROM "Project"
19+
WHERE "projectConfigOverride" IS NULL
20+
OR NOT "projectConfigOverride" ? 'project.requirePublishableClientKey'
21+
LIMIT 10000
22+
)
23+
UPDATE "Project" p
24+
SET "projectConfigOverride" = jsonb_set(
25+
COALESCE(p."projectConfigOverride", '{}'::jsonb),
26+
'{project.requirePublishableClientKey}',
27+
'true'::jsonb,
28+
true
29+
)
30+
FROM to_update tu
31+
WHERE p."id" = tu."id"
32+
RETURNING true AS should_repeat_migration;
33+
-- SPLIT_STATEMENT_SENTINEL
34+
35+
-- Clean up temporary index
36+
DROP INDEX IF EXISTS "temp_project_require_publishable_client_key_idx";

apps/backend/src/app/api/latest/auth/oauth/authorize/[provider_id]/route.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { checkApiKeySet } from "@/lib/internal-api-keys";
1+
import { checkApiKeySet, throwCheckApiKeySetError } from "@/lib/internal-api-keys";
22
import { getSoleTenancyFromProjectBranch } from "@/lib/tenancies";
33
import { decodeAccessToken, oauthCookieSchema } from "@/lib/tokens";
44
import { getProjectBranchFromClientId, getProvider } from "@/oauth";
@@ -60,8 +60,9 @@ export const GET = createSmartRouteHandler({
6060
throw new KnownErrors.InvalidOAuthClientIdOrSecret(query.client_id);
6161
}
6262

63-
if (!(await checkApiKeySet(tenancy.project.id, { publishableClientKey: query.client_secret }))) {
64-
throw new KnownErrors.InvalidPublishableClientKey(tenancy.project.id);
63+
const keyCheck = await checkApiKeySet(tenancy.project.id, { publishableClientKey: query.client_secret });
64+
if (keyCheck.status === "error") {
65+
throwCheckApiKeySetError(keyCheck.error, tenancy.project.id, new KnownErrors.InvalidPublishableClientKey(tenancy.project.id));
6566
}
6667

6768
const providerRaw = Object.entries(tenancy.config.auth.oauth.providers).find(([providerId, _]) => providerId === params.provider_id);

apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { usersCrudHandlers } from "@/app/api/latest/users/crud";
2+
import { checkApiKeySet, throwCheckApiKeySetError } from "@/lib/internal-api-keys";
23
import { createOAuthUserAndAccount, findExistingOAuthAccount, handleOAuthEmailMergeStrategy, linkOAuthAccountToUser } from "@/lib/oauth";
34
import { isAcceptedNativeAppUrl, validateRedirectUrl } from "@/lib/redirect-urls";
45
import { Tenancy, getTenancy } from "@/lib/tenancies";
@@ -126,6 +127,11 @@ const handler = createSmartRouteHandler({
126127

127128
const provider = { id: providerRaw[0], ...providerRaw[1] };
128129

130+
const keyCheck = await checkApiKeySet(tenancy.project.id, { publishableClientKey: outerInfo.publishableClientKey });
131+
if (keyCheck.status === "error") {
132+
throwCheckApiKeySetError(keyCheck.error, tenancy.project.id, new KnownErrors.InvalidPublishableClientKey(tenancy.project.id));
133+
}
134+
129135
const providerObj = await getProvider(provider as any);
130136
let callbackResult: Awaited<ReturnType<typeof providerObj.getCallback>>;
131137
try {

apps/backend/src/app/api/latest/auth/oauth/token/route.tsx

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { oauthServer } from "@/oauth";
1+
import { checkApiKeySet, throwCheckApiKeySetError } from "@/lib/internal-api-keys";
2+
import { getSoleTenancyFromProjectBranch } from "@/lib/tenancies";
3+
import { getProjectBranchFromClientId, oauthServer } from "@/oauth";
24
import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler";
35
import { InvalidClientError, InvalidGrantError, InvalidRequestError, Request as OAuthRequest, Response as OAuthResponse, ServerError } from "@node-oauth/oauth2-server";
46
import { KnownErrors } from "@stackframe/stack-shared/dist/known-errors";
@@ -15,6 +17,8 @@ export const POST = createSmartRouteHandler({
1517
request: yupObject({
1618
body: yupObject({
1719
grant_type: yupString().oneOf(["authorization_code", "refresh_token"]).defined(),
20+
client_id: yupString().optional(),
21+
client_secret: yupString().optional(),
1822
}).unknown().defined(),
1923
}).defined(),
2024
response: yupObject({
@@ -24,6 +28,25 @@ export const POST = createSmartRouteHandler({
2428
headers: yupMixed().defined(),
2529
}),
2630
async handler(req, fullReq) {
31+
// Pre-validate the publishable client key to provide specific error messages
32+
// before the OAuth library processes the request
33+
const clientId = req.body.client_id;
34+
const clientSecret = req.body.client_secret;
35+
36+
if (clientId) {
37+
const tenancy = await getSoleTenancyFromProjectBranch(...getProjectBranchFromClientId(clientId), true);
38+
if (tenancy) {
39+
if (clientSecret) {
40+
const keyCheck = await checkApiKeySet(tenancy.project.id, { publishableClientKey: clientSecret });
41+
if (keyCheck.status === "error") {
42+
throwCheckApiKeySetError(keyCheck.error, tenancy.project.id, new KnownErrors.InvalidOAuthClientIdOrSecret());
43+
}
44+
} else if (tenancy.config.project.requirePublishableClientKey) {
45+
throw new KnownErrors.PublishableClientKeyRequiredForProject(tenancy.project.id);
46+
}
47+
}
48+
}
49+
2750
const oauthRequest = new OAuthRequest({
2851
headers: {
2952
...fullReq.headers,

0 commit comments

Comments
 (0)