feat(auth): sign in with the deployment's SAML identity providers

[jeroenrinzema]

Sign in to the console with the deployment's own SAML 2.0 identity providers.
It is a driver like the others: AUTH_DRIVER=basic,saml, configured by the
operator, offered beside the password login or on its own.

Stacked on #311. Review that first; this diff is against it. #311's own body
says SAML was considered and dropped — this is that decision revisited, and it
turned out to need no seams that were not already there.

Where a directory speaks both protocols, OpenID Connect is still the one to
reach for: it has discovery, so a key rotation is picked up on its own, and it
carries an email_verified claim the platform can act on. This is for the
directories that only speak SAML.

What it reuses, and what it could not

The identity half was free. auth.Exchanger resolves the admin, provisions
membership and mints the session, so a SAML login lands exactly where every
other driver's does. admin_identities is keyed on (issuer, subject) and its
CHECK constraint already permits 'saml', so there is no migration.
Nothing in internal/rbac or internal/http/auth/exchange.go changes.

Signature verification is crewjam/saml's, and so are InResponseTo,
Recipient, Destination, the audience restriction and the assertion's
validity window. Rolling any of that by hand is how XML signature wrapping bugs
get written.

Three things did not carry over from #311.

The browser binding cookie had to change

This is the one piece of #311's design that breaks outright. Its binding cookie
is SameSite=Lax, with a comment explaining that Strict would strip it from the
top-level navigation back from the provider. That works because OpenID Connect
returns by GET. SAML returns by the HTTP-POST binding — a cross-site
top-level form POST, which Lax cookies are not sent on. A Lax cookie here
would have been absent from every assertion the deployment ever received.

So the SAML binding is SameSite=None, which browsers only accept on a Secure
cookie. A deployment whose PUBLIC_URL is not https is therefore refused the
driver at boot, rather than run with a binding that silently never arrives.
There is no unprefixed development fallback for the same reason.

Redeeming the flow is not enough

The identity provider does not sign RelayState. Making it single-use stops a
replay of the same POST, but not a captured assertion replayed inside a login
the attacker starts themselves, which would carry a RelayState that has never
been spent. Assertion IDs are recorded in Redis for as long as the assertion
would otherwise stay valid — its own NotOnOrAfter, not a fixed window, so the
keyspace stays bounded.

There is no email_verified

Nothing an assertion can carry attests an address the way an OpenID Connect
claim does. So the attestation is the operator's: configuring a provider says it
is authoritative for the addresses it asserts, which is true of a corporate
directory and is why trust_email defaults to true. allowed_domains is what
bounds it, exactly as in #311. trust_email: false is there for a directory
whose users can edit their own address; logins through it then only ever reach
an account it provisioned itself.

This is the one place where the SAML driver is weaker than the OpenID Connect
one, and it is worth a reviewer's attention. #311 is explicit that
EmailVerified must come from the provider's own claim because that is what
linkByEmail acts on. SAML has no such claim, so the choice is between
verification-by-configuration and never linking a SAML login to an existing
admin — which would silently create duplicate accounts for people who already
have one.

Configuration

One provider from the environment:

AUTH_DRIVER=basic,saml
AUTH_SAML_ENTITY_ID=http://www.okta.com/exk...
AUTH_SAML_METADATA_URL=https://example.okta.com/app/exk.../sso/saml/metadata

Or, where the deployment cannot reach the metadata — an egress policy, or a
provider that only offers a file — the two fields it would have read from it:

AUTH_SAML_SSO_URL=https://example.okta.com/app/exk.../sso/saml
AUTH_SAML_CERTIFICATE="$(cat idp.pem)"

Setting both forms is refused rather than merged, at both levels: metadata URL
versus explicit fields, and AUTH_SAML_* versus auth.saml.providers.

The entity id is taken verbatim and compared exactly. It is an opaque URI —
Entra publishes an https URL, others publish a urn: with no host — so unlike
an OpenID Connect issuer it is never parsed, and there is no origin to bind the
metadata document to. What binds it instead: the operator chose the URL, it must
pass the outbound policy (which refuses plaintext), and the document must name
the entity id the operator configured.

Several providers are declared in the configuration file, as in #311. The
service provider key material sits on auth.saml rather than on a provider,
because a deployment is one service provider however many directories it
federates with.

Endpoints

GET /api/auth/saml/{provider}/start → provider → POST .../acs.
GET .../metadata publishes this deployment's own service provider metadata, so
an operator registers a URL rather than copying an entity id, an ACS URL and a
certificate between two screens.

