Add TLS Cert input (#1784)

* Make imports more consistent

* Rough up adding TLS certs to silo create form

* Add test for adding a silo cert

* TSC onchange and remove unnecessary `readBlobAsText`

* Ensure certificate name is unique

* Don't validate file input on blur

* don't need the onBlur noop if we're not validating on blur

* use TextField directly for unique name, make it required, delete unused file

* polish e2e tests: extract common chooseFile, wrap in test.step(), use getByRole where possible

* code tweaks, mostly names

* undo upgrade to playwright 1.39 for now. it break unrelated tests

---------

Co-authored-by: David Crespo <[email protected]>

app/components/form/fields/TextField.tsx

@@ -53,7 +53,6 @@ export interface TextFieldProps<
   description?: string
   placeholder?: string
   units?: string
-  // TODO: think about this doozy of a type
   validate?: Validate<FieldPathValue<TFieldValues, TName>, TFieldValues>
   control: Control<TFieldValues>
 }

app/components/form/fields/TlsCertsField.tsx

@@ -0,0 +1,147 @@
+/*
+ * 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 } from 'react-hook-form'
+import { useController } from 'react-hook-form'
+import type { Merge } from 'type-fest'
+
+import type { CertificateCreate } from '@oxide/api'
+import { Button, Error16Icon, FieldLabel, MiniTable, Modal } from '@oxide/ui'
+
+import { DescriptionField, FileField, TextField, validateName } from 'app/components/form'
+import type { SiloCreateFormValues } from 'app/forms/silo-create'
+import { useForm } from 'app/hooks'
+
+export function TlsCertsField({ control }: { control: Control<SiloCreateFormValues> }) {
+  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}
+                >
+                  <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 and key are required fields. they will always be present if we get here
+              cert: await values.cert!.text(),
+              key: await values.key!.text(),
+            }
+            onChange([...items, certCreate])
+            setShowAddCert(false)
+          }}
+          allNames={items.map((item) => item.name)}
+        />
+      )}
+    </>
+  )
+}
+
+export type CertFormValues = Merge<
+  CertificateCreate,
+  { key: File | null; cert: File | null } // swap strings for Files
+>
+
+const defaultValues: CertFormValues = {
+  description: '',
+  name: '',
+  service: 'external_api',
+  key: null,
+  cert: null,
+}
+
+type AddCertModalProps = {
+  onDismiss: () => void
+  onSubmit: (values: CertFormValues) => void
+  allNames: string[]
+}
+
+const AddCertModal = ({ onDismiss, onSubmit, allNames }: AddCertModalProps) => {
+  const { control, handleSubmit } = useForm<CertFormValues>({ defaultValues })
+
+  return (
+    <Modal isOpen onDismiss={onDismiss} title="Add TLS certificate">
+      <Modal.Body>
+        <form autoComplete="off" onSubmit={handleSubmit(onSubmit)}>
+          <Modal.Section>
+            <TextField
+              name="name"
+              control={control}
+              required
+              // this field is identical to NameField (which just does
+              // validateName for you) except we also want to check that the
+              // name is not in the list of certs you've already added
+              validate={(name) => {
+                if (allNames.includes(name)) {
+                  return 'A certificate with this name already exists'
+                }
+                return validateName(name, 'Name', true)
+              }}
+            />
+            <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>
+  )
+}

app/components/form/index.ts

@@ -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'

app/forms/idp/create.tsx

@@ -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'

app/forms/idp/shared.tsx

@@ -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,

app/forms/image-upload.tsx

@@ -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'

app/forms/silo-create.tsx

@@ -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,
@@ -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 SiloCreateFormValues = Omit<SiloCreate, 'mappedFleetRoles'> & {
   siloAdminGetsFleetAdmin: boolean
   siloViewerGetsFleetViewer: boolean
 }
 
-const defaultValues: FormValues = {
+const defaultValues: SiloCreateFormValues = {
   name: '',
   description: '',
   discoverable: true,
@@ -117,6 +119,8 @@ export function CreateSiloSideModalForm() {
           Grant fleet viewer role to silo viewers
         </CheckboxField>
       </div>
+      <FormDivider />
+      <TlsCertsField control={form.control} />
     </SideModalForm>
   )
 }

app/test/e2e/image-upload.e2e.ts

@@ -8,22 +8,13 @@
 import type { Page } from '@playwright/test'
 import { expect, test } from '@playwright/test'
 
-import { MiB } from '@oxide/util'
-
-import { expectNotVisible, expectRowVisible, expectVisible, sleep } from './utils'
-
-async function chooseFile(page: Page, size = 15 * MiB) {
-  const fileChooserPromise = page.waitForEvent('filechooser')
-  await page.getByText('Image file', { 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(size, 'a'),
-  })
-}
+import {
+  chooseFile,
+  expectNotVisible,
+  expectRowVisible,
+  expectVisible,
+  sleep,
+} from './utils'
 
 // playwright isn't quick enough to catch each step going from ready to running
 // to complete in time, so we just assert that they all start out ready and end
@@ -56,7 +47,7 @@ async function fillForm(page: Page, name: string) {
   await page.fill('role=textbox[name="Description"]', 'image description')
   await page.fill('role=textbox[name="OS"]', 'Ubuntu')
   await page.fill('role=textbox[name="Version"]', 'Dapper Drake')
-  await chooseFile(page)
+  await chooseFile(page, page.getByLabel('Image file'))
 }
 
 test.describe('Image upload', () => {
@@ -117,7 +108,7 @@ test.describe('Image upload', () => {
     await expectNotVisible(page, [nameRequired])
 
     // now set the file, clear it, and submit again
-    await chooseFile(page)
+    await chooseFile(page, page.getByLabel('Image file'))
     await expectNotVisible(page, [fileRequired])
 
     await page.click('role=button[name="Clear file"]')

app/test/e2e/silos.e2e.ts

@@ -7,9 +7,11 @@
  */
 import { expect, test } from '@playwright/test'
 
