Repository navigation
Commit 425c743
fix(plugin-auth): SSO domain verification accepts the TXT record request-domain-verification names (#25161)
Fixes #25111
Clause-②: no — the verify door now accepts the record the platform
already tells the operator to publish, as `@better-auth/sso` designed
it. The one record it stops accepting is `undefined=TOKEN`, which only
the defect produced and nothing instructs (table below).
Metadata-protocol: none
## What was wrong
`POST /api/v1/auth/admin/sso/request-domain-verification` tells the
operator to publish a TXT record named
`_better-auth-token-PROVIDER.DOMAIN` with the value
`_better-auth-token-PROVIDER=TOKEN` (`register-sso-provider.ts`,
`dnsRecordValue`). The vendor route behind the bridge, `POST
/api/v1/auth/sso/verify-domain` (`@better-auth/sso` 1.7.12,
`verifyDomain`), accepts a TXT record at that name whose trimmed value
equals either `activeVerification.identifier + '=' +
activeVerification.value` or the bare `activeVerification.value`.
`activeVerification` is the row
`internalAdapter.findVerificationValue(identifier)` reads back: a
`findMany` on `verification` keyed on `identifier`, newest first, limit
1.
`sys_verification.identifier` is declared `internal`, so the engine
strips it from that read, and `internal-field-readback.ts` re-attached
only `value`. The vendor therefore compared the published record against
`undefined=TOKEN`.
## Measured (the dispatch's four mechanism assumptions)
Base `9cfc57533`, better-auth / `@better-auth/sso` 1.7.12. Real
`ObjectQL` over `@objectstack/driver-sql` (better-sqlite3) with this
plugin's manifest, so the strip is live. Real `AuthManager` with the
real `sso({ domainVerification: { enabled: true } })`, and the plugin's
real route registration on a Hono app for the bridge. DNS is a double on
`node:dns/promises.resolveTxt`. This is the new suite run against the
unfixed source:
| TXT record at `_better-auth-token-PROVIDER.DOMAIN` | vendor
`/sso/verify-domain` | bridge `/admin/sso/verify-domain` |
|---|---|---|
| `_better-auth-token-PROVIDER=TOKEN` (the instructed record) | **502**
`DOMAIN_VERIFICATION_FAILED` | **502** `DOMAIN_VERIFICATION_FAILED` |
| `undefined=TOKEN` | **204** | **200**, `verified: true` |
| bare `TOKEN` | 204 | not run |
| wrong token, or another provider's label in the value, or the
instructed value at another provider's name | 502 | 502 |
1. **Assumption 1 holds.** The vendor's real handler received the row
without `identifier`. The `undefined=TOKEN` record verified, and only an
undefined identifier can produce that string.
2. **Assumption 2 holds.** The read-back census is the mechanism, and
its note that no flow reads `identifier` back was false. I re-traced it
on 1.7.12: across better-auth, `@better-auth/sso`, `/oauth-provider` and
`/scim`, `verifyDomain` is the only code that reads
`verification.identifier` off a row. The other `.identifier` hits are
the oauth-provider resource model, a doc comment and test-utils.
3. **Assumption 3, explained from the vendor code.** The bare token
verifies because the vendor accepts `VALUE` beside `IDENTIFIER=VALUE` by
design (`normalizedRecord === verificationRecord || normalizedRecord ===
verificationValue`), and `value` was always re-attached.
4. **Assumption 4, run.** The bridge re-dispatches to the vendor route
and gives the same answers: 502 for the instructed record, and 200 for
`undefined=TOKEN`.
## The remedy, under the four axes
Two candidates were measured against the vendor code:
- **A. The read-back seam** (this PR): re-attach
`sys_verification.identifier` to the rows the adapter hands better-auth.
- **B. The instruction text**: change `dnsRecordValue` to the bare
token.
**What the verification proves under each.** The vendor queries DNS only
at `identifier + '.' + domain`. That `identifier` is computed from the
provider id (`getVerificationIdentifier`); it is never read from the
row. The token is a fresh random value stored under that provider's
identifier. So under both A and B the provider binding is the queried
name plus the per-provider token, and the vendor already accepts the
bare token. The dispatch's premise that the bare-token instruction
"would make the published record prove less" does not hold against the
vendor code: the label inside the value repeats what the queried name
already binds. Neither remedy weakens the proof, so this is not the
`needs_decision` case.
**Chosen: A.**
- **Real business need.** An operator follows the record the platform
shows them, which `content/docs/permissions/sso.mdx` repeats ("publish
`dnsRecordName` / `dnsRecordValue`"). A makes that record verify. B
would change the shown record and leave the vendor's `IDENTIFIER=VALUE`
branch dead on this store, with `undefined=TOKEN` still verifying.
- **Long-term soundness (contract-first, no workarounds).** The defect
is in the producer of the row the vendor reads, the storage seam, not in
the instruction. The census is the platform's mechanism for handing an
internal column back to the vendor's adapter. B would bend a correct
instruction around a seam defect, which is a consumer-side
accommodation.
- **Keeping AI authors from going wrong.** A makes the census true: one
named reader, traced on the installed version. The next agent who reads
it is no longer told "no flow reads identifier back". B would leave that
false census in place.
- **Startup focus.** A is one row in an existing table: no new surface,
no new gate, no docs change. B changes the value of a public response
field (`dnsRecordValue`) that operators and the docs read.
**What A hands to whom.** Every verification read better-auth issues is
keyed on `identifier` (`findVerificationValue`,
`consumeVerificationValue`), so A hands the vendor a value it passed in
itself. A `findOne` keyed on it is answered from the lookup with no
privileged read. No vendor route returns a verification row to its
caller: I checked every `findVerificationValue` /
`consumeVerificationValue` result variable in those four packages
against `ctx.json(` and found none passed whole or spread. The generic
data API still strips the column, and the suite's precondition case pins
that.
## What the verify door accepts, before and after
| record at the instructed name | before | after |
|---|---|---|
| `_better-auth-token-PROVIDER=TOKEN` (instructed) | refused 502 |
**accepted** |
| bare `TOKEN` | accepted | accepted (unchanged; the vendor's second
form) |
| `undefined=TOKEN` | accepted | **refused 502** |
| wrong token / another provider's label / another provider's name |
refused 502 | refused 502 |
The `Clause-②: no` line above follows the claim. The narrowing is only
`undefined=TOKEN`, a string that exists only because of the defect.
## Change
- `packages/plugins/plugin-auth/src/internal-field-readback.ts`: the
`sys_verification` readback row gains `identifier` with
`absenceProvesStrip: true`, because the column is `required: true`. The
census comment now names the one reader, traced on 1.7.12, and says why
handing the column back discloses nothing. The fail-closed consequence
text also names domain verification.
-
`packages/plugins/plugin-auth/src/sso-domain-verification-instructed-record.test.ts`
(new): the pins on the vendor's real handler, both doors. Refusals
assert `code` and `status`, the DNS name the vendor queried, and that
the provider stays unverified at rest.
- `packages/plugins/plugin-auth/src/internal-field-readback.test.ts`:
three seam pins. Both columns come back on a stripped findMany row, one
batched read per column. A read keyed on `identifier` answers it from
the lookup. The seam fails closed with no accessor.
- `.changeset/25111-sso-domain-verification-instructed-record.md`: patch
for `@objectstack/plugin-auth`.
`register-sso-provider.ts` is unchanged, and so is the instruction.
## Tests
- Before the fix, new suite on unfixed source: `Tests 4 failed | 8
passed (12)`. The four failures are the instructed record (502 on both
doors) and `undefined=TOKEN` (204 / 200).
- After the fix (commit `7f0292dde`), the new suite plus
`internal-field-readback.test.ts` and
`internal-credential-readback.test.ts`: `Test Files 3 passed (3)`,
`Tests 46 passed (46)`.
- Full `@objectstack/plugin-auth` suite and typecheck on the final head
`731668228` (the fix merged with `origin/main` `23540582f`): `Test Files
183 passed (183)`, `Tests 3466 passed | 10 skipped (3476)`. `typecheck`:
`tsc --noEmit`, the examples program, and `check:test-typecheck: OK` (10
files / 88 errors / 23 pinned signatures held, unchanged). Lock verdict
`command-exit 0`. Both new test files are in the `tsconfig.test.json`
program (`--listFilesOnly`).
## Ablation (run once, then restored)
Run on the fix commit `7f0292dde`. The ablation used `node
scripts/ablation-replace.mjs --delete` on the anchor for the new
`identifier` readback row, wrapping `vitest run` over the new suite and
`internal-field-readback.test.ts`. The subject is imported from `src/`
by relative path, so no build sits between the mutation and the run.
- The mutation landed: anchor x1 to x0, blob `e8e98f4e53e8` to
`4d7810f74bf0`.
- Result: `Tests 7 failed | 37 passed (44)`. The instructed record went
back to 502 on both doors, `undefined=TOKEN` went back to 204 / 200, and
the three seam pins failed.
- Restored: blob `e8e98f4e53e8` equals HEAD, `git diff HEAD` is empty,
and `git status --porcelain` is empty.
## Gates
Local subset, on the final head `731668228`. `node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derives 67 commands for these 4 paths (merge
base `23540582f`). All 67 were run, each exit code captured before any
pipe, and all 67 exited 0. `--ran` reconciliation: `67 derived, 67 run,
0 NOT-MEASURED, 0 UNRUN`.
`pnpm check:dual-build-cjs-loads` deserves a note. On the pre-merge head
`7f0292dde` it exited 3 (`PREREQUISITE NOT MET`) because not every
package had a `dist/`. The workspace build that `check:type-check-debt`
runs supplied them, and on the final head it measured green (112 require
entry points across 69 packages load).
The five artifact-roster gates whose roster sits under a changed
directory also exit 0 on the final head: `check-changeset-fixed`,
`check:auth-mount-ledger`, `check:authz-resolver`,
`check:error-code-casing`, `check:filter-alias-parity`.
Every family CI runs is cited from this PR's check-runs, not re-run
here.
## Acceptance notes
- `packages/platform-objects/src/identity/sys-verification.object.ts`
still says `identifier` "has none [readback row], because no better-auth
flow reads it back off a row". That sentence was already false (this
card) and is now plainly so. It is the engine lane's file, outside this
card's file surface, so it is not edited here. Carrier: none.
- `objectql-adapter.ts`'s `findMany` passes no lookup to the seam, so
`identifier` costs one id-batched privileged read on each verification
`findMany` / `consumeOne`. A `findOne` keyed on it is answered from the
lookup. These are one-time flows (OTP, magic link, reset, OAuth state,
domain verification), not per-request reads. Passing `findMany`'s
`where` as the lookup would make that read free. It is not done here
because `objectql-adapter.ts` is outside the file surface.
- The new suite's tests run in about 1.4 s. Its module import is about
18 s on the shared dev box, the same shape as the sibling real-engine
suites (`internal-credential-readback.test.ts`).
---
_Generated by [Claude
Code](https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 06900c9 commit 425c743
4 files changed
Lines changed: 409 additions & 10 deletions
File tree
- .changeset
- packages/plugins/plugin-auth/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
184 | 184 | | |
185 | 185 | | |
186 | 186 | | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
187 | 225 | | |
188 | 226 | | |
189 | 227 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
195 | 195 | | |
196 | 196 | | |
197 | 197 | | |
198 | | - | |
| 198 | + | |
| 199 | + | |
199 | 200 | | |
200 | 201 | | |
201 | 202 | | |
| |||
205 | 206 | | |
206 | 207 | | |
207 | 208 | | |
208 | | - | |
209 | | - | |
210 | | - | |
211 | | - | |
212 | | - | |
213 | | - | |
214 | | - | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
215 | 227 | | |
216 | 228 | | |
217 | 229 | | |
| |||
222 | 234 | | |
223 | 235 | | |
224 | 236 | | |
225 | | - | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
226 | 241 | | |
227 | 242 | | |
228 | 243 | | |
| |||
252 | 267 | | |
253 | 268 | | |
254 | 269 | | |
255 | | - | |
| 270 | + | |
| 271 | + | |
256 | 272 | | |
257 | 273 | | |
258 | 274 | | |
| |||
0 commit comments