Skip to content

Commit

Permalink
Automatic ACS URL in IdP forms w/ copy button (#2510)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
charliepark and david-crespo authored Nov 8, 2024
1 parent 8da7b6d commit 48693a2
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 29 deletions.
51 changes: 44 additions & 7 deletions app/forms/idp/create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*
* Copyright Oxide Computer Company
*/
import { useEffect, useState } from 'react'
import { useForm } from 'react-hook-form'
import { useNavigate } from 'react-router-dom'

Expand All @@ -18,12 +19,14 @@ import { SideModalForm } from '~/components/form/SideModalForm'
import { HL } from '~/components/HL'
import { useSiloSelector } from '~/hooks/use-params'
import { addToast } from '~/stores/toast'
import { Checkbox } from '~/ui/lib/Checkbox'
import { FormDivider } from '~/ui/lib/Divider'
import { SideModal } from '~/ui/lib/SideModal'
import { readBlobAsBase64 } from '~/util/file'
import { pb } from '~/util/path-builder'

import { MetadataSourceField, type IdpCreateFormValues } from './shared'
import { getDelegatedDomain } from './util'

const defaultValues: IdpCreateFormValues = {
type: 'saml',
Expand Down Expand Up @@ -62,6 +65,23 @@ export function CreateIdpSideModalForm() {
})

const form = useForm({ defaultValues })
const name = form.watch('name')

const [generateUrl, setGenerateUrl] = useState(true)

useEffect(() => {
// When creating a SAML identity provider connection, the ACS URL that the user enters
// should always be of the form: http(s)://<silo>.sys.<suffix>/login/<silo>/saml/<name>
// where <silo> is the Silo name, <suffix> is the delegated domain assigned to the rack,
// and <name> is the name of the IdP connection
// The user can override this by unchecking the "Automatically generate ACS URL" checkbox
// and entering a custom ACS URL, though if they check the box again, we will regenerate
// the ACS URL.
const suffix = getDelegatedDomain(window.location)
if (generateUrl) {
form.setValue('acsUrl', `https://${silo}.sys.${suffix}/login/${silo}/saml/${name}`)
}
}, [form, name, silo, generateUrl])

return (
<SideModalForm
Expand Down Expand Up @@ -127,13 +147,30 @@ export function CreateIdpSideModalForm() {
required
control={form.control}
/>
<TextField
name="acsUrl"
label="ACS URL"
description="Service provider endpoint for the IdP to send the SAML response"
required
control={form.control}
/>
<div className="flex flex-col gap-2">
<TextField
name="acsUrl"
label="ACS URL"
description={
<div className="children:inline-block">
<span>
Oxide endpoint for the identity provider to send the SAML response.{' '}
</span>
<span>
URL is generated from the current hostname, silo name, and provider name
according to a standard format.
</span>
</div>
}
required
control={form.control}
disabled={generateUrl}
copyable
/>
<Checkbox checked={generateUrl} onChange={(e) => setGenerateUrl(e.target.checked)}>
Use standard ACS URL
</Checkbox>
</div>
<TextField
name="sloUrl"
label="Single Logout (SLO) URL"
Expand Down
1 change: 1 addition & 0 deletions app/forms/idp/edit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export function EditIdpSideModalForm() {
required
control={form.control}
disabled
copyable
/>

<FormDivider />
Expand Down
29 changes: 29 additions & 0 deletions app/forms/idp/util.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
*
* Copyright Oxide Computer Company
*/
import { describe, expect, it } from 'vitest'

import { getDelegatedDomain } from './util'

describe('getDomainSuffix', () => {
it('handles arbitrary URLs by falling back to placeholder', () => {
expect(getDelegatedDomain({ hostname: 'localhost' })).toBe('placeholder')
expect(getDelegatedDomain({ hostname: 'console-preview.oxide.computer' })).toBe(
'placeholder'
)
})

it('handles 1 subdomain after sys', () => {
const location = { hostname: 'oxide.sys.r3.oxide-preview.com' }
expect(getDelegatedDomain(location)).toBe('r3.oxide-preview.com')
})

it('handles 2 subdomains after sys', () => {
const location = { hostname: 'oxide.sys.rack2.eng.oxide.computer' }
expect(getDelegatedDomain(location)).toBe('rack2.eng.oxide.computer')
})
})
17 changes: 17 additions & 0 deletions app/forms/idp/util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
*
* Copyright Oxide Computer Company
*/

// note: this lives in its own file for fast refresh reasons

/**
* When given a full URL hostname for an Oxide silo, return the domain
* (everything after `<silo>.sys.`). Placeholder logic should only apply
* in local dev or Vercel previews.
*/
export const getDelegatedDomain = (location: { hostname: string }) =>
location.hostname.split('.sys.')[1] || 'placeholder'
6 changes: 3 additions & 3 deletions app/ui/lib/Combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export const Combobox = ({
placeholder={placeholder}
disabled={disabled || isLoading}
className={cn(
`h-10 w-full rounded !border-none px-3 py-[0.5rem] !outline-none text-sans-md text-default placeholder:text-quaternary`,
`h-10 w-full rounded !border-none px-3 py-2 !outline-none text-sans-md text-default placeholder:text-quaternary`,
disabled
? 'cursor-not-allowed text-disabled bg-disabled !border-default'
: 'bg-default',
Expand All @@ -208,7 +208,7 @@ export const Combobox = ({
/>
{items.length > 0 && (
<ComboboxButton
className="flex items-center border-l px-3 bg-default border-secondary"
className="my-1.5 flex items-center border-l px-3 bg-default border-secondary"
aria-hidden
>
<SelectArrows6Icon title="Select" className="w-2 text-tertiary" />
Expand All @@ -218,7 +218,7 @@ export const Combobox = ({
{(items.length > 0 || allowArbitraryValues) && (
<ComboboxOptions
anchor="bottom start"
// 14px gap is presumably because it's measured from inside the outline or something
// 13px gap is presumably because it's measured from inside the outline or something
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`}
modal={false}
>
Expand Down
2 changes: 1 addition & 1 deletion app/ui/lib/Listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export const Listbox = <Value extends string = string>({
id={id}
name={name}
className={cn(
`flex h-10 w-full items-center justify-between rounded border text-sans-md`,
`flex h-11 w-full items-center justify-between rounded border text-sans-md`,
hasError
? 'focus-error border-error-secondary hover:border-error'
: 'border-default hover:border-hover',
Expand Down
37 changes: 28 additions & 9 deletions app/ui/lib/TextInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
import { announce } from '@react-aria/live-announcer'
import cn from 'classnames'
import React, { useEffect } from 'react'
import type { Merge } from 'type-fest'

import { CopyToClipboard } from './CopyToClipboard'

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

export const TextInput = React.forwardRef<
HTMLInputElement,
Expand All @@ -47,10 +56,12 @@ export const TextInput = React.forwardRef<
(
{
type = 'text',
value,
error,
className,
disabled,
fieldClassName,
copyable,
as: asProp,
...fieldProps
},
Expand All @@ -60,7 +71,7 @@ export const TextInput = React.forwardRef<
return (
<div
className={cn(
'flex rounded border',
'flex items-center rounded border',
error
? 'border-error-secondary hover:border-error'
: 'border-default hover:border-hover',
Expand All @@ -72,16 +83,24 @@ export const TextInput = React.forwardRef<
// @ts-expect-error this is fine, it's just mad because Component is a variable
ref={ref}
type={type}
value={value}
className={cn(
`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`,
error && 'focus-error',
fieldClassName,
disabled && 'text-disabled bg-disabled'
disabled && 'text-disabled bg-disabled',
copyable && 'pr-0'
)}
aria-invalid={error}
disabled={disabled}
{...fieldProps}
/>
{copyable && (
<CopyToClipboard
text={value || ''}
className="!h-10 rounded-none border-l border-solid px-4 bg-disabled border-default"
/>
)}
</div>
)
}
Expand Down
59 changes: 50 additions & 9 deletions test/e2e/silos.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { expect, test } from '@playwright/test'
import {
chooseFile,
clickRowAction,
closeToast,
expectNotVisible,
expectRowVisible,
expectVisible,
Expand Down Expand Up @@ -170,28 +171,68 @@ test('Default silo', async ({ page }) => {
page.getByText('Silo viewerFleet viewer'),
])
})

test('Identity providers', async ({ page }) => {
await page.goto('/system/silos/maze-war')

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

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

await expectVisible(page, [
'role=dialog[name="Identity provider"]',
'role=heading[name="mock-idp"]',
// random stuff that's not in the table
'text="Entity ID"',
'text="Single Logout (SLO) URL"',
])
const dialog = page.getByRole('dialog', { name: 'Identity provider' })

await expect(dialog).toBeVisible()
await expect(page.getByRole('heading', { name: 'mock-idp' })).toBeVisible()
// random stuff that's not in the table
await expect(page.getByText('Entity ID')).toBeVisible()
await expect(page.getByText('Single Logout (SLO) URL')).toBeVisible()

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

await page.getByRole('button', { name: 'Cancel' }).click()
await expectNotVisible(page, ['role=dialog[name="Identity provider"]'])

await expect(dialog).toBeHidden()

// test creating identity provider
await page.getByRole('link', { name: 'New provider' }).click()

await expect(dialog).toBeVisible()

const nameField = dialog.getByLabel('Name', { exact: true })
const acsUrlField = dialog.getByLabel('ACS URL', { exact: true })

await nameField.fill('test-provider')
// ACS URL should be populated with generated value
const acsUrl = 'https://maze-war.sys.placeholder/login/maze-war/saml/test-provider'
await expect(acsUrlField).toHaveValue(acsUrl)

// uncheck the box and change the value
await dialog.getByRole('checkbox', { name: 'Use standard ACS URL' }).click()
await acsUrlField.fill('https://example.com')
await expect(acsUrlField).toHaveValue('https://example.com')

// re-check the box and verify that the value is regenerated
await dialog.getByRole('checkbox', { name: 'Use standard ACS URL' }).click()
await expect(acsUrlField).toHaveValue(acsUrl)

await page.getByRole('button', { name: 'Create provider' }).click()

await closeToast(page)
await expect(dialog).toBeHidden()

// new provider should appear in table
await expectRowVisible(page.getByRole('table'), {
name: 'test-provider',
Type: 'saml',
description: '—',
})

await page.getByRole('link', { name: 'test-provider' }).click()
await expect(nameField).toHaveValue('test-provider')
await expect(nameField).toBeDisabled()
await expect(acsUrlField).toHaveValue(acsUrl)
await expect(acsUrlField).toBeDisabled()
})

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

0 comments on commit 48693a2

Please sign in to comment.