From 054c590813cb0ee008130d33d8bec7d3e6d1ee14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Tue, 31 Jan 2023 08:40:23 +0000 Subject: [PATCH] fix: limit creation of other users PATs (adapting) (#3019) https://linear.app/unleash/issue/2-656/limit-the-ability-of-creating-a-token-on-behalf-of-another-user Adapts to the refactor that reverts the initial experimental idea of Service Accounts before they existed in the current implementation: Managing other user's PATs. --- .../ServiceAccountModal.tsx | 6 +- .../ServiceAccountCreateTokenDialog.tsx | 4 +- .../ServiceAccountTokens.tsx | 22 ++++---- .../providers/AccessProvider/permissions.ts | 3 - .../useServiceAccountTokensApi.ts | 56 +++++++++++++++++++ .../useServiceAccountTokens.ts | 36 ++++++++++++ src/lib/types/permissions.ts | 3 - ...30130113337-revert-user-pat-permissions.js | 21 +++++++ 8 files changed, 130 insertions(+), 21 deletions(-) create mode 100644 frontend/src/hooks/api/actions/useServiceAccountTokensApi/useServiceAccountTokensApi.ts create mode 100644 frontend/src/hooks/api/getters/useServiceAccountTokens/useServiceAccountTokens.ts create mode 100644 src/migrations/20230130113337-revert-user-pat-permissions.js diff --git a/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountModal.tsx b/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountModal.tsx index 3806d07c46..79c225d727 100644 --- a/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountModal.tsx +++ b/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountModal.tsx @@ -29,7 +29,7 @@ import { IPersonalAPITokenFormErrors, PersonalAPITokenForm, } from 'component/user/Profile/PersonalAPITokensTab/CreatePersonalAPIToken/PersonalAPITokenForm/PersonalAPITokenForm'; -import { usePersonalAPITokensApi } from 'hooks/api/actions/usePersonalAPITokensApi/usePersonalAPITokensApi'; +import { useServiceAccountTokensApi } from 'hooks/api/actions/useServiceAccountTokensApi/useServiceAccountTokensApi'; import { INewPersonalAPIToken } from 'interfaces/personalAPIToken'; import { ServiceAccountTokens } from './ServiceAccountTokens/ServiceAccountTokens'; import { IServiceAccount } from 'interfaces/service-account'; @@ -127,7 +127,7 @@ export const ServiceAccountModal = ({ const { serviceAccounts, roles, refetch } = useServiceAccounts(); const { addServiceAccount, updateServiceAccount, loading } = useServiceAccountsApi(); - const { createUserPersonalAPIToken } = usePersonalAPITokensApi(); + const { createServiceAccountToken } = useServiceAccountTokensApi(); const { setToastData, setToastApiError } = useToast(); const { uiConfig } = useUiConfig(); @@ -190,7 +190,7 @@ export const ServiceAccountModal = ({ getServiceAccountPayload() ); if (tokenGeneration === TokenGeneration.NOW) { - const token = await createUserPersonalAPIToken(id, { + const token = await createServiceAccountToken(id, { description: patDescription, expiresAt: patExpiresAt, }); diff --git a/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountCreateTokenDialog/ServiceAccountCreateTokenDialog.tsx b/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountCreateTokenDialog/ServiceAccountCreateTokenDialog.tsx index be7da49971..8caac142ff 100644 --- a/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountCreateTokenDialog/ServiceAccountCreateTokenDialog.tsx +++ b/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountCreateTokenDialog/ServiceAccountCreateTokenDialog.tsx @@ -6,7 +6,7 @@ import { IPersonalAPITokenFormErrors, PersonalAPITokenForm, } from 'component/user/Profile/PersonalAPITokensTab/CreatePersonalAPIToken/PersonalAPITokenForm/PersonalAPITokenForm'; -import { ICreatePersonalApiTokenPayload } from 'hooks/api/actions/usePersonalAPITokensApi/usePersonalAPITokensApi'; +import { ICreateServiceAccountTokenPayload } from 'hooks/api/actions/useServiceAccountTokensApi/useServiceAccountTokensApi'; import { IPersonalAPIToken } from 'interfaces/personalAPIToken'; const DEFAULT_EXPIRATION = ExpirationOption['30DAYS']; @@ -15,7 +15,7 @@ interface IServiceAccountCreateTokenDialogProps { open: boolean; setOpen: React.Dispatch>; tokens: IPersonalAPIToken[]; - onCreateClick: (newToken: ICreatePersonalApiTokenPayload) => void; + onCreateClick: (newToken: ICreateServiceAccountTokenPayload) => void; } export const ServiceAccountCreateTokenDialog = ({ diff --git a/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountTokens.tsx b/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountTokens.tsx index 112ecbf14a..9799c59199 100644 --- a/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountTokens.tsx +++ b/frontend/src/component/admin/serviceAccounts/ServiceAccountsTable/ServiceAccountModal/ServiceAccountTokens/ServiceAccountTokens.tsx @@ -16,7 +16,7 @@ import { HighlightCell } from 'component/common/Table/cells/HighlightCell/Highli import { TextCell } from 'component/common/Table/cells/TextCell/TextCell'; import { SearchHighlightProvider } from 'component/common/Table/SearchHighlightContext/SearchHighlightContext'; import { PAT_LIMIT } from '@server/util/constants'; -import { usePersonalAPITokens } from 'hooks/api/getters/usePersonalAPITokens/usePersonalAPITokens'; +import { useServiceAccountTokens } from 'hooks/api/getters/useServiceAccountTokens/useServiceAccountTokens'; import { useSearch } from 'hooks/useSearch'; import { INewPersonalAPIToken, @@ -32,9 +32,9 @@ import { useConditionallyHiddenColumns } from 'hooks/useConditionallyHiddenColum import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { Dialogue } from 'component/common/Dialogue/Dialogue'; import { - ICreatePersonalApiTokenPayload, - usePersonalAPITokensApi, -} from 'hooks/api/actions/usePersonalAPITokensApi/usePersonalAPITokensApi'; + ICreateServiceAccountTokenPayload, + useServiceAccountTokensApi, +} from 'hooks/api/actions/useServiceAccountTokensApi/useServiceAccountTokensApi'; import useToast from 'hooks/useToast'; import { formatUnknownError } from 'utils/formatUnknownError'; import { IServiceAccount } from 'interfaces/service-account'; @@ -90,12 +90,12 @@ export const ServiceAccountTokens = ({ const { setToastData, setToastApiError } = useToast(); const isSmallScreen = useMediaQuery(theme.breakpoints.down('md')); const isExtraSmallScreen = useMediaQuery(theme.breakpoints.down('sm')); - const { tokens = [], refetchTokens } = usePersonalAPITokens( + const { tokens = [], refetchTokens } = useServiceAccountTokens( serviceAccount.id ); const { refetch } = useServiceAccounts(); - const { createUserPersonalAPIToken, deleteUserPersonalAPIToken } = - usePersonalAPITokensApi(); + const { createServiceAccountToken, deleteServiceAccountToken } = + useServiceAccountTokensApi(); const [initialState] = useState(() => ({ sortBy: readOnly ? [{ id: 'seenAt' }] : [defaultSort], @@ -108,9 +108,11 @@ export const ServiceAccountTokens = ({ const [newToken, setNewToken] = useState(); const [selectedToken, setSelectedToken] = useState(); - const onCreateClick = async (newToken: ICreatePersonalApiTokenPayload) => { + const onCreateClick = async ( + newToken: ICreateServiceAccountTokenPayload + ) => { try { - const token = await createUserPersonalAPIToken( + const token = await createServiceAccountToken( serviceAccount.id, newToken ); @@ -131,7 +133,7 @@ export const ServiceAccountTokens = ({ const onDeleteClick = async () => { if (selectedToken) { try { - await deleteUserPersonalAPIToken( + await deleteServiceAccountToken( serviceAccount.id, selectedToken?.id ); diff --git a/frontend/src/component/providers/AccessProvider/permissions.ts b/frontend/src/component/providers/AccessProvider/permissions.ts index 1382b050ab..1e7ee2c0c4 100644 --- a/frontend/src/component/providers/AccessProvider/permissions.ts +++ b/frontend/src/component/providers/AccessProvider/permissions.ts @@ -36,6 +36,3 @@ export const DELETE_SEGMENT = 'DELETE_SEGMENT'; export const APPLY_CHANGE_REQUEST = 'APPLY_CHANGE_REQUEST'; export const APPROVE_CHANGE_REQUEST = 'APPROVE_CHANGE_REQUEST'; export const SKIP_CHANGE_REQUEST = 'SKIP_CHANGE_REQUEST'; -export const READ_USER_PAT = 'READ_USER_PAT'; -export const CREATE_USER_PAT = 'CREATE_USER_PAT'; -export const DELETE_USER_PAT = 'DELETE_USER_PAT'; diff --git a/frontend/src/hooks/api/actions/useServiceAccountTokensApi/useServiceAccountTokensApi.ts b/frontend/src/hooks/api/actions/useServiceAccountTokensApi/useServiceAccountTokensApi.ts new file mode 100644 index 0000000000..f10956a5b8 --- /dev/null +++ b/frontend/src/hooks/api/actions/useServiceAccountTokensApi/useServiceAccountTokensApi.ts @@ -0,0 +1,56 @@ +import { INewPersonalAPIToken } from 'interfaces/personalAPIToken'; +import useAPI from '../useApi/useApi'; + +export interface ICreateServiceAccountTokenPayload { + description: string; + expiresAt: Date; +} + +export const useServiceAccountTokensApi = () => { + const { makeRequest, createRequest, errors, loading } = useAPI({ + propagateErrors: true, + }); + + const createServiceAccountToken = async ( + serviceAccountId: number, + payload: ICreateServiceAccountTokenPayload + ): Promise => { + const req = createRequest( + `api/admin/service-account/${serviceAccountId}/token`, + { + method: 'POST', + body: JSON.stringify(payload), + } + ); + try { + const response = await makeRequest(req.caller, req.id); + return await response.json(); + } catch (e) { + throw e; + } + }; + + const deleteServiceAccountToken = async ( + serviceAccountId: number, + id: string + ) => { + const req = createRequest( + `api/admin/service-account/${serviceAccountId}/token/${id}`, + { + method: 'DELETE', + } + ); + try { + await makeRequest(req.caller, req.id); + } catch (e) { + throw e; + } + }; + + return { + createServiceAccountToken, + deleteServiceAccountToken, + errors, + loading, + }; +}; diff --git a/frontend/src/hooks/api/getters/useServiceAccountTokens/useServiceAccountTokens.ts b/frontend/src/hooks/api/getters/useServiceAccountTokens/useServiceAccountTokens.ts new file mode 100644 index 0000000000..e70a8729fd --- /dev/null +++ b/frontend/src/hooks/api/getters/useServiceAccountTokens/useServiceAccountTokens.ts @@ -0,0 +1,36 @@ +import { IPersonalAPIToken } from 'interfaces/personalAPIToken'; +import { formatApiPath } from 'utils/formatPath'; +import handleErrorResponses from '../httpErrorResponseHandler'; +import { useConditionalSWR } from '../useConditionalSWR/useConditionalSWR'; +import useUiConfig from '../useUiConfig/useUiConfig'; + +export interface IUseServiceAccountTokensOutput { + tokens?: IPersonalAPIToken[]; + refetchTokens: () => void; + loading: boolean; + error?: Error; +} + +export const useServiceAccountTokens = (id: number) => { + const { uiConfig, isEnterprise } = useUiConfig(); + + const { data, error, mutate } = useConditionalSWR( + Boolean(uiConfig.flags.serviceAccounts) && isEnterprise(), + { tokens: [] }, + formatApiPath(`api/admin/service-account/${id}/token`), + fetcher + ); + + return { + tokens: data ? data.pats : undefined, + loading: !error && !data, + refetchTokens: () => mutate(), + error, + }; +}; + +const fetcher = (path: string) => { + return fetch(path) + .then(handleErrorResponses('Service Account Tokens')) + .then(res => res.json()); +}; diff --git a/src/lib/types/permissions.ts b/src/lib/types/permissions.ts index 7c37d41134..00b3a98b6c 100644 --- a/src/lib/types/permissions.ts +++ b/src/lib/types/permissions.ts @@ -42,6 +42,3 @@ export const DELETE_SEGMENT = 'DELETE_SEGMENT'; export const APPROVE_CHANGE_REQUEST = 'APPROVE_CHANGE_REQUEST'; export const APPLY_CHANGE_REQUEST = 'APPLY_CHANGE_REQUEST'; export const SKIP_CHANGE_REQUEST = 'SKIP_CHANGE_REQUEST'; -export const READ_USER_PAT = 'READ_USER_PAT'; -export const CREATE_USER_PAT = 'CREATE_USER_PAT'; -export const DELETE_USER_PAT = 'DELETE_USER_PAT'; diff --git a/src/migrations/20230130113337-revert-user-pat-permissions.js b/src/migrations/20230130113337-revert-user-pat-permissions.js new file mode 100644 index 0000000000..a8d0d0c9e9 --- /dev/null +++ b/src/migrations/20230130113337-revert-user-pat-permissions.js @@ -0,0 +1,21 @@ +exports.up = function (db, cb) { + db.runSql( + ` + DELETE FROM permissions WHERE permission = 'READ_USER_PAT'; + DELETE FROM permissions WHERE permission = 'CREATE_USER_PAT'; + DELETE FROM permissions WHERE permission = 'DELETE_USER_PAT'; + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + INSERT INTO permissions (permission, display_name, type) VALUES ('READ_USER_PAT', 'Read PATs for a specific user', 'root'); + INSERT INTO permissions (permission, display_name, type) VALUES ('CREATE_USER_PAT', 'Create a PAT for a specific user', 'root'); + INSERT INTO permissions (permission, display_name, type) VALUES ('DELETE_USER_PAT', 'Delete a PAT for a specific user', 'root'); + `, + cb, + ); +};