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
46 changes: 46 additions & 0 deletions .changeset/sso-domain-verification-error-code-casing.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
---
"@objectstack/plugin-auth": minor
---

The two SSO domain-verification admin routes now answer a registered ADR-0112
error code. `POST /admin/sso/request-domain-verification` and
`POST /admin/sso/verify-domain` shape their failure as
`code: parsed?.code || <our default>`, and the default half — the code
ObjectStack itself authors when @better-auth/sso returns none — was lowercase
(#10716, found by #10658):

| route | wrote | writes instead |
| --- | --- | --- |
| `POST /admin/sso/request-domain-verification` | `request_domain_verification_failed` | `DOMAIN_VERIFICATION_FAILED` |
| `POST /admin/sso/verify-domain` | `verify_domain_failed` | `DOMAIN_VERIFICATION_FAILED` |

If you match on either lowercase spelling, match on `DOMAIN_VERIFICATION_FAILED`
instead — the two routes are distinguished by their path, as they already were
for every other failure they can answer.

`DOMAIN_VERIFICATION_FAILED` is reused, not invented: it is already registered
for `@objectstack/plugin-auth` in the error-code ledger, so this PR adds nothing
to `packages/spec` and the emitted vocabulary gets no new member. A new spelling
(`VERIFY_DOMAIN_FAILED`) would have needed a ledger registration to be a legal
`error.code` at all, and — measured while fixing this — an unregistered code in
an `||` fallback slot is currently invisible to BOTH error-code gates, so it
would have shipped as exactly the silent fourth state ADR-0112 D3 exists to
prevent.

**The vendor pass-through arm is unchanged.** `parsed?.code` still reaches the
caller verbatim, so @better-auth/sso's own diagnosis (`NO_PENDING_VERIFICATION`,
`DOMAIN_VERIFICATION_FAILED`) is never overwritten by ours — the half that would
be silently lost by a handler that stamped our code unconditionally, and it is
pinned in both directions by
`packages/plugins/plugin-auth/src/sso-domain-verification-error-codes.test.ts`.
Statuses and messages are untouched on every path.

ADR-0087 disposition, in prose because the marker vocabulary has no slot for
this shape: nothing is registered and nothing needs to be. The declared wire
contract is `error.code ∈ StandardErrorCode ∪ ERROR_CODE_LEDGER`, and neither
lowercase spelling was ever a member of it — they were undeclared values a
blind gate let through, so this brings the implementation onto the published
contract rather than changing that contract. There is no metadata surface for
`objectstack migrate meta` to rewrite: error codes live in responses, not in
stored metadata. The table above is here for anyone who matched the undeclared
spelling anyway, which is why this ships as `minor` rather than `patch`.
11 changes: 9 additions & 2 deletions packages/plugins/plugin-auth/src/register-sso-provider.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -408,7 +408,11 @@ export async function runRequestDomainVerification(
if (resp.status === 404 && !parsed?.code) {
return { status: 400, body: { success: false, error: { code: 'DOMAIN_VERIFICATION_DISABLED', message: 'Domain verification is not enabled for this environment (set OS_SSO_DOMAIN_VERIFICATION).' } } };
}
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'request_domain_verification_failed', message: parsed?.message || 'Failed to request domain verification' } } };
// ADR-0112: `parsed?.code` is better-auth's own code passing through — never
// overwritten. The DEFAULT is ours, so it is the registered SCREAMING ledger
// entry for this feature (`DOMAIN_VERIFICATION_FAILED`, @objectstack/plugin-auth)
// rather than a bespoke lowercase spelling (#10716).
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'DOMAIN_VERIFICATION_FAILED', message: parsed?.message || 'Failed to request domain verification' } } };
}

