Introduce experimental permission validation based on ToDo resource (#186)

pepakriz · web-flow · commit d33cf6b68613 · 2024-03-26T21:32:10.000+01:00
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -16,9 +16,6 @@ jobs:
         with:
           node-version: ${{ matrix.node-version }}
 
-      - name: EditorConfig Lint
-        uses: docker://mstruebing/editorconfig-checker:2.0.3
-
       - name: Install
         run: yarn install --frozen-lockfile && yarn run generate
 
diff --git a/README.md b/README.md
@@ -102,6 +102,7 @@ GITLAB_AUTH_TOKEN="<token>" yarn run start
 | `HTTP_SERVER_ENABLE`           | `false`              | It'll enable experimental API and dashboard support        |
 | `HTTP_SERVER_PORT`             | `4000`               | It'll use different http server port                       |
 | `WEB_HOOK_TOKEN`               | ``                   | It'll enable experimental web hook support                 |
+| `PERMISSION_VALIDATION`        | `false`              | It'll enable experimental permission validation            |
 
 ## Development
 
diff --git a/server/src/Config.ts b/server/src/Config.ts
@@ -16,6 +16,7 @@ export const defaultConfig = {
 	WEB_HOOK_TOKEN: '',
 	DRY_RUN: false,
 	HTTP_PROXY: '',
+	ENABLE_PERMISSION_VALIDATION: false,
 };
 
 export const getConfig = (): Config => ({
@@ -66,6 +67,10 @@ export const getConfig = (): Config => ({
 	WEB_HOOK_TOKEN: env.get('WEB_HOOK_TOKEN').default(defaultConfig.WEB_HOOK_TOKEN).asString(),
 	DRY_RUN: env.get('DRY_RUN').default(`${defaultConfig.DRY_RUN}`).asBoolStrict(),
 	HTTP_PROXY: env.get('HTTP_PROXY').default('').asString(),
+	ENABLE_PERMISSION_VALIDATION: env
+		.get('ENABLE_PERMISSION_VALIDATION')
+		.default(`${defaultConfig.ENABLE_PERMISSION_VALIDATION}`)
+		.asBoolStrict(),
 });
 
 export type Config = typeof defaultConfig;
diff --git a/server/src/GitlabApi.ts b/server/src/GitlabApi.ts
@@ -2,6 +2,7 @@ import fetch, { FetchError, RequestInit, Response } from 'node-fetch';
 import queryString, { ParsedUrlQueryInput } from 'querystring';
 import { sleep } from './Utils';
 import { HttpsProxyAgent } from 'https-proxy-agent';
+import * as console from 'console';
 
 export interface User {
 	id: number;
@@ -40,13 +41,55 @@ interface MergeRequestAssignee {
 	id: number;
 }
 
+export interface ProtectedBranch {
+	id: number;
+	name: string;
+	merge_access_levels: (
+		| {
+				access_level: number;
+				user_id: null;
+				group_id: null;
+		  }
+		| {
+				access_level: null;
+				user_id: number;
+				group_id: null;
+		  }
+		| {
+				access_level: null;
+				user_id: null;
+				group_id: number;
+		  }
+	)[];
+}
+
+interface Author {
+	id: number;
+	username: string;
+	name: string;
+	web_url: string;
+	state: 'active' | 'awaiting';
+}
+
+export interface Member extends Author {
+	access_level: number;
+	expires_at: string | null;
+}
+
+export interface ToDo {
+	id: number;
+	project: {
+		id: number;
+	};
+	author: Author;
+	target: MergeRequest;
+}
+
 export interface MergeRequest {
 	id: number;
 	iid: number;
 	title: string;
-	author: {
-		id: number;
-	};
+	author: Author;
 	assignee: MergeRequestAssignee | null;
 	assignees: MergeRequestAssignee[];
 	project_id: number;
@@ -159,6 +202,37 @@ export class GitlabApi {
 		return this.sendRequestWithSingleResponse(`/api/v4/users/${userId}`, RequestMethod.Get);
 	}
 
+	public async getProtectedBranch(
+		projectId: number,
+		name: string,
+	): Promise<ProtectedBranch | null> {
+		return this.sendRequestWithSingleResponse(
+			`/api/v4/projects/${projectId}/protected_branches/${encodeURIComponent(name)}`,
+			RequestMethod.Get,
+		);
+	}
+
+	public async getMember(projectId: number, userId: number): Promise<Member | null> {
+		return this.sendRequestWithSingleResponse(
+			`/api/v4/projects/${projectId}/members/${userId}`,
+			RequestMethod.Get,
+		);
+	}
+
+	public async getMergeRequestTodos(): Promise<ToDo[]> {
+		return this.sendRequestWithMultiResponse(
+			`/api/v4/todos?type=MergeRequest&action=assigned&state=pending`,
+			RequestMethod.Get,
+		);
+	}
+
+	public async markTodoAsDone(todoId: number): Promise<void> {
+		return this.sendRequestWithSingleResponse(
+			`/api/v4/todos/${todoId}/mark_as_done`,
+			RequestMethod.Post,
+		);
+	}
+
 	public async getAssignedOpenedMergeRequests(): Promise<MergeRequest[]> {
 		return this.sendRequestWithMultiResponse(
 			`/api/v4/merge_requests?scope=assigned_to_me&state=opened`,
@@ -190,16 +264,6 @@ export class GitlabApi {
 		);
 	}
 
-	public async getMergeRequestPipelines(
-		projectId: number,
-		mergeRequestIid: number,
-	): Promise<MergeRequestPipeline[]> {
-		return this.sendRequestWithMultiResponse(
-			`/api/v4/projects/${projectId}/merge_requests/${mergeRequestIid}/pipelines`,
-			RequestMethod.Get,
-		);
-	}
-
 	public async getPipelineJobs(projectId: number, pipelineId: number): Promise<PipelineJob[]> {
 		return this.sendRequestWithMultiResponse(
 			`/api/v4/projects/${projectId}/pipelines/${pipelineId}/jobs?per_page=100`,
@@ -287,6 +351,10 @@ export class GitlabApi {
 		body?: ParsedUrlQueryInput,
 	): Promise<any> {
 		const response = await this.sendRawRequest(url, method, body);
+		if (response.status === 404) {
+			return null;
+		}
+
 		await this.validateResponseStatus(response);
 
 		const data = await response.json();
diff --git a/server/src/MergeRequestAcceptor.ts b/server/src/MergeRequestAcceptor.ts
@@ -186,7 +186,7 @@ const incompletePipelineStatuses = [
 ];
 
 const containsLabel = (labels: string[], label: BotLabels) => labels.includes(label);
-const containsAssignedUser = (mergeRequest: MergeRequest, user: User) => {
+export const containsAssignedUser = (mergeRequest: MergeRequest, user: User) => {
 	const userIds = mergeRequest.assignees.map((assignee) => assignee.id);
 	return userIds.includes(user.id);
 };
diff --git a/server/src/MergeRequestCheckerLoop.ts b/server/src/MergeRequestCheckerLoop.ts
@@ -1,4 +1,4 @@
-import { GitlabApi, MergeRequest, User } from './GitlabApi';
+import { GitlabApi, MergeRequest, ToDo, User } from './GitlabApi';
 import { prepareMergeRequestForMerge } from './MergeRequestReceiver';
 import { Config } from './Config';
 import { Worker } from './Worker';
@@ -69,17 +69,26 @@ export class MergeRequestCheckerLoop {
 	}
 
 	private async task() {
+		if (this.config.ENABLE_PERMISSION_VALIDATION) {
+			console.log('[loop] Checking new todos');
+			const mergeRequestTodos = await this.gitlabApi.getMergeRequestTodos();
+			const possibleToAcceptMergeRequests = mergeRequestTodos.map((mergeRequestTodo: ToDo) =>
+				prepareMergeRequestForMerge(this.gitlabApi, this.user, this.worker, this.config, {
+					mergeRequestTodo,
+				}),
+			);
+
+			await Promise.all(possibleToAcceptMergeRequests);
+			return;
+		}
+
 		console.log('[loop] Checking assigned merge requests');
 		const assignedMergeRequests = await this.gitlabApi.getAssignedOpenedMergeRequests();
 		const possibleToAcceptMergeRequests = assignedMergeRequests.map(
 			(mergeRequest: MergeRequest) =>
-				prepareMergeRequestForMerge(
-					this.gitlabApi,
-					this.user,
-					this.worker,
-					this.config,
+				prepareMergeRequestForMerge(this.gitlabApi, this.user, this.worker, this.config, {
 					mergeRequest,
-				),
+				}),
 		);
 
 		await Promise.all(possibleToAcceptMergeRequests);
diff --git a/server/src/MergeRequestReceiver.ts b/server/src/MergeRequestReceiver.ts
@@ -1,10 +1,11 @@
-import { DetailedMergeStatus, GitlabApi, MergeRequest, User } from './GitlabApi';
+import { DetailedMergeStatus, GitlabApi, MergeRequest, ToDo, User } from './GitlabApi';
 import { assignToAuthorAndResetLabels } from './AssignToAuthor';
 import { sendNote } from './SendNote';
 import {
 	acceptMergeRequest,
 	AcceptMergeRequestResultKind,
 	BotLabels,
+	containsAssignedUser,
 	runAcceptingMergeRequest,
 } from './MergeRequestAcceptor';
 import { resolveMergeRequestResult } from './MergeRequestResultResolver';
@@ -18,8 +19,28 @@ export const prepareMergeRequestForMerge = async (
 	user: User,
 	worker: Worker,
 	config: Config,
-	mergeRequest: MergeRequest,
+	mergeRequestData:
+		| {
+				mergeRequestTodo: ToDo;
+		  }
+		| {
+				mergeRequest: MergeRequest;
+		  },
 ) => {
+	const { mergeRequest, author } = (() => {
+		if ('mergeRequestTodo' in mergeRequestData) {
+			return {
+				mergeRequest: mergeRequestData.mergeRequestTodo.target,
+				author: mergeRequestData.mergeRequestTodo.author,
+			};
+		}
+
+		return {
+			mergeRequest: mergeRequestData.mergeRequest,
+			author: null,
+		};
+	})();
+
 	const jobId = `accept-merge-${mergeRequest.id}`;
 	const jobPriority = mergeRequest.labels.includes(config.HIGH_PRIORITY_LABEL)
 		? JobPriority.HIGH
@@ -50,6 +71,64 @@ export const prepareMergeRequestForMerge = async (
 		return;
 	}
 
+	if (!containsAssignedUser(mergeRequest, user)) {
+		if ('mergeRequestTodo' in mergeRequestData) {
+			await gitlabApi.markTodoAsDone(mergeRequestData.mergeRequestTodo.id);
+		}
+
+		return;
+	}
+
+	// Validate permissions
+	if (author !== null) {
+		const protectedBranch = await gitlabApi.getProtectedBranch(
+			mergeRequest.target_project_id,
+			mergeRequest.target_branch,
+		);
+		if (protectedBranch !== null) {
+			const member = await gitlabApi.getMember(mergeRequest.target_project_id, author.id);
+			if (member === null) {
+				await Promise.all([
+					assignToAuthorAndResetLabels(gitlabApi, mergeRequest, user),
+					sendNote(
+						gitlabApi,
+						mergeRequest,
+						`I can't merge it because the merge request was made by ${author.username} who is unauthorized for this instruction.`,
+					),
+				]);
+
+				return;
+			}
+
+			const hasAccessLevel = protectedBranch.merge_access_levels.find((mergeAccessLevel) => {
+				if (mergeAccessLevel.user_id !== null && member.id === mergeAccessLevel.user_id) {
+					return true;
+				}
+
+				if (
+					mergeAccessLevel.access_level !== null &&
+					member.access_level >= mergeAccessLevel.access_level
+				) {
+					return true;
+				}
+
+				return false;
+			});
+			if (!hasAccessLevel) {
+				await Promise.all([
+					assignToAuthorAndResetLabels(gitlabApi, mergeRequest, user),
+					sendNote(
+						gitlabApi,
+						mergeRequest,
+						`I can't merge it because the merge request was made by ${author.username} who doesn't pass the protection of the target branch.`,
+					),
+				]);
+
+				return;
+			}
+		}
+	}
+
 	if (mergeRequest.detailed_merge_status === DetailedMergeStatus.DraftStatus) {
 		console.log(`[loop][MR][${mergeRequest.iid}] Merge request is a draft, assigning back`);
 
@@ -155,6 +234,9 @@ export const prepareMergeRequestForMerge = async (
 			}
 
 			console.log(`Finishing job: ${JSON.stringify(result)}`);
+			if ('mergeRequestTodo' in mergeRequestData) {
+				await gitlabApi.markTodoAsDone(mergeRequestData.mergeRequestTodo.id);
+			}
 			success();
 			await resolveMergeRequestResult(gitlabApi, result);
 		},
diff --git a/server/src/WebHookServer.ts b/server/src/WebHookServer.ts
@@ -62,7 +62,7 @@ const processMergeRequestHook = async (
 			data.project.id,
 			data.object_attributes.iid,
 		);
-		await prepareMergeRequestForMerge(gitlabApi, user, worker, config, mergeRequest);
+		await prepareMergeRequestForMerge(gitlabApi, user, worker, config, { mergeRequest });
 		return;
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ const processMergeRequestHook = async (`
`62`	`62`	`data.project.id,`
`63`	`63`	`data.object_attributes.iid,`
`64`	`64`	`);`
`65`		`- await prepareMergeRequestForMerge(gitlabApi, user, worker, config, mergeRequest);`
	`65`	`+ await prepareMergeRequestForMerge(gitlabApi, user, worker, config, { mergeRequest });`
`66`	`66`	`return;`
`67`	`67`	`}`
`68`	`68`