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

`POST /admin/sso/verify-domain` now answers the DISABLED condition the way its
sibling always has. When SSO domain verification is off for an environment,
`@better-auth/sso` never mounts the inner endpoint and answers `404` with no
code. Both bridge routes recognise that shape, and they used to answer it
differently (#10859):

| route | answered | answers instead |
| --- | --- | --- |
| `POST /admin/sso/request-domain-verification` | `400` `DOMAIN_VERIFICATION_DISABLED` | unchanged |
| `POST /admin/sso/verify-domain` | `404` `DOMAIN_VERIFICATION_FAILED` | `400` `DOMAIN_VERIFICATION_DISABLED` |

`verify-domain` rewrote only the `message` for that branch and let the code fall
through to its generic failure default, so the response carried "the feature is
off" copy under a code that means "verification failed". A caller can only act
on the machine-readable half, and the two halves disagreed. The status moves
with the code: the inner `404` describes the INNER endpoint, which is unmounted,
whereas this bridge route is mounted unconditionally — passing that status
through said "no such endpoint" about a resource that exists.

If you match on `DOMAIN_VERIFICATION_FAILED` (or on `404`) to detect the
disabled case on `verify-domain`, match on `DOMAIN_VERIFICATION_DISABLED` (or on
`400`) instead — the same pair `request-domain-verification` has always
answered. The distinction is worth having: `DISABLED` means "turn on
`OS_SSO_DOMAIN_VERIFICATION`", `FAILED` means "the DNS TXT record is not visible
yet, retry".

**No `packages/spec` change, and the emitted vocabulary gains no member.** Both
codes are already registered for `@objectstack/plugin-auth` in the error-code
ledger, with exactly these meanings (`DOMAIN_VERIFICATION_DISABLED` — "domain
verification is off on this deployment"). This route was emitting a *declared*
code whose registered meaning is a different condition, so this is
declared-vs-enforced restoration rather than a new contract decision.

**A genuine verification failure still answers the failure code, and the vendor
pass-through arm is untouched on both routes.** The rewrite is keyed to the
disabled shape specifically — `404` *without* a code. A `404` that carries
`@better-auth/sso`'s own code is the vendor's diagnosis and reaches the caller
verbatim, status included, as does every non-404 failure. That direction is the
load-bearing one — an implementation keyed to "any 404", or to `!resp.ok`, would
satisfy the disabled case while destroying the diagnosis a caller acts on — and
it is pinned in both directions in
`packages/plugins/plugin-auth/src/sso-domain-verification-error-codes.test.ts`.

Shipped as `minor`, following the same call the casing rename on these two
routes made (#10716). The argument for it: the vocabulary is unchanged, and the
old pairing was self-contradictory rather than a contract anyone could have
relied on deliberately. The argument against it, stated here rather than
settled: unlike that rename — whose old spellings were undeclared values no
schema admitted — `DOMAIN_VERIFICATION_FAILED` *is* a declared, registered code,
so a client keyed to it for this case was keyed to something the published
contract admitted, and both halves of the answer change. A reviewer who reads
that as `major` is not reading it wrong; this PR does not decide it silently.
16 changes: 13 additions & 3 deletions packages/plugins/plugin-auth/src/register-sso-provider.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -457,11 +457,21 @@ export async function runVerifyDomain(
if (resp.ok) {
return { status: 200, body: { success: true, data: { providerId, verified: true, message: 'Domain ownership verified — this provider can now sign users in.' } } };
}
// The feature is OFF for this env: `@better-auth/sso` never mounted the inner
// endpoint, so its `404` describes the INNER route. THIS route is mounted
// unconditionally, so passing that status through says "no such endpoint"
// about a resource that exists. Answer the sibling's answer instead — same
// condition, same code, same status (#10859). Before this, the branch
// rewrote only the `message` and let the code fall through to the generic
// default below, so the machine-readable field said "verification failed"
// while the human-readable one said "the feature is off"; a caller can only
// act on the first.
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).' } } };
}
// Friendlier copy for the expected failure modes.
let message = parsed?.message || 'Domain verification failed';
if (resp.status === 404 && !parsed?.code) {
message = 'Domain verification is not enabled for this environment (set OS_SSO_DOMAIN_VERIFICATION).';
} else if (parsed?.code === 'NO_PENDING_VERIFICATION') {
if (parsed?.code === 'NO_PENDING_VERIFICATION') {
message = 'No pending verification — click “Request Domain Verification” first to get the DNS record.';
} 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.';
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,13 +64,18 @@ describe('#10716 SSO domain verification — our default is a registered ADR-011
});

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));
// RE-FIXTURED by #10859. This case used to be driven by `fakeHandle(404,
// undefined)` — which is the DISABLED shape, not a verification failure, and
// now answers `DOMAIN_VERIFICATION_DISABLED` (see the #10859 describe below).
// Pointing the generic-default leg at that shape would have left the default
// arm of `verify-domain` with no coverage at all once the disabled branch
// returned early, so it is driven by a real uncoded failure instead — the
// same `502` its sibling above uses.
const res = await runVerifyDomain(fakeHandle(502, { message: 'upstream exploded' }), post(VERIFY_URL));

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

