Skip to content

fix(plugin-auth): Register SAML Provider registers a complete form (it answered 400 for every one) - #30402

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-0zrk6i
Nov 5, 2026
Merged

fix(plugin-auth): Register SAML Provider registers a complete form (it answered 400 for every one)#30402
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-0zrk6i

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Requested by Bill · project thread

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

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (not touched)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request. (the designated thread branch, as the Claim records)
  • 5. A change that publishes carries a changeset (patch, @objectstack/plugin-auth), and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed. (not touched)
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Utp6CboMj2jstF8f9snYQE

claude added 4 commits November 5, 2026 08:53
@claude
fix(plugin-auth): Register SAML Provider sends the IdP EntityID the v…
457f3a6 
…endor requires

Claude-Session: https://claude.ai/code/session_01Utp6CboMj2jstF8f9snYQE
Co-authored-by: Claude <noreply@anthropic.com>
@claude
test(plugin-auth): refusal pin asserts the named field, not the prose
95c7fd2 
Claude-Session: https://claude.ai/code/session_01Utp6CboMj2jstF8f9snYQE
Co-authored-by: Claude <noreply@anthropic.com>
@claude
chore(changeset): plugin-auth patch for Register SAML Provider
6b0f05c 
Claude-Session: https://claude.ai/code/session_01Utp6CboMj2jstF8f9snYQE
Co-authored-by: Claude <noreply@anthropic.com>
@claude
chore(changeset): Clause line carries the measured arm only
815fcdc 
Claude-Session: https://claude.ai/code/session_01Utp6CboMj2jstF8f9snYQE
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 5, 2026
@objectstack-fleet objectstack-fleet Bot mentioned this pull request Nov 5, 2026
Closed
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 103 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0e4dfbe27a2a312864cd8d14e4954b6af14e2a22 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 442b4519672d2ad896492b3c985196b23afed49d — the merge of head 815fcdcdada86647a59b69548c0e534921bf92d3 into base 0e4dfbe27a2a312864cd8d14e4954b6af14e2a22, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 442b4519672d2ad896492b3c985196b23afed49d && git checkout 442b4519672d2ad896492b3c985196b23afed49d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0e4dfbe27a2a312864cd8d14e4954b6af14e2a22 815fcdcdada86647a59b69548c0e534921bf92d3 && git checkout -B drift-repro 0e4dfbe27a2a312864cd8d14e4954b6af14e2a22 && git merge --no-ff 815fcdcdada86647a59b69548c0e534921bf92d3

node scripts/docs-audit/affected-docs.mjs --json 0e4dfbe27a2a312864cd8d14e4954b6af14e2a22

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet objectstack-fleet Bot mentioned this pull request Nov 5, 2026
Closed
@objectstack-fleet

objectstack-fleet Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF

席位意见：ACCEPT（#30352）。Setup → SSO 提供方 → 注册 SAML 时，桥接层现在把表单里的 IdP 发行方作为 samlConfig.idpMetadata.entityID 发给 @better-auth/sso，填完整的表单能注册成功（写入一行 sys_sso_provider），不再对每个表单都答 400。

席位核对

Head-sha: 815fcdcdada86647a59b69548c0e534921bf92d3 · read 2026-11-05T10:03Z

Item Reading
Shape Draft → main, author objectstack-fleet[bot] (relay pr_create); attribution block, then signature line, Fixes #30352, Clause-② / Metadata-protocol at line start. 3 files, +131/−2. ⛔ packages/spec/**, ui/** and seat 3's auth-manager-before-hook.ts untouched.
Ruling Card #30352 (the #30300 dev's measurement, report 6652003986); triage first-touch 6652854060 ("Fix site: plugin-auth's SAML register bridge"); claim 6653764017; dev report 6654769801.
Gates (check-runs on this head) 48 completed: 35 success, 13 skipped (path-gated ui/docs/opt-in), 0 failed; mergeable_state clean.
Seat review Read the diff: runRegisterSamlProviderFromForm adds idpMetadata: { entityID: issuer } to the samlConfig it sends, from the form's issuer field that the bridge already reads; the metadata-XML path is unchanged. The pin goes through the real route with the real @better-auth/sso plugin, not a vendor mock. Clause-②: no holds: a form that was always refused now registers, and no caller input that was accepted is now refused. No contract review owed (not narrowing, not governed, no spec path).
Follow-up An IdP whose Entity ID is a URN (urn:…) is filed as #30404, serial after this PR.
Landing Not governed (check-governed-merges --pr 30402: 0 of 3 paths), 133 lines ≤ 3000, no !. Seat lands: pr_ready + automerge_enable.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 5, 2026 10:04
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 5, 2026 10:04
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 5, 2026
@objectstack-fleet objectstack-fleet Bot mentioned this pull request Nov 5, 2026
Open
Merged via the queue into main with commit 26c2efd Nov 5, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-0zrk6i branch November 5, 2026 10:22
@objectstack-fleet objectstack-fleet Bot mentioned this pull request Nov 5, 2026
Merged
9 tasks done
@github-actions github-actions Bot mentioned this pull request Nov 5, 2026
Closed
@objectstack-fleet objectstack-fleet Bot mentioned this pull request Nov 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation size/m tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

plugin-auth: Setup's Register SAML Provider answers 400 SAML_REGISTER_FAILED for every complete form (the bridge sends no idpMetadata.entityID)

1 participant

@claude