Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
# Changelog

## v1.8.4

- **`AUTH_MODE=dual` would have refused to start against a correctly-secured
core.** v1.8.3 fixed the wrong-endpoint bug in the preflight but left a second
copy of the same probe in the boot path, still using `/recipe/users/count` —
which SuperTokens core 12 does not implement. That guard *throws* on anything
that is not a 401, so the 404 would have been read as "the core is running
without API_KEYS" and stopped the container from booting.

Unlike the preflight, where the bug printed an alarming line, here it would
have blocked the cutover entirely and blamed the operator for a URL this code
got wrong.

The probe now lives in `server/supertokens/coreProbe.js` and is imported by
both callers, so they cannot drift again — asserted by a test. The boot guard
now throws **only** on a confirmed-open core (a known endpoint answering an
unkeyed request with 200); every other outcome warns and lets the boot
proceed.


## v1.8.3

- **`supertokens:check` no longer reports a correctly-secured core as wide
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ LABEL org.opencontainers.image.licenses="MIT"
# only on a pushed vX.Y.Z tag, and docker/metadata-action derives the
# published image's version label from that tag - so this literal only
# affects locally-built images, not what GHCR publishes.
LABEL org.opencontainers.image.version="1.8.3"
LABEL org.opencontainers.image.version="1.8.4"

