From cc689d3178c9de412b71794c64995c52858cc66a Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Tue, 6 Aug 2024 01:52:58 +0800 Subject: [PATCH] feat: added secrets deletion feature on integration removal --- .../ee/services/audit-log/audit-log-types.ts | 1 + backend/src/server/routes/index.ts | 9 +- .../server/routes/v1/integration-router.ts | 12 +- .../integration-delete-secret.ts | 350 ++++++++++++++++++ .../integration/integration-service.ts | 48 ++- .../services/integration/integration-types.ts | 1 + .../DeleteActionModal/DeleteActionModal.tsx | 7 +- .../src/hooks/api/integrations/queries.tsx | 11 +- .../IntegrationsPage/IntegrationsPage.tsx | 10 +- .../IntegrationsSection.tsx | 66 +++- 10 files changed, 493 insertions(+), 22 deletions(-) create mode 100644 backend/src/services/integration-auth/integration-delete-secret.ts diff --git a/backend/src/ee/services/audit-log/audit-log-types.ts b/backend/src/ee/services/audit-log/audit-log-types.ts index bfc1dbf92..07aeb320e 100644 --- a/backend/src/ee/services/audit-log/audit-log-types.ts +++ b/backend/src/ee/services/audit-log/audit-log-types.ts @@ -337,6 +337,7 @@ interface DeleteIntegrationEvent { targetServiceId?: string; path?: string; region?: string; + shouldDeleteIntegrationSecrets?: boolean; }; } diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index e8f80f020..681358193 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -885,8 +885,15 @@ export const registerRoutes = async ( folderDAL, integrationDAL, integrationAuthDAL, - secretQueueService + secretQueueService, + integrationAuthService, + projectBotService, + secretV2BridgeDAL, + secretImportDAL, + secretDAL, + kmsService }); + const serviceTokenService = serviceTokenServiceFactory({ projectEnvDAL, serviceTokenDAL, diff --git a/backend/src/server/routes/v1/integration-router.ts b/backend/src/server/routes/v1/integration-router.ts index 97a7f4d7a..6526dd940 100644 --- a/backend/src/server/routes/v1/integration-router.ts +++ b/backend/src/server/routes/v1/integration-router.ts @@ -170,6 +170,12 @@ export const registerIntegrationRouter = async (server: FastifyZodProvider) => { params: z.object({ integrationId: z.string().trim().describe(INTEGRATION.DELETE.integrationId) }), + querystring: z.object({ + shouldDeleteIntegrationSecrets: z + .enum(["true", "false"]) + .optional() + .transform((val) => val === "true") + }), response: { 200: z.object({ integration: IntegrationsSchema @@ -183,7 +189,8 @@ export const registerIntegrationRouter = async (server: FastifyZodProvider) => { actorAuthMethod: req.permission.authMethod, actor: req.permission.type, actorOrgId: req.permission.orgId, - id: req.params.integrationId + id: req.params.integrationId, + shouldDeleteIntegrationSecrets: req.query.shouldDeleteIntegrationSecrets }); await server.services.auditLog.createAuditLog({ @@ -205,7 +212,8 @@ export const registerIntegrationRouter = async (server: FastifyZodProvider) => { targetService: integration.targetService, targetServiceId: integration.targetServiceId, path: integration.path, - region: integration.region + region: integration.region, + shouldDeleteIntegrationSecrets: req.query.shouldDeleteIntegrationSecrets // eslint-disable-next-line }) as any } diff --git a/backend/src/services/integration-auth/integration-delete-secret.ts b/backend/src/services/integration-auth/integration-delete-secret.ts new file mode 100644 index 000000000..2a33677dd --- /dev/null +++ b/backend/src/services/integration-auth/integration-delete-secret.ts @@ -0,0 +1,350 @@ +import { Octokit } from "@octokit/rest"; + +import { TIntegrationAuths, TIntegrations } from "@app/db/schemas"; +import { decryptSymmetric128BitHexKeyUTF8 } from "@app/lib/crypto"; +import { BadRequestError, NotFoundError } from "@app/lib/errors"; +import { logger } from "@app/lib/logger"; + +import { IntegrationMetadataSchema } from "../integration/integration-schema"; +import { TKmsServiceFactory } from "../kms/kms-service"; +import { KmsDataKey } from "../kms/kms-types"; +import { TProjectBotServiceFactory } from "../project-bot/project-bot-service"; +import { TSecretDALFactory } from "../secret/secret-dal"; +import { TSecretFolderDALFactory } from "../secret-folder/secret-folder-dal"; +import { TSecretImportDALFactory } from "../secret-import/secret-import-dal"; +import { fnSecretsV2FromImports } from "../secret-import/secret-import-fns"; +import { TSecretV2BridgeDALFactory } from "../secret-v2-bridge/secret-v2-bridge-dal"; +import { TIntegrationAuthServiceFactory } from "./integration-auth-service"; +import { Integrations } from "./integration-list"; + +const MAX_SYNC_SECRET_DEPTH = 5; + +/** + * Return the secrets in a given [folderId] including secrets from + * nested imported folders recursively. + */ +const getIntegrationSecretsV2 = async ( + dto: { + projectId: string; + environment: string; + folderId: string; + depth: number; + decryptor: (value: Buffer | null | undefined) => string; + }, + secretV2BridgeDAL: Pick, + folderDAL: Pick, + secretImportDAL: Pick +) => { + const content: Record = {}; + if (dto.depth > MAX_SYNC_SECRET_DEPTH) { + logger.info( + `getIntegrationSecrets: secret depth exceeded for [projectId=${dto.projectId}] [folderId=${dto.folderId}] [depth=${dto.depth}]` + ); + return content; + } + + // process secrets in current folder + const secrets = await secretV2BridgeDAL.findByFolderId(dto.folderId); + + secrets.forEach((secret) => { + const secretKey = secret.key; + content[secretKey] = true; + }); + + // check if current folder has any imports from other folders + const secretImports = await secretImportDAL.find({ folderId: dto.folderId, isReplication: false }); + + // if no imports then return secrets in the current folder + if (!secretImports.length) return content; + const importedSecrets = await fnSecretsV2FromImports({ + decryptor: dto.decryptor, + folderDAL, + secretDAL: secretV2BridgeDAL, + secretImportDAL, + allowedImports: secretImports + }); + + for (let i = importedSecrets.length - 1; i >= 0; i -= 1) { + for (let j = 0; j < importedSecrets[i].secrets.length; j += 1) { + const importedSecret = importedSecrets[i].secrets[j]; + if (!content[importedSecret.key]) { + content[importedSecret.key] = true; + } + } + } + return content; +}; + +/** + * Return the secrets in a given [folderId] including secrets from + * nested imported folders recursively. + */ +const getIntegrationSecrets = async ( + dto: { + projectId: string; + environment: string; + folderId: string; + key: string; + depth: number; + }, + secretDAL: Pick, + folderDAL: Pick, + secretImportDAL: Pick +) => { + let content: Record = {}; + if (dto.depth > MAX_SYNC_SECRET_DEPTH) { + logger.info( + `getIntegrationSecrets: secret depth exceeded for [projectId=${dto.projectId}] [folderId=${dto.folderId}] [depth=${dto.depth}]` + ); + return content; + } + + // process secrets in current folder + const secrets = await secretDAL.findByFolderId(dto.folderId); + secrets.forEach((secret) => { + const secretKey = decryptSymmetric128BitHexKeyUTF8({ + ciphertext: secret.secretKeyCiphertext, + iv: secret.secretKeyIV, + tag: secret.secretKeyTag, + key: dto.key + }); + + content[secretKey] = true; + }); + + // check if current folder has any imports from other folders + const secretImport = await secretImportDAL.find({ folderId: dto.folderId, isReplication: false }); + + // if no imports then return secrets in the current folder + if (!secretImport) return content; + + const importedFolders = await folderDAL.findByManySecretPath( + secretImport.map(({ importEnv, importPath }) => ({ + envId: importEnv.id, + secretPath: importPath + })) + ); + + for await (const folder of importedFolders) { + if (folder) { + // get secrets contained in each imported folder by recursively calling + // this function against the imported folder + const importedSecrets = await getIntegrationSecrets( + { + environment: dto.environment, + projectId: dto.projectId, + folderId: folder.id, + key: dto.key, + depth: dto.depth + 1 + }, + secretDAL, + folderDAL, + secretImportDAL + ); + + // add the imported secrets to the current folder secrets + content = { ...importedSecrets, ...content }; + } + } + + return content; +}; + +export const deleteGithubSecrets = async ({ + integration, + secrets, + accessToken +}: { + integration: Omit; + secrets: Record; + accessToken: string; +}) => { + interface GitHubSecret { + name: string; + created_at: string; + updated_at: string; + visibility?: "all" | "private" | "selected"; + selected_repositories_url?: string | undefined; + } + + const octokit = new Octokit({ + auth: accessToken + }); + + enum GithubScope { + Repo = "github-repo", + Org = "github-org", + Env = "github-env" + } + + let encryptedSecrets: GitHubSecret[]; + + switch (integration.scope) { + case GithubScope.Org: { + encryptedSecrets = ( + await octokit.request("GET /orgs/{org}/actions/secrets", { + org: integration.owner as string + }) + ).data.secrets; + break; + } + case GithubScope.Env: { + encryptedSecrets = ( + await octokit.request("GET /repositories/{repository_id}/environments/{environment_name}/secrets", { + repository_id: Number(integration.appId), + environment_name: integration.targetEnvironmentId as string + }) + ).data.secrets; + break; + } + default: { + encryptedSecrets = ( + await octokit.request("GET /repos/{owner}/{repo}/actions/secrets", { + owner: integration.owner as string, + repo: integration.app as string + }) + ).data.secrets; + break; + } + } + + for await (const encryptedSecret of encryptedSecrets) { + if (encryptedSecret.name in secrets) { + switch (integration.scope) { + case GithubScope.Org: { + await octokit.request("DELETE /orgs/{org}/actions/secrets/{secret_name}", { + org: integration.owner as string, + secret_name: encryptedSecret.name + }); + break; + } + case GithubScope.Env: { + await octokit.request( + "DELETE /repositories/{repository_id}/environments/{environment_name}/secrets/{secret_name}", + { + repository_id: Number(integration.appId), + environment_name: integration.targetEnvironmentId as string, + secret_name: encryptedSecret.name + } + ); + break; + } + default: { + await octokit.request("DELETE /repos/{owner}/{repo}/actions/secrets/{secret_name}", { + owner: integration.owner as string, + repo: integration.app as string, + secret_name: encryptedSecret.name + }); + break; + } + } + } + } +}; + +export const deleteIntegrationSecrets = async ({ + integration, + integrationAuth, + integrationAuthService, + projectBotService, + secretV2BridgeDAL, + folderDAL, + secretDAL, + secretImportDAL, + kmsService +}: { + integration: Omit & { + projectId: string; + environment: { + id: string; + name: string; + slug: string; + }; + secretPath: string; + }; + integrationAuth: TIntegrationAuths; + integrationAuthService: Pick; + projectBotService: Pick; + secretV2BridgeDAL: Pick; + folderDAL: Pick; + secretImportDAL: Pick; + secretDAL: Pick; + kmsService: Pick; +}) => { + const { shouldUseSecretV2Bridge, botKey } = await projectBotService.getBotKey(integration.projectId); + const { decryptor: secretManagerDecryptor } = await kmsService.createCipherPairWithDataKey({ + type: KmsDataKey.SecretManager, + projectId: integration.projectId + }); + + const folder = await folderDAL.findBySecretPath( + integration.projectId, + integration.environment.slug, + integration.secretPath + ); + + if (!folder) { + throw new NotFoundError({ + message: "Folder not found." + }); + } + + const { accessToken } = await integrationAuthService.getIntegrationAccessToken( + integrationAuth, + shouldUseSecretV2Bridge, + botKey + ); + + const secrets = shouldUseSecretV2Bridge + ? await getIntegrationSecretsV2( + { + environment: integration.environment.id, + projectId: integration.projectId, + folderId: folder.id, + depth: 1, + decryptor: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : "") + }, + secretV2BridgeDAL, + folderDAL, + secretImportDAL + ) + : await getIntegrationSecrets( + { + environment: integration.environment.id, + projectId: integration.projectId, + folderId: folder.id, + key: botKey as string, + depth: 1 + }, + secretDAL, + folderDAL, + secretImportDAL + ); + + const suffixedSecrets: typeof secrets = {}; + const metadata = IntegrationMetadataSchema.parse(integration.metadata); + + if (metadata) { + Object.keys(secrets).forEach((key) => { + const prefix = metadata?.secretPrefix || ""; + const suffix = metadata?.secretSuffix || ""; + const newKey = prefix + key + suffix; + suffixedSecrets[newKey] = secrets[key]; + }); + } + + switch (integration.integration) { + case Integrations.GITHUB: { + await deleteGithubSecrets({ + integration, + accessToken, + secrets: Object.keys(suffixedSecrets).length !== 0 ? suffixedSecrets : secrets + }); + break; + } + default: + throw new BadRequestError({ + message: "Invalid integration" + }); + } +}; diff --git a/backend/src/services/integration/integration-service.ts b/backend/src/services/integration/integration-service.ts index da9cfc71f..02e520c6e 100644 --- a/backend/src/services/integration/integration-service.ts +++ b/backend/src/services/integration/integration-service.ts @@ -6,8 +6,15 @@ import { BadRequestError } from "@app/lib/errors"; import { TProjectPermission } from "@app/lib/types"; import { TIntegrationAuthDALFactory } from "../integration-auth/integration-auth-dal"; +import { TIntegrationAuthServiceFactory } from "../integration-auth/integration-auth-service"; +import { deleteIntegrationSecrets } from "../integration-auth/integration-delete-secret"; +import { TKmsServiceFactory } from "../kms/kms-service"; +import { TProjectBotServiceFactory } from "../project-bot/project-bot-service"; +import { TSecretDALFactory } from "../secret/secret-dal"; import { TSecretQueueFactory } from "../secret/secret-queue"; import { TSecretFolderDALFactory } from "../secret-folder/secret-folder-dal"; +import { TSecretImportDALFactory } from "../secret-import/secret-import-dal"; +import { TSecretV2BridgeDALFactory } from "../secret-v2-bridge/secret-v2-bridge-dal"; import { TIntegrationDALFactory } from "./integration-dal"; import { TCreateIntegrationDTO, @@ -19,9 +26,15 @@ import { type TIntegrationServiceFactoryDep = { integrationDAL: TIntegrationDALFactory; integrationAuthDAL: TIntegrationAuthDALFactory; - folderDAL: Pick; + integrationAuthService: TIntegrationAuthServiceFactory; + folderDAL: Pick; permissionService: Pick; + projectBotService: TProjectBotServiceFactory; secretQueueService: Pick; + secretV2BridgeDAL: Pick; + secretImportDAL: Pick; + kmsService: Pick; + secretDAL: Pick; }; export type TIntegrationServiceFactory = ReturnType; @@ -31,7 +44,13 @@ export const integrationServiceFactory = ({ integrationAuthDAL, folderDAL, permissionService, - secretQueueService + secretQueueService, + integrationAuthService, + projectBotService, + secretV2BridgeDAL, + secretImportDAL, + kmsService, + secretDAL }: TIntegrationServiceFactoryDep) => { const createIntegration = async ({ app, @@ -161,7 +180,14 @@ export const integrationServiceFactory = ({ return updatedIntegration; }; - const deleteIntegration = async ({ actorId, id, actor, actorAuthMethod, actorOrgId }: TDeleteIntegrationDTO) => { + const deleteIntegration = async ({ + actorId, + id, + actor, + actorAuthMethod, + actorOrgId, + shouldDeleteIntegrationSecrets + }: TDeleteIntegrationDTO) => { const integration = await integrationDAL.findById(id); if (!integration) throw new BadRequestError({ message: "Integration auth not found" }); @@ -174,6 +200,22 @@ export const integrationServiceFactory = ({ ); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Delete, ProjectPermissionSub.Integrations); + const integrationAuth = await integrationAuthDAL.findById(integration.integrationAuthId); + + if (shouldDeleteIntegrationSecrets) { + await deleteIntegrationSecrets({ + integration, + integrationAuth, + projectBotService, + integrationAuthService, + secretV2BridgeDAL, + folderDAL, + secretImportDAL, + secretDAL, + kmsService + }); + } + const deletedIntegration = await integrationDAL.transaction(async (tx) => { // delete integration const deletedIntegrationResult = await integrationDAL.deleteById(id, tx); diff --git a/backend/src/services/integration/integration-types.ts b/backend/src/services/integration/integration-types.ts index abbccbe90..cfb6d70a4 100644 --- a/backend/src/services/integration/integration-types.ts +++ b/backend/src/services/integration/integration-types.ts @@ -63,6 +63,7 @@ export type TUpdateIntegrationDTO = { export type TDeleteIntegrationDTO = { id: string; + shouldDeleteIntegrationSecrets?: boolean; } & Omit; export type TSyncIntegrationDTO = { diff --git a/frontend/src/components/v2/DeleteActionModal/DeleteActionModal.tsx b/frontend/src/components/v2/DeleteActionModal/DeleteActionModal.tsx index 8e4bcbe71..299b23bb5 100644 --- a/frontend/src/components/v2/DeleteActionModal/DeleteActionModal.tsx +++ b/frontend/src/components/v2/DeleteActionModal/DeleteActionModal.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { ReactNode, useEffect, useState } from "react"; import { useToggle } from "@app/hooks"; @@ -16,6 +16,7 @@ type Props = { subTitle?: string; onDeleteApproved: () => Promise; buttonText?: string; + children?: ReactNode; }; export const DeleteActionModal = ({ @@ -26,7 +27,8 @@ export const DeleteActionModal = ({ onDeleteApproved, title, subTitle = "This action is irreversible.", - buttonText = "Delete" + buttonText = "Delete", + children }: Props): JSX.Element => { const [inputData, setInputData] = useState(""); const [isLoading, setIsLoading] = useToggle(); @@ -97,6 +99,7 @@ export const DeleteActionModal = ({ placeholder={`Type ${deleteKey} here`} /> + {children} diff --git a/frontend/src/hooks/api/integrations/queries.tsx b/frontend/src/hooks/api/integrations/queries.tsx index 81d0f00ca..de4c1a914 100644 --- a/frontend/src/hooks/api/integrations/queries.tsx +++ b/frontend/src/hooks/api/integrations/queries.tsx @@ -110,8 +110,15 @@ export const useCreateIntegration = () => { export const useDeleteIntegration = () => { const queryClient = useQueryClient(); - return useMutation<{}, {}, { id: string; workspaceId: string }>({ - mutationFn: ({ id }) => apiRequest.delete(`/api/v1/integration/${id}`), + return useMutation< + {}, + {}, + { id: string; workspaceId: string; shouldDeleteIntegrationSecrets: boolean } + >({ + mutationFn: ({ id, shouldDeleteIntegrationSecrets }) => + apiRequest.delete( + `/api/v1/integration/${id}?shouldDeleteIntegrationSecrets=${shouldDeleteIntegrationSecrets}` + ), onSuccess: (_, { workspaceId }) => { queryClient.invalidateQueries(workspaceKeys.getWorkspaceIntegrations(workspaceId)); queryClient.invalidateQueries(workspaceKeys.getWorkspaceAuthorization(workspaceId)); diff --git a/frontend/src/views/IntegrationsPage/IntegrationsPage.tsx b/frontend/src/views/IntegrationsPage/IntegrationsPage.tsx index 5601a7f68..e44fd2539 100644 --- a/frontend/src/views/IntegrationsPage/IntegrationsPage.tsx +++ b/frontend/src/views/IntegrationsPage/IntegrationsPage.tsx @@ -106,9 +106,13 @@ export const IntegrationsPage = withProjectPermission( handleProviderIntegration(provider); }; - const handleIntegrationDelete = async (integrationId: string, cb: () => void) => { + const handleIntegrationDelete = async ( + integrationId: string, + shouldDeleteIntegrationSecrets: boolean, + cb: () => void + ) => { try { - await deleteIntegration({ id: integrationId, workspaceId }); + await deleteIntegration({ id: integrationId, workspaceId, shouldDeleteIntegrationSecrets }); if (cb) cb(); createNotification({ type: "success", @@ -152,7 +156,7 @@ export const IntegrationsPage = withProjectPermission( isLoading={isIntegrationLoading} integrations={integrations} environments={environments} - onIntegrationDelete={({ id }, cb) => handleIntegrationDelete(id, cb)} + onIntegrationDelete={handleIntegrationDelete} workspaceId={workspaceId} /> ; integrations?: TIntegration[]; isLoading?: boolean; - onIntegrationDelete: (integration: TIntegration, cb: () => void) => void; + onIntegrationDelete: ( + integrationId: string, + shouldDeleteIntegrationSecrets: boolean, + cb: () => void + ) => Promise; workspaceId: string; }; @@ -37,10 +42,12 @@ export const IntegrationsSection = ({ workspaceId }: Props) => { const { popUp, handlePopUpOpen, handlePopUpClose, handlePopUpToggle } = usePopUp([ - "deleteConfirmation" + "deleteConfirmation", + "deleteSecretsConfirmation" ] as const); const { mutate: syncIntegration } = useSyncIntegration(); + const [shouldDeleteSecrets, setShouldDeleteSecrets] = useToggle(false); return (
@@ -249,7 +256,10 @@ export const IntegrationsSection = ({
handlePopUpOpen("deleteConfirmation", integration)} + onClick={() => { + setShouldDeleteSecrets.off(); + handlePopUpOpen("deleteConfirmation", integration); + }} ariaLabel="delete" isDisabled={!isAllowed} colorSchema="danger" @@ -281,11 +291,49 @@ export const IntegrationsSection = ({ (popUp?.deleteConfirmation?.data as TIntegration)?.integration || "" } - onDeleteApproved={async () => - onIntegrationDelete(popUp?.deleteConfirmation.data as TIntegration, () => - handlePopUpClose("deleteConfirmation") - ) - } + onDeleteApproved={async () => { + if (shouldDeleteSecrets) { + handlePopUpOpen("deleteSecretsConfirmation"); + return; + } + + await onIntegrationDelete( + (popUp?.deleteConfirmation.data as TIntegration).id, + false, + () => handlePopUpClose("deleteConfirmation") + ); + }} + > + {(popUp?.deleteConfirmation?.data as TIntegration)?.integration === "github" && ( +
+ setShouldDeleteSecrets.toggle()} + > + Delete secrets in destination + +
+ )} + + handlePopUpToggle("deleteSecretsConfirmation", isOpen)} + deleteKey="confirm" + onDeleteApproved={async () => { + await onIntegrationDelete( + (popUp?.deleteConfirmation.data as TIntegration).id, + true, + () => { + handlePopUpClose("deleteSecretsConfirmation"); + handlePopUpClose("deleteConfirmation"); + } + ); + }} />
);