it('both defaults are SCREAMING_SNAKE and registered for this package — reused, not invented', () => {
Expand DownExpand Up@@ -116,3 +121,83 @@ describe('#10716 the vendor pass-through arm is untouched', () => {
expect(res.status).toBe(400);
});
});

describe('#10859 the DISABLED condition gets ONE answer across both routes', () => {
/**
* The feature being off is a different condition from a verification failing,
* and the two SSO domain-verification routes used to answer it differently:
* the sibling rewrote to `400 DOMAIN_VERIFICATION_DISABLED`, while
* `verify-domain` rewrote only the MESSAGE and let the code fall through to
* the generic failure default — so its machine-readable field said
* "verification failed" while its human-readable one said "the feature is
* off". A caller can only act on the first.
*
* Both halves are asserted per ADR-0112 (`code` AND `status`), and the
* counter-direction below is the load-bearing half: stamping `DISABLED` on
* every failure would pass a one-directional suite while destroying exactly
* the diagnosis the caller acts on.
*/
const DISABLED = 'DOMAIN_VERIFICATION_DISABLED';
/** The inner endpoint is unmounted when the feature is off: 404, no code. */
const disabledInner = () => fakeHandle(404, undefined);

it('verify-domain: the disabled condition answers the dedicated code at the sibling’s status', async () => {
const res = await runVerifyDomain(disabledInner(), post(VERIFY_URL));

expect(res.body.success).toBe(false);
expect(res.body.error?.code).toBe(DISABLED);
expect(res.status).toBe(400);
expect(res.body.error?.message).toContain('OS_SSO_DOMAIN_VERIFICATION');
});

it('both routes answer the SAME code and the SAME status for the SAME condition', async () => {
// The card's governing invariant, asserted directly rather than inferred
// from the two per-route cases above: one answer for one condition.
const verify = await runVerifyDomain(disabledInner(), post(VERIFY_URL));
const request = await runRequestDomainVerification(disabledInner(), post(REQUEST_URL));

expect(verify.body.error?.code).toBe(request.body.error?.code);
expect(verify.status).toBe(request.status);
expect(verify.body.error?.code).toBe(DISABLED);
expect(verify.status).toBe(400);
});

it('DOMAIN_VERIFICATION_DISABLED is registered for this package — reused, not invented', () => {
expect(DISABLED).toMatch(/^[A-Z][A-Z0-9_]*$/);
expect(ERROR_CODE_LEDGER['@objectstack/plugin-auth']).toContain(DISABLED);
});

// ── the load-bearing direction: DISABLED is NOT stamped on every failure ──

it('verify-domain: a genuine verification failure still answers the FAILURE code', async () => {
// Uncoded, but not the disabled shape. An implementation that keyed on
// `!resp.ok` instead of the 404-without-a-code shape would answer DISABLED
// here and tell the admin to flip an env var that is already on.
const res = await runVerifyDomain(fakeHandle(502, { message: 'upstream exploded' }), post(VERIFY_URL));

expect(res.body.error?.code).toBe(OUR_DEFAULT);
expect(res.body.error?.code).not.toBe(DISABLED);
expect(res.status).toBe(502);
});

it('verify-domain: a 404 that CARRIES a vendor code is the vendor’s diagnosis, not DISABLED', async () => {
// The disabled shape is 404 *without* a code. A 404 that carries one is the
// vendor answering, and both its code and its status pass through untouched
// — the arm #10716 pinned, re-pinned here at the status this branch tests.
const res = await runVerifyDomain(fakeHandle(404, { code: VENDOR_CODE, message: 'vendor copy' }), post(VERIFY_URL));

expect(res.body.error?.code).toBe(VENDOR_CODE);
expect(res.body.error?.code).not.toBe(DISABLED);
expect(res.status).toBe(404);
});

it('request-domain-verification: a genuine failure still answers the FAILURE code', async () => {
// The sibling's counter-direction, so the parity assertion above cannot be
// satisfied by both routes collapsing onto DISABLED.
const res = await runRequestDomainVerification(fakeHandle(502, { message: 'upstream exploded' }), post(REQUEST_URL));

expect(res.body.error?.code).toBe(OUR_DEFAULT);
expect(res.body.error?.code).not.toBe(DISABLED);
expect(res.status).toBe(502);
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -258,7 +258,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 DOMAIN_VERIFICATION_FAILED while SSO is off',
note: 'admin passes the gate and lands on 400 DOMAIN_VERIFICATION_DISABLED while SSO is off (#10859 — it answered 404 DOMAIN_VERIFICATION_FAILED before, out of step with the sibling above)',
},

// ── #9652: ban / unban moved from the vendor to an ObjectStack mount ────
Expand Down
Loading