From fddb457dfb893d3241ea23223574c1aa4ca3e1ee Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Tue, 12 Dec 2023 13:43:00 -0800 Subject: [PATCH 01/13] Add child_account scope to Create and View PAT drawers --- packages/manager/src/factories/oauth.ts | 2 + .../APITokens/CreateAPITokenDrawer.test.tsx | 50 +++++++- .../APITokens/CreateAPITokenDrawer.tsx | 107 ++++++++++-------- .../APITokens/ViewAPITokenDrawer.test.tsx | 36 +++++- .../Profile/APITokens/ViewAPITokenDrawer.tsx | 95 +++++++++------- .../src/features/Profile/APITokens/utils.ts | 2 + 6 files changed, 199 insertions(+), 93 deletions(-) diff --git a/packages/manager/src/factories/oauth.ts b/packages/manager/src/factories/oauth.ts index be6b64dd528..b087b2e878b 100644 --- a/packages/manager/src/factories/oauth.ts +++ b/packages/manager/src/factories/oauth.ts @@ -13,6 +13,7 @@ export const appTokenFactory = Factory.Sync.makeFactory({ export interface Access { Account: number; + // 'Child Account': number; Databases: number; Domains: number; Events: number; @@ -30,6 +31,7 @@ export interface Access { export const accessFactory = Factory.Sync.makeFactory({ Account: 0, + // 'Child Account': 0, Databases: 0, Domains: 0, Events: 0, diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index d31df383ce0..f30dd29ca8a 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -3,6 +3,7 @@ import userEvent from '@testing-library/user-event'; import * as React from 'react'; import { appTokenFactory } from 'src/factories'; +import { accountUserFactory } from 'src/factories/accountUsers'; import { rest, server } from 'src/mocks/testServer'; import { renderWithTheme } from 'src/utilities/testHelpers'; @@ -38,6 +39,7 @@ describe('Create API Token Drawer', () => { expect(cancelBtn).toBeEnabled(); expect(cancelBtn).toBeVisible(); }); + it('Should see secret modal with secret when you type a label and submit the form successfully', async () => { server.use( rest.post('*/profile/tokens', (req, res, ctx) => { @@ -58,19 +60,63 @@ describe('Create API Token Drawer', () => { expect(props.showSecret).toBeCalledWith('secret-value') ); }); + + // TODO: Parent/Child - remove this test when Parent/Child feature is released. it('Should default to read/write for all scopes', () => { const { getByLabelText } = renderWithTheme( ); - const selectAllReadWriteRadioButton = getByLabelText( + const selectAllReadWritePermRadioButton = getByLabelText( 'Select read/write for all' ); - expect(selectAllReadWriteRadioButton).toBeChecked(); + expect(selectAllReadWritePermRadioButton).toBeChecked(); + }); + + it('Should default to None for all scopes with the parent/child feature flag on', () => { + const { getByLabelText } = renderWithTheme( + , + { flags: { parentChildAccountAccess: true } } + ); + const selectAllNonePermRadioButton = getByLabelText('Select none for all'); + expect(selectAllNonePermRadioButton).toBeChecked(); }); + it('Should default to 6 months for expiration', () => { const { getByText } = renderWithTheme(); getByText('In 6 months'); }); + + // TODO: fix - cannot find text + it.skip('Should show the Child Account scope for a parent user account with the parent/child feature flag on', () => { + server.use( + // rest.get('*/profile', (req, res, ctx) => { + // return res(ctx.json(profileFactory.build({ username: 'parent-user' }))); + // }), + rest.get('*/account/users/parent-user', (req, res, ctx) => { + return res(ctx.json(accountUserFactory.build({ user_type: 'parent' }))); + }) + ); + + const { getByText } = renderWithTheme(, { + flags: { parentChildAccountAccess: true }, + }); + expect(getByText('Child Account')).toBeInTheDocument(); + }); + + // TODO: fix - cannot find text + it.skip('Should not show the Child Account scope for a non-parent user account with the parent/child feature flag on', () => { + server.use( + rest.get('*/account/users/test-user', (req, res, ctx) => { + return res(ctx.json(accountUserFactory.build({ user_type: null }))); + }) + ); + + const { getByText } = renderWithTheme(, { + flags: { parentChildAccountAccess: true }, + }); + expect(getByText('Child Account')).not.toBeInTheDocument(); + }); + it('Should close when Cancel is pressed', () => { const { getByText } = renderWithTheme(); const cancelButton = getByText(/Cancel/); diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx index 87e1d178331..c842a2dca23 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx @@ -5,6 +5,8 @@ import * as React from 'react'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { Drawer } from 'src/components/Drawer'; import Select, { Item } from 'src/components/EnhancedSelect/Select'; +import { FormControl } from 'src/components/FormControl'; +import { FormHelperText } from 'src/components/FormHelperText'; import { Notice } from 'src/components/Notice/Notice'; import { Radio } from 'src/components/Radio/Radio'; import { TableBody } from 'src/components/TableBody'; @@ -12,10 +14,11 @@ import { TableCell } from 'src/components/TableCell'; import { TableHead } from 'src/components/TableHead'; import { TableRow } from 'src/components/TableRow'; import { TextField } from 'src/components/TextField'; -import { FormControl } from 'src/components/FormControl'; -import { FormHelperText } from 'src/components/FormHelperText'; import { ISO_DATETIME_NO_TZ_FORMAT } from 'src/constants'; import { AccessCell } from 'src/features/ObjectStorage/AccessKeyLanding/AccessCell'; +import { useFlags } from 'src/hooks/useFlags'; +import { useAccountUser } from 'src/queries/accountUsers'; +import { useProfile } from 'src/queries/profile'; import { useCreatePersonalAccessTokenMutation } from 'src/queries/tokens'; import { getErrorMap } from 'src/utilities/errorUtils'; @@ -82,12 +85,17 @@ export const CreateAPITokenDrawer = (props: Props) => { const expiryTups = genExpiryTups(); const { onClose, open, showSecret } = props; + const flags = useFlags(); + const initialValues = { expiry: expiryTups[0][1], label: '', - scopes: scopeStringToPermTuples('*'), + scopes: scopeStringToPermTuples(flags.parentChildAccountAccess ? '' : '*'), }; + const { data: profile } = useProfile(); + const { data: user } = useAccountUser(profile?.username ?? ''); + const { error, isLoading, @@ -241,50 +249,57 @@ export const CreateAPITokenDrawer = (props: Props) => { return null; } return ( - - - {basePermNameMap[scopeTup[0]]} - - - - - - - - - - - + + {basePermNameMap[scopeTup[0]]} + + + + + + + + + + + + ) ); })} diff --git a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx index 5f2c0b8e7b3..22da74bfa19 100644 --- a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx @@ -6,7 +6,13 @@ import { renderWithTheme } from 'src/utilities/testHelpers'; import { ViewAPITokenDrawer } from './ViewAPITokenDrawer'; import { basePerms } from './utils'; +const nonParentPerms = basePerms.filter((value) => value !== 'child_account'); + const token = appTokenFactory.build({ label: 'my-token', scopes: '*' }); +const limitedToken = appTokenFactory.build({ + label: 'my-limited-token', + scopes: '', +}); const props = { onClose: vi.fn(), @@ -21,9 +27,25 @@ describe('View API Token Drawer', () => { expect(getByText(token.label)).toBeVisible(); }); - it('should all permissions as read/write with wildcard scopes', () => { - const { getByTestId } = renderWithTheme(); - for (const permissionName of basePerms) { + it('should show all permissions as none with no scopes', () => { + const { getByTestId } = renderWithTheme( + , + { flags: { parentChildAccountAccess: false } } + ); + for (const permissionName of nonParentPerms) { + expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute( + 'aria-label', + `This token has 0 access for ${permissionName}` + ); + } + }); + + it('should show all permissions as read/write with wildcard scopes', () => { + const { getByTestId } = renderWithTheme( + , + {} + ); + for (const permissionName of nonParentPerms) { expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute( 'aria-label', `This token has 2 access for ${permissionName}` @@ -38,7 +60,7 @@ describe('View API Token Drawer', () => { token={appTokenFactory.build({ scopes: 'account:read_write' })} /> ); - for (const permissionName of basePerms) { + for (const permissionName of nonParentPerms) { // We only expect account to have read/write for this test const expectedScopeLevel = permissionName === 'account' ? 2 : 0; expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute( @@ -76,7 +98,7 @@ describe('View API Token Drawer', () => { volumes: 1, } as const; - for (const permissionName of basePerms) { + for (const permissionName of nonParentPerms) { const expectedScopeLevel = expectedScopeLevels[permissionName]; expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute( 'aria-label', @@ -84,4 +106,8 @@ describe('View API Token Drawer', () => { ); } }); + + it('shows the child account perm in the table for a parent user account', () => { + // TODO: test child_account perm; use basePerms + }); }); diff --git a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx index 12493e8ffa0..6b32ccc0351 100644 --- a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx +++ b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx @@ -7,6 +7,9 @@ import { TableCell } from 'src/components/TableCell'; import { TableHead } from 'src/components/TableHead'; import { TableRow } from 'src/components/TableRow'; import { AccessCell } from 'src/features/ObjectStorage/AccessKeyLanding/AccessCell'; +import { useFlags } from 'src/hooks/useFlags'; +import { useAccountUser } from 'src/queries/accountUsers'; +import { useProfile } from 'src/queries/profile'; import { StyledAccessCell, @@ -24,6 +27,11 @@ interface Props { export const ViewAPITokenDrawer = (props: Props) => { const { onClose, open, token } = props; + const flags = useFlags(); + + const { data: profile } = useProfile(); + const { data: user } = useAccountUser(profile?.username ?? ''); + const permissions = scopeStringToPermTuples(token?.scopes ?? ''); return ( @@ -53,47 +61,54 @@ export const ViewAPITokenDrawer = (props: Props) => { return null; } return ( - - - {basePermNameMap[scopeTup[0]]} - - - null} - scope="0" - scopeDisplay={scopeTup[0]} - viewOnly={true} - /> - - - null} - scope="1" - scopeDisplay={scopeTup[0]} - viewOnly={true} - /> - - - null} - scope="2" - scopeDisplay={scopeTup[0]} - viewOnly={true} - /> - - + + {basePermNameMap[scopeTup[0]]} + + + null} + scope="0" + scopeDisplay={scopeTup[0]} + viewOnly={true} + /> + + + null} + scope="1" + scopeDisplay={scopeTup[0]} + viewOnly={true} + /> + + + null} + scope="2" + scopeDisplay={scopeTup[0]} + viewOnly={true} + /> + + + ) ); })} diff --git a/packages/manager/src/features/Profile/APITokens/utils.ts b/packages/manager/src/features/Profile/APITokens/utils.ts index f6dbd4c2a7f..7df9027bab9 100644 --- a/packages/manager/src/features/Profile/APITokens/utils.ts +++ b/packages/manager/src/features/Profile/APITokens/utils.ts @@ -6,6 +6,7 @@ export type Permission = [string, number]; export const basePerms = [ 'account', + 'child_account', 'databases', 'domains', 'events', @@ -23,6 +24,7 @@ export const basePerms = [ export const basePermNameMap: Record = { account: 'Account', + child_account: 'Child Account', databases: 'Databases', domains: 'Domains', events: 'Events', From 1f2d5939e8b888919a4f5b9b4cae7cad3dcecf79 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Wed, 13 Dec 2023 15:37:24 -0800 Subject: [PATCH 02/13] Make new scope name match mocks --- .../Profile/APITokens/CreateAPITokenDrawer.test.tsx | 8 ++++---- .../features/Profile/APITokens/CreateAPITokenDrawer.tsx | 7 ++++--- .../src/features/Profile/APITokens/ViewAPITokenDrawer.tsx | 7 ++++--- packages/manager/src/features/Profile/APITokens/utils.ts | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index f30dd29ca8a..f3b3ffdafd3 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -87,7 +87,7 @@ describe('Create API Token Drawer', () => { }); // TODO: fix - cannot find text - it.skip('Should show the Child Account scope for a parent user account with the parent/child feature flag on', () => { + it.skip('Should show the Child Account Access scope for a parent user account with the parent/child feature flag on', () => { server.use( // rest.get('*/profile', (req, res, ctx) => { // return res(ctx.json(profileFactory.build({ username: 'parent-user' }))); @@ -100,11 +100,11 @@ describe('Create API Token Drawer', () => { const { getByText } = renderWithTheme(, { flags: { parentChildAccountAccess: true }, }); - expect(getByText('Child Account')).toBeInTheDocument(); + expect(getByText('Child Account Access')).toBeInTheDocument(); }); // TODO: fix - cannot find text - it.skip('Should not show the Child Account scope for a non-parent user account with the parent/child feature flag on', () => { + it.skip('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', () => { server.use( rest.get('*/account/users/test-user', (req, res, ctx) => { return res(ctx.json(accountUserFactory.build({ user_type: null }))); @@ -114,7 +114,7 @@ describe('Create API Token Drawer', () => { const { getByText } = renderWithTheme(, { flags: { parentChildAccountAccess: true }, }); - expect(getByText('Child Account')).not.toBeInTheDocument(); + expect(getByText('Child Account Access')).not.toBeInTheDocument(); }); it('Should close when Cancel is pressed', () => { diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx index c842a2dca23..6394b0ff675 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx @@ -249,12 +249,13 @@ export const CreateAPITokenDrawer = (props: Props) => { return null; } return ( - // When the feature flag is on, display the Child Account scope for parent user accounts only. + // When the feature flag is on, display the Child Account Access scope for parent user accounts only. (!flags.parentChildAccountAccess && - basePermNameMap[scopeTup[0]] === 'Child Account') || + basePermNameMap[scopeTup[0]] === 'Child Account Access') || (flags.parentChildAccountAccess && user?.user_type !== 'parent' && - basePermNameMap[scopeTup[0]] === 'Child Account') ? null : ( + basePermNameMap[scopeTup[0]] === + 'Child Account Access') ? null : ( { return null; } return ( - // When the feature flag is on, display the Child Account scope for parent user accounts only. + // When the feature flag is on, display the Child Account Access scope for parent user accounts only. (!flags.parentChildAccountAccess && - basePermNameMap[scopeTup[0]] === 'Child Account') || + basePermNameMap[scopeTup[0]] === 'Child Account Access') || (flags.parentChildAccountAccess && user?.user_type !== 'parent' && - basePermNameMap[scopeTup[0]] === 'Child Account') ? null : ( + basePermNameMap[scopeTup[0]] === + 'Child Account Access') ? null : ( = { account: 'Account', - child_account: 'Child Account', + child_account: 'Child Account Access', databases: 'Databases', domains: 'Domains', events: 'Events', From adb0d0226f118a195a22e8a472876da7341ad90b Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Wed, 13 Dec 2023 15:38:03 -0800 Subject: [PATCH 03/13] Fix APITokens utils failing tests --- .../src/features/Profile/APITokens/utils.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/manager/src/features/Profile/APITokens/utils.test.ts b/packages/manager/src/features/Profile/APITokens/utils.test.ts index 83181c2fbbf..fdf56838f95 100644 --- a/packages/manager/src/features/Profile/APITokens/utils.test.ts +++ b/packages/manager/src/features/Profile/APITokens/utils.test.ts @@ -26,6 +26,7 @@ describe('APIToken utils', () => { const result = scopeStringToPermTuples('*'); const expected = [ ['account', 2], + ['child_account', 2], ['databases', 2], ['domains', 2], ['events', 2], @@ -49,6 +50,7 @@ describe('APIToken utils', () => { const result = scopeStringToPermTuples(''); const expected = [ ['account', 0], + ['child_account', 0], ['databases', 0], ['domains', 0], ['events', 0], @@ -73,6 +75,7 @@ describe('APIToken utils', () => { const result = scopeStringToPermTuples('account:none'); const expected = [ ['account', 0], + ['child_account', 0], ['databases', 0], ['domains', 0], ['events', 0], @@ -97,6 +100,7 @@ describe('APIToken utils', () => { const result = scopeStringToPermTuples('account:read_only'); const expected = [ ['account', 1], + ['child_account', 0], ['databases', 0], ['domains', 0], ['events', 0], @@ -121,6 +125,7 @@ describe('APIToken utils', () => { const result = scopeStringToPermTuples('account:read_write'); const expected = [ ['account', 2], + ['child_account', 0], ['databases', 0], ['domains', 0], ['events', 0], @@ -147,6 +152,7 @@ describe('APIToken utils', () => { ); const expected = [ ['account', 0], + ['child_account', 0], ['databases', 0], ['domains', 1], ['events', 0], @@ -175,6 +181,7 @@ describe('APIToken utils', () => { const result = scopeStringToPermTuples('account:none,tokens:read_write'); const expected = [ ['account', 2], + ['child_account', 0], ['databases', 0], ['domains', 0], ['events', 0], @@ -203,6 +210,7 @@ describe('APIToken utils', () => { const result = scopeStringToPermTuples('account:read_only,tokens:none'); const expected = [ ['account', 1], + ['child_account', 0], ['databases', 0], ['domains', 0], ['events', 0], @@ -227,6 +235,7 @@ describe('APIToken utils', () => { it('should return 0 if all scopes are 0', () => { const scopes: Permission[] = [ ['account', 0], + ['child_account', 0], ['databases', 0], ['domains', 0], ['events', 0], @@ -246,6 +255,7 @@ describe('APIToken utils', () => { it('should return 1 if all scopes are 1', () => { const scopes: Permission[] = [ ['account', 1], + ['child_account', 1], ['databases', 1], ['domains', 1], ['events', 1], @@ -265,6 +275,7 @@ describe('APIToken utils', () => { it('should return 2 if all scopes are 2', () => { const scopes: Permission[] = [ ['account', 2], + ['child_account', 2], ['databases', 2], ['domains', 2], ['events', 2], @@ -284,6 +295,7 @@ describe('APIToken utils', () => { it('should return null if all scopes are different', () => { const scopes: Permission[] = [ ['account', 1], + ['child_account', 0], ['databases', 0], ['domains', 2], ['events', 0], From 9ebe44d45620c806b1db6c4396b1e19733270efa Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 10:34:36 -0800 Subject: [PATCH 04/13] Fix one failing unit test with await --- .../APITokens/CreateAPITokenDrawer.test.tsx | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index f3b3ffdafd3..ad654f40833 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -86,35 +86,39 @@ describe('Create API Token Drawer', () => { getByText('In 6 months'); }); - // TODO: fix - cannot find text - it.skip('Should show the Child Account Access scope for a parent user account with the parent/child feature flag on', () => { + it('Should show the Child Account Access scope for a parent user account with the parent/child feature flag on', async () => { server.use( - // rest.get('*/profile', (req, res, ctx) => { - // return res(ctx.json(profileFactory.build({ username: 'parent-user' }))); - // }), rest.get('*/account/users/parent-user', (req, res, ctx) => { return res(ctx.json(accountUserFactory.build({ user_type: 'parent' }))); }) ); - const { getByText } = renderWithTheme(, { - flags: { parentChildAccountAccess: true }, - }); - expect(getByText('Child Account Access')).toBeInTheDocument(); + const { findByText } = renderWithTheme( + , + { + flags: { parentChildAccountAccess: true }, + } + ); + const childScope = await findByText('Child Account Access'); + expect(childScope).toBeInTheDocument(); }); // TODO: fix - cannot find text - it.skip('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', () => { + it('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', async () => { server.use( rest.get('*/account/users/test-user', (req, res, ctx) => { return res(ctx.json(accountUserFactory.build({ user_type: null }))); }) ); - const { getByText } = renderWithTheme(, { - flags: { parentChildAccountAccess: true }, - }); - expect(getByText('Child Account Access')).not.toBeInTheDocument(); + const { findByText } = renderWithTheme( + , + { + flags: { parentChildAccountAccess: true }, + } + ); + const childScope = await findByText('Child Account Access'); + expect(childScope).not.toBeInTheDocument(); }); it('Should close when Cancel is pressed', () => { From d21fbaf83681b1f300b0a31d92399c6ccef0abb6 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 10:48:17 -0800 Subject: [PATCH 05/13] Update comment --- .../features/Profile/APITokens/CreateAPITokenDrawer.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index ad654f40833..7738d4ec9d9 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -103,7 +103,7 @@ describe('Create API Token Drawer', () => { expect(childScope).toBeInTheDocument(); }); - // TODO: fix - cannot find text + // TODO: fix failing test it('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', async () => { server.use( rest.get('*/account/users/test-user', (req, res, ctx) => { From df607a6e7a88bafa0ee5dbd0b78cc2a32a10ae14 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 13:17:13 -0800 Subject: [PATCH 06/13] Fix the unit tests for real, hopefully --- .../Profile/APITokens/CreateAPITokenDrawer.test.tsx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index 7738d4ec9d9..a0c941e14bb 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -88,7 +88,7 @@ describe('Create API Token Drawer', () => { it('Should show the Child Account Access scope for a parent user account with the parent/child feature flag on', async () => { server.use( - rest.get('*/account/users/parent-user', (req, res, ctx) => { + rest.get('*/account/users/*', (req, res, ctx) => { return res(ctx.json(accountUserFactory.build({ user_type: 'parent' }))); }) ); @@ -103,21 +103,20 @@ describe('Create API Token Drawer', () => { expect(childScope).toBeInTheDocument(); }); - // TODO: fix failing test - it('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', async () => { + it('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', () => { server.use( - rest.get('*/account/users/test-user', (req, res, ctx) => { + rest.get('*/account/users/*', (req, res, ctx) => { return res(ctx.json(accountUserFactory.build({ user_type: null }))); }) ); - const { findByText } = renderWithTheme( + const { queryByText } = renderWithTheme( , { flags: { parentChildAccountAccess: true }, } ); - const childScope = await findByText('Child Account Access'); + const childScope = queryByText('Child Account Access'); expect(childScope).not.toBeInTheDocument(); }); From 3bcdf12b075c63e22eb5230f2d416bad0f6104cb Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 13:59:20 -0800 Subject: [PATCH 07/13] Mock the useAccountUser hook in test, thanks @jdamore-linode --- .../APITokens/CreateAPITokenDrawer.test.tsx | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index a0c941e14bb..3e5175a621c 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -9,6 +9,19 @@ import { renderWithTheme } from 'src/utilities/testHelpers'; import { CreateAPITokenDrawer } from './CreateAPITokenDrawer'; +// Mock the useAccountUser hooks to immediately return the expected data, circumventing the HTTP request and loading state. +const queryMocks = vi.hoisted(() => ({ + useAccountUser: vi.fn().mockReturnValue({}), +})); + +vi.mock('src/queries/accountUsers', async () => { + const actual = await vi.importActual('src/queries/accountUsers'); + return { + ...actual, + useAccountUser: queryMocks.useAccountUser, + }; +}); + const props = { onClose: vi.fn(), open: true, @@ -87,11 +100,9 @@ describe('Create API Token Drawer', () => { }); it('Should show the Child Account Access scope for a parent user account with the parent/child feature flag on', async () => { - server.use( - rest.get('*/account/users/*', (req, res, ctx) => { - return res(ctx.json(accountUserFactory.build({ user_type: 'parent' }))); - }) - ); + queryMocks.useAccountUser.mockReturnValue({ + data: accountUserFactory.build({ user_type: 'parent' }), + }); const { findByText } = renderWithTheme( , @@ -104,11 +115,9 @@ describe('Create API Token Drawer', () => { }); it('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', () => { - server.use( - rest.get('*/account/users/*', (req, res, ctx) => { - return res(ctx.json(accountUserFactory.build({ user_type: null }))); - }) - ); + queryMocks.useAccountUser.mockReturnValue({ + data: accountUserFactory.build({ user_type: null }), + }); const { queryByText } = renderWithTheme( , @@ -116,6 +125,7 @@ describe('Create API Token Drawer', () => { flags: { parentChildAccountAccess: true }, } ); + const childScope = queryByText('Child Account Access'); expect(childScope).not.toBeInTheDocument(); }); From c27bd98c6d6c15ff7b209a9f946d6c40e77906fd Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 14:31:32 -0800 Subject: [PATCH 08/13] Update test for View PAT drawer --- .../APITokens/ViewAPITokenDrawer.test.tsx | 73 ++++++++++++++++--- 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx index 22da74bfa19..09588df06c0 100644 --- a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx @@ -1,11 +1,25 @@ import * as React from 'react'; import { appTokenFactory } from 'src/factories'; +import { accountUserFactory } from 'src/factories/accountUsers'; import { renderWithTheme } from 'src/utilities/testHelpers'; import { ViewAPITokenDrawer } from './ViewAPITokenDrawer'; import { basePerms } from './utils'; +// Mock the useAccountUser hooks to immediately return the expected data, circumventing the HTTP request and loading state. +const queryMocks = vi.hoisted(() => ({ + useAccountUser: vi.fn().mockReturnValue({}), +})); + +vi.mock('src/queries/accountUsers', async () => { + const actual = await vi.importActual('src/queries/accountUsers'); + return { + ...actual, + useAccountUser: queryMocks.useAccountUser, + }; +}); + const nonParentPerms = basePerms.filter((value) => value !== 'child_account'); const token = appTokenFactory.build({ label: 'my-token', scopes: '*' }); @@ -27,28 +41,28 @@ describe('View API Token Drawer', () => { expect(getByText(token.label)).toBeVisible(); }); - it('should show all permissions as none with no scopes', () => { + it('should show all permissions as read/write with wildcard scopes', () => { const { getByTestId } = renderWithTheme( - , - { flags: { parentChildAccountAccess: false } } + , + {} ); for (const permissionName of nonParentPerms) { expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute( 'aria-label', - `This token has 0 access for ${permissionName}` + `This token has 2 access for ${permissionName}` ); } }); - it('should show all permissions as read/write with wildcard scopes', () => { + it('should show all permissions as none with no scopes', () => { const { getByTestId } = renderWithTheme( - , - {} + , + { flags: { parentChildAccountAccess: false } } ); for (const permissionName of nonParentPerms) { expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute( 'aria-label', - `This token has 2 access for ${permissionName}` + `This token has 0 access for ${permissionName}` ); } }); @@ -107,7 +121,46 @@ describe('View API Token Drawer', () => { } }); - it('shows the child account perm in the table for a parent user account', () => { - // TODO: test child_account perm; use basePerms + it('should show Child Account Access scope with read/write perms for a parent user account with the parent/child feature flag on', async () => { + queryMocks.useAccountUser.mockReturnValue({ + data: accountUserFactory.build({ user_type: 'parent' }), + }); + + const { findByText, getByTestId } = renderWithTheme( + , + { + flags: { parentChildAccountAccess: true }, + } + ); + + const childScope = await findByText('Child Account Access'); + const expectedScopeLevels = { + child_account: 2, + } as const; + const childPermissionName = 'child_account'; + + expect(childScope).toBeInTheDocument(); + expect(getByTestId(`perm-${childPermissionName}`)).toHaveAttribute( + 'aria-label', + `This token has ${expectedScopeLevels[childPermissionName]} access for ${childPermissionName}` + ); + }); + + it('should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', () => { + queryMocks.useAccountUser.mockReturnValue({ + data: accountUserFactory.build({ user_type: null }), + }); + + const { queryByText } = renderWithTheme(, { + flags: { parentChildAccountAccess: true }, + }); + + const childScope = queryByText('Child Account Access'); + expect(childScope).not.toBeInTheDocument(); }); }); From 5b78f50395300c160c4dbaf96e1a4e763ca82e43 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 14:47:15 -0800 Subject: [PATCH 09/13] Clean up --- packages/manager/src/factories/oauth.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/manager/src/factories/oauth.ts b/packages/manager/src/factories/oauth.ts index b087b2e878b..be6b64dd528 100644 --- a/packages/manager/src/factories/oauth.ts +++ b/packages/manager/src/factories/oauth.ts @@ -13,7 +13,6 @@ export const appTokenFactory = Factory.Sync.makeFactory({ export interface Access { Account: number; - // 'Child Account': number; Databases: number; Domains: number; Events: number; @@ -31,7 +30,6 @@ export interface Access { export const accessFactory = Factory.Sync.makeFactory({ Account: 0, - // 'Child Account': 0, Databases: 0, Domains: 0, Events: 0, From bd78972e63bd066a289050930fdbb2726cb16386 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 14:49:45 -0800 Subject: [PATCH 10/13] Added changeset: Add `child_account` oauth scope to Personal Access Token drawers --- .../.changeset/pr-9992-upcoming-features-1702939785550.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-9992-upcoming-features-1702939785550.md diff --git a/packages/manager/.changeset/pr-9992-upcoming-features-1702939785550.md b/packages/manager/.changeset/pr-9992-upcoming-features-1702939785550.md new file mode 100644 index 00000000000..711c7142e3e --- /dev/null +++ b/packages/manager/.changeset/pr-9992-upcoming-features-1702939785550.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Add `child_account` oauth scope to Personal Access Token drawers ([#9992](https://github.com/linode/manager/pull/9992)) From c92b95bedccedda8c344c98b70a863c6740fd9bd Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Mon, 18 Dec 2023 14:56:14 -0800 Subject: [PATCH 11/13] More test clean up --- .../Profile/APITokens/CreateAPITokenDrawer.test.tsx | 13 +++++-------- .../Profile/APITokens/ViewAPITokenDrawer.test.tsx | 11 ++++------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index 3e5175a621c..eea453fd7b1 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -99,18 +99,15 @@ describe('Create API Token Drawer', () => { getByText('In 6 months'); }); - it('Should show the Child Account Access scope for a parent user account with the parent/child feature flag on', async () => { + it('Should show the Child Account Access scope for a parent user account with the parent/child feature flag on', () => { queryMocks.useAccountUser.mockReturnValue({ data: accountUserFactory.build({ user_type: 'parent' }), }); - const { findByText } = renderWithTheme( - , - { - flags: { parentChildAccountAccess: true }, - } - ); - const childScope = await findByText('Child Account Access'); + const { getByText } = renderWithTheme(, { + flags: { parentChildAccountAccess: true }, + }); + const childScope = getByText('Child Account Access'); expect(childScope).toBeInTheDocument(); }); diff --git a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx index 09588df06c0..577d7d717ee 100644 --- a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx @@ -42,10 +42,7 @@ describe('View API Token Drawer', () => { }); it('should show all permissions as read/write with wildcard scopes', () => { - const { getByTestId } = renderWithTheme( - , - {} - ); + const { getByTestId } = renderWithTheme(); for (const permissionName of nonParentPerms) { expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute( 'aria-label', @@ -121,12 +118,12 @@ describe('View API Token Drawer', () => { } }); - it('should show Child Account Access scope with read/write perms for a parent user account with the parent/child feature flag on', async () => { + it('should show Child Account Access scope with read/write perms for a parent user account with the parent/child feature flag on', () => { queryMocks.useAccountUser.mockReturnValue({ data: accountUserFactory.build({ user_type: 'parent' }), }); - const { findByText, getByTestId } = renderWithTheme( + const { getByTestId, getByText } = renderWithTheme( { } ); - const childScope = await findByText('Child Account Access'); + const childScope = getByText('Child Account Access'); const expectedScopeLevels = { child_account: 2, } as const; From 0b91dc50bbfa3a8186aa43ba30976d3ea4d9ab74 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Tue, 19 Dec 2023 14:07:09 -0800 Subject: [PATCH 12/13] Address feedback: clean up filtering in return statement --- .../APITokens/CreateAPITokenDrawer.tsx | 33 ++++++++++-------- .../Profile/APITokens/ViewAPITokenDrawer.tsx | 34 ++++++++++--------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx index 6394b0ff675..cbacbb7b7d6 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx @@ -160,6 +160,15 @@ export const CreateAPITokenDrawer = (props: Props) => { return { label: expiryTup[0], value: expiryTup[1] }; }); + // Filter permissions for all users except parent user accounts. + const allPermissions = form.values.scopes; + const showFilteredPermissions = + (flags.parentChildAccountAccess && user?.user_type !== 'parent') || + Boolean(!flags.parentChildAccountAccess); + const filteredPermissions = allPermissions.filter( + (scopeTup) => basePermNameMap[scopeTup[0]] !== 'Child Account Access' + ); + return ( {errorMap.none && } @@ -244,18 +253,12 @@ export const CreateAPITokenDrawer = (props: Props) => { /> - {form.values.scopes.map((scopeTup) => { - if (!basePermNameMap[scopeTup[0]]) { - return null; - } - return ( - // When the feature flag is on, display the Child Account Access scope for parent user accounts only. - (!flags.parentChildAccountAccess && - basePermNameMap[scopeTup[0]] === 'Child Account Access') || - (flags.parentChildAccountAccess && - user?.user_type !== 'parent' && - basePermNameMap[scopeTup[0]] === - 'Child Account Access') ? null : ( + {(showFilteredPermissions ? filteredPermissions : allPermissions).map( + (scopeTup) => { + if (!basePermNameMap[scopeTup[0]]) { + return null; + } + return ( { /> - ) - ); - })} + ); + } + )} {errorMap.scopes && ( diff --git a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx index 3d14c776bff..e62b455b24d 100644 --- a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx +++ b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx @@ -32,7 +32,15 @@ export const ViewAPITokenDrawer = (props: Props) => { const { data: profile } = useProfile(); const { data: user } = useAccountUser(profile?.username ?? ''); - const permissions = scopeStringToPermTuples(token?.scopes ?? ''); + const allPermissions = scopeStringToPermTuples(token?.scopes ?? ''); + + // Filter permissions for all users except parent user accounts. + const showFilteredPermissions = + (flags.parentChildAccountAccess && user?.user_type !== 'parent') || + Boolean(!flags.parentChildAccountAccess); + const filteredPermissions = allPermissions.filter( + (scopeTup) => basePermNameMap[scopeTup[0]] !== 'Child Account Access' + ); return ( @@ -56,18 +64,12 @@ export const ViewAPITokenDrawer = (props: Props) => { - {permissions.map((scopeTup) => { - if (!basePermNameMap[scopeTup[0]]) { - return null; - } - return ( - // When the feature flag is on, display the Child Account Access scope for parent user accounts only. - (!flags.parentChildAccountAccess && - basePermNameMap[scopeTup[0]] === 'Child Account Access') || - (flags.parentChildAccountAccess && - user?.user_type !== 'parent' && - basePermNameMap[scopeTup[0]] === - 'Child Account Access') ? null : ( + {(showFilteredPermissions ? filteredPermissions : allPermissions).map( + (scopeTup) => { + if (!basePermNameMap[scopeTup[0]]) { + return null; + } + return ( { /> - ) - ); - })} + ); + } + )} From 09afb5a5a090c4dacfdf163a46ac14ab5536289a Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Tue, 19 Dec 2023 15:25:35 -0800 Subject: [PATCH 13/13] Default access to None rather than ReadWrite when creating a PAT --- .../.changeset/pr-9992-changed-1703028128822.md | 5 +++++ .../APITokens/CreateAPITokenDrawer.test.tsx | 14 +------------- .../Profile/APITokens/CreateAPITokenDrawer.tsx | 2 +- 3 files changed, 7 insertions(+), 14 deletions(-) create mode 100644 packages/manager/.changeset/pr-9992-changed-1703028128822.md diff --git a/packages/manager/.changeset/pr-9992-changed-1703028128822.md b/packages/manager/.changeset/pr-9992-changed-1703028128822.md new file mode 100644 index 00000000000..bc20dc6ade7 --- /dev/null +++ b/packages/manager/.changeset/pr-9992-changed-1703028128822.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Changed +--- + +Default access to `None` for all scopes when creating Personal Access Tokens ([#9992](https://github.com/linode/manager/pull/9992)) diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx index eea453fd7b1..90fdb8b8ced 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx @@ -74,22 +74,10 @@ describe('Create API Token Drawer', () => { ); }); - // TODO: Parent/Child - remove this test when Parent/Child feature is released. - it('Should default to read/write for all scopes', () => { + it('Should default to None for all scopes', () => { const { getByLabelText } = renderWithTheme( ); - const selectAllReadWritePermRadioButton = getByLabelText( - 'Select read/write for all' - ); - expect(selectAllReadWritePermRadioButton).toBeChecked(); - }); - - it('Should default to None for all scopes with the parent/child feature flag on', () => { - const { getByLabelText } = renderWithTheme( - , - { flags: { parentChildAccountAccess: true } } - ); const selectAllNonePermRadioButton = getByLabelText('Select none for all'); expect(selectAllNonePermRadioButton).toBeChecked(); }); diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx index cbacbb7b7d6..f4f6d0781c3 100644 --- a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx +++ b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx @@ -90,7 +90,7 @@ export const CreateAPITokenDrawer = (props: Props) => { const initialValues = { expiry: expiryTups[0][1], label: '', - scopes: scopeStringToPermTuples(flags.parentChildAccountAccess ? '' : '*'), + scopes: scopeStringToPermTuples(''), }; const { data: profile } = useProfile();