fix(sso): encrypt SSO provider secrets at rest, and support encrypted SAML assertions by waleedlatif1 · Pull Request #8045 · simstudioai/sim

waleedlatif1 · 2026-09-20T00:16:37Z

Summary

The security fix. The OIDC clientSecret, and the SAML service-provider keys, were stored as plain JSON in sso_provider while every sibling credential is AES-256-GCM encrypted under ENCRYPTION_KEY. A database copy exposed them without the key. They are now encrypted field-by-field at the Better Auth database adapter — the only seam the SSO plugin's reads and writes both pass through, since it loads the provider row itself and hands clientSecret straight to the authorization URL and token exchange
Only the secret fields are wrapped, so the config columns stay parseable JSON for everything reading the public parts. Encrypted values carry a versioned sim.sso.v1: prefix, so a secret that merely looks like ciphertext is never mistaken for it
Existing rows keep working. A value without the prefix is legacy plaintext and is returned unchanged; it becomes encrypted the next time the provider is saved. Nothing is rewritten by deploying
The paths that read or write these columns outside the adapter each handle the envelope: the register route's "keep existing secret" branch, its rollback (verbatim, by design), the providers list's secret hint, and API redaction
Encrypted SAML assertions are now a product feature. The settings UI and registration API take a service-provider certificate and private key: the certificate is published in the SP metadata Sim serves so the IdP can encrypt to it, and the private key is stored encrypted and used to decrypt. GET /api/auth/sso/providers redacts the key material it previously returned in full
Request signing is deliberately not offered. Better Auth's service provider never signs authentication requests and publishes no signing certificate, so a key field for it would do nothing. Documented, including that IdPs which can enforce signed requests must leave that off
The operator registration scripts are gone, with their env vars and docs. Registration goes through the UI; the scripts were a second write path carrying their own copy of the crypto, and the field they wrote for assertion decryption (decryptionPvk) was never read by the SAML library, so it never worked
Also: the new adapter PostgreSQL suite now runs in CI (that list is explicit, so it would otherwise never have run), and a stored OIDC config with no client secret is refused instead of saving a provider without one

Type of Change

Bug fix
New feature

Testing

New unit suites for the secret helper and the adapter decorator: round-trip, legacy plaintext passthrough, a legacy secret shaped like an envelope, idempotent re-encrypt, null/absent/non-JSON configs, nested service-provider keys, other models untouched, transactions wrapped, and decrypt failure raising rather than passing ciphertext on
New PostgreSQL suite against real AES: ciphertext at rest with the surrounding config still readable, the real secret back out, an update that re-reads and writes back without double-wrapping, and a legacy plaintext row still readable
Encrypted assertions: route tests for certificate publication, key storage, the off state clearing key material, both missing-half rejections, non-PEM rejection, and keeping the stored key on edit. UI tests for the key pair appearing only when enabled and the stored key showing masked with Replace
Verified the cryptography end to end with samlify, outside the repo: an IdP encrypting to the certificate produced an EncryptedAssertion that the stored-config shape decrypted. No key material is committed
Verified the tests fail without the fix: disabling decryption turns 10 tests red across four suites
lib/auth + app/api/auth/sso + ee/sso + contracts: 1,639 passed. sim-setup 170 passed. type-check (sim, db, sim-setup), lint, check:audits (47), docs-manifest:check all pass

Rollout

Deploying rewrites nothing: reads tolerate existing plaintext, so rows change only when a provider is next saved. Any backfill should run after the rollout completes, since an instance on the old code cannot decrypt
After this, losing or rotating ENCRYPTION_KEY also breaks SSO sign-in, where a plaintext secret previously survived it. Recovery is re-entering the client secret. The docs changes say so

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

🤖 Generated with Claude Code

The OIDC client secret, and the SAML signing and decryption keys, were stored as plain JSON in sso_provider while every sibling credential is encrypted with ENCRYPTION_KEY, so a copy of the database exposed them without the key. They are now encrypted field-by-field at the Better Auth adapter, which is the only seam the SSO plugin's reads and writes both pass through; the surrounding config stays readable JSON. Values written before this keep working and are encrypted the next time the provider is saved. The providers API also redacts the SAML key material it used to return in full.

vercel · 2026-09-20T00:16:46Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Sep 20, 2026 2:50am UTC

greptile-apps · 2026-09-20T00:19:13Z

Confidence Score: 5/5

The PR appears safe to merge; the previous correctness findings are fixed or withdrawn, and no actionable regression was identified in the changes since the prior review.

Summary

This PR encrypts OIDC and SAML provider secrets at the Better Auth adapter boundary while preserving compatibility with legacy plaintext rows. It also adds support for encrypted SAML assertions and redacts key material returned by provider-management APIs.

Encrypts designated provider-secret fields using a versioned envelope and decrypts them on adapter reads.
Adds validated SAML encryption certificate/private-key configuration and metadata publication.
Preserves stored secrets during edits, including legacy registration-script key layouts.
Redacts OIDC secrets and SAML private keys from provider-list responses.
Adds unit, route, UI, and PostgreSQL coverage and includes the PostgreSQL suite in CI.
Removes the retired direct-registration scripts and updates self-hosting and SSO documentation.

Diagram

