Skip to content

Commit 26c2efd

Browse files
fix(plugin-auth): Register SAML Provider registers a complete form (it answered 400 for every one) (#30402)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wg8twJDyDkhCm8SFYtruR9Rb)_ domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF Fixes #30352 Clause-②: no — a form the bridge always refused registers; nothing a caller sends today is refused Metadata-protocol: none ## 维护者速读 - 改了什么:Setup → SSO Providers → Register SAML Provider 的服务端桥接,现在把表单里的「IdP Entity ID」(`issuer` 字段)作为 `samlConfig.idpMetadata.entityID` 发给 `@better-auth/sso`。表单字段和必填项都没变。 - 为什么改:在这之前,管理员在 Setup 里填完整的 SAML 表单,每次都被 400 `SAML_REGISTER_FAILED` 拒绝("idpMetadata.entityID is required when IdP metadata XML is not provided"),Setup 里根本注册不了任何 SAML 提供方。 - 风险与回滚:只在发给供应商的请求里多带一个 IdP 描述字段,值取自管理员已经在填的字段;漏填字段时仍然在桥接层被拒,并点名缺哪个字段。回滚就是 revert 这个 PR。 - 先看:① `packages/plugins/plugin-auth/src/register-sso-provider.ts` 的 `samlConfig` 那一段(新增 `idpMetadata`);② `register-sso-provider.test.ts` 末尾新增的真实路由测试块。 - 两分钟验证:`pnpm --filter @objectstack/plugin-auth exec vitest run src/register-sso-provider.test.ts`,应看到 10 个用例全部通过,其中「a complete form registers the provider」返回 200,并存下一行提供方记录。 - 需要你做的:无,席位落地。 ## What changes - **`runRegisterSamlProviderFromForm`** (the bridge behind `POST /api/v1/auth/admin/sso/register-saml`) now sends `samlConfig.idpMetadata: { entityID: issuer }`. The Register SAML Provider form has no IdP metadata XML field, so the bridge always lands in the vendor's manual-configuration arm, and `@better-auth/sso` 1.7.12 requires `idpMetadata.entityID` there: its register body schema (`samlIdentityProviderMetadataSchema`, a union of "metadata XML" and "entityID") refused every form at the parse step, and `assertSAMLIdentityProviderAuthority` (run at register and again in `createIdP` at sign-in) refuses the same config at runtime. - **Where the value comes from.** The form field set is read on the server side, from the `register_saml_provider` action params in `packages/platform-objects/src/identity/sys-sso-provider.object.ts`: `providerId`, `issuer` (label "IdP Entity ID", help "The IdP's SAML EntityID (issuer)"), `domain`, `entryPoint`, `cert`, optional `identifierFormat`. The IdP entity ID field exists, so this is a server-only change. - **Unchanged.** The required set, the refusal of an incomplete form (`400 INVALID_REQUEST` naming the missing field, before the vendor is called), the top-level `entryPoint` and `cert` (the vendor reads them when `idpMetadata` names no `singleSignOnService` or `cert`), `spMetadata.entityID` (the SP metadata URL, which keeps the top-level `issuer` from being read as the SP entity ID) and the answer (`providerId`, `acsUrl`, `spMetadataUrl`). ## Why An administrator who sets up enterprise sign-in from Setup → SSO Providers → Register SAML Provider could not register any SAML provider there: every complete form answered `400 SAML_REGISTER_FAILED`. The only working path was the vendor's raw `/sso/register`, which Setup does not expose (measured by the #30300 dev, report 6652003986). ## Tests - **New block in `register-sso-provider.test.ts`: "Register SAML Provider through the REAL route".** The plugin's own `registerAuthRoutes` mounts the bridge on a real Hono app. The auth manager's `handleRequest` is a real `betterAuth()` instance with the real `sso()` plugin and a memory adapter, and the vendor sees the real session cookie of a signed-up user. The platform-admin gate is judged on a platform-admin session shape (its refusals are pinned in `admin-sso-bridge-gate.test.ts`). - A complete form answers 200 with `acsUrl`, and exactly one provider is stored, whose `samlConfig.idpMetadata` is `{ entityID: 'https://idp.acme.example/entity' }`. - A form without the IdP Entity ID answers 400 `INVALID_REQUEST` naming `issuer`, and nothing is stored. - **Updated:** the existing shape case also asserts `idpMetadata: { entityID }` in the re-dispatched body. - Every earlier SAML case used a stand-in for `/sso/register`, which is why this was never seen. The OIDC sibling already had a real-vendor case. ## Verification All at head `815fcdcd` (`main` `870ebce3` merged; nothing newer on `main` at merge time). - **Ablation.** `node scripts/ablation-replace.mjs --delete` on the one `idpMetadata: { entityID: issuer },` line (anchor 1 → 0, blob `341a85ef` → `967366eb`), then the file's suite: 3 failed / 7 passed. The real-route case fails with exactly the card's answer: `{"success":false,"error":{"code":"SAML_REGISTER_FAILED","message":"[body.samlConfig.idpMetadata] idpMetadata.entityID is required when IdP metadata XML is not provided"}}`. Restored (`git diff HEAD` empty). The test file imports the bridge from `src`, so no build sits between the mutation and the run. - **Suites.** `pnpm --filter @objectstack/plugin-auth test`: 223 files passed, 3,937 tests passed, 10 skipped. `pnpm --filter @objectstack/plugin-auth typecheck` (tsc, examples, `check:test-typecheck`): exit 0. - **Lint.** eslint (`--no-inline-config`) on both changed source files: exit 0. `pnpm lint` (repo-wide): exit 0. - **Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 61 commands: 60 exit 0, and `--ran` reconciles 61 derived, 60 run, 1 NOT-MEASURED, 0 UNRUN. `check-plugin-teardown-shape --self-test` first exited 3 on the shallow clone and exits 0 once its pinned commit is fetched. `check:dual-build-cjs-loads` exits 3 locally (PREREQUISITE NOT MET: not every workspace package is built in this worktree), NOT MEASURED, left to CI. ## Acceptance notes - The vendor's register body schema types the top-level `issuer` as a URL, so an IdP whose EntityID is a URN (`urn:...`) is still refused by `@better-auth/sso` at the parse step. Not changed here; noted for the seat. - The form has no IdP metadata XML field, so the vendor's metadata-XML arm is not reachable from Setup; the bridge does not invent one. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. (the designated thread branch, as the Claim records) - [x] 5. A change that publishes carries a changeset (`patch`, `@objectstack/plugin-auth`), and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Utp6CboMj2jstF8f9snYQE --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6fa842a commit 26c2efd

3 files changed

Lines changed: 131 additions & 2 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth): Setup's Register SAML Provider registers a complete form; it answered 400 for every one
6+
7+
Clause-②: no — a form the bridge always refused registers; nothing a caller sends today is refused
8+
Metadata-protocol: none
9+
10+
- **Before.** `POST /api/v1/auth/admin/sso/register-saml` (Setup → SSO Providers → Register SAML Provider) answered `400 SAML_REGISTER_FAILED` "[body.samlConfig.idpMetadata] idpMetadata.entityID is required when IdP metadata XML is not provided" for every complete form. The bridge sent `samlConfig` with no IdP descriptor, which `@better-auth/sso` requires when no IdP metadata XML is given, so no SAML provider could be registered from Setup.
11+
- **Now.** The bridge sends `samlConfig.idpMetadata.entityID` from the form's IdP Entity ID field (`issuer`), and a complete form registers the provider (one `sys_sso_provider` row) and answers the SP ACS and metadata URLs as before. The form's fields, the required set and the refusal of an incomplete form (`400 INVALID_REQUEST` naming the missing field) are unchanged. The IdP SSO URL and signing certificate stay where they were (`samlConfig.entryPoint`, `samlConfig.cert`).

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

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2-
import { describe, it, expect, vi } from 'vitest';
2+
import { describe, it, expect, vi, beforeAll } from 'vitest';
3+
import { Hono } from 'hono';
34
import { betterAuth } from 'better-auth';
45
import { memoryAdapter } from 'better-auth/adapters/memory';
56
import { sso } from '@better-auth/sso';
67
import { runRegisterSamlProviderFromForm, runRegisterSsoProviderFromForm } from './register-sso-provider';
8+
import { AuthPlugin } from './auth-plugin';
9+
import type { PluginContext } from '@objectstack/core';
710