The redirect binding is preferred for the request; a provider advertising only
HTTP-POST gets a self-submitting form, built from this deployment's own
AuthnRequest under a CSP that permits exactly that.

Refused deliberately

• Identity-provider-initiated sign-on (the tile in Okta's dashboard). An
  unsolicited response answers no request this deployment issued, so RelayState,
  the browser binding and InResponseTo all have nothing to check. Supporting it
  would mean accepting assertions with none of those guarantees.
• A transient NameID. It names a session rather than a person, so taking it
  as an identity would provision a new admin on every sign-in and strand the
  real account. Refused with a message that says what to ask the administrator
  for.
• A metadata bundle (EntitiesDescriptor). Choosing one entity out of a
  federation bundle means choosing which provider signs this deployment's
  logins; that is the operator's decision to write down.
• Single logout. Signing out ends the session here.

Console

OpenID Connect and SAML collapse into one "Continue with single sign-on" choice
on the method picker, and the providers behind them into one list of buttons
carrying their own protocol. "Sign in with OpenID Connect" versus "sign in with
SAML" is not a choice anybody signing in wants to make.

Verification

ok  internal/sso                                      0.4s
ok  internal/config                                   0.4s
ok  internal/http/auth                               10.4s
ok  internal/http/auth/verifiers                      5.6s
ok  internal/http/controllers/v1/management          71.7s

Both build tags compile and vet clean, make lint clean (the console's 60
prettier warnings are pre-existing and in untouched files), make generate no
diff, console typechecks, vitest 83 passed.

Tests use a stub identity provider that mints real signed assertions with a
throwaway key, so signature verification is exercised rather than stubbed, and
the AuthnRequest ID is inflated back out of the redirect exactly as a real
provider would read it. Nothing reaches the network beyond loopback.

The login flow is covered end to end — real endpoints, real Postgres and Redis —
across every configuration the driver supports:

	covered by
explicit sso_url + certificate	TestSAMLLogin, TestSAMLLoginRefusals
metadata_url, document served over loopback	TestSAMLLoginThroughPublishedMetadata
provider advertising only HTTP-POST	TestSAMLLoginOverThePostRequestBinding
deployment holding a key pair	TestSAMLSignsItsAuthnRequest, TestSAMLAcceptsAnEncryptedAssertion

Refusals: unsigned, signed-by-another-key, wrong
audience/issuer/destination/InResponseTo, expired, RelayState redeemed twice,
assertion replayed under a fresh RelayState, IdP-initiated, wrong-browser
binding, transient NameID, no address. Plus a binding shared across two tabs.

The signed-request test verifies the signature the way the provider would —
over the query string up to the signature itself — against the certificate the
published metadata carries. The encrypted-assertion test also checks that an
envelope this deployment can open, carrying an assertion signed by somebody
else or not signed at all, is still refused: decryption is not authentication.

One pre-existing failure in this package, TestUpsertOrganizationScheduledRecurring,
is date-dependent and fails identically on feat/admin-oidc-sso with none of
this branch applied. Not touched here.

Reviewer notes

• crewjam/saml v0.5.1 brings goxmldsig, etree and
  mattermost/xml-roundtrip-validator. Only saml.ServiceProvider is used —
  samlsp wants to own session handling, which is what auth.Exchanger is for.
• goxmldsig is pinned to v1.6.0, above what crewjam/saml requires. Its
  v1.4.0 carries a loop-variable signature bypass (GHSA-qpr9-2mgv-qcjp) on
  exactly the path a response is proved on; govulncheck flags it as reachable
  from SAML.Complete. The API is unchanged, so the requirement is raised here
  rather than waiting for an upstream release. govulncheck reports nothing
  against the SAML dependencies after the bump.
• internal/sso/flow.go's Redis mechanics are factored into an unexported
  singleUse that both protocols' flow stores share. The OpenID Connect store's
  behaviour and its key prefix are unchanged; the keyspaces stay separate so a
  value issued for one protocol is not redeemable in the other.
• verifiers.Build now returns a *Federated holding both sets rather than a
  *OIDCSet. That is the only change to feat(auth): sign in with the deployment's OpenID Connect providers #311's Go surface.
• Attribute names have no standard in SAML, so the address is looked for under
  the WS-Federation claim URI, the X.500 OID and the bare words, in that order.
  A name the operator sets is used on its own — falling back would mean a
  deployment that pointed at a specific attribute silently reading another.
• SP metadata is served at a per-provider path even though the document is the
  same for all of them, because the ACS URL in it is not.
• The login page typechecks and its endpoints are covered by Go tests, but it
  has not been rendered in a browser.

[Copilot]

Pull request overview

Adds a first-party SAML 2.0 single sign-on driver to the platform’s existing federated-auth framework (stacked on the prior OIDC work), including server-side flow storage, replay protection, metadata handling, management API endpoints, and console UI support.

Changes:

• Implement SAML verifier/set with Redis-backed flow storage + assertion replay protection, and SAML IdP metadata fetching/parsing with caching.
• Add management API endpoints for SAML (/providers, /start, /acs, /metadata) and wire them into the v1 management controller/OpenAPI clients.
• Update console login UX to collapse OIDC+SAML into a single “single sign-on” choice and merge provider lists, plus documentation/deployment config updates.

Reviewed changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
README.md	Documents SAML SSO configuration, endpoints, and operational/security constraints.
internal/sso/singleuse.go	Introduces shared Redis single-use read/consume helper for OIDC/SAML flows.
internal/sso/saml_metadata.go	Implements SAML metadata fetching, strict parsing, caching, and endpoint/cert extraction.
internal/sso/saml_metadata_test.go	Tests metadata binding, endpoint validation, bundle refusal, and certificate parsing.
internal/sso/saml_flow.go	Adds SAML flow store and assertion replay store (SETNX + TTL).
internal/sso/flow.go	Refactors OIDC flow store to use the shared single-use Redis helper.
internal/http/controllers/v1/management/oapi/resources.yml	Extends OpenAPI spec with SAML auth endpoints and schemas.
internal/http/controllers/v1/management/oapi/resources_gen.go	Regenerates Go OpenAPI client/server bindings to include SAML routes.
internal/http/controllers/v1/management/controller.go	Wires SAML dependencies (flows, assertions, metadata cache) into verifier deps.
internal/http/controllers/v1/management/auth.go	Switches from OIDC-only provider set to a combined federated set (OIDC+SAML).
internal/http/controllers/v1/management/auth_saml.go	Adds SAML start/ACS/providers/metadata handlers and error mapping.
internal/http/controllers/v1/management/auth_saml_test.go	End-to-end SAML controller tests with real signed assertions and edge cases.
internal/http/controllers/v1/management/auth_oidc.go	Adapts OIDC handlers to the new combined federated provider container.
internal/http/auth/verifiers/verifier.go	Extends verifier build/wiring to support SAML alongside OIDC via Federated.
internal/http/auth/verifiers/saml.go	Implements SAML authorize/complete/verify, request binding, proof checks, identity mapping.
internal/http/auth/verifiers/saml_test.go	Unit tests for SAML provider construction and identity extraction behavior.
internal/http/auth/verifiers/saml_set.go	Builds ordered SAML provider sets and validates IDs/duplicates at boot.
internal/http/auth/auth.go	Adds SAML browser-binding cookie helpers (SameSite=None, Secure, __Host-).
internal/config/defaults_test.go	Adds tests for SAML config form resolution and form-mixing refusal.
internal/config/config.go	Introduces SAML config structures (auth.saml) and env/YAML mapping.
go.mod	Adds SAML/XML deps (crewjam/saml, goxmldsig, etree, xml-roundtrip-validator).
go.sum	Records checksums for newly added dependencies.
docker-compose.yml	Adds env var wiring/comments for SAML configuration in compose deployments.
console/src/views/auth/Login.tsx	Collapses OIDC+SAML into one SSO choice and fetches/providers per protocol.
console/src/types.ts	Adds saml auth driver and SsoDriver union used by the login UI.
console/src/oapi/management.generated.ts	Regenerates TS OpenAPI types for SAML endpoints/schemas.
console/src/api.ts	Extends console API helpers to list/start SSO per protocol (/auth/{driver}/...).
console/public/locales/en.json	Adds/renames SSO-related i18n keys, including transient NameID error text.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (3) — in code that hasn't changed since the last review.

internal/http/auth/verifiers/saml.go:269

• When configuring a provider via explicit fields (sso_url + certificate), the SSO URL is currently not validated (scheme/host). This allows an insecure http:// (or otherwise invalid) IdP endpoint to be accepted, contradicting the stated requirement that plaintext should be refused and making /start a potential open redirect to a non-HTTPS destination.

This issue also appears on line 291 of the same file.

	if settings.MetadataURL != "" {
		if err := opts.Metadata.ValidateMetadataURL(settings.MetadataURL); err != nil {
			return nil, err
		}
		return provider, nil
	}

	descriptor, err := sso.DescriptorFromFields(settings.EntityID, settings.SSOURL, settings.Certificate)
	if err != nil {
		return nil, err
	}
	provider.descriptor = descriptor
	return provider, nil

internal/sso/saml_metadata.go:355

• ParseCertificates stops silently when pem.Decode returns nil, so trailing non-PEM garbage after a valid certificate bundle is ignored. This can hide configuration mistakes (e.g., pasting a rotated cert plus extra text) and leave the deployment using fewer certificates than intended.

	for len(rest) > 0 {
		var block *pem.Block
		block, rest = pem.Decode(rest)
		if block == nil {
			break
		}

console/src/views/auth/Login.tsx:160

• SSO provider fetching uses the fixed SSO_DRIVERS order (oidc, saml) rather than the operator-declared driver order returned by /auth/methods. This means provider buttons may be shown in a different order than the configured driver list (e.g., saml before oidc).

    useEffect(() => {
        if (!drivers) return
        const enabled = SSO_DRIVERS.filter((driver) => drivers.includes(driver))
        if (enabled.length === 0) return

internal/http/auth/verifiers/saml.go:301

• missingSAMLCollaborators checks opts.Config.MetadataURL without trimming; if MetadataURL is whitespace it will incorrectly require a metadata cache and refuse to build even though withSAMLProviderDefaults() trims it to empty.

func missingSAMLCollaborators(opts SAMLOptions) []string {
	var missing []string
	if opts.Flows == nil {
		missing = append(missing, "Redis, which holds outstanding authentication requests")
	}
	if opts.Assertions == nil {
		missing = append(missing, "Redis, which records the assertions already redeemed")
	}
	if opts.Metadata == nil && opts.Config.MetadataURL != "" {
		missing = append(missing, "a metadata cache")
	}

[jeroenrinzema]

Working through Copilot's suppressed findings from the second pass. All four held up; fixed in ff5df08.

A typed sso_url was never checked (saml.go:269). A descriptor built from fetched metadata runs through validateDescriptor, which holds its sign-on endpoint to the outbound policy; DescriptorFromFields did not. Nothing downstream tells the two apart, so AUTH_SAML_SSO_URL=http://... was accepted at boot and every AuthnRequest was then carried to the provider in plaintext. Both forms go through the same check now — and since the metadata cache is what carries the policy, it is required whichever form configured the provider, which is also the answer to the fourth finding below.

A truncated PEM bundle was read as a bundle of one (saml_metadata.go:355). pem.Decode returning nil ended the loop quietly, so a bundle whose second block did not survive the trip into the environment left the deployment trusting one key with nothing said. That only surfaces as failed logins at the rotation the second key was there for. Trailing bytes are refused now.

The console asked for providers in a fixed protocol order (Login.tsx:160). driverChoices right above it says it keeps the operator's declared order, and this contradicted it two functions later — AUTH_DRIVER=saml,oidc still put the OpenID Connect buttons first. It iterates the declared order now.

missingSAMLCollaborators read MetadataURL untrimmed (saml.go:301), so a whitespace-only value disagreed with the trimmed settings every other check used. Gone rather than fixed: the field is no longer what decides whether the cache is needed.

Tests added for the first two; go build, go vet, tsc --noEmit and internal/{sso,config,http/auth,http/auth/verifiers,http/controllers/v1/management} all clean.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

internal/http/controllers/v1/management/controller.go:85

• SAMLMetadata uses an internal fetch timeout of 15s (internal/sso/saml_metadata.go), but here it’s wired to the shared provider HTTP client built with oidcProviderTimeout (10s). In practice the HTTP client timeout will win, so SAML metadata fetches will time out earlier than the SAML cache expects.

Consider either using a larger shared timeout for the federated HTTP client, or splitting the OIDC and SAML outbound clients so the SAML metadata fetch can use >=15s consistently.

		SAMLFlows:    sso.NewSAMLFlowStore(rdb, cfg.Redis.KeyPrefix),
		Assertions:   sso.NewAssertionReplayStore(rdb, cfg.Redis.KeyPrefix),
		SAMLMetadata: sso.NewSAMLMetadata(provider, ssrf.Policy{}, 0),
		BaseURL:      cfg.PublicBaseURL(),

internal/http/controllers/v1/management/oapi/resources.yml:6632

• The ACS endpoint expects both SAMLResponse and RelayState to be present for a successful login, but the OpenAPI schema marks both fields as optional. This propagates into generated clients/types and makes it easier to produce invalid requests unintentionally.

Mark both fields as required in the form schema (keep the properties as-is).

            schema:
              type: object
              properties:
                SAMLResponse:
                  type: string

[jeroenrinzema]

Second pass on the suppressed findings from the fourth review. Both are misreadings, but both landed on something that was genuinely unexplained, so 6619a0f writes down what was already true rather than changing behaviour.

The 15s metadata fetch timeout is not in conflict with the 10s client (controller.go:85). It is a backstop, not a target. The fetch context is deliberately detached from every caller — so a login whose browser closed mid-flight does not fail the logins that coalesced behind it — which means it needs a deadline that does not depend on a caller having one. The outbound client normally fires first, and that is fine. This is the arrangement #311 already established for OpenID Connect: discoveryFetchTimeout is also 15s against the same 10s client, and its comment says exactly this. The SAML constant was the one without the comment, which is why it read as a disagreement. It has one now.

The ACS form fields stay optional (resources.yml:6632). Nothing generated binds that body — CompleteSAMLLogin takes the raw *http.Request and hands it to idp.Complete. There is no client to generate either: the POST comes from the identity provider as a top-level browser navigation, not from an SDK.

Marking them required would make a missing field a schema violation, and the person on the other end of that navigation needs a page. A response with no SAMLResponse is refused today with a redirect carrying a reason the login view can say out loud; a 400 problem document is a strictly worse thing to show somebody who is trying to sign in. RelayState is the sharper case — it is absent on precisely the identity-provider-initiated responses this deployment declines, and that refusal has a message written for it that says what to ask the administrator for. A schema rejection would replace it.

The description now says both of those things, so the next reader has the reasoning rather than an omission.

[Copilot]

Pull request overview

Copilot reviewed 41 out of 42 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

console/src/views/auth/Login.tsx:273

• The SSO provider button list uses key={provider.id}. If both OIDC and SAML are enabled, it’s valid for both lists to contain the same id (e.g. default), which will create duplicate React keys and can lead to incorrect rendering/updating. Include the driver in the key.

                    {ssoProviders?.map((provider) => (
                        <Button
                            key={provider.id}
                            type="button"

[jeroenrinzema]

Also fixed the suppressed finding from that pass, and rebased.

Duplicate React keys across protocols (Login.tsx:273). Correct: NewSAMLSet refuses duplicate ids within the SAML set and the OpenID Connect set does the same within its own, but nothing spans the two — and each driver's single-provider form names itself default. A deployment running both in that form rendered two buttons keyed default. They key on ${driver}:${id} now.

Rebased onto main. #311 landed as a squash (e16742b), so the branch was carrying the OpenID Connect commits as duplicates of content already on main — which is where the conflict came from once this PR was retargeted. The seven SAML commits are replayed onto main and the OpenID Connect ones are dropped. One conflict, in console/src/api.ts: #285 removed the Tag import from the same line this branch added SsoDriver to. Kept SsoDriver, dropped Tag.

go build, go vet, tsc --noEmit, prettier and internal/{sso,config,http/auth,http/auth/verifiers,http/controllers/v1/management} all clean after the rebase. PR is mergeable again.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/sso/saml_metadata.go:97

• NewSAMLMetadata accepts a nil *http.Client, but Descriptor()/fetch() unconditionally call m.client.Do(...), which will panic if a caller wires SAMLMetadata without a client (e.g., in future tests or alternate wiring paths). Consider defensively defaulting to a policy-enforcing client (or explicitly rejecting nil) so miswiring fails with an error instead of a nil-pointer panic at login time.

func NewSAMLMetadata(client *http.Client, policy ssrf.Policy, ttl time.Duration) *SAMLMetadata {
	if ttl <= 0 {
		ttl = samlMetadataTTL
	}
	return &SAMLMetadata{

[jeroenrinzema]

Suppressed finding from the seventh pass, fixed in 5f74130.

A clientless metadata cache panicked at the first login (saml_metadata.go:97). Descriptor dereferences the client unconditionally, so a SAMLMetadata wired without one failed as a nil-pointer panic during a sign-in rather than anywhere near the miswiring. Only the verifier test helper passes nil today, and it never fetches, so nothing reachable hit it — but that is a property of the current callers, not of the type.

It defaults now, the way the ttl on the line above it already did. Deliberately not http.DefaultClient: this type exists to hold an outbound document to the deployment's policy, and a default client would quietly dial anywhere — a worse failure than the panic. It builds one under the policy it was handed.

---

One unrelated thing worth flagging: TestUpsertOrganizationScheduledRecurring fails on this branch, and it fails identically on a clean origin/main checkout — scheduled_test.go is untouched here (last changed by #280). The assertion is result.ScheduledAt.After(time.Now()), so it looks time-dependent rather than broken by anything in this PR. Not addressing it here.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated no new comments.