test(sso): cover client secret preservation, and disambiguate the back-out label

Author: waleedlatif1

apps/sim/ee/sso/components/sso-settings.test.tsx

@@ -20,15 +20,24 @@ vi.mock('@sim/emcn', () => ({
       {children}
     </button>
   ),
+  Chip: ({ children, ...props }: { children?: ReactNode }) => (
+    <button type='button' {...props}>
+      {children}
+    </button>
+  ),
   ChipCombobox: () => <div />,
   ChipCopyInput: ({ value }: { value?: string }) => <input readOnly value={value ?? ''} />,
   ChipInput: ({
     value,
     onChange,
+    id,
+    placeholder,
   }: {
     value?: string
     onChange?: ChangeEventHandler<HTMLInputElement>
-  }) => <input value={value ?? ''} onChange={onChange} />,
+    id?: string
+    placeholder?: string
+  }) => <input id={id} placeholder={placeholder} value={value ?? ''} onChange={onChange} />,
   ChipSelect: () => <div />,
   ChipTextarea: ({
     value,
@@ -58,8 +67,11 @@ vi.mock('@/ee/sso/components/verified-domains-section', () => ({
   VerifiedDomainsSection: () => <div />,
 }))
 
+// Surface the real Save/Update action so submit paths are reachable from tests.
 vi.mock('@/components/settings/save-discard-actions', () => ({
-  saveDiscardActions: () => [],
+  saveDiscardActions: ({ saveLabel, onSave }: { saveLabel?: string; onSave?: () => void }) => [
+    { text: saveLabel ?? 'Save', onSelect: onSave },
+  ],
 }))
 
 vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-empty-state', () => ({
@@ -115,13 +127,26 @@ function provider(organizationId: string) {
     organizationId,
     providerType: 'oidc',
     oidcConfig: JSON.stringify({
+      // What the API actually returns: the sentinel plus a display-only hint,
+      // never the secret itself.
       clientId: `client-${suffix}`,
-      clientSecret: `secret-${suffix}`,
+      clientSecret: '[REDACTED]',
+      clientSecretHint: '4f2a',
       scopes: ['openid'],
     }),
   }
 }
 
+function findButton(text: string) {
+  return Array.from(container.querySelectorAll('button')).find(
+    (button) => button.textContent === text
+  )
+}
+
+function startEditing() {
+  act(() => findButton('Edit')?.click())
+}
+
 let container: HTMLDivElement
 let root: Root
 
@@ -137,46 +162,43 @@ beforeAll(() => {
 
 afterAll(resetEnvFlagsMock)
 
-describe('SSO organization transitions', () => {
-  beforeEach(() => {
-    // The component reads getBaseUrl() during render; make sure the env var is
-    // present even when the suite runs without a local .env or after another
-    // test file mutated the environment (auto-restored via unstubEnvs).
-    vi.stubEnv('NEXT_PUBLIC_APP_URL', 'http://localhost:3000')
-    ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
-    container = document.createElement('div')
-    document.body.appendChild(container)
-    root = createRoot(container)
-    mockUseSession.mockReturnValue({ data: { user: { id: 'user-1' } } })
-    mockUseOrganizationBilling.mockReturnValue({
-      data: { data: { subscriptionPlan: 'enterprise' } },
-      isLoading: false,
-    })
-    mockUseConfigureSSO.mockReturnValue({
-      isPending: false,
-      mutateAsync: vi.fn(),
-    })
-    mockUseSSOProviders.mockImplementation(({ organizationId }: { organizationId: string }) => ({
-      data: { providers: [provider(organizationId)] },
-      isLoading: false,
-    }))
+beforeEach(() => {
+  // The component reads getBaseUrl() during render; make sure the env var is
+  // present even when the suite runs without a local .env or after another
+  // test file mutated the environment (auto-restored via unstubEnvs).
+  vi.stubEnv('NEXT_PUBLIC_APP_URL', 'http://localhost:3000')
+  ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
+  container = document.createElement('div')
+  document.body.appendChild(container)
+  root = createRoot(container)
+  mockUseSession.mockReturnValue({ data: { user: { id: 'user-1' } } })
+  mockUseOrganizationBilling.mockReturnValue({
+    data: { data: { subscriptionPlan: 'enterprise' } },
+    isLoading: false,
   })
-
-  afterEach(() => {
-    act(() => root.unmount())
-    container.remove()
-    vi.clearAllMocks()
+  mockUseConfigureSSO.mockReturnValue({
+    isPending: false,
+    mutateAsync: vi.fn(),
   })
+  mockUseSSOProviders.mockImplementation(({ organizationId }: { organizationId: string }) => ({
+    data: { providers: [provider(organizationId)] },
+    isLoading: false,
+  }))
+})
+
+afterEach(() => {
+  act(() => root.unmount())
+  container.remove()
+  vi.clearAllMocks()
+})
 
+describe('SSO organization transitions', () => {
   it('discards org A edit state before rendering org B settings', () => {
     renderSso('org-a')
     expect(container).toHaveTextContent('org-a.example.com')
 
-    const editButton = Array.from(container.querySelectorAll('button')).find(
-      (button) => button.textContent === 'Edit'
-    )
-    expect(editButton).toBeDefined()
-    act(() => editButton?.click())
+    expect(findButton('Edit')).toBeDefined()
+    startEditing()
     expect(container.querySelector('input[value="client-a"]')).not.toBeNull()
 
     renderSso('org-b')
@@ -186,3 +208,81 @@ describe('SSO organization transitions', () => {
     expect(container.querySelector('input[value="client-a"]')).toBeNull()
   })
 })
+
+/**
+ * The stored client secret never reaches the browser — the API sends a sentinel.
+ * Three pieces have to agree for an edit to preserve it: hydration must not put the
+ * sentinel in the form, validation must not demand a value, and submit must send the
+ * sentinel back. If any one drifts, an admin editing an unrelated field either wipes
+ * their secret or saves the literal string "[REDACTED]" as one.
+ */
+describe('SSO client secret preservation', () => {
+  function secretInput() {
+    return container.querySelector<HTMLInputElement>('#sso-client-secret')
+  }
+
+  it('shows the saved secret as a masked hint rather than the sentinel', () => {
+    renderSso('org-a')
+    startEditing()
+
+    expect(container).not.toHaveTextContent('[REDACTED]')
+    expect(secretInput()?.value).toBe('••••••••••••4f2a')
+    expect(findButton('Replace')).toBeDefined()
+  })
+
+  it('keeps the stored secret when the admin edits without replacing it', async () => {
+    const mutateAsync = vi.fn().mockResolvedValue({})
+    mockUseConfigureSSO.mockReturnValue({ isPending: false, mutateAsync })
+
+    renderSso('org-a')
+    startEditing()
+    await act(async () => {
+      findButton('Update')?.click()
+    })
+
+    expect(mutateAsync).toHaveBeenCalledTimes(1)
+    expect(mutateAsync.mock.calls[0][0].clientSecret).toBe('[REDACTED]')
+  })
+
+  it('sends the new value when the admin replaces the secret', async () => {
+    const mutateAsync = vi.fn().mockResolvedValue({})
+    mockUseConfigureSSO.mockReturnValue({ isPending: false, mutateAsync })
+
+    renderSso('org-a')
+    startEditing()
+    act(() => findButton('Replace')?.click())
+
+    const input = secretInput()
+    expect(input).not.toBeNull()
+    act(() => {
+      const setter = Object.getOwnPropertyDescriptor(
+        window.HTMLInputElement.prototype,
+        'value'
+      )?.set
+      setter?.call(input, 'brand-new-secret')
+      input?.dispatchEvent(new Event('input', { bubbles: true }))
+    })
+
+    await act(async () => {
+      findButton('Update')?.click()
+    })
+
+    expect(mutateAsync).toHaveBeenCalledTimes(1)
+    expect(mutateAsync.mock.calls[0][0].clientSecret).toBe('brand-new-secret')
+  })
+
+  /**
+   * The label is deliberately not "Cancel": the header already uses that to discard
+   * the whole edit, and matching it here would make two very different actions
+   * indistinguishable.
+   */
+  it('restores the masked row and drops the typed value when the replace is backed out', () => {
+    renderSso('org-a')
+    startEditing()
+    act(() => findButton('Replace')?.click())
+    act(() => findButton('Keep saved')?.click())
+
+    expect(secretInput()?.value).toBe('••••••••••••4f2a')
+    expect(findButton('Replace')).toBeDefined()
+  })
+})

apps/sim/ee/sso/components/sso-settings.tsx

@@ -162,7 +162,9 @@ function ClientSecretField({
           ) : undefined
         }
       />
-      {hasStoredSecret && <Chip onClick={onCancelReplace}>Cancel</Chip>}
+      {/* Not "Cancel" — the header already owns that label for discarding the
+          whole edit, and these two do very different things. */}
+      {hasStoredSecret && <Chip onClick={onCancelReplace}>Keep saved</Chip>}
     </div>
   )
 }