const token = str(parsed?.domainVerificationToken);
Expand DownExpand Up@@ -462,5 +466,8 @@ export async function runVerifyDomain(
} else if (parsed?.code === 'DOMAIN_VERIFICATION_FAILED') {
message = 'DNS TXT record not found yet. Add the record shown when you requested verification, allow time for DNS to propagate, then retry.';
}
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'verify_domain_failed', message } } };
// ADR-0112, as above: the vendor's code passes through untouched; our default
// is the registered ledger entry (#10716). The `message` above still carries
// the specific diagnosis for each expected failure mode.
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'DOMAIN_VERIFICATION_FAILED', message } } };
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #10716 — the two SSO domain-verification routes answer an ADR-0112 error code.
*
* Both handlers shape their failure as `code: parsed?.code || <our default>`,
* and the two halves of that expression have DIFFERENT owners:
*
* - `parsed?.code` is @better-auth/sso's own code passing through. It is the
* vendor's diagnosis and must reach the caller unchanged.
* - the default is OURS, so ADR-0112 applies to it: SCREAMING_SNAKE, drawn
* from the registered vocabulary rather than invented at the call site.
*
* Both directions are pinned below, and that pairing is the point. Pinning only
* "our code appears" would go green against a handler that overwrites the
* vendor's code unconditionally — which would swallow exactly the diagnosis the
* caller needs (`NO_PENDING_VERIFICATION` tells them to request the DNS record
* first; a blanket "verification failed" does not). Statuses are pinned in both
* directions too: the inner status passes through untouched.
*
* Rejection cases assert `code` AND `status` per ADR-0112 — a bare "it threw"
* would pass against a handler that refuses everyone.
*/

import { describe, it, expect } from 'vitest';
// The `/api` subpath is where the built package exposes the ledger VALUE — the
// root entry re-exports the schemas only. Reading it through the published
// exports map (i.e. `dist`) is deliberate: this asserts against the surface a
// consumer actually gets, not against a source file this test could reach.
import { ERROR_CODE_LEDGER } from '@objectstack/spec/api';
import { runRequestDomainVerification, runVerifyDomain, type AuthRequestHandler } from './register-sso-provider.js';

/** The default this package authors for both routes when the vendor gives no code. */
const OUR_DEFAULT = 'DOMAIN_VERIFICATION_FAILED';
/** A vendor code with its own meaning — the one an overwrite would destroy. */
const VENDOR_CODE = 'NO_PENDING_VERIFICATION';

const REQUEST_URL = 'https://app.example/api/v1/auth/admin/sso/request-domain-verification';
const VERIFY_URL = 'https://app.example/api/v1/auth/admin/sso/verify-domain';

/** A fake inner @better-auth/sso endpoint that answers one canned response. */
function fakeHandle(status: number, body: unknown): AuthRequestHandler {
return async () =>
new Response(body === undefined ? '' : JSON.stringify(body), {
status,
headers: { 'content-type': 'application/json' },
});
}

const post = (url: string) =>
new Request(url, {
method: 'POST',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ providerId: 'p1', domain: 'acme.example' }),
});

describe('#10716 SSO domain verification — our default is a registered ADR-0112 code', () => {
it('request-domain-verification: an uncoded vendor failure answers our SCREAMING default, status passed through', async () => {
const res = await runRequestDomainVerification(fakeHandle(502, { message: 'upstream exploded' }), post(REQUEST_URL));

expect(res.body.success).toBe(false);
expect(res.body.error?.code).toBe(OUR_DEFAULT);
expect(res.status).toBe(502);
});

it('verify-domain: an uncoded vendor failure answers our SCREAMING default, status passed through', async () => {
// The 404-without-a-code shape: SSO domain verification is off for this env.
// This is the response the dogfood admin-route probe observes.
const res = await runVerifyDomain(fakeHandle(404, undefined), post(VERIFY_URL));

expect(res.body.success).toBe(false);
expect(res.body.error?.code).toBe(OUR_DEFAULT);
expect(res.status).toBe(404);
});

it('both defaults are SCREAMING_SNAKE and registered for this package — reused, not invented', () => {
expect(OUR_DEFAULT).toMatch(/^[A-Z][A-Z0-9_]*$/);
// The mechanical half of "we reused a ledger entry": if this code were a new
// spelling it would need a `packages/spec` registration, and an unregistered
// code in a fallback slot is invisible to BOTH error-code gates — a silent
// fourth state, which is precisely what ADR-0112 D3 exists to prevent.
expect(ERROR_CODE_LEDGER['@objectstack/plugin-auth']).toContain(OUR_DEFAULT);
});
});

