Skip to content

Commit 759a53a

Browse files
os-zhuangclaude
andauthored
fix(plugin-auth): stop emitting the retired oidcConfig.mapping.id on OIDC SSO registration (#8193) (#8221)
@better-auth/sso declares oidcConfig.mapping as a strict object. `id` was a real member in 1.6.20 and was honoured at login; the pinned 1.7.0-rc.2 retires it and reads the federated subject from the OIDC `sub` claim directly, so the key the bridge always sent made every OIDC registration answer 400. Emit { email, name } (the strict schema's required members), refuse a non-`sub` user-ID claim loudly rather than discarding it silently, and pin the path with tests that drive the real /sso/register endpoint of a real better-auth instance. Also corrects the stale "verified against 1.6.20" attestation in auth-manager.ts to what was actually re-measured against 1.7.0-rc.2. Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 Co-authored-by: Claude <noreply@anthropic.com>
1 parent e50e479 commit 759a53a

4 files changed

Lines changed: 202 additions & 7 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(plugin-auth): OIDC SSO provider registration works again — stop emitting the retired `oidcConfig.mapping.id` key (#8193)
6+
7+
Registering an external OIDC identity provider through the `sys_sso_provider`
8+
`register_sso_provider` action failed **every time**, with HTTP 400:
9+
10+
```
11+
[body.oidcConfig.mapping] Unrecognized key: "id"
12+
```
13+
14+
Not intermittent and not configuration-dependent — the OIDC half of the
15+
registration bridge was unusable for every deployment, and nothing was
16+
persisted. SAML registration was unaffected.
17+
18+
The bridge unconditionally emitted a claim mapping of
19+
`{ id, email, name }`. `@better-auth/sso` declares `oidcConfig.mapping` as a
20+
**strict** object, so a member it does not declare is rejected outright rather
21+
than ignored.
22+
23+
**`id` was not a key that moved — it was retired upstream.** In 1.6.20 the
24+
mapping was a plain (non-strict) object that did carry `id`, and the plugin
25+
honoured it when resolving the federated user. The pinned 1.7.0-rc.2 removes the
26+
member and reads the federated subject from the OIDC `sub` claim directly, then
27+
cross-checks it against the ID token. There is consequently no new home for the
28+
key: `extraFields` is the one open member of the strict object, but a value
29+
placed at `extraFields.id` is overwritten by `sub` before it is ever used, so
30+
re-homing the key there would have looked configured while doing nothing.
31+
32+
The emitted mapping is now `{ email, name }` — the two members the strict schema
33+
requires — and the email/name claim mappings collected by the form continue to
34+
work exactly as before.
35+
36+
**The user-ID claim mapping is now refused instead of ignored.** Because the
37+
subject claim is no longer configurable at all, a registration that asks for a
38+
non-`sub` user-ID claim is answered with a clear `INVALID_REQUEST` explaining
39+
that the subject is always read from `sub`, rather than being accepted and
40+
silently discarded. Leaving the field empty — or setting it to `sub`, the value
41+
the form suggests — registers as normal.
42+
43+
Pinned by a regression test that drives the real `/sso/register` endpoint of a
44+
real better-auth instance, so the emitted body is judged by the installed
45+
package's own schema and the next dependency bump that moves this surface fails
46+
loudly instead of shipping.

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2610,11 +2610,21 @@ export class AuthManager {
26102610
if (enabled.sso) {
26112611
await this.addOptionalPlugin(plugins, 'sso', async () => {
26122612
const { sso } = await import('@better-auth/sso');
2613-
// NOTE: unlike `oauthProvider`, @better-auth/sso hardcodes its `ssoProvider`
2614-
// model and accepts NO `schema` option (verified against 1.6.20 — no
2615-
// mergeSchema, runtime never reads options.schema). Its table mapping to
2616-
// `sys_sso_provider` must therefore be resolved by the better-auth adapter
2617-
// / a global model map, not per-plugin here (see AUTH_SSO_PROVIDER_SCHEMA).
2613+
// NOTE: the `ssoProvider` model is bridged to `sys_sso_provider` by the
2614+
// better-auth adapter / a global model map, not per-plugin here (see
2615+
// AUTH_SSO_PROVIDER_SCHEMA).
2616+
//
2617+
// That bridge dates from 1.6.20, where @better-auth/sso hardcoded the
2618+
// model and read no `schema` option. Re-checked against the pinned
2619+
// 1.7.0-rc.2 (`node_modules/@better-auth/sso/dist`) on 2026-08-12: that is
2620+
// no longer true — `SSOOptions.schema.ssoProvider` now exists
2621+
// (index-D1yk91me.d.mts) and the runtime honours `modelName` plus a
2622+
// per-field `fieldName` map (index.mjs, the plugin's `schema:` block). The
2623+
// adapter-level bridge is kept as-is here because it is what the rest of
2624+
// the auth stack is wired to; whether to move it onto the plugin option is
2625+
// a separate change, not a silent one. Only the mapping surface below was
2626+
// re-verified in depth — see register-sso-provider.ts for the
2627+
// `oidcConfig.mapping` strict-object findings.
26182628
//
26192629
// `organizationProvisioning.defaultRole` (ADR-0024 V1): a first-time
26202630
// federated login is JIT-provisioned into the user's domain-matched org

packages/plugins/plugin-auth/src/register-sso-provider.test.ts

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22
import { describe, it, expect, vi } from 'vitest';
3-
import { runRegisterSamlProviderFromForm } from './register-sso-provider';
3+
import { betterAuth } from 'better-auth';
4+
import { memoryAdapter } from 'better-auth/adapters/memory';
5+
import { sso } from '@better-auth/sso';
6+
import { runRegisterSamlProviderFromForm, runRegisterSsoProviderFromForm } from './register-sso-provider';
47

58
const makeReq = (body: any) =>
69
new Request('http://localhost:3000/api/v1/auth/admin/sso/register-saml', {
@@ -9,6 +12,108 @@ const makeReq = (body: any) =>
912
body: JSON.stringify(body),
1013
});
1114

15+
const makeOidcReq = (body: any) =>
16+
new Request('http://localhost:3000/api/v1/auth/admin/sso/register', {
17+
method: 'POST',
18+
headers: { 'content-type': 'application/json', origin: 'http://localhost:3000' },
19+
body: JSON.stringify(body),
20+
});
21+
22+
const OIDC_FORM = {
23+
providerId: 'acme',
24+
issuer: 'https://idp.acme.com',
25+
domain: 'acme.com',
26+
clientId: 'cid',
27+
clientSecret: 'csecret',
28+
};
29+
30+
/**
31+
* A REAL `betterAuth()` instance carrying the REAL `@better-auth/sso` plugin, so
32+
* the body this bridge emits is judged by the installed package's own Zod
33+
* schema — not by a hand-copied restatement of it that would drift silently on
34+
* the next dependency bump.
35+
*/
36+
const makeRealAuthHandler = () => {
37+
const auth = betterAuth({
38+
baseURL: 'http://localhost:3000',
39+
basePath: '/api/v1/auth',
40+
secret: 'register-sso-provider-test-secret-0123456789',
41+
database: memoryAdapter({}),
42+
plugins: [sso()],
43+
});
44+
return (request: Request) => auth.handler(request);
45+
};
46+
47+
describe('runRegisterSsoProviderFromForm (OIDC) — the emitted body must satisfy the INSTALLED @better-auth/sso schema', () => {
48+
// Regression pin for the end-to-end break where the bridge always emitted
49+
// `oidcConfig.mapping.id`, which `oidcMappingSchema` (a `z.strictObject` with
50+
// no `id` member since 1.7.0-rc.2) rejects outright — every OIDC registration
51+
// answered `400 [body.oidcConfig.mapping] Unrecognized key: "id"`.
52+
//
53+
// These cases drive the REAL `/sso/register` endpoint. `@better-auth/sso`
54+
// validates the request body BEFORE the endpoint's session gate, so an
55+
// unauthenticated call separates the two failure modes cleanly:
56+
// • body rejected by the schema → 400 VALIDATION_ERROR (the bug)
57+
// • body accepted, stopped by the gate → 401 Unauthorized (the fix)
58+
// Reaching 401 is therefore positive evidence that the emitted body parsed.
59+
it('clears the real body schema and reaches the endpoint session gate', async () => {
60+
const res = await runRegisterSsoProviderFromForm(makeRealAuthHandler(), makeOidcReq(OIDC_FORM));
61+
62+
expect(res.body.error?.message ?? '').not.toMatch(/Unrecognized key/);
63+
expect(res.status).toBe(401);
64+
expect(res.body.error?.code).toBe('SSO_REGISTER_FAILED');
65+
expect(res.body.error?.message).toBe('Unauthorized');
66+
});
67+
68+
it('clears the real body schema with operator-supplied claim mappings too', async () => {
69+
const res = await runRegisterSsoProviderFromForm(
70+
makeRealAuthHandler(),
71+
makeOidcReq({ ...OIDC_FORM, mapEmail: 'upn', mapName: 'display_name', scopes: 'openid email' }),
72+
);
73+
74+
expect(res.body.error?.message ?? '').not.toMatch(/Unrecognized key/);
75+
expect(res.status).toBe(401);
76+
});
77+
78+
it('emits exactly the strict schema’s required members — never the retired `id`', async () => {
79+
let dispatched: any = null;
80+
const handle = vi.fn(async (req: Request) => {
81+
if (req.url.endsWith('/get-session')) return new Response('null', { status: 200 });
82+
dispatched = await req.clone().json();
83+
return new Response(JSON.stringify({ providerId: 'acme' }), { status: 200 });
84+
});
85+
86+
const res = await runRegisterSsoProviderFromForm(handle, makeOidcReq(OIDC_FORM));
87+
88+
expect(res.status).toBe(200);
89+
expect(dispatched.oidcConfig.mapping).toEqual({ email: 'email', name: 'name' });
90+
expect(Object.keys(dispatched.oidcConfig.mapping)).not.toContain('id');
91+
});
92+
93+
// The subject claim is no longer configurable anywhere in the plugin: rc.2
94+
// reads it from `sub` and overwrites any `extraFields.id` with it. Telling the
95+
// caller beats accepting a value we would silently discard.
96+
it('rejects a non-`sub` user-ID claim loudly instead of silently discarding it', async () => {
97+
const handle = vi.fn();
98+
const res = await runRegisterSsoProviderFromForm(handle, makeOidcReq({ ...OIDC_FORM, mapId: 'employee_id' }));
99+
100+
expect(res.status).toBe(400);
101+
expect(res.body.error?.code).toBe('INVALID_REQUEST');
102+
expect(res.body.error?.message).toMatch(/not configurable/);
103+
expect(handle).not.toHaveBeenCalled();
104+
});
105+
106+
it('accepts an explicit `sub` (the value the form field suggests) as the no-op it is', async () => {
107+
const res = await runRegisterSsoProviderFromForm(
108+
makeRealAuthHandler(),
109+
makeOidcReq({ ...OIDC_FORM, mapId: 'sub' }),
110+
);
111+
112+
expect(res.status).toBe(401);
113+
expect(res.body.error?.message).toBe('Unauthorized');
114+
});
115+
});
116+
12117
describe('runRegisterSamlProviderFromForm (ADR-0069 P3)', () => {
13118
it('reshapes flat fields into nested samlConfig + derives the ACS URL, re-dispatching to /sso/register', async () => {
14119
let dispatched: { url: string; body: any } | null = null;

packages/plugins/plugin-auth/src/register-sso-provider.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ async function resolveActiveOrganizationId(
7878
* cookie / bearer + Origin; its body carries the flat form
7979
* fields ({ providerId, issuer, domain, clientId, clientSecret,
8080
* discoveryEndpoint?, scopes?, mapId?, mapEmail?, mapName? }).
81+
* `mapId` is accepted only as the (empty or `sub`) no-op it now
82+
* is — see the mapping block below.
8183
*/
8284
export async function runRegisterSsoProviderFromForm(
8385
handle: AuthRequestHandler,
@@ -119,8 +121,40 @@ export async function runRegisterSsoProviderFromForm(
119121
const oidcConfig: Record<string, unknown> = { clientId, clientSecret };
120122
if (discoveryEndpoint) oidcConfig.discoveryEndpoint = discoveryEndpoint;
121123
oidcConfig.scopes = scopesRaw ? scopesRaw.split(/[\s,]+/).filter(Boolean) : ['openid', 'email', 'profile'];
124+
125+
// `oidcConfig.mapping` is a `z.strictObject` in `@better-auth/sso@1.7.0-rc.2`
126+
// (dist/index.mjs, `oidcMappingSchema`): members { email, emailVerified?,
127+
// name, image?, extraFields? }, with `email` and `name` REQUIRED and NO `id`
128+
// member. Emitting `id` is therefore a hard 400 on EVERY registration:
129+
// [body.oidcConfig.mapping] Unrecognized key: "id"
130+
//
131+
// `id` is not a key that moved — it was RETIRED upstream, so there is nowhere
132+
// to re-home it. 1.6.20 declared the mapping as a plain (non-strict)
133+
// `z.object` that DID carry `id`, and honoured it at login
134+
// (`id: rawUserInfo[mapping.id || "sub"]`). 1.7.0-rc.2 deletes the member and
135+
// hard-wires the federated subject to the OIDC `sub` claim
136+
// (`id: readStringClaim(rawUserInfo, "sub")` / `id: idToken.sub`), then
137+
// cross-checks it (`id_token_subject_missing`,
138+
// `id_token_userinfo_subject_mismatch`). `extraFields` is NOT a substitute:
139+
// it parses, but it is spread BEFORE `id` in the profile literal, so an
140+
// `extraFields.id` is silently overwritten by `sub` — a no-op that reads as
141+
// configured. The subject claim is simply not configurable any more, so a
142+
// caller that asks for a different one is told so instead of being ignored.
143+
const mapId = str(body?.mapId);
144+
if (mapId && mapId !== 'sub') {
145+
return {
146+
status: 400,
147+
body: {
148+
success: false,
149+
error: {
150+
code: 'INVALID_REQUEST',
151+
message:
152+
'The user ID claim is not configurable: the federated subject is always read from the OIDC "sub" claim. Leave the user-ID claim mapping empty (or set it to "sub").',
153+
},
154+
},
155+
};
156+
}
122157
oidcConfig.mapping = {
123-
id: str(body?.mapId) || 'sub',
124158
email: str(body?.mapEmail) || 'email',
125159
name: str(body?.mapName) || 'name',
126160
};

0 commit comments

Comments
 (0)