Skip to content

Commit 48693a2

Browse files
Automatic ACS URL in IdP forms w/ copy button (#2510)
* Add copyable prop to input; set up automatic ACS URL * unnecessary comment * Add tooltip when value is too long to see * No need for useEffect * Revert to useEffect; add tests for getSubdomain * Update design of copyable input * Remove right padding for copyable text * full-height button * slightly wider * Combobox border tweaks * slightly taller Listbox to match other inputs * shuffle files around, make helper not depend on window * constrain TextInput value to be a string * test IdP create and edit * Add checkbox to allow user to use custom ACS URL * Update test to handle unchecking/rechecking standard ACS URL * Update microcopy * use regular useState instead of form for the checkbox --------- Co-authored-by: David Crespo <[email protected]>
1 parent 8da7b6d commit 48693a2

File tree

8 files changed

+173
-29
lines changed

8 files changed

+173
-29
lines changed

app/forms/idp/create.tsx

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*
66
* Copyright Oxide Computer Company
77
*/
8+
import { useEffect, useState } from 'react'
89
import { useForm } from 'react-hook-form'
910
import { useNavigate } from 'react-router-dom'
1011

@@ -18,12 +19,14 @@ import { SideModalForm } from '~/components/form/SideModalForm'
1819
import { HL } from '~/components/HL'
1920
import { useSiloSelector } from '~/hooks/use-params'
2021
import { addToast } from '~/stores/toast'
22+
import { Checkbox } from '~/ui/lib/Checkbox'
2123
import { FormDivider } from '~/ui/lib/Divider'
2224
import { SideModal } from '~/ui/lib/SideModal'
2325
import { readBlobAsBase64 } from '~/util/file'
2426
import { pb } from '~/util/path-builder'
2527

2628
import { MetadataSourceField, type IdpCreateFormValues } from './shared'
29+
import { getDelegatedDomain } from './util'
2730

2831
const defaultValues: IdpCreateFormValues = {
2932
type: 'saml',
@@ -62,6 +65,23 @@ export function CreateIdpSideModalForm() {
6265
})
6366

6467
const form = useForm({ defaultValues })
68+
const name = form.watch('name')
69+
70+
const [generateUrl, setGenerateUrl] = useState(true)
71+
72+
useEffect(() => {
73+
// When creating a SAML identity provider connection, the ACS URL that the user enters
74+
// should always be of the form: http(s)://<silo>.sys.<suffix>/login/<silo>/saml/<name>
75+
// where <silo> is the Silo name, <suffix> is the delegated domain assigned to the rack,
76+
// and <name> is the name of the IdP connection
77+
// The user can override this by unchecking the "Automatically generate ACS URL" checkbox
78+
// and entering a custom ACS URL, though if they check the box again, we will regenerate
79+
// the ACS URL.
80+
const suffix = getDelegatedDomain(window.location)
81+
if (generateUrl) {
82+
form.setValue('acsUrl', `https://${silo}.sys.${suffix}/login/${silo}/saml/${name}`)
83+
}
84+
}, [form, name, silo, generateUrl])
6585

6686
return (
6787
<SideModalForm
@@ -127,13 +147,30 @@ export function CreateIdpSideModalForm() {
127147
required
128148
control={form.control}
129149
/>
130-
<TextField
131-
name="acsUrl"
132-
label="ACS URL"
133-
description="Service provider endpoint for the IdP to send the SAML response"
134-
required
135-
control={form.control}
136-
/>
150+
<div className="flex flex-col gap-2">
151+
<TextField
152+
name="acsUrl"
153+
label="ACS URL"
154+
description={
155+
<div className="children:inline-block">
156+
<span>
157+
Oxide endpoint for the identity provider to send the SAML response.{' '}
158+
</span>
159+
<span>
160+
URL is generated from the current hostname, silo name, and provider name
161+
according to a standard format.
162+
</span>
163+
</div>
164+
}
165+
required
166+
control={form.control}
167+
disabled={generateUrl}
168+
copyable
169+
/>
170+
<Checkbox checked={generateUrl} onChange={(e) => setGenerateUrl(e.target.checked)}>
171+
Use standard ACS URL
172+
</Checkbox>
173+
</div>
137174
<TextField
138175
name="sloUrl"
139176
label="Single Logout (SLO) URL"

app/forms/idp/edit.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export function EditIdpSideModalForm() {
8080
required
8181
control={form.control}
8282
disabled
83+
copyable
8384
/>
8485

8586
<FormDivider />

app/forms/idp/util.spec.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* This Source Code Form is subject to the terms of the Mozilla Public
3+
* License, v. 2.0. If a copy of the MPL was not distributed with this
4+
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
5+
*
6+
* Copyright Oxide Computer Company
7+
*/
8+
import { describe, expect, it } from 'vitest'
9+
10+
import { getDelegatedDomain } from './util'
11+
12+
describe('getDomainSuffix', () => {
13+
it('handles arbitrary URLs by falling back to placeholder', () => {
14+
expect(getDelegatedDomain({ hostname: 'localhost' })).toBe('placeholder')
15+
expect(getDelegatedDomain({ hostname: 'console-preview.oxide.computer' })).toBe(
16+
'placeholder'
17+
)
18+
})
19+
20+
it('handles 1 subdomain after sys', () => {
21+
const location = { hostname: 'oxide.sys.r3.oxide-preview.com' }
22+
expect(getDelegatedDomain(location)).toBe('r3.oxide-preview.com')
23+
})
24+
25+
it('handles 2 subdomains after sys', () => {
26+
const location = { hostname: 'oxide.sys.rack2.eng.oxide.computer' }
27+
expect(getDelegatedDomain(location)).toBe('rack2.eng.oxide.computer')
28+
})
29+
})

app/forms/idp/util.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/*
2+
* This Source Code Form is subject to the terms of the Mozilla Public
3+
* License, v. 2.0. If a copy of the MPL was not distributed with this
4+
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
5+
*
6+
* Copyright Oxide Computer Company
7+
*/
8+
9+
// note: this lives in its own file for fast refresh reasons
10+
11+
/**
12+
* When given a full URL hostname for an Oxide silo, return the domain
13+
* (everything after `<silo>.sys.`). Placeholder logic should only apply
14+
* in local dev or Vercel previews.
15+
*/
16+
export const getDelegatedDomain = (location: { hostname: string }) =>
17+
location.hostname.split('.sys.')[1] || 'placeholder'

app/ui/lib/Combobox.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ export const Combobox = ({
199199
placeholder={placeholder}
200200
disabled={disabled || isLoading}
201201
className={cn(
202-
`h-10 w-full rounded !border-none px-3 py-[0.5rem] !outline-none text-sans-md text-default placeholder:text-quaternary`,
202+
`h-10 w-full rounded !border-none px-3 py-2 !outline-none text-sans-md text-default placeholder:text-quaternary`,
203203
disabled
204204
? 'cursor-not-allowed text-disabled bg-disabled !border-default'
205205
: 'bg-default',
@@ -208,7 +208,7 @@ export const Combobox = ({
208208
/>
209209
{items.length > 0 && (
210210
<ComboboxButton
211-
className="flex items-center border-l px-3 bg-default border-secondary"
211+
className="my-1.5 flex items-center border-l px-3 bg-default border-secondary"
212212
aria-hidden
213213
>
214214
<SelectArrows6Icon title="Select" className="w-2 text-tertiary" />
@@ -218,7 +218,7 @@ export const Combobox = ({
218218
{(items.length > 0 || allowArbitraryValues) && (
219219
<ComboboxOptions
220220
anchor="bottom start"
221-
// 14px gap is presumably because it's measured from inside the outline or something
221+
// 13px gap is presumably because it's measured from inside the outline or something
222222
className={`ox-menu pointer-events-auto ${zIndex} relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border !outline-none border-secondary [--anchor-gap:13px] empty:hidden`}
223223
modal={false}
224224
>

app/ui/lib/Listbox.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ export const Listbox = <Value extends string = string>({
9999
id={id}
100100
name={name}
101101
className={cn(
102-
`flex h-10 w-full items-center justify-between rounded border text-sans-md`,
102+
`flex h-11 w-full items-center justify-between rounded border text-sans-md`,
103103
hasError
104104
? 'focus-error border-error-secondary hover:border-error'
105105
: 'border-default hover:border-hover',

app/ui/lib/TextInput.tsx

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
import { announce } from '@react-aria/live-announcer'
99
import cn from 'classnames'
1010
import React, { useEffect } from 'react'
11+
import type { Merge } from 'type-fest'
12+
13+
import { CopyToClipboard } from './CopyToClipboard'
1114

1215
/**
1316
* This is a little complicated. We only want to allow the `rows` prop if
@@ -32,13 +35,19 @@ export type TextAreaProps =
3235
// it makes a bunch of props required that should be optional. Instead we simply
3336
// take the props of an input field (which are part of the Field props) and
3437
// manually tack on validate.
35-
export type TextInputBaseProps = React.ComponentPropsWithRef<'input'> & {
36-
// error is used to style the wrapper, also to put aria-invalid on the input
37-
error?: boolean
38-
disabled?: boolean
39-
className?: string
40-
fieldClassName?: string
41-
}
38+
export type TextInputBaseProps = Merge<
39+
React.ComponentPropsWithRef<'input'>,
40+
{
41+
// error is used to style the wrapper, also to put aria-invalid on the input
42+
error?: boolean
43+
disabled?: boolean
44+
className?: string
45+
fieldClassName?: string
46+
copyable?: boolean
47+
// by default, number and string[] are allowed, but we want to be simple
48+
value?: string
49+
}
50+
>
4251

4352
export const TextInput = React.forwardRef<
4453
HTMLInputElement,
@@ -47,10 +56,12 @@ export const TextInput = React.forwardRef<
4756
(
4857
{
4958
type = 'text',
59+
value,
5060
error,
5161
className,
5262
disabled,
5363
fieldClassName,
64+
copyable,
5465
as: asProp,
5566
...fieldProps
5667
},
@@ -60,7 +71,7 @@ export const TextInput = React.forwardRef<
6071
return (
6172
<div
6273
className={cn(
63-
'flex rounded border',
74+
'flex items-center rounded border',
6475
error
6576
? 'border-error-secondary hover:border-error'
6677
: 'border-default hover:border-hover',
@@ -72,16 +83,24 @@ export const TextInput = React.forwardRef<
7283
// @ts-expect-error this is fine, it's just mad because Component is a variable
7384
ref={ref}
7485
type={type}
86+
value={value}
7587
className={cn(
7688
`w-full rounded border-none px-3 py-[0.6875rem] !outline-offset-1 text-sans-md text-default bg-default placeholder:text-quaternary focus:outline-none disabled:cursor-not-allowed disabled:text-tertiary disabled:bg-disabled`,
7789
error && 'focus-error',
7890
fieldClassName,
79-
disabled && 'text-disabled bg-disabled'
91+
disabled && 'text-disabled bg-disabled',
92+
copyable && 'pr-0'
8093
)}
8194
aria-invalid={error}
8295
disabled={disabled}
8396
{...fieldProps}
8497
/>
98+
{copyable && (
99+
<CopyToClipboard
100+
text={value || ''}
101+
className="!h-10 rounded-none border-l border-solid px-4 bg-disabled border-default"
102+
/>
103+
)}
85104
</div>
86105
)
87106
}

test/e2e/silos.e2e.ts

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { expect, test } from '@playwright/test'
1010
import {
1111
chooseFile,
1212
clickRowAction,
13+
closeToast,
1314
expectNotVisible,
1415
expectRowVisible,
1516
expectVisible,
@@ -170,28 +171,68 @@ test('Default silo', async ({ page }) => {
170171
page.getByText('Silo viewerFleet viewer'),
171172
])
172173
})
173-
174174
test('Identity providers', async ({ page }) => {
175175
await page.goto('/system/silos/maze-war')
176176

177177
await expectVisible(page, ['role=heading[name*=maze-war]'])
178178

179179
await page.getByRole('link', { name: 'mock-idp' }).click()
180180

181-
await expectVisible(page, [
182-
'role=dialog[name="Identity provider"]',
183-
'role=heading[name="mock-idp"]',
184-
// random stuff that's not in the table
185-
'text="Entity ID"',
186-
'text="Single Logout (SLO) URL"',
187-
])
181+
const dialog = page.getByRole('dialog', { name: 'Identity provider' })
182+
183+
await expect(dialog).toBeVisible()
184+
await expect(page.getByRole('heading', { name: 'mock-idp' })).toBeVisible()
185+
// random stuff that's not in the table
186+
await expect(page.getByText('Entity ID')).toBeVisible()
187+
await expect(page.getByText('Single Logout (SLO) URL')).toBeVisible()
188188

189189
await expect(page.getByRole('textbox', { name: 'Group attribute name' })).toHaveValue(
190190
'groups'
191191
)
192192

193193
await page.getByRole('button', { name: 'Cancel' }).click()
194-
await expectNotVisible(page, ['role=dialog[name="Identity provider"]'])
194+
195+
await expect(dialog).toBeHidden()
196+
197+
// test creating identity provider
198+
await page.getByRole('link', { name: 'New provider' }).click()
199+
200+
await expect(dialog).toBeVisible()
201+
202+
const nameField = dialog.getByLabel('Name', { exact: true })
203+
const acsUrlField = dialog.getByLabel('ACS URL', { exact: true })
204+
205+
await nameField.fill('test-provider')
206+
// ACS URL should be populated with generated value
207+
const acsUrl = 'https://maze-war.sys.placeholder/login/maze-war/saml/test-provider'
208+
await expect(acsUrlField).toHaveValue(acsUrl)
209+
210+
// uncheck the box and change the value
211+
await dialog.getByRole('checkbox', { name: 'Use standard ACS URL' }).click()
212+
await acsUrlField.fill('https://example.com')
213+
await expect(acsUrlField).toHaveValue('https://example.com')
214+
215+
// re-check the box and verify that the value is regenerated
216+
await dialog.getByRole('checkbox', { name: 'Use standard ACS URL' }).click()
217+
await expect(acsUrlField).toHaveValue(acsUrl)
218+
219+
await page.getByRole('button', { name: 'Create provider' }).click()
220+
221+
await closeToast(page)
222+
await expect(dialog).toBeHidden()
223+
224+
// new provider should appear in table
225+
await expectRowVisible(page.getByRole('table'), {
226+
name: 'test-provider',
227+
Type: 'saml',
228+
description: '—',
229+
})
230+
231+
await page.getByRole('link', { name: 'test-provider' }).click()
232+
await expect(nameField).toHaveValue('test-provider')
233+
await expect(nameField).toBeDisabled()
234+
await expect(acsUrlField).toHaveValue(acsUrl)
235+
await expect(acsUrlField).toBeDisabled()
195236
})
196237

197238
test('Silo IP pools', async ({ page }) => {

0 commit comments

Comments
 (0)