Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TLS Cert input #1784

Merged
merged 12 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 172 additions & 0 deletions app/components/form/fields/TlsCertsField.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
/*
* 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 { useState } from 'react'
import type { Control, FieldPath, FieldValues } from 'react-hook-form'
import { useController } from 'react-hook-form'

import { Button, Error16Icon, FieldLabel, MiniTable, Modal } from '@oxide/ui'
import { capitalize } from '@oxide/util'

import {
DescriptionField,
FileField,
TextField,
type TextFieldProps,
} from 'app/components/form'
import type { SiloCreateInput } from 'app/forms/silo-create'
import { useForm } from 'app/hooks'

export function TlsCertsField({ control }: { control: Control<SiloCreateInput> }) {
const [showAddCert, setShowAddCert] = useState(false)

const {
field: { value: items, onChange },
} = useController({ control, name: 'tlsCertificates' })

return (
<>
<div className="max-w-lg">
<FieldLabel id="tls-certificates-label" className="mb-3">
TLS Certificates
</FieldLabel>
{!!items.length && (
<MiniTable.Table className="mb-4">
<MiniTable.Header>
<MiniTable.HeadCell>Name</MiniTable.HeadCell>
{/* For remove button */}
<MiniTable.HeadCell className="w-12" />
</MiniTable.Header>
<MiniTable.Body>
{items.map((item, index) => (
<MiniTable.Row
tabIndex={0}
aria-rowindex={index + 1}
aria-label={`Name: ${item.name}, Description: ${item.description}`}
key={item.name}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we aren't checking for uniqueness around name. Do we want to do so? If not, we should use something like ${item.name}-${index} to ensure the keys do not clash.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we probably should enforce uniqueness. I guess we’d check that on submit in the modal? Refuse to submit if there’s a duplicate.

>
<MiniTable.Cell>{item.name}</MiniTable.Cell>
<MiniTable.Cell>
<button
onClick={() => onChange(items.filter((i) => i.name !== item.name))}
>
<Error16Icon title={`remove ${item.name}`} />
</button>
</MiniTable.Cell>
</MiniTable.Row>
))}
</MiniTable.Body>
</MiniTable.Table>
)}

<Button size="sm" onClick={() => setShowAddCert(true)}>
Add TLS certificate
</Button>
</div>

{showAddCert && (
<AddCertModal
onDismiss={() => setShowAddCert(false)}
onSubmit={async (values) => {
const certCreate: (typeof items)[number] = {
...values,
cert: await values.cert.text(),
key: await values.key.text(),
}
onChange([...items, certCreate])
setShowAddCert(false)
}}
allNames={items.map((item) => item.name)}
/>
)}
</>
)
}

export type TlsCertificate = Omit<
SiloCreateInput['tlsCertificates'][number],
'key' | 'cert'
> & {
key: File
cert: File
}

const defaultValues: Partial<TlsCertificate> = {
description: '',
name: '',
service: 'external_api',
}

function UniqueNameField<
TFieldValues extends FieldValues,
TName extends FieldPath<TFieldValues>
>({
required = true,
name,
label = capitalize(name),
allNames,
...textFieldProps
}: Omit<TextFieldProps<TFieldValues, TName>, 'validate'> & {
label?: string
allNames: string[]
}) {
return (
<TextField
validate={(name) =>
allNames.includes(name) ? 'Certificate with this name already exists' : true
Copy link
Contributor Author

@benjaminleonard benjaminleonard Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did attempt to get this going in the onSubmit of AddCertModal but for whatever reason nothing I put in there was firing. Guessing react-hook-form is intercepting the default onSubmit.

But the benefit of this approach is we don't need to mess around with setError or anything like that. I did consider memoizing allNames but figured it might be overboard for the number of certs we'll have here.

Also the key is guaranteed to be unique because if a name is not, it will never be added.

}
required={required}
label={label}
name={name}
{...textFieldProps}
/>
)
}

const AddCertModal = ({
onDismiss,
onSubmit,
allNames,
}: {
onDismiss: () => void
onSubmit: (values: TlsCertificate) => void
allNames: string[]
}) => {
const { control, handleSubmit } = useForm<TlsCertificate>({ defaultValues })

return (
<Modal isOpen onDismiss={onDismiss} title="Add TLS certificate">
<Modal.Body>
<form
autoComplete="off"
onSubmit={(e) => {
e.stopPropagation()
handleSubmit(onSubmit)(e)
}}
>
<Modal.Section>
<UniqueNameField name="name" control={control} allNames={allNames} />
<DescriptionField name="description" control={control} />
<FileField
id="cert-input"
name="cert"
label="Cert"
required
control={control}
/>
<FileField id="key-input" name="key" label="Key" required control={control} />
</Modal.Section>
</form>
</Modal.Body>
<Modal.Footer
onDismiss={onDismiss}
onAction={handleSubmit(onSubmit)}
actionText="Add Certificate"
/>
</Modal>
)
}
10 changes: 0 additions & 10 deletions app/components/form/fields/index.ts

This file was deleted.

2 changes: 2 additions & 0 deletions app/components/form/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ export * from './fields/NetworkInterfaceField'
export * from './fields/RadioField'
export * from './fields/SubnetListbox'
export * from './fields/TextField'
export * from './fields/TlsCertsField'
export * from './fields/FileField'
benjaminleonard marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 7 additions & 2 deletions app/forms/idp/create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ import { useNavigate } from 'react-router-dom'

import { useApiMutation, useApiQueryClient } from '@oxide/api'