811
const makeReq = (body: any) =>
912
new Request('http://localhost:3000/api/v1/auth/admin/sso/register-saml', {
@@ -152,6 +155,8 @@ describe('runRegisterSamlProviderFromForm (ADR-0069 P3)', () => {
152155
cert: 'MIICert...',
153156
callbackUrl: 'http://localhost:3000/api/v1/auth/sso/saml2/sp/acs/acme-saml',
154157
identifierFormat: 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
158+
// the form's "IdP Entity ID" is the IdP descriptor's EntityID
159+
idpMetadata: { entityID: 'https://idp.acme.com/entity' },
155160
spMetadata: { entityID: 'http://localhost:3000/api/v1/auth/sso/saml2/sp/metadata?providerId=acme-saml' },
156161
},
157162
});
@@ -177,3 +182,108 @@ describe('runRegisterSamlProviderFromForm (ADR-0069 P3)', () => {
177182
expect(res.body.error?.message).toBe('bad cert');
178183
});
179184
});
185+
186+
describe('Register SAML Provider through the REAL route — the INSTALLED @better-auth/sso judges the form', () => {
187+
// The mocked cases above pin the shape the bridge emits; this block pins that
188+
// the vendor ACCEPTS it. Before `idpMetadata.entityID` was sent, every
189+
// complete form answered `400 SAML_REGISTER_FAILED` "[body.samlConfig.
190+
// idpMetadata] idpMetadata.entityID is required when IdP metadata XML is not
191+
// provided", and nothing here could see it: every SAML case used a stand-in
192+
// for `/sso/register`.
193+
//
194+
// Wiring: the plugin's own `registerAuthRoutes` mounts the bridge on a real
195+
// Hono app; the auth manager is reduced to the two seams the route reads.
196+
// `handleRequest` IS the real vendor handler, so the inner `/sso/register`
197+
// runs its real schema, session gate and storage. The ADR-0068 platform-admin
198+
// gate is judged on a platform-admin session shape (its own file,
199+
// admin-sso-bridge-gate.test.ts, pins the refusals), while the vendor sees
200+
// the REAL session cookie of a signed-up user.
201+
const ORIGIN = 'http://localhost:3000';
202+
const BASE = '/api/v1/auth';
203+
const COMPLETE_FORM = {
204+
providerId: 'acme-saml',
205+
issuer: 'https://idp.acme.example/entity',
206+
domain: 'acme.example',
207+
entryPoint: 'https://idp.acme.example/sso',
208+
cert: 'MIIC-PROBE-CERT',
209+
};
210+
211+
let db: Record<string, any[]>;
212+
let app: Hono;
213+
let cookie: string;
214+
215+
beforeAll(async () => {
216+
db = { user: [], session: [], account: [], verification: [], ssoProvider: [] };
217+
const vendor = betterAuth({
218+
baseURL: ORIGIN,
219+
basePath: BASE,
220+
secret: 'register-saml-real-route-secret-0123456789',
221+
database: memoryAdapter(db),
222+
emailAndPassword: { enabled: true },
223+
plugins: [sso()],
224+
});
225+
const signUp = await vendor.handler(
226+
new Request(`${ORIGIN}${BASE}/sign-up/email`, {
227+
method: 'POST',
228+
headers: { 'content-type': 'application/json', origin: ORIGIN },
229+
body: JSON.stringify({ email: 'admin@example.com', password: 'Admin-Pass-123!', name: 'Admin' }),
230+
}),
231+
);
232+
expect(signUp.status, await signUp.clone().text()).toBe(200);
233+
cookie = (signUp.headers.getSetCookie?.() ?? []).map((c) => c.split(';')[0]).join('; ');
234+
expect(cookie).toContain('session_token');
235+
236+
const ctx = {
237+
registerService: vi.fn(),
238+
getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)),
239+
getServices: vi.fn(() => new Map()),
240+
hook: vi.fn(),
241+
trigger: vi.fn(),
242+
logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() },
243+
getKernel: vi.fn(),
244+
} as unknown as PluginContext;
245+
const plugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long!!', plugins: { oidcProvider: false } });
246+
await plugin.init(ctx);
247+
(plugin as any).authManager = {
248+
handleRequest: (req: Request) => vendor.handler(req),
249+
getApi: async () => ({
250+
getSession: async () => ({ user: { id: 'usr_admin', positions: ['user', 'platform_admin'], isPlatformAdmin: true } }),
251+
}),
252+
isSsoDomainVerificationEnabled: () => false,
253+
};
254+
app = new Hono();
255+
(plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx);
256+
});
257+
258+
const post = (form: Record<string, unknown>) =>
259+
app.request(`${ORIGIN}${BASE}/admin/sso/register-saml`, {
260+
method: 'POST',
261+
headers: { 'content-type': 'application/json', origin: ORIGIN, cookie },
262+
body: JSON.stringify(form),
263+
});
264+
265+
it('a complete form registers the provider: 200, one stored provider keyed on the IdP EntityID', async () => {
266+
const res = await post(COMPLETE_FORM);
267+
const body: any = await res.json();
268+
expect(res.status, JSON.stringify(body)).toBe(200);
269+
expect(body.success).toBe(true);
270+
expect(body.data.providerId).toBe('acme-saml');
271+
expect(body.acsUrl).toBe(`${ORIGIN}${BASE}/sso/saml2/sp/acs/acme-saml`);
272+
273+
expect(db.ssoProvider).toHaveLength(1);
274+
const stored = JSON.parse(db.ssoProvider[0].samlConfig);
275+
expect(stored.idpMetadata).toEqual({ entityID: 'https://idp.acme.example/entity' });
276+
expect(stored.entryPoint).toBe('https://idp.acme.example/sso');
277+
});
278+
279+
it('an incomplete form is still refused before the vendor, naming the missing field', async () => {
280+
const { issuer: _omitted, ...withoutEntityId } = COMPLETE_FORM;
281+
const storedBefore = db.ssoProvider.length;
282+
const res = await post({ ...withoutEntityId, providerId: 'acme-saml-2' });
283+
const body: any = await res.json();
284+
expect(res.status).toBe(400);
285+
expect(body.error?.code).toBe('INVALID_REQUEST');
286+
expect(body.error?.message).toContain('issuer');
287+
expect(db.ssoProvider).toHaveLength(storedBefore);
288+
});
289+
});

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,8 @@ export async function runRegisterSsoProviderFromForm(
246246
*
247247
* `@better-auth/sso` (samlify-backed) registers a SAML IdP via the SAME
248248
* `/sso/register` endpoint, with the protocol fields nested under `samlConfig`
249-
* ({ entryPoint, cert, callbackUrl, identifierFormat? }) instead of `oidcConfig`.
249+
* ({ entryPoint, cert, callbackUrl, idpMetadata: { entityID }, spMetadata,
250+
* identifierFormat? }) instead of `oidcConfig`.
250251
* The UI action collects FLAT fields; this helper reshapes them, derives the
251252
* per-provider ACS callback URL (`/sso/saml2/sp/acs/<providerId>`), and
252253
* re-dispatches through `/sso/register` so the admin gate + provisioning run.
@@ -298,6 +299,13 @@ export async function runRegisterSamlProviderFromForm(
298299
entryPoint,
299300
cert,
300301
callbackUrl: acsUrl,
302+
// The IdP descriptor. The form carries no metadata XML, so this is the
303+
// vendor's manual-configuration arm, which requires the IdP's own EntityID
304+
// (`idpMetadata.entityID`) — the form's `issuer` field ("IdP Entity ID").
305+
// Without it `/sso/register` refuses every form with 400. The IdP SSO URL
306+
// and signing cert stay top-level (`entryPoint`, `cert`), which the vendor
307+
// reads when `idpMetadata` names no `singleSignOnService` / `cert`.
308+
idpMetadata: { entityID: issuer },
301309
// better-auth requires an SP descriptor (its inner fields are optional). Use
302310
// the SP metadata URL as our EntityID — the value the IdP keys this SP on.
303311
spMetadata: { entityID: spMetadataUrl },

0 commit comments

Comments
 (0)