From ea6a62e27b7413bf0a8022a759a85d9da68e5c64 Mon Sep 17 00:00:00 2001 From: Jaalah Ramos Date: Mon, 25 Mar 2024 16:24:13 -0400 Subject: [PATCH 1/3] feat: [M3-7888] - Revoke proxy PAT when switching accounts --- .../src/features/Account/AccountLanding.tsx | 10 +++ .../features/Account/SwitchAccountDrawer.tsx | 57 +++++++++++- .../manager/src/features/Account/utils.ts | 16 ++++ .../features/TopMenu/UserMenu/UserMenu.tsx | 11 ++- .../hooks/usePendingRevocationToken.test.ts | 90 +++++++++++++++++++ .../src/hooks/usePendingRevocationToken.ts | 45 ++++++++++ 6 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 packages/manager/src/hooks/usePendingRevocationToken.test.ts create mode 100644 packages/manager/src/hooks/usePendingRevocationToken.ts diff --git a/packages/manager/src/features/Account/AccountLanding.tsx b/packages/manager/src/features/Account/AccountLanding.tsx index aa5667f59fb..57a8c957b1a 100644 --- a/packages/manager/src/features/Account/AccountLanding.tsx +++ b/packages/manager/src/features/Account/AccountLanding.tsx @@ -15,6 +15,7 @@ import { switchAccountSessionContext } from 'src/context/switchAccountSessionCon import { useParentTokenManagement } from 'src/features/Account/SwitchAccounts/useParentTokenManagement'; import { getRestrictedResourceText } from 'src/features/Account/utils'; import { useFlags } from 'src/hooks/useFlags'; +import { usePendingRevocationToken } from 'src/hooks/usePendingRevocationToken'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; import { useAccount } from 'src/queries/account'; import { useProfile } from 'src/queries/profile'; @@ -55,6 +56,10 @@ const AccountLanding = () => { const flags = useFlags(); const [isDrawerOpen, setIsDrawerOpen] = React.useState(false); const sessionContext = React.useContext(switchAccountSessionContext); + const { + getPendingRevocationToken, + pendingRevocationTokenId, + } = usePendingRevocationToken(); const isAkamaiAccount = account?.billing_source === 'akamai'; const isProxyUser = profile?.user_type === 'proxy'; @@ -113,6 +118,10 @@ const AccountLanding = () => { }); } + if (isProxyUser) { + getPendingRevocationToken(); + } + setIsDrawerOpen(true); }; @@ -215,6 +224,7 @@ const AccountLanding = () => { isProxyUser={isProxyUser} onClose={() => setIsDrawerOpen(false)} open={isDrawerOpen} + proxyTokenId={pendingRevocationTokenId} /> ); diff --git a/packages/manager/src/features/Account/SwitchAccountDrawer.tsx b/packages/manager/src/features/Account/SwitchAccountDrawer.tsx index c5171008ab3..8aff8b72e7d 100644 --- a/packages/manager/src/features/Account/SwitchAccountDrawer.tsx +++ b/packages/manager/src/features/Account/SwitchAccountDrawer.tsx @@ -1,4 +1,5 @@ import { createChildAccountPersonalAccessToken } from '@linode/api-v4'; +import { useSnackbar } from 'notistack'; import React from 'react'; import { StyledLinkButton } from 'src/components/Button/StyledLinkButton'; @@ -12,6 +13,7 @@ import { updateCurrentTokenBasedOnUserType, } from 'src/features/Account/utils'; import { useCurrentToken } from 'src/hooks/useAuthentication'; +import { useRevokePersonalAccessTokenMutation } from 'src/queries/tokens'; import { sendSwitchToParentAccountEvent } from 'src/utilities/analytics'; import { getStorage, setStorage } from 'src/utilities/storage'; @@ -25,10 +27,11 @@ interface Props { isProxyUser: boolean; onClose: () => void; open: boolean; + proxyTokenId?: number; } export const SwitchAccountDrawer = (props: Props) => { - const { isProxyUser, onClose, open } = props; + const { isProxyUser, onClose, open, proxyTokenId } = props; const [isParentTokenError, setIsParentTokenError] = React.useState< APIError[] @@ -37,7 +40,12 @@ export const SwitchAccountDrawer = (props: Props) => { [] ); + const { mutateAsync: revokeToken } = useRevokePersonalAccessTokenMutation( + proxyTokenId ?? -1 + ); + const { enqueueSnackbar } = useSnackbar(); const currentTokenWithBearer = useCurrentToken() ?? ''; + const currentParentTokenWithBearer = getStorage('authentication/parent_token/token') ?? ''; @@ -45,6 +53,44 @@ export const SwitchAccountDrawer = (props: Props) => { onClose(); }, [onClose]); + const handleProxyTokenRevocation = React.useCallback(async () => { + try { + await revokeToken(); + enqueueSnackbar(`Successfully revoked ${proxyTokenId}.`, { + variant: 'success', + }); + } catch (error) { + enqueueSnackbar('Failed to revoke token.', { + variant: 'error', + }); + } + }, [enqueueSnackbar, proxyTokenId, revokeToken]); + + const handleSwitchAccount = async ({ + currentTokenWithBearer, + euuid, + event, + handleClose, + isProxyUser, + }: { + currentTokenWithBearer?: AuthState['token']; + euuid: string; + event: React.MouseEvent; + handleClose: (e: React.SyntheticEvent) => void; + isProxyUser: boolean; + }) => { + if (isProxyUser) { + await handleProxyTokenRevocation(); + } + handleSwitchToChildAccount({ + currentTokenWithBearer, + euuid, + event, + handleClose, + isProxyUser, + }); + }; + /** * Headers are required for proxy users when obtaining a proxy token. * For 'proxy' userType, use the stored parent token in the request. @@ -142,7 +188,7 @@ export const SwitchAccountDrawer = (props: Props) => { [getProxyToken, refreshPage] ); - const handleSwitchToParentAccount = React.useCallback(() => { + const handleSwitchToParentAccount = React.useCallback(async () => { if (!isParentTokenValid()) { const expiredTokenError: APIError = { field: 'token', @@ -154,6 +200,9 @@ export const SwitchAccountDrawer = (props: Props) => { return; } + // Revoke proxy token before switching to parent account. + await handleProxyTokenRevocation(); + updateCurrentTokenBasedOnUserType({ userType: 'parent' }); // Reset flag for proxy user to display success toast once. @@ -161,7 +210,7 @@ export const SwitchAccountDrawer = (props: Props) => { handleClose(); refreshPage(); - }, [handleClose, refreshPage]); + }, [handleClose, handleProxyTokenRevocation, refreshPage]); return ( @@ -199,7 +248,7 @@ export const SwitchAccountDrawer = (props: Props) => { } isProxyUser={isProxyUser} onClose={handleClose} - onSwitchAccount={handleSwitchToChildAccount} + onSwitchAccount={handleSwitchAccount} /> ); diff --git a/packages/manager/src/features/Account/utils.ts b/packages/manager/src/features/Account/utils.ts index 3d70d71b204..29ff0cbe72a 100644 --- a/packages/manager/src/features/Account/utils.ts +++ b/packages/manager/src/features/Account/utils.ts @@ -135,3 +135,19 @@ export const updateCurrentTokenBasedOnUserType = ({ setStorage('authentication/expire', userExpiry); } }; + +/** + * Finds a personal access token stored locally for revocation, + * typically used when switching between accounts. Searching local storage + * for the token is necessary because the token is not persisted in state. + */ +export async function getPersonalAccessTokenForRevocation( + tokens: Token[], + currentTokenWithBearer: string +): Promise { + return tokens.find( + (token) => + token.token && + currentTokenWithBearer.replace('Bearer ', '').startsWith(token.token) + ); +} diff --git a/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx b/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx index ff626948db6..f0c50c0ecd5 100644 --- a/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx +++ b/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx @@ -22,6 +22,7 @@ import { SwitchAccountButton } from 'src/features/Account/SwitchAccountButton'; import { SwitchAccountDrawer } from 'src/features/Account/SwitchAccountDrawer'; import { useParentTokenManagement } from 'src/features/Account/SwitchAccounts/useParentTokenManagement'; import { useFlags } from 'src/hooks/useFlags'; +import { usePendingRevocationToken } from 'src/hooks/usePendingRevocationToken'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; import { useAccount } from 'src/queries/account'; import { useGrants, useProfile } from 'src/queries/profile'; @@ -29,7 +30,6 @@ import { sendSwitchAccountEvent } from 'src/utilities/analytics'; import { getStorage, setStorage } from 'src/utilities/storage'; import { getCompanyNameOrEmail } from './utils'; - interface MenuLink { display: string; hide?: boolean; @@ -59,6 +59,10 @@ export const UserMenu = React.memo(() => { null ); const [isDrawerOpen, setIsDrawerOpen] = React.useState(false); + const { + getPendingRevocationToken, + pendingRevocationTokenId, + } = usePendingRevocationToken(); const { data: account } = useAccount(); const { data: profile } = useProfile(); @@ -201,6 +205,10 @@ export const UserMenu = React.memo(() => { }); } + if (isProxyUser) { + getPendingRevocationToken(); + } + setIsDrawerOpen(true); }; @@ -340,6 +348,7 @@ export const UserMenu = React.memo(() => { isProxyUser={isProxyUser} onClose={() => setIsDrawerOpen(false)} open={isDrawerOpen} + proxyTokenId={pendingRevocationTokenId} /> ); diff --git a/packages/manager/src/hooks/usePendingRevocationToken.test.ts b/packages/manager/src/hooks/usePendingRevocationToken.test.ts new file mode 100644 index 00000000000..195cf56a5ba --- /dev/null +++ b/packages/manager/src/hooks/usePendingRevocationToken.test.ts @@ -0,0 +1,90 @@ +import { act, renderHook, waitFor } from '@testing-library/react'; + +import { queryClientFactory } from 'src/queries/base'; +import { wrapWithTheme } from 'src/utilities/testHelpers'; + +import { usePendingRevocationToken } from './usePendingRevocationToken'; // Adjust path as needed +import { Token } from '@linode/api-v4'; + +const queryClient = queryClientFactory(); + +const queryMocks = vi.hoisted(() => ({ + getPersonalAccessTokenForRevocation: vi.fn( + (tokens, currentTokenWithBearer) => { + const tokenValue = currentTokenWithBearer.replace('Bearer ', ''); + const foundToken = tokens.find( + (token: Token) => token.token === tokenValue + ); + return Promise.resolve(foundToken); + } + ), + useCurrentToken: vi.fn(() => 'Bearer 232345245345'), + usePersonalAccessTokensQuery: vi.fn().mockReturnValue({ + data: { + data: [{ id: 123, token: '232345245345' }], + }, + }), +})); + +vi.mock('src/queries/tokens', async () => { + const actual = await vi.importActual('src/queries/tokens'); + return { + ...actual, + usePersonalAccessTokensQuery: queryMocks.usePersonalAccessTokensQuery, + }; +}); + +vi.mock('src/features/Account/utils', async () => { + const actual = await vi.importActual('src/features/Account/utils'); + return { + ...actual, + getPersonalAccessTokenForRevocation: + queryMocks.getPersonalAccessTokenForRevocation, + }; +}); + +vi.mock('src/hooks/useAuthentication', async () => { + const actual = await vi.importActual('src/hooks/useAuthentication'); + return { + ...actual, + useCurrentToken: queryMocks.useCurrentToken, + }; +}); + +describe('usePendingRevocationToken', () => { + it('should set pendingRevocationTokenId when personal access tokens are available', async () => { + const { result } = renderHook(() => usePendingRevocationToken(), { + wrapper: (ui) => wrapWithTheme(ui, { queryClient }), + }); + + await waitFor(() => { + expect(result.current.pendingRevocationTokenId).toBeUndefined(); + }); + + await act(async () => { + await result.current.getPendingRevocationToken(); + }); + + await waitFor(() => { + expect(result.current.pendingRevocationTokenId).toEqual(123); + }); + }); + + it('should not set pendingRevocationTokenId when no matching tokens are available', async () => { + // Adjust the mock to return a token that doesn't match + queryMocks.useCurrentToken.mockReturnValue('Bearer nonMatchingToken'); + + const { result } = renderHook(() => usePendingRevocationToken(), { + wrapper: (ui) => wrapWithTheme(ui, { queryClient }), + }); + + await act(async () => { + await result.current.getPendingRevocationToken(); + }); + + // Now expecting undefined because there should be no match + await waitFor(() => { + expect(result.current.pendingRevocationTokenId).toBeUndefined(); + }); + }); +}); diff --git a/packages/manager/src/hooks/usePendingRevocationToken.ts b/packages/manager/src/hooks/usePendingRevocationToken.ts new file mode 100644 index 00000000000..bf694e61cbc --- /dev/null +++ b/packages/manager/src/hooks/usePendingRevocationToken.ts @@ -0,0 +1,45 @@ +import React from 'react'; + +import { getPersonalAccessTokenForRevocation } from 'src/features/Account/utils'; +import { useCurrentToken } from 'src/hooks/useAuthentication'; +import { usePersonalAccessTokensQuery } from 'src/queries/tokens'; + +/** + * Custom hook to manage the ID of a personal access token pending revocation. + * + * This hook provides functionality to determine which personal access token + * should be considered for revocation based on the current authentication state + * and the list of personal access tokens associated with the account. It utilizes + * the current bearer token to identify the corresponding personal access token + * and sets its ID for potential revocation actions. + * + * @returns {object} An object containing: + * - `getPendingRevocationToken`: A function to fetch and set the pending revocation token ID based on current conditions. + * - `pendingRevocationTokenId`: The ID of the token currently marked for pending revocation, or `undefined` if none. + */ +export const usePendingRevocationToken = () => { + const [ + pendingRevocationTokenId, + setPendingRevocationTokenId, + ] = React.useState(undefined); + const currentTokenWithBearer = useCurrentToken() ?? ''; + const { data: personalAccessTokens } = usePersonalAccessTokensQuery(); + + const getPendingRevocationToken = async () => { + if (!personalAccessTokens?.data) { + return; + } + + const token = await getPersonalAccessTokenForRevocation( + personalAccessTokens?.data, + currentTokenWithBearer + ); + + setPendingRevocationTokenId(token?.id); + }; + + return { + getPendingRevocationToken, + pendingRevocationTokenId, + }; +}; From 67df038cde9c4090da15a9c26006ab69b698c567 Mon Sep 17 00:00:00 2001 From: Jaalah Ramos Date: Mon, 25 Mar 2024 16:35:16 -0400 Subject: [PATCH 2/3] Added changeset: Revoke proxy PAT when switching accounts --- .../.changeset/pr-10313-upcoming-features-1711398916591.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-10313-upcoming-features-1711398916591.md diff --git a/packages/manager/.changeset/pr-10313-upcoming-features-1711398916591.md b/packages/manager/.changeset/pr-10313-upcoming-features-1711398916591.md new file mode 100644 index 00000000000..731d90565e7 --- /dev/null +++ b/packages/manager/.changeset/pr-10313-upcoming-features-1711398916591.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Revoke proxy PAT when switching accounts ([#10313](https://github.com/linode/manager/pull/10313)) From 8da4f27fcc9618c83c9df91bec6b239db206dae3 Mon Sep 17 00:00:00 2001 From: Jaalah Ramos Date: Wed, 27 Mar 2024 10:46:43 -0400 Subject: [PATCH 3/3] Update hook to return full token --- .../manager/src/features/Account/AccountLanding.tsx | 4 ++-- .../src/features/Account/SwitchAccountDrawer.tsx | 13 +++++++------ .../src/features/TopMenu/UserMenu/UserMenu.tsx | 4 ++-- .../src/hooks/usePendingRevocationToken.test.ts | 10 +++++----- .../manager/src/hooks/usePendingRevocationToken.ts | 12 ++++++------ 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/packages/manager/src/features/Account/AccountLanding.tsx b/packages/manager/src/features/Account/AccountLanding.tsx index 57a8c957b1a..f87cce9eaed 100644 --- a/packages/manager/src/features/Account/AccountLanding.tsx +++ b/packages/manager/src/features/Account/AccountLanding.tsx @@ -58,7 +58,7 @@ const AccountLanding = () => { const sessionContext = React.useContext(switchAccountSessionContext); const { getPendingRevocationToken, - pendingRevocationTokenId, + pendingRevocationToken, } = usePendingRevocationToken(); const isAkamaiAccount = account?.billing_source === 'akamai'; @@ -224,7 +224,7 @@ const AccountLanding = () => { isProxyUser={isProxyUser} onClose={() => setIsDrawerOpen(false)} open={isDrawerOpen} - proxyTokenId={pendingRevocationTokenId} + proxyToken={pendingRevocationToken} /> ); diff --git a/packages/manager/src/features/Account/SwitchAccountDrawer.tsx b/packages/manager/src/features/Account/SwitchAccountDrawer.tsx index 8aff8b72e7d..5114cdb8f95 100644 --- a/packages/manager/src/features/Account/SwitchAccountDrawer.tsx +++ b/packages/manager/src/features/Account/SwitchAccountDrawer.tsx @@ -27,11 +27,12 @@ interface Props { isProxyUser: boolean; onClose: () => void; open: boolean; - proxyTokenId?: number; + proxyToken?: Token; } export const SwitchAccountDrawer = (props: Props) => { - const { isProxyUser, onClose, open, proxyTokenId } = props; + const { isProxyUser, onClose, open, proxyToken } = props; + const proxyTokenLabel = proxyToken?.label; const [isParentTokenError, setIsParentTokenError] = React.useState< APIError[] @@ -41,7 +42,7 @@ export const SwitchAccountDrawer = (props: Props) => { ); const { mutateAsync: revokeToken } = useRevokePersonalAccessTokenMutation( - proxyTokenId ?? -1 + proxyToken?.id ?? -1 ); const { enqueueSnackbar } = useSnackbar(); const currentTokenWithBearer = useCurrentToken() ?? ''; @@ -56,15 +57,15 @@ export const SwitchAccountDrawer = (props: Props) => { const handleProxyTokenRevocation = React.useCallback(async () => { try { await revokeToken(); - enqueueSnackbar(`Successfully revoked ${proxyTokenId}.`, { + enqueueSnackbar(`Successfully revoked ${proxyTokenLabel}.`, { variant: 'success', }); } catch (error) { - enqueueSnackbar('Failed to revoke token.', { + enqueueSnackbar(`Failed to revoke ${proxyTokenLabel}.`, { variant: 'error', }); } - }, [enqueueSnackbar, proxyTokenId, revokeToken]); + }, [enqueueSnackbar, proxyTokenLabel, revokeToken]); const handleSwitchAccount = async ({ currentTokenWithBearer, diff --git a/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx b/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx index f0c50c0ecd5..3e4b023f376 100644 --- a/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx +++ b/packages/manager/src/features/TopMenu/UserMenu/UserMenu.tsx @@ -61,7 +61,7 @@ export const UserMenu = React.memo(() => { const [isDrawerOpen, setIsDrawerOpen] = React.useState(false); const { getPendingRevocationToken, - pendingRevocationTokenId, + pendingRevocationToken, } = usePendingRevocationToken(); const { data: account } = useAccount(); @@ -348,7 +348,7 @@ export const UserMenu = React.memo(() => { isProxyUser={isProxyUser} onClose={() => setIsDrawerOpen(false)} open={isDrawerOpen} - proxyTokenId={pendingRevocationTokenId} + proxyToken={pendingRevocationToken} /> ); diff --git a/packages/manager/src/hooks/usePendingRevocationToken.test.ts b/packages/manager/src/hooks/usePendingRevocationToken.test.ts index 195cf56a5ba..2049effb82b 100644 --- a/packages/manager/src/hooks/usePendingRevocationToken.test.ts +++ b/packages/manager/src/hooks/usePendingRevocationToken.test.ts @@ -52,13 +52,13 @@ vi.mock('src/hooks/useAuthentication', async () => { }); describe('usePendingRevocationToken', () => { - it('should set pendingRevocationTokenId when personal access tokens are available', async () => { + it('should set pendingRevocationToken id when personal access tokens are available', async () => { const { result } = renderHook(() => usePendingRevocationToken(), { wrapper: (ui) => wrapWithTheme(ui, { queryClient }), }); await waitFor(() => { - expect(result.current.pendingRevocationTokenId).toBeUndefined(); + expect(result.current.pendingRevocationToken?.id).toBeUndefined(); }); await act(async () => { @@ -66,11 +66,11 @@ describe('usePendingRevocationToken', () => { }); await waitFor(() => { - expect(result.current.pendingRevocationTokenId).toEqual(123); + expect(result.current.pendingRevocationToken?.id).toEqual(123); }); }); - it('should not set pendingRevocationTokenId when no matching tokens are available', async () => { + it('should not set pendingRevocationToken?.id when no matching tokens are available', async () => { // Adjust the mock to return a token that doesn't match queryMocks.useCurrentToken.mockReturnValue('Bearer nonMatchingToken'); @@ -84,7 +84,7 @@ describe('usePendingRevocationToken', () => { // Now expecting undefined because there should be no match await waitFor(() => { - expect(result.current.pendingRevocationTokenId).toBeUndefined(); + expect(result.current.pendingRevocationToken?.id).toBeUndefined(); }); }); }); diff --git a/packages/manager/src/hooks/usePendingRevocationToken.ts b/packages/manager/src/hooks/usePendingRevocationToken.ts index bf694e61cbc..ba41f077681 100644 --- a/packages/manager/src/hooks/usePendingRevocationToken.ts +++ b/packages/manager/src/hooks/usePendingRevocationToken.ts @@ -1,3 +1,4 @@ +import { Token } from '@linode/api-v4'; import React from 'react'; import { getPersonalAccessTokenForRevocation } from 'src/features/Account/utils'; @@ -18,10 +19,9 @@ import { usePersonalAccessTokensQuery } from 'src/queries/tokens'; * - `pendingRevocationTokenId`: The ID of the token currently marked for pending revocation, or `undefined` if none. */ export const usePendingRevocationToken = () => { - const [ - pendingRevocationTokenId, - setPendingRevocationTokenId, - ] = React.useState(undefined); + const [pendingRevocationToken, setPendingRevocationToken] = React.useState< + Token | undefined + >(undefined); const currentTokenWithBearer = useCurrentToken() ?? ''; const { data: personalAccessTokens } = usePersonalAccessTokensQuery(); @@ -35,11 +35,11 @@ export const usePendingRevocationToken = () => { currentTokenWithBearer ); - setPendingRevocationTokenId(token?.id); + setPendingRevocationToken(token); }; return { getPendingRevocationToken, - pendingRevocationTokenId, + pendingRevocationToken, }; };