import { DescriptionField, NameField, SideModalForm, TextField } from 'app/components/form'
import { FileField } from 'app/components/form/fields'
import {
DescriptionField,
FileField,
NameField,
SideModalForm,
TextField,
} from 'app/components/form'
import { useForm, useSiloSelector, useToast } from 'app/hooks'
import { readBlobAsBase64 } from 'app/util/file'
import { pb } from 'app/util/path-builder'
Expand Down
3 changes: 1 addition & 2 deletions app/forms/idp/shared.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import type { Merge } from 'type-fest'
import type { IdpMetadataSource, SamlIdentityProviderCreate } from '@oxide/api'
import { Radio, RadioGroup } from '@oxide/ui'

import { TextField } from 'app/components/form'
import { FileField } from 'app/components/form/fields'
import { FileField, TextField } from 'app/components/form'

export type IdpCreateFormValues = { type: 'saml' } & Merge<
SamlIdentityProviderCreate,
Expand Down
2 changes: 1 addition & 1 deletion app/forms/image-upload.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ import { GiB, KiB, invariant } from '@oxide/util'

import {
DescriptionField,
FileField,
NameField,
RadioField,
SideModalForm,
TextField,
} from 'app/components/form'
import { FileField } from 'app/components/form/fields'
import { useForm, useProjectSelector } from 'app/hooks'
import { readBlobAsBase64 } from 'app/util/file'
import { pb } from 'app/util/path-builder'
Expand Down
8 changes: 6 additions & 2 deletions app/forms/silo-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useNavigate } from 'react-router-dom'

import type { SiloCreate } from '@oxide/api'
import { useApiMutation, useApiQueryClient } from '@oxide/api'
import { FormDivider } from '@oxide/ui'

import {
CheckboxField,
Expand All @@ -17,16 +18,17 @@ import {
RadioField,
SideModalForm,
TextField,
TlsCertsField,
} from 'app/components/form'
import { useForm, useToast } from 'app/hooks'
import { pb } from 'app/util/path-builder'

type FormValues = Omit<SiloCreate, 'mappedFleetRoles'> & {
export type SiloCreateInput = Omit<SiloCreate, 'mappedFleetRoles'> & {
siloAdminGetsFleetAdmin: boolean
siloViewerGetsFleetViewer: boolean
}

const defaultValues: FormValues = {
const defaultValues: SiloCreateInput = {
name: '',
description: '',
discoverable: true,
Expand Down Expand Up @@ -117,6 +119,8 @@ export function CreateSiloSideModalForm() {
Grant fleet viewer role to silo viewers
</CheckboxField>
</div>
<FormDivider />
<TlsCertsField control={form.control} />
</SideModalForm>
)
}
61 changes: 60 additions & 1 deletion app/test/e2e/silos.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,25 @@
*
* Copyright Oxide Computer Company
*/
import { expect, test } from '@playwright/test'
import { type Page, expect, test } from '@playwright/test'

import { MiB } from '@oxide/util'

import { expectNotVisible, expectRowVisible, expectVisible } from './utils'

async function chooseFile(fieldName: string, page: Page) {
const fileChooserPromise = page.waitForEvent('filechooser')
await page.getByLabel(fieldName, { exact: true }).click()
const fileChooser = await fileChooserPromise
await fileChooser.setFiles({
name: 'my-image.iso',
mimeType: 'application/octet-stream',
// fill with nonzero content, otherwise we'll skip the whole thing, which
// makes the test too fast for playwright to catch anything
buffer: Buffer.alloc(0.1 * MiB, 'a'),
})
}

test('Silos page', async ({ page }) => {
await page.goto('/system/silos')

Expand All @@ -31,6 +46,50 @@ test('Silos page', async ({ page }) => {
await page.click('role=radio[name="Local only"]')
await page.fill('role=textbox[name="Admin group name"]', 'admins')
await page.click('role=checkbox[name="Grant fleet admin role to silo admins"]')

// Add a TLS cert
await page.click('role=button[name="Add TLS certificate"]')

const certRequired = 'role=dialog[name="Add TLS certificate"] >> text="Cert is required"'
const keyRequired = 'role=dialog[name="Add TLS certificate"] >> text="Key is required"'
await expectNotVisible(page, [certRequired, keyRequired])
await page.click('role=button[name="Add Certificate"]')
// Check that the modal cannot be submitted without cert and
// key and that an error is displayed
await expectVisible(page, [certRequired, keyRequired])

await chooseFile('Cert', page)
await chooseFile('Key', page)
await page.fill(
'role=dialog[name="Add TLS certificate"] >> role=textbox[name="Name"]',
'test-cert'
)

await page.click('role=button[name="Add Certificate"]')

// Check cert appears in the mini-table
await expectVisible(page, ['role=cell[name="test-cert"]'])

await page.click('role=button[name="remove test-cert"]')
// Cert should not appear after it has been deleted
await expectNotVisible(page, ['role=cell[name="test-cert"]'])

await page.click('role=button[name="Add TLS certificate"]')

// Adding another after the first cert is deleted
await page.fill(
'role=dialog[name="Add TLS certificate"] >> role=textbox[name="Name"]',
'test-cert-2'
)
await page.fill(
'role=dialog[name="Add TLS certificate"] >> role=textbox[name="Description"]',
'definitely a cert'
)
await chooseFile('Cert', page)
await chooseFile('Key', page)
await page.click('role=button[name="Add Certificate"]')
await expectVisible(page, ['role=cell[name="test-cert-2"]'])

await page.click('role=button[name="Create silo"]')

// it's there in the table
Expand Down
Loading