sequenceDiagram
  participant Admin
  participant API as SSO Registration API
  participant Auth as Better Auth
  participant Adapter as Secret Adapter
  participant DB as sso_provider
  participant IdP

  Admin->>API: Save OIDC secret or SAML key pair
  API->>API: Validate authorization and key pair
  API->>Auth: Register/update provider
  Auth->>Adapter: create/update provider config
  Adapter->>Adapter: Encrypt designated secret fields
  Adapter->>DB: Store parseable JSON with encrypted envelopes

  IdP->>Auth: SSO sign-in / encrypted assertion
  Auth->>Adapter: Read provider
  Adapter->>DB: Load provider config
  DB-->>Adapter: Encrypted secret fields
  Adapter->>Adapter: Decrypt envelopes
  Adapter-->>Auth: Plaintext runtime configuration
  Auth->>IdP: OIDC exchange / SAML assertion processing

_{Reviews (8) · Last reviewed commit: "fix(sso): recover gracefully from an unr..."}

cubic-dev-ai

All reported issues were addressed across 19 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

Detecting ciphertext by its iv:ciphertext:authTag shape was ambiguous: a client secret is an arbitrary string chosen at the identity provider, so one shaped like an envelope would have been read back as ciphertext and broken that provider. Encrypted values now carry a versioned prefix. The providers list also lets a decryption failure surface instead of reporting the provider as having no config, and the helper moved next to the adapter that uses it.

waleedlatif1 · 2026-09-20T01:03:59Z

@greptile

waleedlatif1 · 2026-09-20T01:04:00Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-20T01:04:06Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 19 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

…ion scripts Encrypted assertions are now configured in the settings UI and the registration API: the service provider certificate is published in the SP metadata Sim serves, and the matching private key is stored encrypted and handed to the SAML library for decryption. Request signing is deliberately absent — Better Auth's service provider neither signs requests nor publishes a signing certificate, so a key field for it would do nothing. The operator registration scripts are removed along with the env vars and docs that drove them; the UI covers registration, and the scripts were a second write path with their own copy of the crypto. The field they wrote for assertion decryption was never read by the SAML library, so it never worked. Also: run the new adapter PostgreSQL suite in CI, reject a stored OIDC config whose client secret is missing rather than saving a provider without one, and cover the nested service-provider keys in both encryption at rest and API redaction.

waleedlatif1 · 2026-09-20T01:44:55Z

@greptile

waleedlatif1 · 2026-09-20T01:44:56Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-20T01:45:02Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 27 files

_{Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic}

The registration log emitted the resolved provider config with only the client secret and IdP certificate redacted, so enabling encrypted assertions wrote the service provider's private key to a log line. It is redacted now, with a test that fails if it ever reaches one again. Both halves of the encryption pair are also parsed and compared rather than checked for PEM markers: a valid certificate paired with a valid key from a different pair used to save cleanly and then fail every sign-in. The tests generate real key material per run instead of committing any.

waleedlatif1 · 2026-09-20T01:54:10Z

@greptile

waleedlatif1 · 2026-09-20T01:54:11Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-20T01:54:18Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 27 files

_{Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic}

Comparing derived public keys accepted a private key PEM in the certificate field, since a private key satisfies that comparison — and the metadata document was then built by stripping the PEM armor off whatever was pasted, which would have published the private key as the service provider certificate. The certificate is now parsed with X509Certificate, which rejects key PEMs outright, and the document carries that parsed certificate's own DER bytes rather than re-serialized input.

waleedlatif1 · 2026-09-20T02:07:33Z

@greptile

waleedlatif1 · 2026-09-20T02:07:34Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-20T02:07:40Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 27 files

_{You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic}

…a legacy key Both SAML toggles now use ChipSwitch, the canonical control, instead of the legacy Switch primitive; the encrypted-assertions toggle is new here and the signed-assertions one beside it moves with it so the card stays consistent. An update that keeps the stored decryption key also falls back to the flat decryptionPvk that rows from the retired registration script carry, so an admin enabling encryption on such a provider is not asked to re-paste a key they already hold. The pair is validated against the submitted certificate either way.

waleedlatif1 · 2026-09-20T02:15:54Z

@greptile

waleedlatif1 · 2026-09-20T02:15:55Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-20T02:16:00Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 27 files

_{You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic}

The chip switch takes no id, so the labels' htmlFor pointed at elements that no longer exist and clicking one did nothing. They are plain labels beside an aria-labelled control now, matching inbox-enable-toggle. The new field hint also uses the text-caption token rather than text-xs.

waleedlatif1 · 2026-09-20T02:22:55Z

@greptile

waleedlatif1 · 2026-09-20T02:22:56Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-20T02:23:02Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 27 files

_{Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic}

An update that keeps the stored key returned 500 when the key could not be decrypted; it now returns the same kind of 400 the OIDC path does, asking for the key again. Replace gained the Keep saved action its client-secret counterpart has, so opening the field is no longer a one-way door, and the form recognizes a key left flat by the retired registration script, matching the fallback the server already had.

waleedlatif1 · 2026-09-20T02:30:53Z

@greptile

waleedlatif1 · 2026-09-20T02:30:54Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-20T02:31:01Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 27 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic}

…crypted Listing providers now reports an undecryptable config as a provider with no config rather than failing the request. The failure is already logged where it happens, and the form stays reachable — it still offers Replace, which is the only way back now that the operator scripts are gone. A 500 there would have taken the page down with no recovery path. Also reworks the client-secret reuse branch to validate after reading rather than throwing into its own catch, and covers SAML body validation now that its contract branch carries a refinement.

vercel Bot deployed to Preview September 20, 2026 00:18 View deployment

greptile-apps Bot reviewed Sep 20, 2026

View reviewed changes

Comment thread apps/sim/lib/auth/sso/provider-secrets.ts Outdated