Skip to content

Commit 091d08a

Browse files
committed
fix(sso): reject a blank replacement instead of overwriting the stored secret
1 parent 46f5b23 commit 091d08a

2 files changed

Lines changed: 49 additions & 16 deletions

File tree

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

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,20 @@ describe('SSO client secret preservation', () => {
221221
return container.querySelector<HTMLInputElement>('#sso-client-secret')
222222
}
223223

224+
/** Sets the input through the native setter so React's onChange fires. */
225+
function typeSecret(value: string) {
226+
const input = secretInput()
227+
expect(input).not.toBeNull()
228+
act(() => {
229+
const setter = Object.getOwnPropertyDescriptor(
230+
window.HTMLInputElement.prototype,
231+
'value'
232+
)?.set
233+
setter?.call(input, value)
234+
input?.dispatchEvent(new Event('input', { bubbles: true }))
235+
})
236+
}
237+
224238
it('shows the saved secret as a masked hint rather than the sentinel', () => {
225239
renderSso('org-a')
226240
startEditing()
@@ -252,16 +266,7 @@ describe('SSO client secret preservation', () => {
252266
startEditing()
253267
act(() => findButton('Replace')?.click())
254268

255-
const input = secretInput()
256-
expect(input).not.toBeNull()
257-
act(() => {
258-
const setter = Object.getOwnPropertyDescriptor(
259-
window.HTMLInputElement.prototype,
260-
'value'
261-
)?.set
262-
setter?.call(input, 'brand-new-secret')
263-
input?.dispatchEvent(new Event('input', { bubbles: true }))
264-
})
269+
typeSecret('brand-new-secret')
265270

266271
await act(async () => {
267272
findButton('Update')?.click()
@@ -271,6 +276,28 @@ describe('SSO client secret preservation', () => {
271276
expect(mutateAsync.mock.calls[0][0].clientSecret).toBe('brand-new-secret')
272277
})
273278

279+
/**
280+
* A whitespace-only value must not reach the server. Validation is skipped only
281+
* while the stored secret is being kept; once Replace is clicked the field is a
282+
* real input, so blank input has to fail rather than overwrite a working secret.
283+
*/
284+
it('refuses to submit a whitespace-only replacement', async () => {
285+
const mutateAsync = vi.fn().mockResolvedValue({})
286+
mockUseConfigureSSO.mockReturnValue({ isPending: false, mutateAsync })
287+
288+
renderSso('org-a')
289+
startEditing()
290+
act(() => findButton('Replace')?.click())
291+
typeSecret(' ')
292+
293+
await act(async () => {
294+
findButton('Update')?.click()
295+
})
296+
297+
expect(mutateAsync).not.toHaveBeenCalled()
298+
expect(container).toHaveTextContent('Client Secret is required.')
299+
})
300+
274301
/**
275302
* The label is deliberately not "Cancel": the header already uses that to discard
276303
* the whole edit, and matching it here would make two very different actions

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,13 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) {
350350

351351
if (providerType === 'oidc') {
352352
newErrors.clientId = validateRequired('Client ID', data.clientId)
353-
newErrors.clientSecret = hasStoredClientSecret
354-
? []
355-
: validateRequired('Client Secret', data.clientSecret)
353+
// Skipped only while the stored secret is being kept. Once Replace is
354+
// clicked the field is a real input again, so a blank or whitespace-only
355+
// value has to fail rather than quietly overwrite a working secret.
356+
newErrors.clientSecret =
357+
hasStoredClientSecret && !isReplacingClientSecret
358+
? []
359+
: validateRequired('Client Secret', data.clientSecret)
356360
if (!data.scopes || !data.scopes.trim()) {
357361
newErrors.scopes = ['Scopes are required for OIDC providers']
358362
}
@@ -409,11 +413,13 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) {
409413
},
410414
clientId: formData.clientId,
411415
// Blank on an edit means the admin did not retype it: send the
412-
// sentinel so the server keeps the stored secret.
416+
// sentinel so the server keeps the stored secret. Trimmed because a
417+
// pasted secret often carries a trailing newline, and because a
418+
// whitespace-only value must never be stored as the secret.
413419
clientSecret:
414-
hasStoredClientSecret && !formData.clientSecret
420+
hasStoredClientSecret && !formData.clientSecret.trim()
415421
? REDACTED_MARKER
416-
: formData.clientSecret,
422+
: formData.clientSecret.trim(),
417423
scopes: formData.scopes.split(',').map((s) => s.trim()),
418424
...(formData.authorizationEndpoint.trim()
419425
? { authorizationEndpoint: formData.authorizationEndpoint.trim() }

0 commit comments

Comments
 (0)