From fe899c6f49e4246c3fbb74cd10f9830ba6d4e06c Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 17 Oct 2024 15:29:18 -0700 Subject: [PATCH 01/18] Add copyable prop to input; set up automatic ACS URL --- app/components/form/fields/TextField.tsx | 1 + app/forms/idp/create.tsx | 18 ++++++++++++++++++ app/forms/idp/edit.tsx | 1 + app/ui/lib/TextInput.tsx | 12 +++++++++++- 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index 9163a02e48..5728f34175 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -48,6 +48,7 @@ export interface TextFieldProps< control: Control /** Alters the value of the input during the field's onChange event. */ transform?: (value: string) => FieldPathValue + /** shortens input field and places a click-to-copy button on the input */ } export function TextField< diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index 4fa8e10fa7..8ff7022541 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -5,6 +5,7 @@ * * Copyright Oxide Computer Company */ +import { useEffect } from 'react' import { useForm } from 'react-hook-form' import { useNavigate } from 'react-router-dom' @@ -22,6 +23,11 @@ import { pb } from '~/util/path-builder' import { MetadataSourceField, type IdpCreateFormValues } from './shared' +const host = window.document.location.host.toString() +const subdomain = host.includes('localhost') + ? 'r3.oxide-preview.com' + : host.split('.').slice(2).join('.') + const defaultValues: IdpCreateFormValues = { type: 'saml', name: '', @@ -59,6 +65,16 @@ export function CreateIdpSideModalForm() { }) const form = useForm({ defaultValues }) + const name = form.watch('name') + + useEffect(() => { + // When creating a SAML identity provider connection, the ACS URL that the user enters + // should always be of the form: http(s)://.sys./login//saml/ + // where is the Silo name, is the subdomain assigned to the rack, + // and is the name of the IdP connection + const acsUrl = `https://${silo}.sys.${subdomain}/login/${silo}/saml/${name}` + form.setValue('acsUrl', acsUrl) + }, [form, name, silo]) return ( {/* TODO: help text */} diff --git a/app/forms/idp/edit.tsx b/app/forms/idp/edit.tsx index a5cbc1e07a..531f6cb869 100644 --- a/app/forms/idp/edit.tsx +++ b/app/forms/idp/edit.tsx @@ -79,6 +79,7 @@ export function EditIdpSideModalForm() { required control={form.control} disabled + copyable /> {/* TODO: help text */} & { disabled?: boolean className?: string fieldClassName?: string + copyable?: boolean } export const TextInput = React.forwardRef< @@ -51,6 +54,7 @@ export const TextInput = React.forwardRef< className, disabled, fieldClassName, + copyable, as: asProp, ...fieldProps }, @@ -60,7 +64,7 @@ export const TextInput = React.forwardRef< return (
+ {copyable && ( + + )}
) } From 46720b8c43f4f5995dde26b6d649299febed9c69 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 17 Oct 2024 15:34:37 -0700 Subject: [PATCH 02/18] unnecessary comment --- app/components/form/fields/TextField.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index 5728f34175..9163a02e48 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -48,7 +48,6 @@ export interface TextFieldProps< control: Control /** Alters the value of the input during the field's onChange event. */ transform?: (value: string) => FieldPathValue - /** shortens input field and places a click-to-copy button on the input */ } export function TextField< From 5aaa096e72ae58d2c71debe2c491b0901b3944a1 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 18 Oct 2024 11:52:59 -0700 Subject: [PATCH 03/18] Add tooltip when value is too long to see --- app/ui/lib/TextInput.tsx | 44 ++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/app/ui/lib/TextInput.tsx b/app/ui/lib/TextInput.tsx index 786c1ee68f..bb635b507b 100644 --- a/app/ui/lib/TextInput.tsx +++ b/app/ui/lib/TextInput.tsx @@ -10,6 +10,7 @@ import cn from 'classnames' import React, { useEffect } from 'react' import { CopyToClipboard } from './CopyToClipboard' +import { Tooltip } from './Tooltip' /** * This is a little complicated. We only want to allow the `rows` prop if @@ -50,6 +51,7 @@ export const TextInput = React.forwardRef< ( { type = 'text', + value, error, className, disabled, @@ -61,6 +63,24 @@ export const TextInput = React.forwardRef< ref ) => { const Component = asProp || 'input' + const component = ( + + ) + const copyableValue = value?.toString() || '' return (
- + {/* don't bother with the tooltip if the string is short */} + {copyable && copyableValue.length > 50 ? ( + + {component} + + ) : ( + component + )} {copyable && ( )} From 08a53e331070de643b7b3befcb7679043da60222 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 18 Oct 2024 12:05:40 -0700 Subject: [PATCH 04/18] No need for useEffect --- app/forms/idp/create.tsx | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index 8ff7022541..601abc6505 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -5,7 +5,6 @@ * * Copyright Oxide Computer Company */ -import { useEffect } from 'react' import { useForm } from 'react-hook-form' import { useNavigate } from 'react-router-dom' @@ -67,15 +66,6 @@ export function CreateIdpSideModalForm() { const form = useForm({ defaultValues }) const name = form.watch('name') - useEffect(() => { - // When creating a SAML identity provider connection, the ACS URL that the user enters - // should always be of the form: http(s)://.sys./login//saml/ - // where is the Silo name, is the subdomain assigned to the rack, - // and is the name of the IdP connection - const acsUrl = `https://${silo}.sys.${subdomain}/login/${silo}/saml/${name}` - form.setValue('acsUrl', acsUrl) - }, [form, name, silo]) - return ( .sys./login//saml/ + // where is the Silo name, is the subdomain assigned to the rack, + // and is the name of the IdP connection + value={`https://${silo}.sys.${subdomain}/login/${silo}/saml/${name}`} required disabled control={form.control} From 2e7b1f18529ac1301955dc9385d87bcefa5eee6c Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 18 Oct 2024 14:25:05 -0700 Subject: [PATCH 05/18] Revert to useEffect; add tests for getSubdomain --- app/forms/idp/create.tsx | 20 +++++++++++--------- app/util/browser.spec.ts | 33 +++++++++++++++++++++++++++++++++ app/util/browser.ts | 15 +++++++++++++++ 3 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 app/util/browser.spec.ts create mode 100644 app/util/browser.ts diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index 601abc6505..03e02fb582 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -5,6 +5,7 @@ * * Copyright Oxide Computer Company */ +import { useEffect } from 'react' import { useForm } from 'react-hook-form' import { useNavigate } from 'react-router-dom' @@ -17,15 +18,13 @@ import { TextField } from '~/components/form/fields/TextField' import { SideModalForm } from '~/components/form/SideModalForm' import { useSiloSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' +import { getSubdomain } from '~/util/browser' import { readBlobAsBase64 } from '~/util/file' import { pb } from '~/util/path-builder' import { MetadataSourceField, type IdpCreateFormValues } from './shared' -const host = window.document.location.host.toString() -const subdomain = host.includes('localhost') - ? 'r3.oxide-preview.com' - : host.split('.').slice(2).join('.') +const subdomain = getSubdomain() const defaultValues: IdpCreateFormValues = { type: 'saml', @@ -66,6 +65,14 @@ export function CreateIdpSideModalForm() { const form = useForm({ defaultValues }) const name = form.watch('name') + useEffect(() => { + // When creating a SAML identity provider connection, the ACS URL that the user enters + // should always be of the form: http(s)://.sys./login//saml/ + // where is the Silo name, is the subdomain assigned to the rack, + // and is the name of the IdP connection + form.setValue('acsUrl', `https://${silo}.sys.${subdomain}/login/${silo}/saml/${name}`) + }, [form, name, silo]) + return ( .sys./login//saml/ - // where is the Silo name, is the subdomain assigned to the rack, - // and is the name of the IdP connection - value={`https://${silo}.sys.${subdomain}/login/${silo}/saml/${name}`} required disabled control={form.control} diff --git a/app/util/browser.spec.ts b/app/util/browser.spec.ts new file mode 100644 index 0000000000..4576254d4d --- /dev/null +++ b/app/util/browser.spec.ts @@ -0,0 +1,33 @@ +/* + * 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 { getSubdomain } from './browser' + +describe('getSubdomain', () => { + it('should handle localhost', () => { + Object.defineProperty(window, 'location', { + value: { hostname: 'localhost' }, + }) + expect(getSubdomain()).toBe('localhost') + }) + + it('should handle 1 subdomains after sys', () => { + Object.defineProperty(window, 'location', { + value: { hostname: 'https://oxide.sys.r3.oxide-preview.com' }, + }) + expect(getSubdomain()).toBe('r3.oxide-preview.com') + }) + + it('should handle 2 subdomains after sys', () => { + Object.defineProperty(window, 'location', { + value: { hostname: 'https://oxide.sys.rack2.eng.oxide.computer' }, + }) + expect(getSubdomain()).toBe('rack2.eng.oxide.computer') + }) +}) diff --git a/app/util/browser.ts b/app/util/browser.ts new file mode 100644 index 0000000000..162ce94567 --- /dev/null +++ b/app/util/browser.ts @@ -0,0 +1,15 @@ +/* + * 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 + */ + +/** When given a full URL hostname for an Oxide silo, return the domain + * (everything after `.sys.`) + */ +export const getSubdomain = () => { + const hostname = window.location.hostname + return hostname === 'localhost' ? 'localhost' : hostname.split('.sys.')[1] +} From e1c7d5f29957d95d0652d7ff5b875842aa950e1f Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 24 Oct 2024 12:19:24 -0700 Subject: [PATCH 06/18] Update design of copyable input --- app/ui/lib/TextInput.tsx | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/app/ui/lib/TextInput.tsx b/app/ui/lib/TextInput.tsx index bb635b507b..d723a84f5b 100644 --- a/app/ui/lib/TextInput.tsx +++ b/app/ui/lib/TextInput.tsx @@ -10,7 +10,6 @@ import cn from 'classnames' import React, { useEffect } from 'react' import { CopyToClipboard } from './CopyToClipboard' -import { Tooltip } from './Tooltip' /** * This is a little complicated. We only want to allow the `rows` prop if @@ -92,19 +91,16 @@ export const TextInput = React.forwardRef< className )} > - {/* don't bother with the tooltip if the string is short */} - {copyable && copyableValue.length > 50 ? ( - - {component} - - ) : ( - component - )} + {component} {copyable && ( - +
+
+ +
+
)}
) From 1cfbeae0fbb3c4456a89f1aa092151ec16a9ffe6 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 25 Oct 2024 13:55:01 -0700 Subject: [PATCH 07/18] Remove right padding for copyable text --- app/ui/lib/TextInput.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/ui/lib/TextInput.tsx b/app/ui/lib/TextInput.tsx index d723a84f5b..1b11e26e4a 100644 --- a/app/ui/lib/TextInput.tsx +++ b/app/ui/lib/TextInput.tsx @@ -72,7 +72,8 @@ export const TextInput = React.forwardRef< `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} From f894637ce1d3ee8212ac9b4cd85bf29629876444 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 28 Oct 2024 09:12:44 -0700 Subject: [PATCH 08/18] full-height button --- app/forms/idp/create.tsx | 2 +- app/ui/lib/TextInput.tsx | 47 +++++++++++++++++----------------------- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index 03e02fb582..090040b440 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -125,8 +125,8 @@ export function CreateIdpSideModalForm() { label="ACS URL" description="Service provider endpoint for the IdP to send the SAML response" required - disabled control={form.control} + disabled copyable /> {/* TODO: help text */} diff --git a/app/ui/lib/TextInput.tsx b/app/ui/lib/TextInput.tsx index 1b11e26e4a..87f77f28bb 100644 --- a/app/ui/lib/TextInput.tsx +++ b/app/ui/lib/TextInput.tsx @@ -62,24 +62,6 @@ export const TextInput = React.forwardRef< ref ) => { const Component = asProp || 'input' - const component = ( - - ) const copyableValue = value?.toString() || '' return (
- {component} + {copyable && ( -
-
- -
-
+ )}
) From 158c45697b232958c8b5f12de2318fc2a5bc71b8 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 28 Oct 2024 09:23:11 -0700 Subject: [PATCH 09/18] slightly wider --- app/ui/lib/TextInput.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/ui/lib/TextInput.tsx b/app/ui/lib/TextInput.tsx index 87f77f28bb..5c8a89e902 100644 --- a/app/ui/lib/TextInput.tsx +++ b/app/ui/lib/TextInput.tsx @@ -93,7 +93,7 @@ export const TextInput = React.forwardRef< {copyable && ( )} From 32b2ab4a9ad9803b002e927f3414989bde9efba0 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 28 Oct 2024 11:32:15 -0700 Subject: [PATCH 10/18] Combobox border tweaks --- app/ui/lib/Combobox.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index bc7de2ff8a..de189c52b6 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -161,7 +161,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', @@ -170,7 +170,7 @@ export const Combobox = ({ /> {items.length > 0 && ( @@ -180,8 +180,8 @@ export const Combobox = ({ {items.length > 0 && ( {!allowArbitraryValues && filteredItems.length === 0 && ( From 37f83178f1e91fa8b967663738285de190951f3d Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 28 Oct 2024 12:46:38 -0700 Subject: [PATCH 11/18] slightly taller Listbox to match other inputs --- app/ui/lib/Listbox.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/ui/lib/Listbox.tsx b/app/ui/lib/Listbox.tsx index 6055e15fe4..978f2548fc 100644 --- a/app/ui/lib/Listbox.tsx +++ b/app/ui/lib/Listbox.tsx @@ -99,7 +99,7 @@ export const Listbox = ({ 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', From 771e164f73ecbd6498d11fc824eb26e3018a8bef Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 30 Oct 2024 16:17:26 -0500 Subject: [PATCH 12/18] shuffle files around, make helper not depend on window --- app/forms/idp/create.tsx | 11 +++++------ app/forms/idp/util.spec.ts | 29 +++++++++++++++++++++++++++++ app/forms/idp/util.ts | 17 +++++++++++++++++ app/util/browser.spec.ts | 33 --------------------------------- app/util/browser.ts | 15 --------------- 5 files changed, 51 insertions(+), 54 deletions(-) create mode 100644 app/forms/idp/util.spec.ts create mode 100644 app/forms/idp/util.ts delete mode 100644 app/util/browser.spec.ts delete mode 100644 app/util/browser.ts diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index 090040b440..0ac055c5e7 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -18,13 +18,11 @@ import { TextField } from '~/components/form/fields/TextField' import { SideModalForm } from '~/components/form/SideModalForm' import { useSiloSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' -import { getSubdomain } from '~/util/browser' import { readBlobAsBase64 } from '~/util/file' import { pb } from '~/util/path-builder' import { MetadataSourceField, type IdpCreateFormValues } from './shared' - -const subdomain = getSubdomain() +import { getDelegatedDomain } from './util' const defaultValues: IdpCreateFormValues = { type: 'saml', @@ -67,10 +65,11 @@ export function CreateIdpSideModalForm() { useEffect(() => { // When creating a SAML identity provider connection, the ACS URL that the user enters - // should always be of the form: http(s)://.sys./login//saml/ - // where is the Silo name, is the subdomain assigned to the rack, + // should always be of the form: http(s)://.sys./login//saml/ + // where is the Silo name, is the delegated domain assigned to the rack, // and is the name of the IdP connection - form.setValue('acsUrl', `https://${silo}.sys.${subdomain}/login/${silo}/saml/${name}`) + const suffix = getDelegatedDomain(window.location) + form.setValue('acsUrl', `https://${silo}.sys.${suffix}/login/${silo}/saml/${name}`) }, [form, name, silo]) return ( diff --git a/app/forms/idp/util.spec.ts b/app/forms/idp/util.spec.ts new file mode 100644 index 0000000000..71c88a81c1 --- /dev/null +++ b/app/forms/idp/util.spec.ts @@ -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') + }) +}) diff --git a/app/forms/idp/util.ts b/app/forms/idp/util.ts new file mode 100644 index 0000000000..9d18f059ec --- /dev/null +++ b/app/forms/idp/util.ts @@ -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 `.sys.`). Placeholder logic should only apply + * in local dev or Vercel previews. + */ +export const getDelegatedDomain = (location: { hostname: string }) => + location.hostname.split('.sys.')[1] || 'placeholder' diff --git a/app/util/browser.spec.ts b/app/util/browser.spec.ts deleted file mode 100644 index 4576254d4d..0000000000 --- a/app/util/browser.spec.ts +++ /dev/null @@ -1,33 +0,0 @@ -/* - * 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 { getSubdomain } from './browser' - -describe('getSubdomain', () => { - it('should handle localhost', () => { - Object.defineProperty(window, 'location', { - value: { hostname: 'localhost' }, - }) - expect(getSubdomain()).toBe('localhost') - }) - - it('should handle 1 subdomains after sys', () => { - Object.defineProperty(window, 'location', { - value: { hostname: 'https://oxide.sys.r3.oxide-preview.com' }, - }) - expect(getSubdomain()).toBe('r3.oxide-preview.com') - }) - - it('should handle 2 subdomains after sys', () => { - Object.defineProperty(window, 'location', { - value: { hostname: 'https://oxide.sys.rack2.eng.oxide.computer' }, - }) - expect(getSubdomain()).toBe('rack2.eng.oxide.computer') - }) -}) diff --git a/app/util/browser.ts b/app/util/browser.ts deleted file mode 100644 index 162ce94567..0000000000 --- a/app/util/browser.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * 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 - */ - -/** When given a full URL hostname for an Oxide silo, return the domain - * (everything after `.sys.`) - */ -export const getSubdomain = () => { - const hostname = window.location.hostname - return hostname === 'localhost' ? 'localhost' : hostname.split('.sys.')[1] -} From d1120a3f7ffcb42c5b9670f4848e6bd718312178 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 30 Oct 2024 16:25:07 -0500 Subject: [PATCH 13/18] constrain TextInput value to be a string --- app/ui/lib/TextInput.tsx | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/app/ui/lib/TextInput.tsx b/app/ui/lib/TextInput.tsx index 5c8a89e902..81614c881d 100644 --- a/app/ui/lib/TextInput.tsx +++ b/app/ui/lib/TextInput.tsx @@ -8,6 +8,7 @@ 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' @@ -34,14 +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 - copyable?: boolean -} +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, @@ -62,7 +68,6 @@ export const TextInput = React.forwardRef< ref ) => { const Component = asProp || 'input' - const copyableValue = value?.toString() || '' return (
{copyable && ( )} From f07b3fad20b09bfe6923b96b3b4ad1989c6dad4d Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 30 Oct 2024 16:39:29 -0500 Subject: [PATCH 14/18] test IdP create and edit --- test/e2e/silos.e2e.ts | 50 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index b37b2e862a..bb58af81da 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -10,6 +10,7 @@ import { expect, test } from '@playwright/test' import { chooseFile, clickRowAction, + closeToast, expectNotVisible, expectRowVisible, expectVisible, @@ -170,7 +171,6 @@ test('Default silo', async ({ page }) => { page.getByText('Silo viewerFleet viewer'), ]) }) - test('Identity providers', async ({ page }) => { await page.goto('/system/silos/maze-war') @@ -178,16 +178,48 @@ test('Identity providers', async ({ page }) => { 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 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') + + 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) + + 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 }) => { From 78730ca8aaa450e021c9791f37464bdbfc631b23 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 7 Nov 2024 14:44:38 -0800 Subject: [PATCH 15/18] Add checkbox to allow user to use custom ACS URL --- app/forms/idp/create.tsx | 41 +++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index 56f3e8786c..f4649fea12 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -11,6 +11,7 @@ import { useNavigate } from 'react-router-dom' import { useApiMutation, useApiQueryClient } from '@oxide/api' +import { CheckboxField } from '~/components/form/fields/CheckboxField' import { DescriptionField } from '~/components/form/fields/DescriptionField' import { FileField } from '~/components/form/fields/FileField' import { NameField } from '~/components/form/fields/NameField' @@ -66,14 +67,22 @@ export function CreateIdpSideModalForm() { const form = useForm({ defaultValues }) const name = form.watch('name') + const acsUrlForm = useForm({ defaultValues: { generateUrl: true } }) + const generateUrl = acsUrlForm.watch('generateUrl') + useEffect(() => { // When creating a SAML identity provider connection, the ACS URL that the user enters // should always be of the form: http(s)://.sys./login//saml/ // where is the Silo name, is the delegated domain assigned to the rack, // and 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) - form.setValue('acsUrl', `https://${silo}.sys.${suffix}/login/${silo}/saml/${name}`) - }, [form, name, silo]) + if (generateUrl) { + form.setValue('acsUrl', `https://${silo}.sys.${suffix}/login/${silo}/saml/${name}`) + } + }, [form, name, silo, generateUrl]) return ( - +
+ + acsUrlForm.setValue('generateUrl', e.target.checked)} + > + Use standard ACS URL + +
Date: Thu, 7 Nov 2024 15:03:09 -0800 Subject: [PATCH 16/18] Update test to handle unchecking/rechecking standard ACS URL --- test/e2e/silos.e2e.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index eba0c960d0..7f0e3ba0fc 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -200,13 +200,22 @@ test('Identity providers', async ({ page }) => { await expect(dialog).toBeVisible() const nameField = dialog.getByLabel('Name', { exact: true }) - const acsUrlField = dialog.getByLabel('ACS URL') + 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) From c10403dcd35a119c559d3cb8f2dc090554a916d7 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 7 Nov 2024 15:40:09 -0800 Subject: [PATCH 17/18] Update microcopy --- app/forms/idp/create.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index f4649fea12..d472a9a2ae 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -152,7 +152,17 @@ export function CreateIdpSideModalForm() { + + Oxide endpoint for the identity provider to send the SAML response.{' '} + + + URL is generated from the current hostname, silo name, and provider name + according to a standard format. + +
+ } required control={form.control} disabled={generateUrl} From 8112dce3a41224ae0e9c97211d42051d3982e1ac Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 7 Nov 2024 21:55:55 -0600 Subject: [PATCH 18/18] use regular useState instead of form for the checkbox --- app/forms/idp/create.tsx | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/app/forms/idp/create.tsx b/app/forms/idp/create.tsx index d472a9a2ae..95f9d13d7f 100644 --- a/app/forms/idp/create.tsx +++ b/app/forms/idp/create.tsx @@ -5,13 +5,12 @@ * * Copyright Oxide Computer Company */ -import { useEffect } from 'react' +import { useEffect, useState } from 'react' import { useForm } from 'react-hook-form' import { useNavigate } from 'react-router-dom' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { CheckboxField } from '~/components/form/fields/CheckboxField' import { DescriptionField } from '~/components/form/fields/DescriptionField' import { FileField } from '~/components/form/fields/FileField' import { NameField } from '~/components/form/fields/NameField' @@ -20,6 +19,7 @@ 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' @@ -67,8 +67,7 @@ export function CreateIdpSideModalForm() { const form = useForm({ defaultValues }) const name = form.watch('name') - const acsUrlForm = useForm({ defaultValues: { generateUrl: true } }) - const generateUrl = acsUrlForm.watch('generateUrl') + const [generateUrl, setGenerateUrl] = useState(true) useEffect(() => { // When creating a SAML identity provider connection, the ACS URL that the user enters @@ -168,14 +167,9 @@ export function CreateIdpSideModalForm() { disabled={generateUrl} copyable /> - acsUrlForm.setValue('generateUrl', e.target.checked)} - > + setGenerateUrl(e.target.checked)}> Use standard ACS URL - +