Repository navigation
Commit 30002df
fix(plugin-auth): a refused enterprise OIDC sign-in names its cause (#31194)
<!-- claude-projects-attribution: {"github_login":"marchtian"} -->
_Requested by **田騏** · [project
thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiXg46wP2iyXrx3QP2KhstCt)_
Fixes #30965
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:企业 OIDC 单点登录被拒时,跳转里的 `error_description` 从一律的
`token_not_verified` 改成具体原因(过期、未生效、受众不对、签发方不对、签名密钥未知、签名无效),服务器同时写一行 warn
日志(providerId、协议、原因)。
- 为什么改:Okta / Entra ID 接入时任何配置错误都显示同一句,管理员无法自查(cloud#3666 实测)。按裁定 D + A
+ V1 只做 OIDC,SAML 等上游。
- 风险与回滚:better-auth 自己的日志改走 AuthManager 的 logger(级别不变),没配 logger
时仍走控制台;`error=` 不变。回滚即还原本 PR,无数据迁移。
- 先看:`sso-oidc-refusal-cause.ts`(分类与改写,风险在
`runSsoOidcCallbackScope`)、`auth-manager-request-routing.ts`(作用域挂载点)、`sso-oidc-refusal-cause.test.ts`(端到端用例)。
- 两分钟验证:`pnpm --filter @objectstack/plugin-auth exec vitest run
src/sso-oidc-refusal-cause.test.ts`,预期 15 个用例全过。
- 需要你做的:无,席位落地。
## What changes
`@better-auth/sso` 1.7.12 answers every refused ID token on the OIDC
callback with one redirect,
`error=invalid_provider&error_description=token_not_verified`. The
`jose` error that names the cause reaches only the vendor's logger
(`ctx.context.logger.error(error)` in `handleOIDCCallback`). Per the
ruling (D + A + V1, comment 6709033929), this PR reads it there and does
OIDC only:
- **`sso-oidc-refusal-cause.ts`** (new). `createBetterAuthLogSink` is
the `logger` plugin-auth now hands `betterAuth`.
`runSsoOidcCallbackScope` wraps one `/sso/callback/:providerId` request
in an `AsyncLocalStorage` scope. Inside that scope the sink holds back
an `error` line whose message is a `jose` error with a classified `code`
(and `claim`), and records the cause. When the vendor answers the opaque
redirect, the scope does three things: it replaces the
`error_description` segment with the cause and leaves every other byte
and header as they were, it writes one `warn` line (`providerId`,
`protocol: 'oidc'`, `cause`), and it drops the held vendor line, which
the warn line replaces. Any other answer releases the held lines
unchanged.
- **Closed list**, exported as `SSO_OIDC_REFUSAL_CAUSES`:
`id_token_expired` (`ERR_JWT_EXPIRED`), `id_token_not_yet_valid`
(`ERR_JWT_CLAIM_VALIDATION_FAILED` + `nbf`),
`id_token_audience_mismatch` (+ `aud`), `id_token_issuer_mismatch` (+
`iss`), `id_token_signing_key_unknown` (`ERR_JWKS_NO_MATCHING_KEY`),
`id_token_signature_invalid` (`ERR_JWS_SIGNATURE_VERIFICATION_FAILED`).
Anything else keeps `token_not_verified` and still gets the warn line.
Classification never reads message text.
- **Wiring**: `auth-manager-auth-instance.ts` passes the sink;
`auth-manager-request-routing.ts` opens the scope for the per-provider
callback path through a new `AuthManager.runSsoOidcCallback` host
member. The shared `/sso/callback` endpoint is not scoped: it is reached
only when `sso({ redirectURI })` is set, and plugin-auth does not set
it.
- **How existing better-auth log lines move**: they now reach the
`AuthManager` logger instead of the console, at the level better-auth
chose (`error` stays `error`, `warn` stays `warn`; no regrading under
the "Degradation log levels" rule, because the vendor already graded
them). `info`/`debug` stay unpublished (the vendor's default level
`warn` is unchanged). With no logger configured they go to the console
as before. A message that is an error object is rendered as its name and
message, as the vendor's console logger rendered it, so a `jose` claim
error's `payload` (the token's claims) never reaches a line. Further
arguments pass through unchanged.
`driver-fault-auth-log-carriers.test.ts` now reads the library's
carriers from both the console and the manager logger, and its
assertions are otherwise unchanged.
- **Docs**: `content/docs/permissions/sso.mdx` gains "Why an OIDC
sign-in was refused", with one row per cause and what to check.
- **Changeset**: `@objectstack/plugin-auth` patch. `Metadata-protocol:
none` takes `patch` (AGENTS.md Post-Task Checklist step 3). The new
export is a runtime constant, not an authorable key.
Not here: SAML (the vendor discards that cause inside a bare `catch`,
and it goes to a follow-up card upstream-first), `packages/spec`,
ADR-0112. The values are lowercase `error_description` tokens in the
vendor's dialect for that slot, not ledger codes (ruling V1).
## Tests
`sso-oidc-refusal-cause.test.ts` drives a real `AuthManager` with the
real `sso()` plugin through `/sign-in/sso` →
`/sso/callback/:providerId`. Only the IdP's token and JWKS endpoints are
stubbed. The ID tokens are signed in the test with generated RS256 keys,
so every refusal comes from the vendor's real verifier.
- CONTROL: a valid token signs in (302 to `callbackURL`, session cookie,
no refusal line).
- One case per cause: the redirect carries `error=invalid_provider` and
the cause, there is exactly one warn line `{ providerId, protocol:
'oidc', cause }`, and no vendor `error` line.
- Unclassified (`azp` naming another client): keeps
`token_not_verified`, writes the warn line, and the vendor's diagnosis
still reaches the log.
- No-leak: neither the Location nor any logger channel carries the
token, its payload segment, the subject, the email or the name. A known
account and an unknown one get identical redirects and lines.
- Unit: classification by code and claim only, the path scope, a
byte-exact rewrite with headers kept, held lines released on other
answers, and forwarding outside a scope.
<details><summary>Ablations (each restored, blob == HEAD, `git diff
HEAD` empty, via scripts/ablation-replace.mjs)</summary>
Run on the 16-case file before the docs-table pin was dropped (heads
b73a45f for 1 and 2, 07fe9fc for 3); the per-cause and no-leak cases
are unchanged since.
1. Side channel off (`const cause = undefined` in the sink): 7 failed /
9 passed (all six per-cause E2E cases and the rewrite unit case).
2. Sink not handed to `betterAuth` (the `logger:` line deleted): 7
failed / 9 passed (six per-cause cases plus the unclassified case). This
is the shape a vendor bump that stops passing the error object would
take.
3. Error-message rendering leaks the error's own properties
(`JSON.stringify({ ...message })`): 2 failed / 14 passed, including the
held-line no-leak pin.
A first version that also reshaped the vendor's further arguments was
measured against `driver-fault-auth-log-carriers.test.ts` (the error
object's `cause` stopped surviving). The arguments now pass through as
written.
</details>
<details><summary>Local runs (final head b0a55dc)</summary>
- `pnpm --filter @objectstack/plugin-auth test` at 07fe9fc (after
merging origin/main): Test Files 235 passed (235), Tests 4184 passed, 10
skipped. Since then only a comment move in
`auth-manager-request-routing.ts` and the removal of one test case; the
new file reruns 15/15 at b0a55dc.
- `pnpm --filter @objectstack/plugin-auth typecheck`: exit 0 (includes
`check:test-typecheck`, debt unchanged).
- `npx eslint --no-inline-config packages/plugins/plugin-auth`: exit 0.
- Derived gates at b0a55dc (`node scripts/pm/dispatch-gates.mjs
--commands`): 95 derived, 95 run, every one `:: exit 0`; `--ran`
reconciliation: 95 run, 0 NOT-MEASURED, 0 UNRUN. Three gates first
answered exit 3 for missing prerequisites (`check-plugin-teardown-shape
--self-test` on a shallow clone, `check:skill-examples` and
`check:dual-build-cjs-loads` without a full build); fixed by fetching
the fixture commit and a full `pnpm build` (119/119), then rerun green.
- A first gate pass flagged `check:cross-package-test-inputs`: a
docs-table pin read `content/docs/permissions/sso.mdx` from the
plugin-auth test. That pin was dropped rather than widening the
package's declared inputs.
</details>
## Acceptance notes
- The cloud#3666 `it.todo` cases for OIDC (wrong audience, expired
token, foreign issuer, retired key) can go live against this behaviour;
the cloud repo is not edited here. The SAML todos stay todo until the
follow-up card lands.
- The mechanism depends on the vendor handing its logger the `jose`
error object. Ablation 2 shows the per-cause tests go red if that stops,
so a bump cannot silently fall back.
- The docs table and `SSO_OIDC_REFUSAL_CAUSES` are kept equal by hand: a
pin would read outside the package, which
`check:cross-package-test-inputs` requires declaring in
`scripts/cross-package-test-inputs.mjs` and `turbo.json`, outside this
card's file surface.
## Verification
Human Minimum: 1 n/a (no spec), 2 yes, 3 yes, 4 yes, 5 yes, 6 n/a, 7
yes, 8 yes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ihL2dEm9wh1JnKi42YPDL)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent d57be87 commit 30002df
9 files changed
Lines changed: 766 additions & 7 deletions
File tree
- .changeset
- content/docs/permissions
- 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 | |
|---|---|---|---|
| |||
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
291 | 312 | | |
292 | 313 | | |
293 | 314 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
| 74 | + | |
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
| |||
206 | 207 | | |
207 | 208 | | |
208 | 209 | | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
209 | 216 | | |
210 | 217 | | |
211 | 218 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| 70 | + | |
70 | 71 | | |
71 | 72 | | |
72 | 73 | | |
| |||
85 | 86 | | |
86 | 87 | | |
87 | 88 | | |
| 89 | + | |
88 | 90 | | |
89 | 91 | | |
90 | 92 | | |
91 | | - | |
| 93 | + | |
92 | 94 | | |
93 | 95 | | |
94 | 96 | | |
| |||
210 | 212 | | |
211 | 213 | | |
212 | 214 | | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
213 | 219 | | |
214 | 220 | | |
215 | 221 | | |
| |||
230 | 236 | | |
231 | 237 | | |
232 | 238 | | |
233 | | - | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
234 | 242 | | |
235 | 243 | | |
236 | 244 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
250 | 250 | | |
251 | 251 | | |
252 | 252 | | |
| 253 | + | |
253 | 254 | | |
254 | 255 | | |
255 | 256 | | |
| |||
3938 | 3939 | | |
3939 | 3940 | | |
3940 | 3941 | | |
3941 | | - | |
| 3942 | + | |
3942 | 3943 | | |
3943 | 3944 | | |
3944 | 3945 | | |
3945 | 3946 | | |
3946 | 3947 | | |
| 3948 | + | |
3947 | 3949 | | |
3948 | 3950 | | |
3949 | 3951 | | |
3950 | 3952 | | |
| 3953 | + | |
| 3954 | + | |
| 3955 | + | |
| 3956 | + | |
| 3957 | + | |
| 3958 | + | |
| 3959 | + | |
| 3960 | + | |
| 3961 | + | |
3951 | 3962 | | |
3952 | 3963 | | |
3953 | 3964 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
17 | 24 | | |
18 | 25 | | |
19 | 26 | | |
| |||
350 | 357 | | |
351 | 358 | | |
352 | 359 | | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
353 | 366 | | |
354 | | - | |
| 367 | + | |
355 | 368 | | |
356 | 369 | | |
357 | | - | |
358 | | - | |
| 370 | + | |
| 371 | + | |
359 | 372 | | |
360 | 373 | | |
361 | 374 | | |
362 | 375 | | |
363 | | - | |
| 376 | + | |
364 | 377 | | |
365 | 378 | | |
366 | 379 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
93 | 93 | | |
94 | 94 | | |
95 | 95 | | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
96 | 103 | | |
97 | 104 | | |
98 | 105 | | |
| |||
0 commit comments