VOLUME ["/app/data"]
EXPOSE 3000
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "rackstack-server",
"version": "1.8.3",
"version": "1.8.4",
"private": true,
"type": "module",
"scripts": {
Expand Down
56 changes: 56 additions & 0 deletions server/supertokens/coreProbe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Asking a SuperTokens core whether it requires authentication.
//
// ONE implementation, imported by both the boot-time guard (init.js) and the
// operator preflight (preflight.js). It lives in its own module because it
// previously did not: the same probe was written twice, the path was corrected
// in the preflight, and the copy in the boot path was left behind - where a
// wrong answer does not print a scary line, it stops the container from
// starting. If you add a third caller, import this; do not copy it.

/**
* Endpoints gated by the core's API key, newest path first.
*
* NOT `/hello` - that answers unauthenticated by design as a health check, so
* a 200 there proves nothing about whether the core is locked down.
*
* More than one because the path is tenant-scoped on modern cores
* (`/<tenantId>/users/count`, per supertokens-node's own querier) and was not
* on older ones. v1.8.2 shipped a single guessed path that core 12 does not
* implement, so every probe returned 404.
*/
export const AUTHED_ENDPOINTS = Object.freeze([
'/public/users/count', // cores with multitenancy (the default tenant)
'/recipe/users/count', // older cores
]);

/**
* Probes the first endpoint this core actually implements.
*
* Returns `{ status, path }`. `status` is `null` when every candidate 404s,
* which means "this core exposes no path we know how to ask" - NOT "the core
* answered". That distinction is the entire point of this module: a 404 is
* evidence about our URL, not about the core's authentication, and inferring
* a security verdict from one is how v1.8.2 reported a correctly-secured core
* as running wide open.
*/
export async function probeAuthedEndpoint({
connectionURI, apiKey, fetchImpl = fetch, timeoutMs = 5000,
}) {
const base = connectionURI.replace(/\/$/, '');
const headers = { 'api-version': '3.0' };
if (apiKey) headers['api-key'] = apiKey;

for (const path of AUTHED_ENDPOINTS) {
// eslint-disable-next-line no-await-in-loop
const res = await fetchImpl(`${base}${path}`, {
method: 'GET', headers, signal: AbortSignal.timeout(timeoutMs),
});
if (res.status !== 404) return { status: res.status, path };
}
return { status: null, path: null };
}

/** A refusal, however the core spells it. */
export function isRefused(status) {
return status === 401 || status === 403;
}
56 changes: 34 additions & 22 deletions server/supertokens/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import { isSuperTokensEnabled } from '../authMode.js';
import { buildProviders, resolvePublicOrigin } from './providers.js';
import { buildSignInUpOverride } from './mapping.js';
import { probeAuthedEndpoint, isRefused } from './coreProbe.js';

// SuperTokens' own default API base path. It is also why the runbook widens
// the GitHub OAuth registration to /auth: SuperTokens serves its callbacks at
Expand Down Expand Up @@ -135,26 +136,25 @@ export function isLoopback(uri) {
/**
* Confirms the core actually refuses unauthenticated callers.
*
* Probes an endpoint that requires an API key when one is configured. A 401
* means the core is closed and all is well; a 200 means it answered a caller
* holding no key at all, which is the state where anyone who can reach it can
* mint a session for any user id.
* Throws ONLY on a confirmed-open core - an endpoint we know exists answering
* an unkeyed request with 200. Every other outcome warns and lets the boot
* proceed, because this guard sits on the startup path and a wrong answer here
* does not print a scary line, it stops the container.
*
* `/hello` is deliberately NOT used - it answers unauthenticated by design as
* a health check, so probing it would prove nothing.
* That distinction was missing in v1.8.2: this probed a single hardcoded path
* that core 12 does not implement, and treated the resulting 404 as proof the
* core was open - so setting AUTH_MODE=dual against a perfectly well-secured
* core would have refused to start, blaming the operator for a URL this code
* got wrong. The probe now lives in ./coreProbe.js and is shared with the
* preflight, so the two cannot drift again.
*/
export async function assertCoreRejectsAnonymous({ connectionURI, hasKey, fetchImpl = fetch }) {
const url = `${connectionURI.replace(/\/$/, '')}/recipe/users/count`;
let response;
let probe;
try {
response = await fetchImpl(url, {
method: 'GET',
headers: { 'api-version': '3.0' },
signal: AbortSignal.timeout(5000),
});
probe = await probeAuthedEndpoint({ connectionURI, fetchImpl });
} catch (e) {
// Unreachable, DNS failure, timeout. Cannot establish anything; the core
// may simply still be starting. Warn rather than refuse - see the caller.
// may simply still be starting. Warn rather than refuse.
console.warn(
`[auth] could not verify that the SuperTokens core at ${connectionURI} requires `
+ `authentication (${e.message}). If it is running without API_KEYS, anyone who can `
Expand All @@ -163,16 +163,28 @@ export async function assertCoreRejectsAnonymous({ connectionURI, hasKey, fetchI
return 'unverified';
}

if (response.status === 401) return 'closed';
if (isRefused(probe.status)) return 'closed';

throw new Error(
`The SuperTokens core at ${connectionURI} answered an unauthenticated request with `
+ `HTTP ${response.status}, which means it is running without API_KEYS. Anyone who can `
+ 'reach it can mint a session for any user id, including every value in SUPER_ADMIN_IDS, '
+ 'without any request reaching RackStack. Set API_KEYS on the core to the same value as '
+ `SUPERTOKENS_API_KEY here${hasKey ? '' : ' (which is also unset)'}, and do not publish `
+ 'its port.',
if (probe.status === 200) {
throw new Error(
`The SuperTokens core at ${connectionURI} answered an unauthenticated request to `
+ `${probe.path} with HTTP 200, which means it is running without API_KEYS. Anyone who `
+ 'can reach it can mint a session for any user id, including every value in '
+ 'SUPER_ADMIN_IDS, without any request reaching RackStack. Set API_KEYS on the core to '
+ `the same value as SUPERTOKENS_API_KEY here${hasKey ? '' : ' (which is also unset)'}, `
+ 'and do not publish its port.',
);
}

// A 404 from every candidate, or any other unexpected status, says our URL
// is wrong for this core version - not that the core is open. Refusing to
// boot on that would be punishing the operator for our own mistake.
console.warn(
`[auth] could not verify that the SuperTokens core at ${connectionURI} requires `
+ `authentication (${probe.status === null ? 'no known endpoint answered' : `unexpected HTTP ${probe.status}`}). `
+ 'This is NOT evidence that it is open, but do check it by hand.',
);
return 'unverified';
}

/**
Expand Down
41 changes: 3 additions & 38 deletions server/supertokens/preflight.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import { buildProviders, resolvePublicOrigin, PROVIDER_IDS } from './providers.js';
import { isLoopback } from './init.js';
import { AUTHED_ENDPOINTS, probeAuthedEndpoint, isRefused } from './coreProbe.js';

const PASS = 'PASS';
const FAIL = 'FAIL';
Expand All @@ -26,48 +27,12 @@ function result(status, name, detail) {
return { status, name, detail };
}

/**
* Endpoints that require an API key when one is configured, newest path first.
*
* NOT `/hello` - that answers unauthenticated by design as a health check, so a
* 200 there proves nothing about whether the core is locked down.
*
* More than one, because the path is tenant-scoped on modern cores
* (`/<tenantId>/users/count`, per supertokens-node's own querier) and was not
* on older ones. The first version of this shipped a single guessed path,
* `/recipe/users/count`, which does not exist on core 12 - so every probe came
* back 404 and the check reported a correctly-locked-down core as running wide
* open. See the 404 handling below: that false alarm is the reason this is a
* list and not a constant.
*/
const AUTHED_ENDPOINTS = Object.freeze([
'/public/users/count', // core with multitenancy (the default tenant)
'/recipe/users/count', // older cores
]);

async function probe(url, { apiKey, fetchImpl, timeoutMs = 5000 }) {
const headers = { 'api-version': '3.0' };
if (apiKey) headers['api-key'] = apiKey;
return fetchImpl(url, { method: 'GET', headers, signal: AbortSignal.timeout(timeoutMs) });
}

/**
* Probes the first endpoint this core actually implements.
*
* Returns `{ status, path }`, or `{ status: null }` when every candidate 404s -
* which means "this core does not expose any path we know how to ask", NOT
* "the core answered". The distinction is the whole point: a 404 is evidence
* about our URL, not about the core's authentication.
*/
async function probeAuthedEndpoint({ connectionURI, apiKey, fetchImpl }) {
for (const path of AUTHED_ENDPOINTS) {
// eslint-disable-next-line no-await-in-loop
const res = await probe(`${connectionURI}${path}`, { apiKey, fetchImpl });
if (res.status !== 404) return { status: res.status, path };
}
return { status: null, path: null };
}

/**
* Runs every deployment check and returns the results.
*
Expand Down Expand Up @@ -158,7 +123,7 @@ export async function runPreflight({
// is exactly why it is checked here rather than left to be noticed.
try {
const anon = await probeAuthedEndpoint({ connectionURI, fetchImpl });
if (anon.status === 401 || anon.status === 403) {
if (isRefused(anon.status)) {
checks.push(result(PASS, 'core requires authentication', `anonymous request rejected (${anon.status})`));
} else if (anon.status === 200) {
checks.push(result(
Expand Down Expand Up @@ -239,7 +204,7 @@ export async function runPreflight({
const authed = await probeAuthedEndpoint({ connectionURI, apiKey, fetchImpl });
if (authed.status === 200) {
checks.push(result(PASS, 'SUPERTOKENS_API_KEY', 'accepted by the core'));
} else if (authed.status === 401 || authed.status === 403) {
} else if (isRefused(authed.status)) {
checks.push(result(
FAIL, 'SUPERTOKENS_API_KEY',
`The core rejected it (HTTP ${authed.status}). It must be byte-identical to a value in `
Expand Down
47 changes: 46 additions & 1 deletion tests/supertokens.security.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,52 @@ describe('rejectRawOAuthTokens (authentication bypass guard)', () => {
fetchImpl: async (url) => { probed = url; return { status: 401 }; },
});
expect(probed).not.toContain('/hello');
expect(probed).toContain('/recipe/');
// A key-gated endpoint. The exact path is version-dependent, so assert the
// property rather than a literal - the literal is what broke in v1.8.2.
expect(probed).toContain('users/count');
});

it('BOOTS against a core whose endpoints all 404, rather than calling it open', async () => {
// The v1.8.3 near-miss. The preflight's 404 handling was fixed, but this
// guard kept its own copy of the probe with the old hardcoded path - and
// unlike the preflight, a wrong answer here does not print a scary line,
// it stops the container from starting.
//
// So on core 12 (which does not implement /recipe/users/count) setting
// AUTH_MODE=dual against a perfectly well-secured core would have refused
// to boot, blaming the operator for a URL this code got wrong. The probe
// now lives in coreProbe.js and is shared, so the two cannot drift again.
const { assertCoreRejectsAnonymous } = await import('../server/supertokens/init.js');
const noKnownEndpoint = async () => ({ status: 404 });

await expect(assertCoreRejectsAnonymous({
connectionURI: 'http://core:3567', hasKey: true, fetchImpl: noKnownEndpoint,
})).resolves.toBe('unverified');
});

it('finds the tenant-scoped path a modern core actually implements', async () => {
const { assertCoreRejectsAnonymous } = await import('../server/supertokens/init.js');
const core12 = async (url) => (url.endsWith('/public/users/count')
? { status: 401 }
: { status: 404 });

await expect(assertCoreRejectsAnonymous({
connectionURI: 'http://core:3567', hasKey: true, fetchImpl: core12,
})).resolves.toBe('closed');
});

it('shares one probe implementation with the preflight', async () => {
// The bug was duplication, so this asserts the de-duplication rather than
// the behaviour: both callers must import from coreProbe.js. A future
// caller that re-copies the paths reintroduces exactly this class of bug.
const init = readFileSync(new URL('../server/supertokens/init.js', import.meta.url), 'utf8');
const preflight = readFileSync(new URL('../server/supertokens/preflight.js', import.meta.url), 'utf8');

expect(init).toMatch(/from '\.\/coreProbe\.js'/);
expect(preflight).toMatch(/from '\.\/coreProbe\.js'/);
// Neither may hardcode a probe path of its own.
expect(init).not.toMatch(/users\/count/);
expect(preflight).not.toMatch(/'\/(public|recipe)\/users\/count'/);
});

it('refuses to boot against a core too old for this SDK', async () => {
Expand Down
Loading