From ca6368a23c7d5ec384ffc2fc0f99c9c155b2c40b Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Fri, 11 Dec 2020 06:16:39 -0600 Subject: [PATCH 1/2] AIOSEC-85 Verify project group membership before processing gitlab deletion --- node-packages/commons/src/api.ts | 19 ++++++++++- .../src/handlers/gitlabProjectDelete.ts | 33 ++++++++++++++++--- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/node-packages/commons/src/api.ts b/node-packages/commons/src/api.ts index b79c55ec81..bebb4270c9 100644 --- a/node-packages/commons/src/api.ts +++ b/node-packages/commons/src/api.ts @@ -653,6 +653,23 @@ export async function getProjectByName(project: string): Promise { return result.project; } +export const allProjectsInGroup = (groupInput: { + id?: string; + name?: string; +}): Promise => + graphqlapi.query( + ` + query($groupInput: GroupInput!) { + allProjectsInGroup(input: $groupInput) { + ...${projectFragment} + } + } + `, + { + groupInput + } + ); + export async function getMicrosoftTeamsInfoForProject( project: string, contentType = 'DEPLOYMENT' ): Promise { @@ -1456,4 +1473,4 @@ export const getProblemHarborScanMatches = () => graphqlapi.query( regex } }` -); \ No newline at end of file +); diff --git a/services/webhooks2tasks/src/handlers/gitlabProjectDelete.ts b/services/webhooks2tasks/src/handlers/gitlabProjectDelete.ts index 38d0120226..9d1ce137cf 100644 --- a/services/webhooks2tasks/src/handlers/gitlabProjectDelete.ts +++ b/services/webhooks2tasks/src/handlers/gitlabProjectDelete.ts @@ -1,5 +1,6 @@ +import R from 'ramda'; import { sendToLagoonLogs } from '@lagoon/commons/dist/logs'; -import { deleteProject } from '@lagoon/commons/dist/api'; +import { allProjectsInGroup, deleteProject, sanitizeGroupName } from '@lagoon/commons/dist/api'; import { WebhookRequestData } from '../types'; @@ -9,15 +10,37 @@ export async function gitlabProjectDelete(webhook: WebhookRequestData) { event, uuid, body, - body: { path: name } + body: { path: projectName, path_with_namespace } } = webhook; try { const meta = { - project: name + project: projectName }; - await deleteProject(name); + const groupName = sanitizeGroupName(path_with_namespace.replace(`/${projectName}`, '')); + const projectsInGroup = await allProjectsInGroup({ name: groupName }); + const projectExists = R.pipe( + R.prop('allProjectsInGroup'), + R.pluck('name'), + R.contains(projectName), + // @ts-ignore + )(projectsInGroup); + + if (projectExists) { + await deleteProject(projectName); + + sendToLagoonLogs( + 'info', + '', + uuid, + `${webhooktype}:${event}:handled`, + meta, + `deleted project ${projectName}` + ); + + return; + } sendToLagoonLogs( 'info', @@ -25,7 +48,7 @@ export async function gitlabProjectDelete(webhook: WebhookRequestData) { uuid, `${webhooktype}:${event}:handled`, meta, - `deleted project ${name}` + `project "${projectName}" not a member of group "${groupName}"` ); return; From 61e28e2bb1e68df7918dca5fd77781f2c24d8589 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Fri, 11 Dec 2020 06:59:57 -0600 Subject: [PATCH 2/2] AIOSEC-85 Verify gitlab system hook event names instead of payload structure --- node-packages/commons/src/gitlabApi.ts | 20 +++++++++++++++++++ .../webhook-handler/src/extractWebhookData.ts | 3 ++- services/webhooks2tasks/src/processQueue.ts | 5 ++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/node-packages/commons/src/gitlabApi.ts b/node-packages/commons/src/gitlabApi.ts index 9633db1d7c..b9cde73a38 100644 --- a/node-packages/commons/src/gitlabApi.ts +++ b/node-packages/commons/src/gitlabApi.ts @@ -18,6 +18,26 @@ const options = { const gitlabapi = axios.create(options); +export const secureGitlabSystemHooks = [ + 'group_create', + 'group_rename', + 'group_destroy', + 'project_create', + 'project_transfer', + 'project_rename', + 'project_update', + 'project_destroy', + 'user_create', + 'user_rename', + 'user_destroy', + 'user_add_to_group', + 'user_remove_from_group', + 'user_add_to_team', + 'user_remove_from_team', + 'key_create', + 'key_destroy', +]; + class NetworkError extends Error { constructor(message: string) { super(message); diff --git a/services/webhook-handler/src/extractWebhookData.ts b/services/webhook-handler/src/extractWebhookData.ts index c96d3dc6b9..5187b80a75 100644 --- a/services/webhook-handler/src/extractWebhookData.ts +++ b/services/webhook-handler/src/extractWebhookData.ts @@ -2,6 +2,7 @@ import uuid4 from 'uuid4'; import url from 'url'; import R from 'ramda'; import { IncomingMessage } from 'http'; +import { secureGitlabSystemHooks } from '@lagoon/commons/dist/gitlabApi'; import type { RawData, WebhookRequestData } from './types'; @@ -45,7 +46,7 @@ export function extractWebhookData(req: IncomingMessage, body: string): WebhookR giturl = R.path(['project', 'git_ssh_url'], bodyObj); // This is a system webhook - if (!giturl) { + if (R.contains(event, secureGitlabSystemHooks)) { // Ensure the system hook came from gitlab if (!('x-gitlab-token' in req.headers) || req.headers['x-gitlab-token'] !== process.env.GITLAB_SYSTEM_HOOK_TOKEN) { throw new Error('Gitlab system hook secret verification failed'); diff --git a/services/webhooks2tasks/src/processQueue.ts b/services/webhooks2tasks/src/processQueue.ts index 34ce759a2e..4c7754577a 100644 --- a/services/webhooks2tasks/src/processQueue.ts +++ b/services/webhooks2tasks/src/processQueue.ts @@ -1,5 +1,7 @@ +import R from 'ramda'; import { ChannelWrapper } from 'amqp-connection-manager'; import { ConsumeMessage } from 'amqplib'; +import { secureGitlabSystemHooks } from '@lagoon/commons/dist/gitlabApi'; import { processProjects } from './webhooks/projects'; import { processDataSync } from './webhooks/dataSync'; import { processBackup } from './webhooks/backup'; @@ -11,13 +13,14 @@ export async function processQueue (rabbitMsg: ConsumeMessage, channelWrapperWeb const { webhooktype, + event, giturl, } = webhook; // GitLab supports System Hooks which trigger on changes like creating new // organizations or users. Since these don't have associated projects, they // must be handled separately. - if (webhooktype == 'gitlab' && !giturl) { + if (webhooktype == 'gitlab' && R.contains(event, secureGitlabSystemHooks)) { processDataSync(rabbitMsg, channelWrapperWebhooks); } else if (webhooktype == 'resticbackup') { processBackup(rabbitMsg, channelWrapperWebhooks);