describe('#10716 the vendor pass-through arm is untouched', () => {
it('request-domain-verification: the vendor code and status reach the caller unchanged', async () => {
const res = await runRequestDomainVerification(
fakeHandle(400, { code: VENDOR_CODE, message: 'vendor copy' }),
post(REQUEST_URL),
);

expect(res.body.error?.code).toBe(VENDOR_CODE);
expect(res.body.error?.code).not.toBe(OUR_DEFAULT);
expect(res.status).toBe(400);
expect(res.body.error?.message).toBe('vendor copy');
});

it('verify-domain: the vendor code and status reach the caller unchanged', async () => {
const res = await runVerifyDomain(fakeHandle(400, { code: VENDOR_CODE, message: 'vendor copy' }), post(VERIFY_URL));

expect(res.body.error?.code).toBe(VENDOR_CODE);
expect(res.body.error?.code).not.toBe(OUR_DEFAULT);
expect(res.status).toBe(400);
// This route substitutes friendlier COPY for known vendor codes — the code
// itself still passes through, which is the half that matters on the wire.
expect(res.body.error?.message).toContain('Request Domain Verification');
});

it('request-domain-verification: the disabled branch keeps its own dedicated code', async () => {
// Unchanged by #10716 and pinned so the rename cannot have blurred the two:
// "the feature is off" is a different answer from "verification failed".
const res = await runRequestDomainVerification(fakeHandle(404, undefined), post(REQUEST_URL));

expect(res.body.error?.code).toBe('DOMAIN_VERIFICATION_DISABLED');
expect(res.status).toBe(400);
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -249,7 +249,7 @@ function expectationsFor(targetUserId: string): Record<string, RouteExpectation>
'POST /api/v1/auth/admin/sso/verify-domain': {
bucket: 'objectstack-gate',
body: { providerId: 'refusal-probe-oidc' },
note: 'admin passes the gate and lands on 404 verify_domain_failed while SSO is off',
note: 'admin passes the gate and lands on 404 DOMAIN_VERIFICATION_FAILED while SSO is off',
},

// ── #9652: ban / unban moved from the vendor to an ObjectStack mount ────
Expand Down
54 changes: 39 additions & 15 deletions scripts/check-error-code-casing.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -162,27 +162,28 @@ const CODE_POSITION_PATTERNS = [
* an allowlist nobody re-reads. A NEW lowercase code never joins it: the gate
* refuses that, and the remedy for a fresh finding is a registered SCREAMING
* code, never a line here.
*
* [#10716] It is now EMPTY, and that is this list reaching its designed end
* rather than a list that never had a use: both entries it was created for
* (`request_domain_verification_failed`, `verify_domain_failed`) were renamed
* onto the registered ledger code `DOMAIN_VERIFICATION_FAILED` by the services
* lane, and the owning PR deleted them here — the coordination #10658 asked
* for, so the gate ends up green with zero exceptions. Emptiness costs no
* coverage: --self-test drives the shrink-only semantics against a FIXTURE
* registry, and pins the live one AT zero so a future exception cannot be
* added quietly.
*/
const KNOWN_LOWERCASE_CODES = new Map([
[
'packages/plugins/plugin-auth/src/register-sso-provider.ts::request_domain_verification_failed',
'wire-visible; rename owned by #10716 (services lane)',
],
[
'packages/plugins/plugin-auth/src/register-sso-provider.ts::verify_domain_failed',
'wire-visible; rename owned by #10716 (services lane); pinned by name in the dogfood suite',
],
]);
const KNOWN_LOWERCASE_CODES = new Map([]);

/** Split findings into the two the wire already carries and everything else. */
export function partitionKnown(violations) {
export function partitionKnown(violations, registry = KNOWN_LOWERCASE_CODES) {
const known = [];
const fresh = [];
for (const v of violations) {
(KNOWN_LOWERCASE_CODES.has(`${v.file}::${v.literal}`) ? known : fresh).push(v);
(registry.has(`${v.file}::${v.literal}`) ? known : fresh).push(v);
}
const reached = new Set(known.map((v) => `${v.file}::${v.literal}`));
const stale = [...KNOWN_LOWERCASE_CODES.keys()].filter((k) => !reached.has(k)).sort();
const stale = [...registry.keys()].filter((k) => !reached.has(k)).sort();
return { known, fresh, stale };
}

Expand DownExpand Up@@ -302,8 +303,19 @@ function selfTest() {
// [#10658] The shrink-only registry, in both directions. The second one is
// the load-bearing half: when the owning card's rename lands, a stale line
// must FAIL rather than sit there as a quiet allowlist entry.
//
// [#10716] These drive a FIXTURE registry rather than the live one, which is
// now empty. The semantics being pinned belong to the MECHANISM (an entry
// that stops matching is stale and fails; a new code is never absorbed), and
// they have to survive the live list reaching zero — otherwise emptying it
// would have silently taken the coverage with it. The live list gets its own
// assertion below.
const SSO = 'packages/plugins/plugin-auth/src/register-sso-provider.ts';
const row = (literal) => ({ file: SSO, line: 1, literal, form: 'fallback' });
const fixtureRegistry = new Map([
[`${SSO}::request_domain_verification_failed`, 'fixture — the shape the live list had before #10716'],
[`${SSO}::verify_domain_failed`, 'fixture — the shape the live list had before #10716'],
]);
const partitionCases = [
[[row('request_domain_verification_failed'), row('verify_domain_failed')], { known: 2, fresh: 0, stale: 0 }, 'both known rows still present'],
[
Expand All@@ -315,20 +327,32 @@ function selfTest() {
[[], { known: 0, fresh: 0, stale: 2 }, 'an empty tree makes every entry stale'],
];
for (const [input, want, label] of partitionCases) {
const got = partitionKnown(input);
const got = partitionKnown(input, fixtureRegistry);
const shape = { known: got.known.length, fresh: got.fresh.length, stale: got.stale.length };
if (shape.known !== want.known || shape.fresh !== want.fresh || shape.stale !== want.stale) {
console.error(` ✗ self-test "${label}": expected ${JSON.stringify(want)}, got ${JSON.stringify(shape)}`);
failed++;
}
}

// [#10716] The live registry, at zero. It is closed to new entries by the rule
// above, so "closed" is checked rather than merely written down: a wire-visible
// code that genuinely needs deferring is a call for the ADR-0112 owner to make
// in the open, not a line someone adds back here on the way past.
if (KNOWN_LOWERCASE_CODES.size !== 0) {
console.error(
` ✗ self-test "the live registry stays empty": KNOWN_LOWERCASE_CODES holds ${KNOWN_LOWERCASE_CODES.size} entry/entries — ` +
`this list is closed (#10658/#10716); a new deferral is an ADR-0112 decision, not a line here.`,
);
failed++;
}

if (failed) {
console.error(`\n✗ check-error-code-casing self-test failed (${failed} case(s)).`);
process.exit(1);
}
console.log(
`✓ check-error-code-casing self-test: ${cases.length} recognizer case(s) + ${partitionCases.length} registry case(s) pass.`,
`✓ check-error-code-casing self-test: ${cases.length} recognizer case(s) + ${partitionCases.length + 1} registry case(s) pass.`,
);
}

Expand Down
Loading