fix(sso): keep the settings page reachable when a secret cannot be decrypted

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.

Author: waleedlatif1

apps/sim/app/api/auth/sso/providers/route.test.ts

@@ -94,14 +94,20 @@ describe('GET /api/auth/sso/providers', () => {
     expect(mockDecryptSecret).not.toHaveBeenCalled()
   })
 
-  it('fails the request when a stored secret cannot be decrypted', async () => {
+  it('still lists a provider whose secret cannot be decrypted', async () => {
     mockDecryptSecret.mockRejectedValue(new Error('auth tag mismatch'))
     queueTableRows(schemaMock.ssoProvider, [providerRow])
 
     const res = await GET(createMockRequest('GET'))
 
-    /** Reporting a key problem as a provider with no config would hide it behind a 200. */
-    expect(res.status).toBe(500)
+    /**
+     * The settings form stays reachable, which is where an admin replaces the
+     * secret; the decryption failure is logged rather than 500ing the page.
+     */
+    expect(res.status).toBe(200)
+    const { providers } = await res.json()
+    expect(providers).toHaveLength(1)
+    expect(providers[0]).toMatchObject({ providerId: 'acme-okta', oidcConfig: null })
   })
 
   it('redacts SAML key material and keeps the certificate', async () => {

apps/sim/app/api/auth/sso/providers/route.ts

@@ -44,14 +44,16 @@ function buildClientSecretHint(clientSecret: unknown): string | null {
  */
 async function redactOidcConfig(oidcConfig: string | null): Promise<string | null> {
   if (!oidcConfig) return oidcConfig
-  /**
-   * Outside the catch: a config that will not decrypt is a key problem, and
-   * reporting it as a provider with no config would hide it behind a healthy
-   * 200. Unreadable JSON stays tolerated below, as it was before.
-   */
-  const decrypted = await decryptProviderConfig(oidcConfig, 'oidcConfig')
   try {
-    const parsed = JSON.parse(decrypted as string)
+    /**
+     * A secret that will not decrypt — a lost or rotated `ENCRYPTION_KEY` —
+     * reports as a provider with no config rather than failing the request.
+     * `decryptProviderConfig` has already logged the cause, and listing the
+     * provider is what keeps the settings form reachable: it still offers
+     * Replace, which is how an admin restores a working secret. A 500 here
+     * would take the whole page down and leave no way back.
+     */
+    const parsed = JSON.parse((await decryptProviderConfig(oidcConfig, 'oidcConfig')) as string)
     const hint = buildClientSecretHint(parsed.clientSecret)
     parsed.clientSecret = REDACTED_MARKER
     if (hint) parsed.clientSecretHint = hint

apps/sim/app/api/auth/sso/register/route.test.ts

@@ -381,6 +381,34 @@ describe('POST /api/auth/sso/register', () => {
     expect(mockUpdateSSOProvider).not.toHaveBeenCalled()
   })
 
+  /** The SAML branch carries a superRefine now; these prove the union still narrows cleanly. */
+  it.each([
+    ['an empty certificate', { cert: '' }, /Certificate is required for SAML/],
+    ['a malformed entry point', { entryPoint: 'not-a-url' }, /[Ee]ntry point/],
+    /** A missing field reports the type error; the point is that it narrows to SAML at all. */
+    ['a missing certificate', { cert: undefined }, /expected string/],
+  ])('rejects a SAML body with %s', async (_label, overrides, expected) => {
+    queueMembers([{ organizationId: 'org1', role: 'owner' }])
+    queueProviders([])
+
+    const res = await POST(
+      request({
+        providerType: 'saml',
+        providerId: 'acme-saml',
+        issuer: 'https://idp.acme.com',
+        domain: 'acme.com',
+        orgId: 'org1',
+        entryPoint: 'https://idp.acme.com/sso',
+        cert: 'IDP-CERT',
+        ...overrides,
+      })
+    )
+
+    expect(res.status).toBe(400)
+    await expect(res.json()).resolves.toMatchObject({ error: expect.stringMatching(expected) })
+    expect(mockRegisterSSOProvider).not.toHaveBeenCalled()
+  })
+
   describe('SAML encrypted assertions', () => {
     /**
      * Real key material, because the route parses both halves and checks they

apps/sim/app/api/auth/sso/register/route.ts

@@ -384,26 +384,28 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
             { status: 400 }
           )
         }
+        let storedSecret: unknown
         try {
           const stored = await decryptProviderConfig(existing.oidcConfig, 'oidcConfig')
-          const storedSecret = JSON.parse(stored as string).clientSecret
-          /**
-           * A stored config without a usable secret cannot be reused: letting it
-           * through would save the provider with no client secret at all, and the
-           * failure would only appear at the next sign-in.
-           */
-          if (typeof storedSecret !== 'string' || storedSecret === '') {
-            throw new Error('stored OIDC config has no client secret')
-          }
-          clientSecret = storedSecret
+          storedSecret = JSON.parse(stored as string).clientSecret
         } catch {
+          storedSecret = null
+        }
+
+        /**
+         * Unreadable, or readable but holding no secret: either way there is
+         * nothing to carry forward, and saving without one would surface only at
+         * the next sign-in.
+         */
+        if (typeof storedSecret !== 'string' || storedSecret === '') {
           return NextResponse.json(
             {
               error: 'Cannot update: failed to read existing secret. Re-enter your client secret.',
             },
             { status: 400 }
           )
         }
+        clientSecret = storedSecret
       }
 
       const oidcConfig: any = {