-import { expectNotVisible, expectRowVisible, expectVisible } from './utils'
+import { MiB } from '@oxide/util'
 
-test('Silos page', async ({ page }) => {
+import { chooseFile, expectNotVisible, expectRowVisible, expectVisible } from './utils'
+
+test('Create silo', async ({ page }) => {
   await page.goto('/system/silos')
 
   await expectVisible(page, ['role=heading[name*="Silos"]'])
@@ -31,6 +33,55 @@ 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
+  const openCertModalButton = page.getByRole('button', { name: 'Add TLS certificate' })
+  await openCertModalButton.click()
+
+  const certDialog = page.getByRole('dialog', { name: 'Add TLS certificate' })
+
+  const certRequired = certDialog.getByText('Cert is required')
+  const keyRequired = certDialog.getByText('Key is required')
+  const nameRequired = certDialog.getByText('Name is required')
+  await expectNotVisible(page, [certRequired, keyRequired, nameRequired])
+
+  const certSubmit = page.getByRole('button', { name: 'Add Certificate' })
+  await certSubmit.click()
+
+  // Validation error for missing name + key and cert files
+  await expectVisible(page, [certRequired, keyRequired, nameRequired])
+
+  await chooseFile(page, page.getByLabel('Cert', { exact: true }), 0.1 * MiB)
+  await chooseFile(page, page.getByLabel('Key'), 0.1 * MiB)
+  const certName = certDialog.getByRole('textbox', { name: 'Name' })
+  await certName.fill('test-cert')
+
+  await certSubmit.click()
+
+  // Check cert appears in the mini-table
+  const certCell = page.getByRole('cell', { name: 'test-cert', exact: true })
+  await expect(certCell).toBeVisible()
+
+  // check unique name validation
+  await openCertModalButton.click()
+  await certName.fill('test-cert')
+  await certSubmit.click()
+  await expect(
+    certDialog.getByText('A certificate with this name already exists')
+  ).toBeVisible()
+
+  // Change the name so it's unique
+  await certName.fill('test-cert-2')
+  await chooseFile(page, page.getByLabel('Cert', { exact: true }), 0.1 * MiB)
+  await chooseFile(page, page.getByLabel('Key'), 0.1 * MiB)
+  await certSubmit.click()
+  await expect(page.getByRole('cell', { name: 'test-cert-2', exact: true })).toBeVisible()
+
+  // now delete the first
+  await page.getByRole('button', { name: 'remove test-cert', exact: true }).click()
+  // Cert should not appear after it has been deleted
+  await expect(certCell).toBeHidden()
+
   await page.click('role=button[name="Create silo"]')
 
   // it's there in the table

app/test/e2e/utils.ts

@@ -9,6 +9,7 @@ import type { Browser, Locator, Page } from '@playwright/test'
 import { expect } from '@playwright/test'
 
 import { MSW_USER_COOKIE } from '@oxide/api-mocks'
+import { MiB } from '@oxide/util'
 
 export * from '@playwright/test'
 
@@ -148,3 +149,16 @@ export async function expectObscured(locator: Locator) {
 }
 
 export const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms))
+
+export async function chooseFile(page: Page, inputLocator: Locator, size = 15 * MiB) {
+  const fileChooserPromise = page.waitForEvent('filechooser')
+  await inputLocator.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(size, 'a'),
+  })
+}