Repository navigation
Commit 1c3a4d9
fix(auth): TOTP enrollment names the deployment app name as its issuer, not Better Auth (#21752)
Fixes #21731
Clause-②: no
A TOTP enrollment now names the deployment, not the auth library.
`/two-factor/enable` and `/two-factor/get-totp-uri` answered
`otpauth://totp/Better%20Auth:...&issuer=Better+Auth`. They now carry
the deployment's app name: `OS_APP_NAME`, else the configured app name,
else `ObjectStack`. An explicitly set `branding.workspace_name` still
outranks it. Existing enrollments keep working, because the issuer is a
display label beside an unchanged secret.
## What changed
- **`@objectstack/plugin-auth`.** `AuthManager.createAuthInstance()`
passes `getAppName()` to better-auth as its `appName`. That is the one
app-name authority every auth email already reads. No second read was
added, and `twoFactor({ issuer })` is left unset (see the better-auth
reading below).
- **`@objectstack/cli` (producer side).** `objectstack serve` constructs
`AuthPlugin` with `appName: resolveDeploymentAppName(...)`.
`resolveDeploymentAppName` is the email capability's existing chain
(`OS_APP_NAME` > `config.email.appName` >
`config.email.defaultTemplateContext.appName` > `config.appName` >
`ObjectStack`), factored out of `resolveEmailCapabilityArg` unchanged.
Both consumers now call that one resolver with the same arguments.
- **Docs.** The `OS_APP_NAME` row in
`content/docs/deployment/environment-variables.mdx` now says what the
variable names. It used to say "the OAuth app display name", which was
never true.
- **Changeset.** `.changeset/21731-totp-issuer-app-name.md`, `patch` for
both packages. It says that an existing enrollment keeps working.
## Why the landing reaches `packages/cli`: the triage premise was false
under `serve`
The card and triage both assumed that `getAppName()` honours
`OS_APP_NAME`. Under `objectstack serve` it did not. `serve` built
`AuthPlugin` with no `appName`, so `getAppName()` answered
`ObjectStack`. Auth emails hid this: their own `appName` outranks the
email service's template context (`plugin-email/src/email-service.ts`,
the `{ ...defaultTemplateContext, ...input.data }` merge), and the
template context was the only place that carried `OS_APP_NAME`.
These readings come from the public door, an `os dev` boot (`pnpm
dev:todo -- --fresh`, `OS_AUTH_TWO_FACTOR=true`, `OS_APP_NAME='Acme
Ops'`, seeded admin, secrets redacted):
| build | `POST /api/v1/auth/two-factor/enable` totpURI | password-reset
email subject (`sys_email`) |
|---|---|---|
| the `serve.ts` line ablated, `auth-manager.ts` fix in place |
`otpauth://totp/ObjectStack:admin%40objectos.ai?...&issuer=ObjectStack`
| `Reset your ObjectStack password` |
| this PR |
`otpauth://totp/Acme%20Ops:admin%40objectos.ai?...&issuer=Acme+Ops` |
(see the liveness table) |
Without the `serve` half, the card's first pin (the issuer equals
`OS_APP_NAME`) fails at the public door. The same root cause also fixes
the auth emails' `{{appName}}` under `serve`.
## better-auth 1.7.3, read off its dist
- `appName` is read once, at init:
`dist/context/create-context.mjs:131`, `appName: options.appName ||
"Better Auth"`.
- That context value is read in exactly two places, both otpauth URIs:
- `dist/plugins/two-factor/index.mjs:170` (enable): `issuer ||
options?.issuer || ctx.context.appName`. The first term is the request
body's own `issuer`.
- `dist/plugins/two-factor/totp/index.mjs:133` (get-totp-uri):
`options?.issuer || ctx.context.appName`. Here `options` is
`totpOptions`, and its type omits `issuer`
(`dist/plugins/two-factor/types.d.mts:25` declares `totpOptions` as
`TOTPOptions` with `issuer` omitted).
- No other runtime read of `appName` exists in `better-auth`,
`@better-auth/core`, `sso`, `scim`, `oauth-provider`, `telemetry` or
`kysely-adapter`. `core` has 3 hits and all of them are `.d.mts` docs.
The other packages have 0.
- **No stored or wire identifier changes.** The cookie prefix is the
literal `"better-auth"` (`dist/cookies/index.mjs:28`). The core type doc
says the prefix defaults from `appName`, but in 1.7.3 it does not.
better-auth reads no `APP_NAME` env either.
- So `appName` is the one typed key that names both routes. `twoFactor({
issuer })` alone would leave `get-totp-uri` on `Better Auth`, so the
triage condition for setting it explicitly does not hold.
## Liveness: what follows a branding change
| reading (public door) | result |
|---|---|
| boot with `OS_BRANDING_WORKSPACE_NAME='Env Brand'` and
`OS_APP_NAME='Acme Ops'`, first enable | `issuer=Env+Brand`: the
boot-time branding value reaches the issuer |
| after boot, `PUT /api/settings/branding {"workspace_name":"Renamed
Co"}` (200), then enable and get-totp-uri | both still `issuer=Acme+Ops`
|
| same process, password-reset email | subject `Reset your Renamed Co
password` |
`ctx.context.appName` is fixed when the better-auth instance is built. A
rename after that reaches the issuer at the next rebuild (a restart, or
an auth-settings change that calls `applyConfigPatch`). Auth emails
follow it on the next send. No route in `auth-manager.ts` follows the
live value on both TOTP routes without rebuilding the instance: a
getter-backed `twoFactor({ issuer })` would follow it on `enable` only.
The divergence is recorded in the report's open question for the PM, and
the code comment and changeset state it.
## Tests (HEAD `e7e32f4f`)
- `packages/plugins/plugin-auth/src/two-factor-totp-issuer.test.ts`
(new, real better-auth pipeline through `AuthManager.handleRequest`, on
the in-memory engine the sibling 2FA suites use):
- the configured app name is the issuer and the label prefix on
`enable`, and `ObjectStack` when unset;
- `get-totp-uri` names the same issuer for the same secret;
- a branding override set before the instance is built outranks the
configured name;
- **still-works:** an enrollment made under the `Better Auth` label and
confirmed is then served by a process configured as `Acme Ops` over the
same store. The codes from the OLD URI's secret complete a sign-in
challenge, and `get-totp-uri` returns the same secret relabelled. The
stored row does not contain the label.
- `packages/cli/src/commands/serve-auth-app-name.contract.test.ts` (new,
unit tier):
- `resolveDeploymentAppName` equals the email chain's answer on all five
rungs;
- a comment-masked scan of `serve.ts` finds exactly one `new
AuthPlugin(` call, passing `appName: resolveDeploymentAppName(...)` with
the same argument text as the one `resolveEmailCapabilityArg(...)` call.
- `pnpm --filter @objectstack/plugin-auth exec vitest run
--maxWorkers=2`: 119 files, 2509 passed, 10 skipped.
- `pnpm --filter @objectstack/plugin-auth typecheck`: exit 0. It
includes `check:test-typecheck`, and the new file compiles clean, with
no ledger entry.
- `pnpm --filter @objectstack/cli exec vitest run --project unit
--maxWorkers=2`: 258 files, 3777 passed. The integration tier is left to
CI, because the diff touches no spawn entry and no integration-tier
file.
- `pnpm --filter @objectstack/cli typecheck`: exit 0.
## Ablations (each through `scripts/ablation-replace.mjs`, restore
proven blob == HEAD)
- `auth-manager.ts` without the `appName` line: all 5 new plugin-auth
tests turn red, for example `expected { labelIssuer: 'Better Auth', ...
} to match object { labelIssuer: 'Acme Ops', ... }`. Restored to blob
`a6696db0`, which equals HEAD.
- `serve.ts` without the `AuthPlugin` `appName` line: the construction
scan turns red (`AuthPlugin must receive appName from
resolveDeploymentAppName: expected +0 to be 1`). Restored to blob
`121c8125`, which equals HEAD.
- The same `serve.ts` mutation was rebuilt into `packages/cli/dist` and
booted (`ablation-dist-preflight --absent` passed). The result is the
`ObjectStack` row in the first table. The restore leg rebuilt `dist` and
the preflight found the marker present again, with a clean tree.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 99 commands from this branch, and
every one exited 0. `--ran` reports: 99 derived, 99 run, 0 NOT-MEASURED,
0 UNRUN. ESLint (`--no-inline-config`) on the 4 changed TS files reports
0 errors and 0 warnings. That run covers only the changed files.
`eslint.config.mjs` lints `**/*.{ts,...}` and never enables type-aware
linting, so this diff cannot move a verdict on an untouched file.
## Acceptance notes
- `AuthPluginOptions` does not declare `appName`, although `AuthPlugin`
forwards it to `AuthManager` by spread. `serve` imports the plugin
through a variable specifier, so the call is untyped. A typed host
writing `new AuthPlugin({ appName })` gets a loud excess-property error,
not a silent drop. Declaring it would widen a published type, so it is
left out of this PR, which is `Clause-②: no`. Carrier: none.
- Watch item for a better-auth upgrade: if a later version makes the
cookie prefix default from `appName`, as its type doc already claims,
the `appName` passed here would rename session cookies on upgrade.
Re-check `dist/cookies` when bumping. Carrier: whoever bumps
better-auth.
- `plugin-dev` and the `verify` harness construct `AuthPlugin` without
`appName`. Neither reads `OS_APP_NAME` anywhere, so no surface there
disagrees with another. Noted only.
- `/two-factor/enable` accepts a request-body `issuer` (vendor
behaviour, first term above). It only relabels the caller's own
enrollment and is display-only. Noted only.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 866b439 commit 1c3a4d9
6 files changed
Lines changed: 414 additions & 6 deletions
File tree
- .changeset
- content/docs/deployment
- packages
- cli/src/commands
- 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
123 | 123 | | |
124 | 124 | | |
125 | 125 | | |
126 | | - | |
| 126 | + | |
127 | 127 | | |
128 | 128 | | |
129 | 129 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4083 | 4083 | | |
4084 | 4084 | | |
4085 | 4085 | | |
| 4086 | + | |
| 4087 | + | |
| 4088 | + | |
| 4089 | + | |
| 4090 | + | |
4086 | 4091 | | |
4087 | 4092 | | |
4088 | 4093 | | |
| |||
6405 | 6410 | | |
6406 | 6411 | | |
6407 | 6412 | | |
| 6413 | + | |
| 6414 | + | |
| 6415 | + | |
| 6416 | + | |
| 6417 | + | |
| 6418 | + | |
| 6419 | + | |
| 6420 | + | |
| 6421 | + | |
| 6422 | + | |
| 6423 | + | |
| 6424 | + | |
| 6425 | + | |
| 6426 | + | |
| 6427 | + | |
| 6428 | + | |
| 6429 | + | |
| 6430 | + | |
| 6431 | + | |
| 6432 | + | |
| 6433 | + | |
6408 | 6434 | | |
6409 | 6435 | | |
6410 | 6436 | | |
| |||
6520 | 6546 | | |
6521 | 6547 | | |
6522 | 6548 | | |
6523 | | - | |
6524 | | - | |
| 6549 | + | |
6525 | 6550 | | |
6526 | 6551 | | |
6527 | 6552 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
768 | 768 | | |
769 | 769 | | |
770 | 770 | | |
771 | | - | |
772 | | - | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
773 | 775 | | |
774 | 776 | | |
775 | 777 | | |
| |||
1463 | 1465 | | |
1464 | 1466 | | |
1465 | 1467 | | |
| 1468 | + | |
| 1469 | + | |
| 1470 | + | |
| 1471 | + | |
| 1472 | + | |
| 1473 | + | |
| 1474 | + | |
| 1475 | + | |
| 1476 | + | |
| 1477 | + | |
| 1478 | + | |
| 1479 | + | |
| 1480 | + | |
| 1481 | + | |
| 1482 | + | |
1466 | 1483 | | |
1467 | 1484 | | |
1468 | 1485 | | |
| |||
5652 | 5669 | | |
5653 | 5670 | | |
5654 | 5671 | | |
| 5672 | + | |
| 5673 | + | |
| 5674 | + | |
| 5675 | + | |
5655 | 5676 | | |
5656 | 5677 | | |
5657 | 5678 | | |
5658 | 5679 | | |
5659 | 5680 | | |
5660 | 5681 | | |
5661 | | - | |
| 5682 | + | |
| 5683 | + | |
| 5684 | + | |
| 5685 | + | |
5662 | 5686 | | |
5663 | 5687 | | |
5664 | 5688 | | |
| |||
0 commit comments