From 32be720e8a9b656fce9f5b294444c0b3254cc65e Mon Sep 17 00:00:00 2001 From: Kris West Date: Wed, 12 Mar 2025 12:14:56 +0000 Subject: [PATCH 01/23] fix: user should be looked up by email rather than name Git user.name may not be set to the GitHub username. GitHub uses the email address for lookup --- .../push-action/checkUserPushPermission.ts | 30 +++++++++++-------- src/proxy/processors/push-action/parsePush.ts | 14 ++++++--- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/proxy/processors/push-action/checkUserPushPermission.ts b/src/proxy/processors/push-action/checkUserPushPermission.ts index c712693e5..62674b2bb 100644 --- a/src/proxy/processors/push-action/checkUserPushPermission.ts +++ b/src/proxy/processors/push-action/checkUserPushPermission.ts @@ -7,24 +7,30 @@ const exec = async (req: any, action: Action): Promise => { const repoName = action.repo.split('/')[1].replace('.git', ''); let isUserAllowed = false; - let user = action.user; - // Find the user associated with this Git Account - const list = await getUsers({ gitAccount: action.user }); - - console.log(`Users for this git account: ${JSON.stringify(list)}`); - - if (list.length == 1) { - user = list[0].username; - isUserAllowed = await isUserPushAllowed(repoName, user); + // n.b. action.user will be set to whatever the user had set in their user.name config in their git client. + // it is not necessarily the GitHub username. GitHub looks users by email address as should we + const userEmail = action.userEmail; + let username = "unknown"; + + // Find the user associated with this email address + const list = await getUsers({ email: action.userEmail }); + + if (list.length > 1) { + console.warn(`Multiple users found with email address ${userEmail}, using the first only`); + } else if (list.length == 0){ + console.error(`No user with email address ${userEmail} found`); + } else { + username = list[0].username + isUserAllowed = await isUserPushAllowed(repoName, username); } - console.log(`User ${user} permission on Repo ${repoName} : ${isUserAllowed}`); + console.log(`User ${username} <${userEmail}> permission on Repo ${repoName} : ${isUserAllowed}`); if (!isUserAllowed) { console.log('User not allowed to Push'); step.error = true; - step.log(`User ${user} is not allowed to push on repo ${action.repo}, ending`); + step.log(`User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`); console.log('setting error'); @@ -37,7 +43,7 @@ const exec = async (req: any, action: Action): Promise => { return action; } - step.log(`User ${user} is allowed to push on repo ${action.repo}`); + step.log(`User ${username} <${userEmail}> is allowed to push on repo ${action.repo}`); action.addStep(step); return action; }; diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 42adcb48e..436d1896c 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -35,9 +35,10 @@ async function exec(req: any, action: Action): Promise { action.commitFrom = action.commitData[action.commitData.length - 1].parent; } - const user = action.commitData[action.commitData.length - 1].committer; - console.log(`Push Request received from user ${user}`); - action.user = user; + const {committer, committerEmail} = action.commitData[action.commitData.length - 1].committer; + console.log(`Push Request received from user ${committer} with email ${committerEmail}`); + action.user = committer; + action.userEmail = committerEmail; step.content = { meta: meta, @@ -110,6 +111,9 @@ const getCommitData = (contents: CommitContent[]) => { const authorEmail = author?.split(' ').reverse()[2].slice(1, -1); console.log({ authorEmail }); + const committerEmail = committer.split(' ').reverse()[2].slice(1, -1); + console.log({ committerEmail }); + console.log({ tree, parent, @@ -118,6 +122,7 @@ const getCommitData = (contents: CommitContent[]) => { commitTimestamp, message, authorEmail, + committerEmail }); if (!tree || !parent || !author || !committer || !commitTimestamp || !message || !authorEmail) { @@ -131,7 +136,8 @@ const getCommitData = (contents: CommitContent[]) => { committer: committer.split('<')[0].trim(), commitTimestamp, message, - authorEmail: authorEmail, + authorEmail, + committerEmail }; }) .value(); From 85f4b4e465cdb816295f9e502e96604e2f810907 Mon Sep 17 00:00:00 2001 From: kriswest Date: Thu, 13 Mar 2025 10:04:39 +0000 Subject: [PATCH 02/23] fix: error in destructuring of action commit data --- src/proxy/processors/push-action/parsePush.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 436d1896c..8d722dd36 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -35,7 +35,7 @@ async function exec(req: any, action: Action): Promise { action.commitFrom = action.commitData[action.commitData.length - 1].parent; } - const {committer, committerEmail} = action.commitData[action.commitData.length - 1].committer; + const {committer, committerEmail} = action.commitData[action.commitData.length - 1]; console.log(`Push Request received from user ${committer} with email ${committerEmail}`); action.user = committer; action.userEmail = committerEmail; From 3c53683ad68ee0d3d0785d38de5cccf634e1bc4a Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 21 Mar 2025 17:42:44 +0000 Subject: [PATCH 03/23] fix: lookup users via email rather than user.name git client config --- src/service/routes/push.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/service/routes/push.js b/src/service/routes/push.js index 9750375ca..1aea6ba64 100644 --- a/src/service/routes/push.js +++ b/src/service/routes/push.js @@ -42,14 +42,14 @@ router.post('/:id/reject', async (req, res) => { const push = await db.getPush(id); console.log({ push }); - // Get the Internal Author of the push via their Git Account name - const gitAccountauthor = push.user; - const list = await db.getUsers({ gitAccount: gitAccountauthor }); + // Get the committer of the push via their email + const committerEmail = push.userEmail; + const list = await db.getUsers({ email: committerEmail }); console.log({ list }); if (list.length === 0) { res.status(401).send({ - message: `The git account ${gitAccountauthor} could not be found`, + message: `The user with email ${committerEmail} could not be found`, }); return; } @@ -97,14 +97,14 @@ router.post('/:id/authorise', async (req, res) => { const push = await db.getPush(id); console.log({ push }); - // Get the Internal Author of the push via their Git Account name - const gitAccountauthor = push.user; - const list = await db.getUsers({ gitAccount: gitAccountauthor }); + // Get the committer of the push via their email address + const committerEmail = push.userEmail; + const list = await db.getUsers({ email: committerEmail }); console.log({ list }); if (list.length === 0) { res.status(401).send({ - message: `The git account ${gitAccountauthor} could not be found`, + message: `The email address ${committerEmail} could not be found`, }); return; } From 2b04db45558b804741b6a13ab676688a5118a0b3 Mon Sep 17 00:00:00 2001 From: kriswest Date: Thu, 27 Mar 2025 11:36:38 +0000 Subject: [PATCH 04/23] fix: typo in comment --- src/proxy/processors/push-action/pullRemote.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy/processors/push-action/pullRemote.ts b/src/proxy/processors/push-action/pullRemote.ts index c7559643f..d81037e19 100644 --- a/src/proxy/processors/push-action/pullRemote.ts +++ b/src/proxy/processors/push-action/pullRemote.ts @@ -22,7 +22,7 @@ const exec = async (req: any, action: Action): Promise => { } const cmd = `git clone ${action.url}`; - step.log(`Exectuting ${cmd}`); + step.log(`Executing ${cmd}`); const authHeader = req.headers?.authorization; const [username, password] = Buffer.from(authHeader.split(' ')[1], 'base64') From 1b4bd588d9829e6fe7572be6aa87095cf35374e5 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 1 Apr 2025 14:22:18 +0100 Subject: [PATCH 05/23] test: use unique emails for users in tests and remove afterwards --- test/addRepoTest.test.js | 5 ++++- test/testUserCreation.test.js | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index 04983f63c..b86fa222c 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -28,7 +28,7 @@ describe('add new repo', async () => { await db.deleteUser('u1'); await db.deleteUser('u2'); await db.createUser('u1', 'abc', 'test@test.com', 'test', true); - await db.createUser('u2', 'abc', 'test@test.com', 'test', true); + await db.createUser('u2', 'abc', 'test2@test.com', 'test', true); }); it('login', async function () { @@ -187,5 +187,8 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); + await db.deleteRepo('test-repo'); + await db.deleteUser('u1'); + await db.deleteUser('u2'); }); }); diff --git a/test/testUserCreation.test.js b/test/testUserCreation.test.js index c07dd0e7b..631be0069 100644 --- a/test/testUserCreation.test.js +++ b/test/testUserCreation.test.js @@ -55,8 +55,7 @@ describe('user creation', async () => { }); it('login as new user', async function () { - // we don't know the users tempoary password - so force update a - // pasword + // we don't know the users temporary password - so force update a password const user = await db.findUser('login-test-user'); await bcrypt.hash('test1234', 10, async function (err, hash) { From f5e952c4a7ecb0a664f68473d9044b61d7a6f21d Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 8 Apr 2025 09:25:48 +0100 Subject: [PATCH 06/23] chore: prettier --- src/proxy/actions/Action.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/proxy/actions/Action.ts b/src/proxy/actions/Action.ts index 78dbc2ef0..e7d183061 100644 --- a/src/proxy/actions/Action.ts +++ b/src/proxy/actions/Action.ts @@ -1,5 +1,5 @@ -import { getProxyUrl } from "../../config"; -import { Step } from "./Step"; +import { getProxyUrl } from '../../config'; +import { Step } from './Step'; /** * Represents a commit. @@ -7,6 +7,7 @@ import { Step } from "./Step"; export interface Commit { message: string; committer: string; + committerEmail: string; tree: string; parent: string; author: string; @@ -45,6 +46,7 @@ class Action { message?: string; author?: string; user?: string; + userEmail?: string; attestation?: string; lastStep?: Step; proxyGitPath?: string; @@ -62,15 +64,15 @@ class Action { this.type = type; this.method = method; this.timestamp = timestamp; - this.project = repo.split("/")[0]; - this.repoName = repo.split("/")[1]; + this.project = repo.split('/')[0]; + this.repoName = repo.split('/')[1]; this.url = `${getProxyUrl()}/${repo}`; this.repo = repo; } /** * Add a step to the action. - * @param {Step} step + * @param {Step} step */ addStep(step: Step): void { this.steps.push(step); From 16def585daf7c38942f2eeed36965f645c0e4add Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Apr 2025 11:03:27 +0100 Subject: [PATCH 07/23] fix: clean-up DB after CLI tests --- packages/git-proxy-cli/test/testCli.test.js | 442 +++++--------------- packages/git-proxy-cli/test/testCliUtils.js | 27 ++ src/db/file/index.ts | 10 +- src/db/file/pushes.ts | 14 +- src/db/index.ts | 1 + src/db/mongo/index.ts | 15 +- src/db/mongo/pushes.ts | 11 +- 7 files changed, 156 insertions(+), 364 deletions(-) diff --git a/packages/git-proxy-cli/test/testCli.test.js b/packages/git-proxy-cli/test/testCli.test.js index 9f6052661..385e38d28 100644 --- a/packages/git-proxy-cli/test/testCli.test.js +++ b/packages/git-proxy-cli/test/testCli.test.js @@ -19,8 +19,8 @@ const GHOST_PUSH_ID = const TEST_REPO_CONFIG = { project: 'finos', name: 'git-proxy-test', - url: 'https://github.com/finos/git-proxy-test.git' -} + url: 'https://github.com/finos/git-proxy-test.git', +}; const TEST_REPO = 'finos/git-proxy-test.git'; describe('test git-proxy-cli', function () { @@ -36,12 +36,7 @@ describe('test git-proxy-cli', function () { 'Options:', 'You need at least one command before moving on', ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it(`print help if invalid command or option is given`, async function () { @@ -53,12 +48,7 @@ describe('test git-proxy-cli', function () { 'Options:', 'Unknown arguments: invalid, invalid', ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it(`print help if "--help" option is given`, async function () { @@ -66,12 +56,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = ['Commands:', 'Options:']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); }); @@ -83,12 +68,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = ['0.1.0']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); }); @@ -100,12 +80,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = ['GitProxy URL:']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); }); @@ -117,12 +92,11 @@ describe('test git-proxy-cli', function () { const testEmail = 'jane.doe@email.com'; before(async function () { - await helper.addUserToDb( - testUser, - testPassword, - testEmail, - 'testGitAccount', - ); + await helper.addUserToDb(testUser, testPassword, testEmail, 'testGitAccount'); + }); + + after(async function () { + await helper.removeUserFromDb(testUser); }); it('login shoud fail when server is down', async function () { @@ -132,12 +106,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = [`Error: Login '${username}':`]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('login shoud fail with invalid credentials', async function () { @@ -149,12 +118,7 @@ describe('test git-proxy-cli', function () { const expectedErrorMessages = [`Error: Login '${username}': '401'`]; try { await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -165,18 +129,11 @@ describe('test git-proxy-cli', function () { const password = 'admin'; const cli = `npx -- @finos/git-proxy-cli login --username ${username} --password ${password}`; const expectedExitCode = 0; - const expectedMessages = [ - `Login "${username}" (admin): OK`, - ]; + const expectedMessages = [`Login "${username}" (admin): OK`]; const expectedErrorMessages = null; try { await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -189,12 +146,7 @@ describe('test git-proxy-cli', function () { const expectedErrorMessages = null; try { await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -211,20 +163,13 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('logout should succeed when server is down (but logged in before)', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -233,12 +178,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('logout should succeed when not authenticated (server is up)', async function () { @@ -250,12 +190,7 @@ describe('test git-proxy-cli', function () { const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -264,20 +199,13 @@ describe('test git-proxy-cli', function () { it('logout shoud be successful when authenticated (server is up)', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli logout`; const expectedExitCode = 0; const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -289,18 +217,21 @@ describe('test git-proxy-cli', function () { describe('test git-proxy-cli :: authorise', function () { const pushId = `auth000000000000000000000000000000000000__${Date.now()}`; - before(async function() { + before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addGitPushToDb(pushId, TEST_REPO); - }) + }); + + after(async function () { + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); it('attempt to authorise should fail when server is down', async function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -310,12 +241,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: Authorise:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to authorise should fail when not authenticated', async function () { @@ -325,15 +251,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli authorise --id ${id}`; const expectedExitCode = 1; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Authorise: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Authorise: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to authorise should fail when not authenticated (server restarted)', async function () { @@ -344,15 +263,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli authorise --id ${id}`; const expectedExitCode = 3; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Authorise: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Authorise: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -361,23 +273,14 @@ describe('test git-proxy-cli', function () { it('attempt to authorise should fail when git push ID not found', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const id = GHOST_PUSH_ID; const cli = `npx -- @finos/git-proxy-cli authorise --id ${id}`; const expectedExitCode = 4; const expectedMessages = null; - const expectedErrorMessages = [ - `Error: Authorise: ID: '${id}': Not Found`, - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = [`Error: Authorise: ID: '${id}': Not Found`]; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -389,18 +292,21 @@ describe('test git-proxy-cli', function () { describe('test git-proxy-cli :: cancel', function () { const pushId = `cancel0000000000000000000000000000000000__${Date.now()}`; - before(async function() { + before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addGitPushToDb(pushId, TEST_REPO); - }) - + }); + + after(async function () { + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); + it('attempt to cancel should fail when server is down', async function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -410,12 +316,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: Cancel:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to cancel should fail when not authenticated', async function () { @@ -426,12 +327,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Error: Cancel: Authentication required']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to cancel should fail when not authenticated (server restarted)', async function () { @@ -442,15 +338,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli cancel --id ${id}`; const expectedExitCode = 3; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Cancel: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Cancel: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); // }); } finally { await helper.closeServer(service.httpServer); @@ -460,21 +349,14 @@ describe('test git-proxy-cli', function () { it('attempt to cancel should fail when git push ID not found', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const id = GHOST_PUSH_ID; const cli = `npx -- @finos/git-proxy-cli cancel --id ${id}`; const expectedExitCode = 4; const expectedMessages = null; const expectedErrorMessages = [`Error: Cancel: ID: '${id}': Not Found`]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -488,9 +370,7 @@ describe('test git-proxy-cli', function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -499,12 +379,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: List:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to ls should fail when not authenticated', async function () { @@ -514,31 +389,19 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Error: List: Authentication required']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to ls should fail when invalid option given', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --invalid`; const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Options:', 'Unknown argument: invalid']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -550,18 +413,21 @@ describe('test git-proxy-cli', function () { describe('test git-proxy-cli :: reject', function () { const pushId = `reject0000000000000000000000000000000000__${Date.now()}`; - before(async function() { + before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addGitPushToDb(pushId, TEST_REPO); - }) - + }); + + after(async function () { + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); + it('attempt to reject should fail when server is down', async function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -571,12 +437,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: Reject:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to reject should fail when not authenticated', async function () { @@ -587,12 +448,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Error: Reject: Authentication required']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to reject should fail when not authenticated (server restarted)', async function () { @@ -603,15 +459,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli reject --id ${id}`; const expectedExitCode = 3; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Reject: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Reject: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -620,21 +469,14 @@ describe('test git-proxy-cli', function () { it('attempt to reject should fail when git push ID not found', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const id = GHOST_PUSH_ID; const cli = `npx -- @finos/git-proxy-cli reject --id ${id}`; const expectedExitCode = 4; const expectedMessages = null; const expectedErrorMessages = [`Error: Reject: ID: '${id}': Not Found`]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -653,12 +495,16 @@ describe('test git-proxy-cli', function () { await helper.addGitPushToDb(pushId, TEST_REPO, gitAccount); }); + after(async function () { + await helper.removeUserFromDb('testuser1'); + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); + it('attempt to ls should list existing push', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --authorised false --blocked true --canceled false --rejected false`; const expectedExitCode = 0; @@ -672,12 +518,7 @@ describe('test git-proxy-cli', function () { 'rejected: false', ]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -686,20 +527,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for authorised', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --authorised true`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -708,20 +542,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for canceled', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --canceled true`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -730,20 +557,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for rejected', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --rejected true`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -752,20 +572,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for non-blocked', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --blocked false`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -774,42 +587,25 @@ describe('test git-proxy-cli', function () { it('authorise push and test if appears on authorised list', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); let cli = `npx -- @finos/git-proxy-cli ls --authorised true --canceled false --rejected false`; let expectedExitCode = 0; let expectedMessages = ['[]']; let expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli authorise --id ${pushId}`; expectedExitCode = 0; expectedMessages = [`Authorise: ID: '${pushId}': OK`]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli ls --authorised true --canceled false --rejected false`; expectedExitCode = 0; expectedMessages = [pushId, TEST_REPO]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -818,42 +614,25 @@ describe('test git-proxy-cli', function () { it('reject push and test if appears on rejected list', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); let cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled false --rejected true`; let expectedExitCode = 0; let expectedMessages = ['[]']; let expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli reject --id ${pushId}`; expectedExitCode = 0; expectedMessages = [`Reject: ID: '${pushId}': OK`]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled false --rejected true`; expectedExitCode = 0; expectedMessages = [pushId, TEST_REPO]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -862,42 +641,25 @@ describe('test git-proxy-cli', function () { it('cancel push and test if appears on canceled list', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); let cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled true --rejected false`; let expectedExitCode = 0; let expectedMessages = ['[]']; let expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli cancel --id ${pushId}`; expectedExitCode = 0; expectedMessages = [`Cancel: ID: '${pushId}': OK`]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled true --rejected false`; expectedExitCode = 0; expectedMessages = [pushId, TEST_REPO]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); await helper.removeCookiesFile(); diff --git a/packages/git-proxy-cli/test/testCliUtils.js b/packages/git-proxy-cli/test/testCliUtils.js index d9e633067..557857619 100644 --- a/packages/git-proxy-cli/test/testCliUtils.js +++ b/packages/git-proxy-cli/test/testCliUtils.js @@ -164,6 +164,14 @@ async function addRepoToDb(newRepo, debug = false) { } } +/** + * Removes a repo from the DB. + * @param {string} repoName The name of the repo to remove. + */ +async function removeRepoFromDb(repoName) { + await db.deleteRepo(repoName); +} + /** * Add a new git push record to the database. * @param {string} id The ID of the git push. @@ -205,6 +213,14 @@ async function addGitPushToDb(id, repo, user = null, debug = false) { } } +/** + * Removes a push from the DB + * @param {string} id + */ +async function removeGitPushFromDb(id) { + await db.deletePush(id); +} + /** * Add new user record to the database. * @param {string} username The user name. @@ -221,13 +237,24 @@ async function addUserToDb(username, password, email, gitAccount, admin = false, } } +/** + * Remove a user record from the database if present. + * @param {string} username The user name. + */ +async function removeUserFromDb(username) { + await db.deleteUser(username); +} + module.exports = { runCli: runCli, startServer: startServer, closeServer: closeServer, addRepoToDb: addRepoToDb, + removeRepoFromDb: removeRepoFromDb, addGitPushToDb: addGitPushToDb, + removeGitPushFromDb: removeGitPushFromDb, addUserToDb: addUserToDb, + removeUserFromDb: removeUserFromDb, createCookiesFileWithExpiredCookie: createCookiesFileWithExpiredCookie, removeCookiesFile: removeCookiesFile, }; diff --git a/src/db/file/index.ts b/src/db/file/index.ts index a31610173..6ac1c2088 100644 --- a/src/db/file/index.ts +++ b/src/db/file/index.ts @@ -6,6 +6,7 @@ export const { getPushes, writeAudit, getPush, + deletePush, authorise, cancel, reject, @@ -26,11 +27,4 @@ export const { canUserApproveRejectPushRepo, } = repo; -export const { - findUser, - findUserByOIDC, - getUsers, - createUser, - deleteUser, - updateUser, -} = users; +export const { findUser, findUserByOIDC, getUsers, createUser, deleteUser, updateUser } = users; diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index 9901940f7..2a54314ea 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -4,7 +4,7 @@ import Datastore from '@seald-io/nedb'; import { Action } from '../../proxy/actions/Action'; import { toClass } from '../helper'; import * as repo from './repo'; -import { PushQuery } from '../types' +import { PushQuery } from '../types'; if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); @@ -51,6 +51,18 @@ export const getPush = async (id: string) => { }); }; +export const deletePush = async (id: string) => { + return new Promise((resolve, reject) => { + db.remove({ id }, (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); +}; + export const writeAudit = async (action: Action) => { return new Promise((resolve, reject) => { const options = { multi: false, upsert: true }; diff --git a/src/db/index.ts b/src/db/index.ts index 0fc681058..ff1189f1b 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -63,6 +63,7 @@ export const { getPushes, writeAudit, getPush, + deletePush, findUser, findUserByOIDC, getUsers, diff --git a/src/db/mongo/index.ts b/src/db/mongo/index.ts index 11f526c2a..a6d7ce6b2 100644 --- a/src/db/mongo/index.ts +++ b/src/db/mongo/index.ts @@ -1,16 +1,15 @@ import * as helper from './helper'; import * as pushes from './pushes'; -import * as repo from './repo'; +import * as repo from './repo'; import * as users from './users'; -export const { - getSessionStore, -} = helper; +export const { getSessionStore } = helper; export const { getPushes, writeAudit, getPush, + deletePush, authorise, cancel, reject, @@ -31,10 +30,4 @@ export const { canUserApproveRejectPushRepo, } = repo; -export const { - findUser, - getUsers, - createUser, - deleteUser, - updateUser, -} = users; +export const { findUser, getUsers, createUser, deleteUser, updateUser } = users; diff --git a/src/db/mongo/pushes.ts b/src/db/mongo/pushes.ts index 609db1252..8778bdf73 100644 --- a/src/db/mongo/pushes.ts +++ b/src/db/mongo/pushes.ts @@ -13,9 +13,7 @@ const defaultPushQuery: PushQuery = { authorised: false, }; -export const getPushes = async ( - query: PushQuery = defaultPushQuery -): Promise => { +export const getPushes = async (query: PushQuery = defaultPushQuery): Promise => { return findDocuments(collectionName, query, { projection: { _id: 0, @@ -44,7 +42,12 @@ export const getPushes = async ( export const getPush = async (id: string): Promise => { const doc = await findOneDocument(collectionName, { id }); - return doc ? toClass(doc, Action.prototype) as Action : null; + return doc ? (toClass(doc, Action.prototype) as Action) : null; +}; + +export const deletePush = async function (id: string) { + const collection = await connect(collectionName); + return collection.deleteOne({ id }); }; export const writeAudit = async (action: Action): Promise => { From 5380e2f8c925be4a1283b23b51649311131d132b Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Apr 2025 15:09:15 +0100 Subject: [PATCH 08/23] test: tests for CLI set better user data and clean up afterwards --- packages/git-proxy-cli/index.js | 12 ++++--- packages/git-proxy-cli/test/testCli.test.js | 36 ++++++++++++--------- packages/git-proxy-cli/test/testCliUtils.js | 4 ++- src/service/routes/push.js | 4 +-- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/packages/git-proxy-cli/index.js b/packages/git-proxy-cli/index.js index b0090a4bf..9350e244a 100755 --- a/packages/git-proxy-cli/index.js +++ b/packages/git-proxy-cli/index.js @@ -7,7 +7,8 @@ const util = require('util'); const GIT_PROXY_COOKIE_FILE = 'git-proxy-cookie'; // GitProxy UI HOST and PORT (configurable via environment variable) -const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 8080 } = process.env; +const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 8080 } = + process.env; const baseUrl = `${uiHost}:${uiPort}`; @@ -175,7 +176,8 @@ async function authoriseGitPush(id) { if (error.response) { switch (error.response.status) { case 401: - errorMessage = 'Error: Authorise: Authentication required'; + errorMessage = + 'Error: Authorise: Authentication required (401): ' + error?.response?.data?.message; process.exitCode = 3; break; case 404: @@ -222,7 +224,8 @@ async function rejectGitPush(id) { if (error.response) { switch (error.response.status) { case 401: - errorMessage = 'Error: Reject: Authentication required'; + errorMessage = + 'Error: Reject: Authentication required (401): ' + error?.response?.data?.message; process.exitCode = 3; break; case 404: @@ -269,7 +272,8 @@ async function cancelGitPush(id) { if (error.response) { switch (error.response.status) { case 401: - errorMessage = 'Error: Cancel: Authentication required'; + errorMessage = + 'Error: Cancel: Authentication required (401): ' + error?.response?.data?.message; process.exitCode = 3; break; case 404: diff --git a/packages/git-proxy-cli/test/testCli.test.js b/packages/git-proxy-cli/test/testCli.test.js index 385e38d28..f5241074a 100644 --- a/packages/git-proxy-cli/test/testCli.test.js +++ b/packages/git-proxy-cli/test/testCli.test.js @@ -22,6 +22,11 @@ const TEST_REPO_CONFIG = { url: 'https://github.com/finos/git-proxy-test.git', }; const TEST_REPO = 'finos/git-proxy-test.git'; +//user for test cases +const TEST_USER = 'testuser'; +const TEST_PASSWORD = 'testpassword'; +const TEST_EMAIL = 'jane.doe@email.com'; +const TEST_GIT_ACCOUNT = 'testGitAccount'; describe('test git-proxy-cli', function () { // *** help *** @@ -87,16 +92,12 @@ describe('test git-proxy-cli', function () { // *** login *** describe('test git-proxy-cli :: login', function () { - const testUser = 'testuser'; - const testPassword = 'testpassword'; - const testEmail = 'jane.doe@email.com'; - before(async function () { - await helper.addUserToDb(testUser, testPassword, testEmail, 'testGitAccount'); + await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); }); after(async function () { - await helper.removeUserFromDb(testUser); + await helper.removeUserFromDb(TEST_USER); }); it('login shoud fail when server is down', async function () { @@ -140,9 +141,9 @@ describe('test git-proxy-cli', function () { }); it('login shoud be successful with valid credentials (non-admin)', async function () { - const cli = `npx -- @finos/git-proxy-cli login --username ${testUser} --password ${testPassword}`; + const cli = `npx -- @finos/git-proxy-cli login --username ${TEST_USER} --password ${TEST_PASSWORD}`; const expectedExitCode = 0; - const expectedMessages = [`Login "${testUser}" <${testEmail}>: OK`]; + const expectedMessages = [`Login "${TEST_USER}" <${TEST_EMAIL}>: OK`]; const expectedErrorMessages = null; try { await helper.startServer(service); @@ -219,11 +220,13 @@ describe('test git-proxy-cli', function () { before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); - await helper.addGitPushToDb(pushId, TEST_REPO); + await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); + await helper.addGitPushToDb(pushId, TEST_USER, TEST_EMAIL, TEST_REPO); }); after(async function () { await helper.removeGitPushFromDb(pushId); + await helper.removeUserFromDb(TEST_USER); await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); }); @@ -294,11 +297,13 @@ describe('test git-proxy-cli', function () { before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); - await helper.addGitPushToDb(pushId, TEST_REPO); + await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); + await helper.addGitPushToDb(pushId, TEST_USER, TEST_EMAIL, TEST_REPO); }); after(async function () { await helper.removeGitPushFromDb(pushId); + await helper.removeUserFromDb(TEST_USER); await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); }); @@ -415,11 +420,13 @@ describe('test git-proxy-cli', function () { before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); - await helper.addGitPushToDb(pushId, TEST_REPO); + await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); + await helper.addGitPushToDb(pushId, TEST_USER, TEST_EMAIL, TEST_REPO); }); after(async function () { await helper.removeGitPushFromDb(pushId); + await helper.removeUserFromDb(TEST_USER); await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); }); @@ -487,17 +494,16 @@ describe('test git-proxy-cli', function () { describe('test git-proxy-cli :: git push administration', function () { const pushId = `0000000000000000000000000000000000000000__${Date.now()}`; - const gitAccount = 'testGitAccount1'; before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); - await helper.addUserToDb('testuser1', 'testpassword', 'test@email.com', gitAccount); - await helper.addGitPushToDb(pushId, TEST_REPO, gitAccount); + await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); + await helper.addGitPushToDb(pushId, TEST_REPO, TEST_USER, TEST_EMAIL); }); after(async function () { - await helper.removeUserFromDb('testuser1'); await helper.removeGitPushFromDb(pushId); + await helper.removeUserFromDb(TEST_USER); await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); }); diff --git a/packages/git-proxy-cli/test/testCliUtils.js b/packages/git-proxy-cli/test/testCliUtils.js index 557857619..b6a9b5be7 100644 --- a/packages/git-proxy-cli/test/testCliUtils.js +++ b/packages/git-proxy-cli/test/testCliUtils.js @@ -177,9 +177,10 @@ async function removeRepoFromDb(repoName) { * @param {string} id The ID of the git push. * @param {string} repo The repository of the git push. * @param {string} user The user who pushed the git push. + * @param {string} userEmail The email of the user who pushed the git push. * @param {boolean} debug Flag to enable logging for debugging. */ -async function addGitPushToDb(id, repo, user = null, debug = false) { +async function addGitPushToDb(id, repo, user = null, userEmail = null, debug = false) { const action = new actions.Action( id, 'push', // type @@ -188,6 +189,7 @@ async function addGitPushToDb(id, repo, user = null, debug = false) { repo, ); action.user = user; + action.userEmail = userEmail; const step = new steps.Step( 'authBlock', // stepName false, // error diff --git a/src/service/routes/push.js b/src/service/routes/push.js index 1aea6ba64..84c096a9c 100644 --- a/src/service/routes/push.js +++ b/src/service/routes/push.js @@ -49,7 +49,7 @@ router.post('/:id/reject', async (req, res) => { if (list.length === 0) { res.status(401).send({ - message: `The user with email ${committerEmail} could not be found`, + message: `There was no registered user with the committer's email address: ${committerEmail}`, }); return; } @@ -104,7 +104,7 @@ router.post('/:id/authorise', async (req, res) => { if (list.length === 0) { res.status(401).send({ - message: `The email address ${committerEmail} could not be found`, + message: `There was no registered user with the committer's email address: ${committerEmail}`, }); return; } From a7fc824e1a809778050bc6e4c3ebda25728c9866 Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Apr 2025 16:00:07 +0100 Subject: [PATCH 09/23] test: don't clean-up test-repo as cypress tests rely on it --- test/addRepoTest.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index b86fa222c..6df809a66 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -187,8 +187,10 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); - await db.deleteRepo('test-repo'); - await db.deleteUser('u1'); - await db.deleteUser('u2'); + + // don't clean up data as cypress tests rely on it being present + // await db.deleteRepo('test-repo'); + // await db.deleteUser('u1'); + // await db.deleteUser('u2'); }); }); From 96cab2d3b1445ceefcbc9874e3a01c169d652ebd Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 15 Apr 2025 15:24:57 +0100 Subject: [PATCH 10/23] test: coverage for checkUserPushPermission --- .../push-action/checkUserPushPermission.ts | 16 ++--- test/testCheckUserPushPermission.test.js | 61 +++++++++++++++++++ 2 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 test/testCheckUserPushPermission.test.js diff --git a/src/proxy/processors/push-action/checkUserPushPermission.ts b/src/proxy/processors/push-action/checkUserPushPermission.ts index 62674b2bb..1e052e33b 100644 --- a/src/proxy/processors/push-action/checkUserPushPermission.ts +++ b/src/proxy/processors/push-action/checkUserPushPermission.ts @@ -11,17 +11,17 @@ const exec = async (req: any, action: Action): Promise => { // n.b. action.user will be set to whatever the user had set in their user.name config in their git client. // it is not necessarily the GitHub username. GitHub looks users by email address as should we const userEmail = action.userEmail; - let username = "unknown"; + let username = 'unknown'; // Find the user associated with this email address const list = await getUsers({ email: action.userEmail }); if (list.length > 1) { console.warn(`Multiple users found with email address ${userEmail}, using the first only`); - } else if (list.length == 0){ + } else if (list.length == 0) { console.error(`No user with email address ${userEmail} found`); } else { - username = list[0].username + username = list[0].username; isUserAllowed = await isUserPushAllowed(repoName, username); } @@ -30,14 +30,16 @@ const exec = async (req: any, action: Action): Promise => { if (!isUserAllowed) { console.log('User not allowed to Push'); step.error = true; - step.log(`User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`); + step.log( + `User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`, + ); console.log('setting error'); step.setError( - `Rejecting push as user ${action.user} ` + - `is not allowed to push on repo ` + - `${action.repo}`, + `Your push has been blocked (${action.userEmail} ` + + `is not allowed to push on repo ` + + `${action.repo})`, ); action.addStep(step); return action; diff --git a/test/testCheckUserPushPermission.test.js b/test/testCheckUserPushPermission.test.js new file mode 100644 index 000000000..92573ba40 --- /dev/null +++ b/test/testCheckUserPushPermission.test.js @@ -0,0 +1,61 @@ +const chai = require('chai'); +const processor = require('../src/proxy/processors/push-action/checkUserPushPermission'); +const { Action } = require('../src/proxy/actions/Action'); +const { expect } = chai; +const db = require('../src/db'); +chai.should(); + +const TEST_ORG = 'finos'; +const TEST_REPO = 'test'; +const TEST_URL = 'https://github.com/finos/user-push-perms-test.git'; +const TEST_USERNAME_1 = 'push-perms-test'; +const TEST_EMAIL_1 = 'push-perms-test@test.com'; +const TEST_USERNAME_2 = 'push-perms-test-2'; +const TEST_EMAIL_2 = 'push-perms-test-2@test.com'; +const TEST_EMAIL_3 = 'push-perms-test-3@test.com'; + +describe('CheckUserPushPermissions...', async () => { + before(async function () { + await db.deleteRepo(TEST_REPO); + await db.deleteUser(TEST_USERNAME_1); + + await db.createRepo({ + project: TEST_ORG, + name: TEST_REPO, + url: TEST_URL, + }); + await db.createUser(TEST_USERNAME_1, 'abc', TEST_EMAIL_1, TEST_USERNAME_1, false); + await db.addUserCanPush(TEST_REPO, TEST_USERNAME_1); + + await db.createUser(TEST_USERNAME_2, 'abc', TEST_EMAIL_2, TEST_USERNAME_2, false); + }); + + after(async function () { + await db.deleteRepo(TEST_REPO); + await db.deleteUser(TEST_USERNAME_1); + await db.deleteUser(TEST_USERNAME_2); + }); + + it('A committer that is approved should be allowed to push...', async () => { + const action = new Action('1', 'type', 'method', 1, TEST_ORG + '/' + TEST_REPO); + action.userEmail = TEST_EMAIL_1; + const { error } = await processor.exec(null, action); + expect(error).to.be.false; + }); + + it('A committer that is NOT approved should NOT be allowed to push...', async () => { + const action = new Action('1', 'type', 'method', 1, TEST_ORG + '/' + TEST_REPO); + action.userEmail = TEST_EMAIL_2; + const { error, errorMessage } = await processor.exec(null, action); + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('An unknown committer should NOT be allowed to push...', async () => { + const action = new Action('1', 'type', 'method', 1, TEST_ORG + '/' + TEST_REPO); + action.userEmail = TEST_EMAIL_3; + const { error, errorMessage } = await processor.exec(null, action); + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); +}); From 7326a4c5263af31298e51d422b48e090f8e9f2d8 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 15 Apr 2025 12:26:48 +0100 Subject: [PATCH 11/23] test: basic db test coverage --- test/testDb.test.js | 126 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 test/testDb.test.js diff --git a/test/testDb.test.js b/test/testDb.test.js new file mode 100644 index 000000000..896be0362 --- /dev/null +++ b/test/testDb.test.js @@ -0,0 +1,126 @@ +// This test needs to run first +const chai = require('chai'); +const db = require('../src/db'); + +const { expect } = chai; + +const TEST_REPO = { + project: 'finos', + name: 'db-test-repo', + url: 'https://github.com/finos/db-test-repo.git', +}; + +const TEST_USER = { + username: 'db-u1', + password: 'abc', + gitAccount: 'db-test-user', + email: 'db-test@test.com', + admin: true, +}; + +const TEST_PUSH = { + steps: [], + error: false, + blocked: true, + allowPush: false, + authorised: false, + canceled: true, + rejected: false, + autoApproved: false, + autoRejected: false, + commitData: [], + id: '0000000000000000000000000000000000000000__1744380874110', + type: 'push', + method: 'get', + timestamp: 1744380903338, + project: 'finos', + repoName: 'db-test-repo.git', + url: 'https://github.com/finos/db-test-repo.git', + repo: 'finos/db-test-repo.git', + user: 'db-test-user', + userEmail: 'db-test@test.com', + lastStep: null, + blockedMessage: + '\n\n\nGitProxy has received your push:\n\nhttp://localhost:8080/requests/0000000000000000000000000000000000000000__1744380874110\n\n\n', + _id: 'GIMEz8tU2KScZiTz', + attestation: null, +}; + +/** + * Clean up response data from the DB by removing an extraneous properties, + * allowing comparison with expect. + * @param {object} example Example element from which columns to retain are extracted + * @param {array} responses Array of responses to clean. + * @return {array} Array of cleaned up responses. + */ +const cleanResponseData = (example, responses) => { + const columns = Object.keys(example); + return responses.map((response) => { + const cleanResponse = {}; + columns.forEach((col) => { + cleanResponse[col] = response[col]; + }); + return cleanResponse; + }); +}; + +// Use this test as a template +describe('Database client', async () => { + before(async function () {}); + + it('should be able to create a repo', async function () { + await db.createRepo(TEST_REPO); + const repos = await db.getRepos(); + const cleanRepos = cleanResponseData(TEST_REPO, repos); + expect(cleanRepos).to.deep.include(TEST_REPO); + }); + + it('should be able to delete a repo', async function () { + await db.deleteRepo(TEST_REPO.name); + const repos = await db.getRepos(); + const cleanRepos = cleanResponseData(TEST_REPO, repos); + expect(cleanRepos).to.not.deep.include(TEST_REPO); + }); + + it('should be able to create a user', async function () { + await db.createUser( + TEST_USER.username, + TEST_USER.password, + TEST_USER.email, + TEST_USER.gitAccount, + TEST_USER.admin, + ); + const users = await db.getUsers(); + console.log('TEST USER:', JSON.stringify(TEST_USER, null, 2)); + console.log('USERS:', JSON.stringify(users, null, 2)); + // remove password as it will have been hashed + // eslint-disable-next-line no-unused-vars + const { password: _, ...TEST_USER_CLEAN } = TEST_USER; + const cleanUsers = cleanResponseData(TEST_USER_CLEAN, users); + console.log('CLEAN USERS:', JSON.stringify(cleanUsers, null, 2)); + expect(cleanUsers).to.deep.include(TEST_USER_CLEAN); + }); + + it('should be able to delete a user', async function () { + await db.deleteUser(TEST_USER.username); + const users = await db.getUsers(); + const cleanUsers = cleanResponseData(TEST_USER, users); + expect(cleanUsers).to.not.deep.include(TEST_USER); + }); + + it('should be able to create a push', async function () { + await db.writeAudit(TEST_PUSH); + const pushes = await db.getPushes(); + const cleanPushes = cleanResponseData(TEST_PUSH, pushes); + expect(cleanPushes).to.deep.include(TEST_PUSH); + }); + + it('should be able to delete a push', async function () { + await db.deletePush(TEST_PUSH.id); + const pushes = await db.getPushes(); + const cleanPushes = cleanResponseData(TEST_PUSH, pushes); + expect(cleanPushes).to.not.deep.include(TEST_PUSH); + }); + + after(async function () {}); +}); From fbc24064eb245c5196b2bc2031284d518617fb00 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 15 Apr 2025 18:38:34 +0100 Subject: [PATCH 12/23] test: coverage for push API calls --- src/service/routes/push.js | 5 +- test/testPush.test.js | 286 +++++++++++++++++++++++++++++++++++-- 2 files changed, 277 insertions(+), 14 deletions(-) diff --git a/src/service/routes/push.js b/src/service/routes/push.js index 84c096a9c..dd746a11f 100644 --- a/src/service/routes/push.js +++ b/src/service/routes/push.js @@ -36,16 +36,13 @@ router.get('/:id', async (req, res) => { router.post('/:id/reject', async (req, res) => { if (req.user) { const id = req.params.id; - console.log({ id }); // Get the push request const push = await db.getPush(id); - console.log({ push }); // Get the committer of the push via their email const committerEmail = push.userEmail; const list = await db.getUsers({ email: committerEmail }); - console.log({ list }); if (list.length === 0) { res.status(401).send({ @@ -86,6 +83,8 @@ router.post('/:id/authorise', async (req, res) => { const questions = req.body.params?.attestation; console.log({ questions }); + // TODO: compare attestation to configuration and ensure all questions are answered + // - we shouldn't go on the definition in the request! const attestationComplete = questions?.every((question) => !!question.checked); console.log({ attestationComplete }); diff --git a/test/testPush.test.js b/test/testPush.test.js index f4e09a4a5..4681f3de5 100644 --- a/test/testPush.test.js +++ b/test/testPush.test.js @@ -8,32 +8,120 @@ chai.use(chaiHttp); chai.should(); const expect = chai.expect; +// dummy repo +const TEST_ORG = 'finos'; +const TEST_REPO = 'test-push'; +const TEST_URL = 'https://github.com/finos/test-push.git'; +// approver user +const TEST_USERNAME_1 = 'push-test'; +const TEST_EMAIL_1 = 'push-test@test.com'; +const TEST_PASSWORD_1 = 'test1234'; +// committer user +const TEST_USERNAME_2 = 'push-test-2'; +const TEST_EMAIL_2 = 'push-test-2@test.com'; +const TEST_PASSWORD_2 = 'test5678'; +// unknown user +const TEST_USERNAME_3 = 'push-test-3'; +const TEST_EMAIL_3 = 'push-test-3@test.com'; + +const TEST_PUSH = { + steps: [], + error: false, + blocked: false, + allowPush: false, + authorised: false, + canceled: false, + rejected: false, + autoApproved: false, + autoRejected: false, + commitData: [], + id: '0000000000000000000000000000000000000000__1744380874110', + type: 'push', + method: 'get', + timestamp: 1744380903338, + project: TEST_ORG, + repoName: TEST_REPO + '.git', + url: TEST_REPO, + repo: TEST_ORG + '/' + TEST_REPO + '.git', + user: TEST_USERNAME_2, + userEmail: TEST_EMAIL_2, + lastStep: null, + blockedMessage: + '\n\n\nGitProxy has received your push:\n\nhttp://localhost:8080/requests/0000000000000000000000000000000000000000__1744380874110\n\n\n', + _id: 'GIMEz8tU2KScZiTz', + attestation: null, +}; + describe('auth', async () => { let app; let cookie; - before(async function () { - app = await service.start(); - await db.deleteUser('login-test-user'); + const setCookie = function (res) { + res.headers['set-cookie'].forEach((x) => { + if (x.startsWith('connect')) { + const value = x.split(';')[0]; + cookie = value; + } + }); + }; + const login = async function (username, password) { + console.log(`logging in as ${username}...`); const res = await chai.request(app).post('/api/auth/login').send({ - username: 'admin', - password: 'admin', + username: username, + password: password, }); - + res.should.have.status(200); expect(res).to.have.cookie('connect.sid'); + setCookie(res); + }; + + const loginAsApprover = () => login(TEST_USERNAME_1, TEST_PASSWORD_1); + const loginAsCommitter = () => login(TEST_USERNAME_2, TEST_PASSWORD_2); + const loginAsAdmin = () => login('admin', 'admin'); + + const logout = async function () { + const res = await chai.request(app).post('/api/auth/logout').set('Cookie', `${cookie}`); res.should.have.status(200); + cookie = null; + }; - // Get the connect cooie - res.headers['set-cookie'].forEach((x) => { - if (x.startsWith('connect')) { - cookie = x.split(';')[0]; - } + before(async function () { + app = await service.start(); + await loginAsAdmin(); + + // set up a repo, user and push to test against + await db.deleteRepo(TEST_REPO); + await db.deleteUser(TEST_USERNAME_1); + await db.createRepo({ + project: TEST_ORG, + name: TEST_REPO, + url: TEST_URL, }); + + // Create a new user for the approver + console.log('creating approver'); + await db.createUser(TEST_USERNAME_1, TEST_PASSWORD_1, TEST_EMAIL_1, TEST_USERNAME_1, false); + await db.addUserCanAuthorise(TEST_REPO, TEST_USERNAME_1); + + // create a new user for the committer + console.log('creating committer'); + await db.createUser(TEST_USERNAME_2, TEST_PASSWORD_2, TEST_EMAIL_2, TEST_USERNAME_2, false); + await db.addUserCanPush(TEST_REPO, TEST_USERNAME_2); + + // logout of admin account + await logout(); }); describe('test push API', async function () { + afterEach(async function () { + await db.deletePush(TEST_PUSH.id); + await logout(); + }); + it('should get 404 for unknown push', async function () { + await loginAsApprover(); + const commitId = '0000000000000000000000000000000000000000__79b4d8953cbc324bcc1eb53d6412ff89666c241f'; // eslint-disable-line max-len const res = await chai @@ -42,6 +130,177 @@ describe('auth', async () => { .set('Cookie', `${cookie}`); res.should.have.status(404); }); + + it('should allow an authorizer to approve a push', async function () { + await db.writeAudit(TEST_PUSH); + await loginAsApprover(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) + .set('Cookie', `${cookie}`) + .set('content-type', 'application/x-www-form-urlencoded') + .send({ + params: { + attestation: [ + { + label: 'I am happy for this to be pushed to the upstream repository', + tooltip: { + text: 'Are you happy for this contribution to be pushed upstream?', + links: [], + }, + checked: true, + }, + ], + }, + }); + res.should.have.status(200); + }); + + it('should NOT allow an authorizer to approve if attestation is incomplete', async function () { + // make the approver also the committer + const testPush = { ...TEST_PUSH }; + testPush.user = TEST_USERNAME_1; + testPush.userEmail = TEST_EMAIL_1; + await db.writeAudit(testPush); + await loginAsApprover(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) + .set('Cookie', `${cookie}`) + .set('content-type', 'application/x-www-form-urlencoded') + .send({ + params: { + attestation: [ + { + label: 'I am happy for this to be pushed to the upstream repository', + tooltip: { + text: 'Are you happy for this contribution to be pushed upstream?', + links: [], + }, + checked: false, + }, + ], + }, + }); + res.should.have.status(401); + }); + + it('should NOT allow an authorizer to approve if committer is unknown', async function () { + // make the approver also the committer + const testPush = { ...TEST_PUSH }; + testPush.user = TEST_USERNAME_3; + testPush.userEmail = TEST_EMAIL_3; + await db.writeAudit(testPush); + await loginAsApprover(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) + .set('Cookie', `${cookie}`) + .set('content-type', 'application/x-www-form-urlencoded') + .send({ + params: { + attestation: [ + { + label: 'I am happy for this to be pushed to the upstream repository', + tooltip: { + text: 'Are you happy for this contribution to be pushed upstream?', + links: [], + }, + checked: true, + }, + ], + }, + }); + res.should.have.status(401); + }); + + it('should NOT allow an authorizer to approve their own push', async function () { + // make the approver also the committer + const testPush = { ...TEST_PUSH }; + testPush.user = TEST_USERNAME_1; + testPush.userEmail = TEST_EMAIL_1; + await db.writeAudit(testPush); + await loginAsApprover(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) + .set('Cookie', `${cookie}`) + .set('content-type', 'application/x-www-form-urlencoded') + .send({ + params: { + attestation: [ + { + label: 'I am happy for this to be pushed to the upstream repository', + tooltip: { + text: 'Are you happy for this contribution to be pushed upstream?', + links: [], + }, + checked: true, + }, + ], + }, + }); + res.should.have.status(401); + }); + + it('should NOT allow a non-authorizer to approve a push', async function () { + await db.writeAudit(TEST_PUSH); + await loginAsCommitter(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) + .set('Cookie', `${cookie}`) + .set('content-type', 'application/x-www-form-urlencoded') + .send({ + params: { + attestation: [ + { + label: 'I am happy for this to be pushed to the upstream repository', + tooltip: { + text: 'Are you happy for this contribution to be pushed upstream?', + links: [], + }, + checked: true, + }, + ], + }, + }); + res.should.have.status(401); + }); + + it('should allow an authorizer to reject a push', async function () { + await db.writeAudit(TEST_PUSH); + await loginAsApprover(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/reject`) + .set('Cookie', `${cookie}`); + res.should.have.status(200); + }); + + it('should NOT allow an authorizer to reject their own push', async function () { + // make the approver also the committer + const testPush = { ...TEST_PUSH }; + testPush.user = TEST_USERNAME_1; + testPush.userEmail = TEST_EMAIL_1; + await db.writeAudit(testPush); + await loginAsApprover(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/reject`) + .set('Cookie', `${cookie}`); + res.should.have.status(401); + }); + + it('should NOT allow a non-authorizer to reject a push', async function () { + await db.writeAudit(TEST_PUSH); + await loginAsCommitter(); + const res = await chai + .request(app) + .post(`/api/v1/push/${TEST_PUSH.id}/reject`) + .set('Cookie', `${cookie}`); + res.should.have.status(401); + }); }); after(async function () { @@ -49,5 +308,10 @@ describe('auth', async () => { res.should.have.status(200); await service.httpServer.close(); + + await db.deleteRepo(TEST_REPO); + await db.deleteUser(TEST_USERNAME_1); + await db.deleteUser(TEST_USERNAME_2); + await db.deletePush(TEST_PUSH.id); }); }); From e8d1fd2f3e1d2ab06f1baede2621eee83a9ea43a Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 10 Jun 2025 18:33:53 +0100 Subject: [PATCH 13/23] test: fix test failures caused by conflicts with main --- .../push-action/checkUserPushPermission.ts | 14 ++++- test/ConfigLoader.test.js | 6 +- .../checkUserPushPermission.test.js | 59 +++++++++++-------- test/testDb.test.js | 14 ++--- 4 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/proxy/processors/push-action/checkUserPushPermission.ts b/src/proxy/processors/push-action/checkUserPushPermission.ts index 1e052e33b..7fdfa233c 100644 --- a/src/proxy/processors/push-action/checkUserPushPermission.ts +++ b/src/proxy/processors/push-action/checkUserPushPermission.ts @@ -17,7 +17,17 @@ const exec = async (req: any, action: Action): Promise => { const list = await getUsers({ email: action.userEmail }); if (list.length > 1) { - console.warn(`Multiple users found with email address ${userEmail}, using the first only`); + console.error(`Multiple users found with email address ${userEmail}, ending`); + step.error = true; + step.log( + `Multiple Users have email <${userEmail}> so we cannot uniquely identify the user, ending`, + ); + + step.setError( + `Your push has been blocked (there are multiple users with email ${action.userEmail})`, + ); + action.addStep(step); + return action; } else if (list.length == 0) { console.error(`No user with email address ${userEmail} found`); } else { @@ -34,8 +44,6 @@ const exec = async (req: any, action: Action): Promise => { `User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`, ); - console.log('setting error'); - step.setError( `Your push has been blocked (${action.userEmail} ` + `is not allowed to push on repo ` + diff --git a/test/ConfigLoader.test.js b/test/ConfigLoader.test.js index df91c49d5..5a4860170 100644 --- a/test/ConfigLoader.test.js +++ b/test/ConfigLoader.test.js @@ -38,6 +38,10 @@ describe('ConfigLoader', () => { }); }); + after(() => { + // restore default config + }); + describe('loadFromFile', () => { it('should load configuration from file', async () => { const testConfig = { @@ -251,7 +255,6 @@ describe('ConfigLoader', () => { configLoader = new ConfigLoader(mockConfig); configLoader.reloadTimer = setInterval(() => {}, 1000); await configLoader.start(); - expect(configLoader.reloadTimer).to.be.null; }); @@ -297,7 +300,6 @@ describe('ConfigLoader', () => { configLoader = new ConfigLoader(mockConfig); configLoader.reloadTimer = setInterval(() => {}, 1000); expect(configLoader.reloadTimer).to.not.be.null; - await configLoader.stop(); expect(configLoader.reloadTimer).to.be.null; }); diff --git a/test/processors/checkUserPushPermission.test.js b/test/processors/checkUserPushPermission.test.js index b140d383b..2f02f77a5 100644 --- a/test/processors/checkUserPushPermission.test.js +++ b/test/processors/checkUserPushPermission.test.js @@ -18,12 +18,15 @@ describe('checkUserPushPermission', () => { getUsersStub = sinon.stub(); isUserPushAllowedStub = sinon.stub(); - const checkUserPushPermission = proxyquire('../../src/proxy/processors/push-action/checkUserPushPermission', { - '../../../db': { - getUsers: getUsersStub, - isUserPushAllowed: isUserPushAllowedStub - } - }); + const checkUserPushPermission = proxyquire( + '../../src/proxy/processors/push-action/checkUserPushPermission', + { + '../../../db': { + getUsers: getUsersStub, + isUserPushAllowed: isUserPushAllowedStub, + }, + }, + ); exec = checkUserPushPermission.exec; }); @@ -39,40 +42,43 @@ describe('checkUserPushPermission', () => { beforeEach(() => { req = {}; - action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo.git' - ); + action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo.git'); action.user = 'git-user'; + action.userEmail = 'db-user@test.com'; stepSpy = sinon.spy(Step.prototype, 'log'); }); it('should allow push when user has permission', async () => { - getUsersStub.resolves([{ username: 'db-user', gitAccount: 'git-user' }]); + getUsersStub.resolves([ + { username: 'db-user', email: 'db-user@test.com', gitAccount: 'git-user' }, + ]); isUserPushAllowedStub.resolves(true); const result = await exec(req, action); expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.false; - expect(stepSpy.calledWith('User db-user is allowed to push on repo test/repo.git')).to.be.true; - expect(logStub.calledWith('User db-user permission on Repo repo : true')).to.be.true; + expect(stepSpy.lastCall.args[0]).to.equal( + 'User db-user is allowed to push on repo test/repo.git', + ); + expect(logStub.lastCall.args[0]).to.equal( + 'User db-user permission on Repo repo : true', + ); }); it('should reject push when user has no permission', async () => { - getUsersStub.resolves([{ username: 'db-user', gitAccount: 'git-user' }]); + getUsersStub.resolves([ + { username: 'db-user', email: 'db-user@test.com', gitAccount: 'git-user' }, + ]); isUserPushAllowedStub.resolves(false); const result = await exec(req, action); expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith('User db-user is not allowed to push on repo test/repo.git, ending')).to.be.true; - expect(result.steps[0].errorMessage).to.include('Rejecting push as user git-user'); - expect(logStub.calledWith('User not allowed to Push')).to.be.true; + expect(result.steps[0].errorMessage).to.equal( + 'Your push has been blocked (db-user@test.com is not allowed to push on repo test/repo.git)', + ); }); it('should reject push when no user found for git account', async () => { @@ -82,21 +88,22 @@ describe('checkUserPushPermission', () => { expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith('User git-user is not allowed to push on repo test/repo.git, ending')).to.be.true; - expect(result.steps[0].errorMessage).to.include('Rejecting push as user git-user'); + expect(result.steps[0].errorMessage).to.include('Your push has been blocked'); }); - it('should handle multiple users for git account by rejecting push', async () => { + it('should handle multiple users for git account by rejecting the push', async () => { getUsersStub.resolves([ - { username: 'user1', gitAccount: 'git-user' }, - { username: 'user2', gitAccount: 'git-user' } + { username: 'user1', email: 'db-user@test.com', gitAccount: 'git-user' }, + { username: 'user2', email: 'db-user@test.com', gitAccount: 'git-user' }, ]); const result = await exec(req, action); expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(logStub.calledWith('Users for this git account: [{"username":"user1","gitAccount":"git-user"},{"username":"user2","gitAccount":"git-user"}]')).to.be.true; + expect(result.steps[0].errorMessage).to.equal( + 'Your push has been blocked (there are multiple users with email db-user@test.com)', + ); }); }); }); diff --git a/test/testDb.test.js b/test/testDb.test.js index a6de33c5c..0deb6d931 100644 --- a/test/testDb.test.js +++ b/test/testDb.test.js @@ -86,13 +86,6 @@ describe('Database clients', async () => { expect(cleanRepos).to.deep.include(TEST_REPO); }); - it('should be able to delete a repo', async function () { - await db.deleteRepo(TEST_REPO.name); - const repos = await db.getRepos(); - const cleanRepos = cleanResponseData(TEST_REPO, repos); - expect(cleanRepos).to.not.deep.include(TEST_REPO); - }); - it('should be able to filter repos', async function () { // uppercase the filter value to confirm db client is lowercasing inputs const repos = await db.getRepos({ name: TEST_REPO.name.toUpperCase() }); @@ -116,6 +109,13 @@ describe('Database clients', async () => { expect(cleanRepo).to.eql(TEST_REPO); }); + it('should be able to delete a repo', async function () { + await db.deleteRepo(TEST_REPO.name); + const repos = await db.getRepos(); + const cleanRepos = cleanResponseData(TEST_REPO, repos); + expect(cleanRepos).to.not.deep.include(TEST_REPO); + }); + it('should NOT be able to create a repo with blank project, name or url', async function () { // test with a null value let threwError = false; From 7e36e2176aa0ab71634ee07f4748f72d45f6f88f Mon Sep 17 00:00:00 2001 From: kriswest Date: Mon, 16 Jun 2025 15:59:59 +0100 Subject: [PATCH 14/23] chore: prettier --- src/config/index.ts | 8 +- src/proxy/processors/push-action/parsePush.ts | 6 +- src/routes.jsx | 52 +++---- src/service/passport/index.js | 6 +- src/service/passport/jwtAuthHandler.js | 139 +++++++++--------- src/service/passport/local.js | 18 +-- src/ui/components/RouteGuard/RouteGuard.tsx | 4 +- src/ui/services/config.js | 7 +- .../RepoList/Components/RepoOverview.jsx | 4 +- test/plugin/plugin.test.js | 46 +++--- test/processors/blockForAuth.test.js | 7 +- test/processors/checkAuthorEmails.test.js | 91 ++++++------ test/processors/checkCommitMessages.test.js | 70 +++++---- test/processors/checkIfWaitingAuth.test.js | 41 ++---- test/processors/getDiff.test.js | 66 ++------- test/processors/gitLeaks.test.js | 110 ++++++++------ test/processors/writePack.test.js | 16 +- test/testAuthMethods.test.js | 8 +- test/testConfig.test.js | 14 +- 19 files changed, 325 insertions(+), 388 deletions(-) diff --git a/src/config/index.ts b/src/config/index.ts index 63174a296..00b00c8e3 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -92,7 +92,7 @@ export const getDatabase = () => { /** * Get the list of enabled authentication methods - * + * * At least one authentication method must be enabled. * @return {Array} List of enabled authentication methods */ @@ -104,7 +104,7 @@ export const getAuthMethods = () => { const enabledAuthMethods = _authentication.filter((auth) => auth.enabled); if (enabledAuthMethods.length === 0) { - throw new Error("No authentication method enabled"); + throw new Error('No authentication method enabled'); } return enabledAuthMethods; @@ -112,7 +112,7 @@ export const getAuthMethods = () => { /** * Get the list of enabled authentication methods for API endpoints - * + * * If no API authentication methods are enabled, all endpoints are public. * @return {Array} List of enabled authentication methods */ @@ -121,7 +121,7 @@ export const getAPIAuthMethods = () => { _apiAuthentication = _userSettings.apiAuthentication; } - const enabledAuthMethods = _apiAuthentication.filter(auth => auth.enabled); + const enabledAuthMethods = _apiAuthentication.filter((auth) => auth.enabled); return enabledAuthMethods; }; diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index ae67c1ef2..4c7fbad22 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -35,7 +35,7 @@ async function exec(req: any, action: Action): Promise { action.commitFrom = action.commitData[action.commitData.length - 1].parent; } - const {committer, committerEmail} = action.commitData[action.commitData.length - 1]; + const { committer, committerEmail } = action.commitData[action.commitData.length - 1]; console.log(`Push Request received from user ${committer} with email ${committerEmail}`); action.user = committer; action.userEmail = committerEmail; @@ -123,7 +123,7 @@ const getCommitData = (contents: CommitContent[]) => { commitTimestamp, message, authorEmail, - committerEmail + committerEmail, }); if ( @@ -146,7 +146,7 @@ const getCommitData = (contents: CommitContent[]) => { commitTimestamp, message, authorEmail, - committerEmail + committerEmail, }; }) .value(); diff --git a/src/routes.jsx b/src/routes.jsx index c409f599e..bf500874e 100644 --- a/src/routes.jsx +++ b/src/routes.jsx @@ -35,11 +35,7 @@ const dashboardRoutes = [ path: '/repo', name: 'Repositories', icon: RepoIcon, - component: (props) => - , + component: (props) => , layout: '/dashboard', visible: true, }, @@ -47,11 +43,9 @@ const dashboardRoutes = [ path: '/repo/:id', name: 'Repo Details', icon: Person, - component: (props) => - , + component: (props) => ( + + ), layout: '/dashboard', visible: false, }, @@ -59,11 +53,9 @@ const dashboardRoutes = [ path: '/push', name: 'Dashboard', icon: Dashboard, - component: (props) => - , + component: (props) => ( + + ), layout: '/dashboard', visible: true, }, @@ -71,11 +63,9 @@ const dashboardRoutes = [ path: '/push/:id', name: 'Open Push Requests', icon: Person, - component: (props) => - , + component: (props) => ( + + ), layout: '/dashboard', visible: false, }, @@ -83,11 +73,7 @@ const dashboardRoutes = [ path: '/profile', name: 'My Account', icon: AccountCircle, - component: (props) => - , + component: (props) => , layout: '/dashboard', visible: true, }, @@ -95,11 +81,9 @@ const dashboardRoutes = [ path: '/admin/user', name: 'Users', icon: Group, - component: (props) => - , + component: (props) => ( + + ), layout: '/dashboard', visible: true, }, @@ -107,11 +91,9 @@ const dashboardRoutes = [ path: '/admin/user/:id', name: 'User', icon: Person, - component: (props) => - , + component: (props) => ( + + ), layout: '/dashboard', visible: false, }, diff --git a/src/service/passport/index.js b/src/service/passport/index.js index b0712d510..5943ddb1e 100644 --- a/src/service/passport/index.js +++ b/src/service/passport/index.js @@ -1,4 +1,4 @@ -const passport = require("passport"); +const passport = require('passport'); const local = require('./local'); const activeDirectory = require('./activeDirectory'); const oidc = require('./oidc'); @@ -20,12 +20,12 @@ const configure = async () => { for (const auth of authMethods) { const strategy = authStrategies[auth.type.toLowerCase()]; - if (strategy && typeof strategy.configure === "function") { + if (strategy && typeof strategy.configure === 'function') { await strategy.configure(passport); } } - if (authMethods.some(auth => auth.type.toLowerCase() === "local")) { + if (authMethods.some((auth) => auth.type.toLowerCase() === 'local')) { await local.createDefaultAdmin(); } diff --git a/src/service/passport/jwtAuthHandler.js b/src/service/passport/jwtAuthHandler.js index da9e3bc47..8f372fd28 100644 --- a/src/service/passport/jwtAuthHandler.js +++ b/src/service/passport/jwtAuthHandler.js @@ -1,6 +1,6 @@ -const axios = require("axios"); -const jwt = require("jsonwebtoken"); -const jwkToPem = require("jwk-to-pem"); +const axios = require('axios'); +const jwt = require('jsonwebtoken'); +const jwkToPem = require('jwk-to-pem'); const config = require('../../config'); /** @@ -10,14 +10,14 @@ const config = require('../../config'); */ async function getJwks(authorityUrl) { try { - const { data } = await axios.get(`${authorityUrl}/.well-known/openid-configuration`); - const jwksUri = data.jwks_uri; + const { data } = await axios.get(`${authorityUrl}/.well-known/openid-configuration`); + const jwksUri = data.jwks_uri; - const { data: jwks } = await axios.get(jwksUri); - return jwks.keys; + const { data: jwks } = await axios.get(jwksUri); + return jwks.keys; } catch (error) { - console.error("Error fetching JWKS:", error); - throw new Error("Failed to fetch JWKS"); + console.error('Error fetching JWKS:', error); + throw new Error('Failed to fetch JWKS'); } } @@ -25,82 +25,87 @@ async function getJwks(authorityUrl) { * Validate a JWT token using the OIDC configuration. * @param {*} token the JWT token * @param {*} authorityUrl the OIDC authority URL - * @param {*} clientID the OIDC client ID + * @param {*} clientID the OIDC client ID * @param {*} expectedAudience the expected audience for the token * @return {Promise} the verified payload or an error */ async function validateJwt(token, authorityUrl, clientID, expectedAudience) { try { - const jwks = await getJwks(authorityUrl); + const jwks = await getJwks(authorityUrl); - const decodedHeader = await jwt.decode(token, { complete: true }); - if (!decodedHeader || !decodedHeader.header || !decodedHeader.header.kid) { - throw new Error("Invalid JWT: Missing key ID (kid)"); - } + const decodedHeader = await jwt.decode(token, { complete: true }); + if (!decodedHeader || !decodedHeader.header || !decodedHeader.header.kid) { + throw new Error('Invalid JWT: Missing key ID (kid)'); + } - const { kid } = decodedHeader.header; - const jwk = jwks.find((key) => key.kid === kid); - if (!jwk) { - throw new Error("No matching key found in JWKS"); - } + const { kid } = decodedHeader.header; + const jwk = jwks.find((key) => key.kid === kid); + if (!jwk) { + throw new Error('No matching key found in JWKS'); + } - const pubKey = jwkToPem(jwk); + const pubKey = jwkToPem(jwk); - const verifiedPayload = jwt.verify(token, pubKey, { - algorithms: ["RS256"], - issuer: authorityUrl, - audience: expectedAudience, - }); + const verifiedPayload = jwt.verify(token, pubKey, { + algorithms: ['RS256'], + issuer: authorityUrl, + audience: expectedAudience, + }); - if (verifiedPayload.azp !== clientID) { - throw new Error("JWT client ID does not match"); - } + if (verifiedPayload.azp !== clientID) { + throw new Error('JWT client ID does not match'); + } - return { verifiedPayload }; + return { verifiedPayload }; } catch (error) { - const errorMessage = `JWT validation failed: ${error.message}\n`; - console.error(errorMessage); - return { error: errorMessage }; + const errorMessage = `JWT validation failed: ${error.message}\n`; + console.error(errorMessage); + return { error: errorMessage }; } } const jwtAuthHandler = () => { return async (req, res, next) => { const apiAuthMethods = config.getAPIAuthMethods(); - const jwtAuthMethod = apiAuthMethods.find((method) => method.type.toLowerCase() === "jwt"); - if (!jwtAuthMethod) { - return next(); - } - - if (req.isAuthenticated()) { - return next(); - } - - const token = req.header("Authorization"); - if (!token) { - return res.status(401).send("No token provided\n"); - } - - const { clientID, authorityURL, expectedAudience } = jwtAuthMethod.jwtConfig; - const audience = expectedAudience || clientID; - - if (!authorityURL) { - return res.status(500).send("OIDC authority URL is not configured\n"); - } - - if (!clientID) { - return res.status(500).send("OIDC client ID is not configured\n"); - } - - const tokenParts = token.split(" "); - const { verifiedPayload, error } = await validateJwt(tokenParts[1], authorityURL, audience, clientID); - if (error) { - return res.status(401).send(error); - } - - req.user = verifiedPayload; + const jwtAuthMethod = apiAuthMethods.find((method) => method.type.toLowerCase() === 'jwt'); + if (!jwtAuthMethod) { return next(); - } -} + } + + if (req.isAuthenticated()) { + return next(); + } + + const token = req.header('Authorization'); + if (!token) { + return res.status(401).send('No token provided\n'); + } + + const { clientID, authorityURL, expectedAudience } = jwtAuthMethod.jwtConfig; + const audience = expectedAudience || clientID; + + if (!authorityURL) { + return res.status(500).send('OIDC authority URL is not configured\n'); + } + + if (!clientID) { + return res.status(500).send('OIDC client ID is not configured\n'); + } + + const tokenParts = token.split(' '); + const { verifiedPayload, error } = await validateJwt( + tokenParts[1], + authorityURL, + audience, + clientID, + ); + if (error) { + return res.status(401).send(error); + } + + req.user = verifiedPayload; + return next(); + }; +}; module.exports = jwtAuthHandler; diff --git a/src/service/passport/local.js b/src/service/passport/local.js index 579d47234..e453f2c41 100644 --- a/src/service/passport/local.js +++ b/src/service/passport/local.js @@ -1,8 +1,8 @@ -const bcrypt = require("bcryptjs"); -const LocalStrategy = require("passport-local").Strategy; -const db = require("../../db"); +const bcrypt = require('bcryptjs'); +const LocalStrategy = require('passport-local').Strategy; +const db = require('../../db'); -const type = "local"; +const type = 'local'; const configure = async (passport) => { passport.use( @@ -10,19 +10,19 @@ const configure = async (passport) => { try { const user = await db.findUser(username); if (!user) { - return done(null, false, { message: "Incorrect username." }); + return done(null, false, { message: 'Incorrect username.' }); } const passwordCorrect = await bcrypt.compare(password, user.password); if (!passwordCorrect) { - return done(null, false, { message: "Incorrect password." }); + return done(null, false, { message: 'Incorrect password.' }); } return done(null, user); } catch (err) { return done(err); } - }) + }), ); passport.serializeUser((user, done) => { @@ -45,9 +45,9 @@ const configure = async (passport) => { * Create the default admin user if it doesn't exist */ const createDefaultAdmin = async () => { - const admin = await db.findUser("admin"); + const admin = await db.findUser('admin'); if (!admin) { - await db.createUser("admin", "admin", "admin@place.com", "none", true); + await db.createUser('admin', 'admin', 'admin@place.com', 'none', true); } }; diff --git a/src/ui/components/RouteGuard/RouteGuard.tsx b/src/ui/components/RouteGuard/RouteGuard.tsx index a729b1660..4efb2c3c1 100644 --- a/src/ui/components/RouteGuard/RouteGuard.tsx +++ b/src/ui/components/RouteGuard/RouteGuard.tsx @@ -46,11 +46,11 @@ const RouteGuard = ({ component: Component, fullRoutePath }: RouteGuardProps) => } if (loginRequired && !user) { - return ; + return ; } if (adminOnly && !user?.admin) { - return ; + return ; } return ; diff --git a/src/ui/services/config.js b/src/ui/services/config.js index 5536e4a35..286aab9a0 100644 --- a/src/ui/services/config.js +++ b/src/ui/services/config.js @@ -32,9 +32,4 @@ const getUIRouteAuth = async (setData) => { }); }; -export { - getAttestationConfig, - getURLShortener, - getEmailContact, - getUIRouteAuth, -}; +export { getAttestationConfig, getURLShortener, getEmailContact, getUIRouteAuth }; diff --git a/src/ui/views/RepoList/Components/RepoOverview.jsx b/src/ui/views/RepoList/Components/RepoOverview.jsx index 826f78c97..56449683c 100644 --- a/src/ui/views/RepoList/Components/RepoOverview.jsx +++ b/src/ui/views/RepoList/Components/RepoOverview.jsx @@ -585,7 +585,9 @@ export default function Repositories(props) { setGitHub(res.data); }) .catch((error) => { - setErrorMessage(`Error fetching GitHub repository ${props.data.project}/${props.data.name}: ${error}`); + setErrorMessage( + `Error fetching GitHub repository ${props.data.project}/${props.data.name}: ${error}`, + ); setSnackbarOpen(true); }); }; diff --git a/test/plugin/plugin.test.js b/test/plugin/plugin.test.js index cee46699e..e8efdc642 100644 --- a/test/plugin/plugin.test.js +++ b/test/plugin/plugin.test.js @@ -23,27 +23,33 @@ describe('loading plugins from packages', function () { spawnSync('npm', ['install'], { cwd: testPackagePath, timeout: 5000 }); }); - it.skip('should load plugins that are the default export (module.exports = pluginObj)', async function () { - const loader = new PluginLoader([join(testPackagePath, 'default-export.js')]); - await loader.load(); - expect(loader.pushPlugins.length).to.equal(1); - expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p))).to.be.true; - expect(loader.pushPlugins[0]).to.be.an.instanceOf(PushActionPlugin); - }).timeout(10000); + it.skip( + 'should load plugins that are the default export (module.exports = pluginObj)', + async function () { + const loader = new PluginLoader([join(testPackagePath, 'default-export.js')]); + await loader.load(); + expect(loader.pushPlugins.length).to.equal(1); + expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p))).to.be.true; + expect(loader.pushPlugins[0]).to.be.an.instanceOf(PushActionPlugin); + }, + ).timeout(10000); - it.skip('should load multiple plugins from a module that match the plugin class (module.exports = { pluginFoo, pluginBar })', async function () { - const loader = new PluginLoader([join(testPackagePath, 'multiple-export.js')]); - await loader.load(); - expect(loader.pushPlugins.length).to.equal(1); - expect(loader.pullPlugins.length).to.equal(1); - expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p))).to.be.true; - expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p, 'isGitProxyPushActionPlugin'))).to - .be.true; - expect(loader.pullPlugins.every((p) => isCompatiblePlugin(p, 'isGitProxyPullActionPlugin'))).to - .be.true; - expect(loader.pushPlugins[0]).to.be.instanceOf(PushActionPlugin); - expect(loader.pullPlugins[0]).to.be.instanceOf(PullActionPlugin); - }).timeout(10000); + it.skip( + 'should load multiple plugins from a module that match the plugin class (module.exports = { pluginFoo, pluginBar })', + async function () { + const loader = new PluginLoader([join(testPackagePath, 'multiple-export.js')]); + await loader.load(); + expect(loader.pushPlugins.length).to.equal(1); + expect(loader.pullPlugins.length).to.equal(1); + expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p))).to.be.true; + expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p, 'isGitProxyPushActionPlugin'))) + .to.be.true; + expect(loader.pullPlugins.every((p) => isCompatiblePlugin(p, 'isGitProxyPullActionPlugin'))) + .to.be.true; + expect(loader.pushPlugins[0]).to.be.instanceOf(PushActionPlugin); + expect(loader.pullPlugins[0]).to.be.instanceOf(PullActionPlugin); + }, + ).timeout(10000); it.skip('should load plugins that are subclassed from plugin classes', async function () { const loader = new PluginLoader([join(testPackagePath, 'subclass.js')]); diff --git a/test/processors/blockForAuth.test.js b/test/processors/blockForAuth.test.js index f566f1b2f..e242cb247 100644 --- a/test/processors/blockForAuth.test.js +++ b/test/processors/blockForAuth.test.js @@ -17,12 +17,12 @@ describe('blockForAuth', () => { beforeEach(() => { req = { protocol: 'https', - headers: { host: 'example.com' } + headers: { host: 'example.com' }, }; action = { id: 'push_123', - addStep: sinon.stub() + addStep: sinon.stub(), }; stepInstance = new Step('temp'); @@ -34,7 +34,7 @@ describe('blockForAuth', () => { const blockForAuth = proxyquire('../../src/proxy/processors/push-action/blockForAuth', { '../../../service/urls': { getServiceUIURL: getServiceUIURLStub }, - '../../actions': { Step: StepSpy } + '../../actions': { Step: StepSpy }, }); exec = blockForAuth.exec; @@ -45,7 +45,6 @@ describe('blockForAuth', () => { }); describe('exec', () => { - it('should generate a correct shareable URL', async () => { await exec(req, action); expect(getServiceUIURLStub.calledOnce).to.be.true; diff --git a/test/processors/checkAuthorEmails.test.js b/test/processors/checkAuthorEmails.test.js index 52d8ffc6e..849842704 100644 --- a/test/processors/checkAuthorEmails.test.js +++ b/test/processors/checkAuthorEmails.test.js @@ -4,7 +4,7 @@ const { expect } = require('chai'); describe('checkAuthorEmails', () => { let action; - let commitConfig + let commitConfig; let exec; let getCommitConfigStub; let stepSpy; @@ -25,9 +25,9 @@ describe('checkAuthorEmails', () => { author: { email: { domain: { allow: null }, - local: { block: null } - } - } + local: { block: null }, + }, + }, }; getCommitConfigStub = sinon.stub().returns(commitConfig); @@ -37,13 +37,16 @@ describe('checkAuthorEmails', () => { action.step = new StepStub(); Object.assign(action.step, step); return action.step; - }) + }), }; - const checkAuthorEmails = proxyquire('../../src/proxy/processors/push-action/checkAuthorEmails', { - '../../../config': { getCommitConfig: getCommitConfigStub }, - '../../actions': { Step: StepStub } - }); + const checkAuthorEmails = proxyquire( + '../../src/proxy/processors/push-action/checkAuthorEmails', + { + '../../../config': { getCommitConfig: getCommitConfigStub }, + '../../actions': { Step: StepStub }, + }, + ); exec = checkAuthorEmails.exec; }); @@ -56,7 +59,7 @@ describe('checkAuthorEmails', () => { it('should allow valid emails when no restrictions', async () => { action.commitData = [ { authorEmail: 'valid@example.com' }, - { authorEmail: 'another.valid@test.org' } + { authorEmail: 'another.valid@test.org' }, ]; await exec({}, action); @@ -68,47 +71,48 @@ describe('checkAuthorEmails', () => { commitConfig.author.email.domain.allow = 'example\\.com$'; action.commitData = [ { authorEmail: 'valid@example.com' }, - { authorEmail: 'invalid@forbidden.org' } + { authorEmail: 'invalid@forbidden.org' }, ]; await exec({}, action); expect(action.step.error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit author e-mails are illegal: invalid@forbidden.org' - )).to.be.true; - expect(StepStub.prototype.setError.calledWith( - 'Your push has been blocked. Please verify your Git configured e-mail address is valid (e.g. john.smith@example.com)' - )).to.be.true; + expect( + stepSpy.calledWith( + 'The following commit author e-mails are illegal: invalid@forbidden.org', + ), + ).to.be.true; + expect( + StepStub.prototype.setError.calledWith( + 'Your push has been blocked. Please verify your Git configured e-mail address is valid (e.g. john.smith@example.com)', + ), + ).to.be.true; }); it('should block emails with forbidden usernames', async () => { commitConfig.author.email.local.block = 'blocked'; action.commitData = [ { authorEmail: 'allowed@example.com' }, - { authorEmail: 'blocked.user@test.org' } + { authorEmail: 'blocked.user@test.org' }, ]; await exec({}, action); expect(action.step.error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit author e-mails are illegal: blocked.user@test.org' - )).to.be.true; + expect( + stepSpy.calledWith( + 'The following commit author e-mails are illegal: blocked.user@test.org', + ), + ).to.be.true; }); it('should handle empty email strings', async () => { - action.commitData = [ - { authorEmail: '' }, - { authorEmail: 'valid@example.com' } - ]; + action.commitData = [{ authorEmail: '' }, { authorEmail: 'valid@example.com' }]; await exec({}, action); expect(action.step.error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit author e-mails are illegal: ' - )).to.be.true; + expect(stepSpy.calledWith('The following commit author e-mails are illegal: ')).to.be.true; }); it('should allow emails when both checks pass', async () => { @@ -116,7 +120,7 @@ describe('checkAuthorEmails', () => { commitConfig.author.email.local.block = 'forbidden'; action.commitData = [ { authorEmail: 'allowed@example.com' }, - { authorEmail: 'also.allowed@example.com' } + { authorEmail: 'also.allowed@example.com' }, ]; await exec({}, action); @@ -127,29 +131,24 @@ describe('checkAuthorEmails', () => { it('should block emails that fail both checks', async () => { commitConfig.author.email.domain.allow = 'example\\.com$'; commitConfig.author.email.local.block = 'forbidden'; - action.commitData = [ - { authorEmail: 'forbidden@wrong.org' } - ]; + action.commitData = [{ authorEmail: 'forbidden@wrong.org' }]; await exec({}, action); expect(action.step.error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit author e-mails are illegal: forbidden@wrong.org' - )).to.be.true; + expect( + stepSpy.calledWith('The following commit author e-mails are illegal: forbidden@wrong.org'), + ).to.be.true; }); it('should handle emails without domain', async () => { - action.commitData = [ - { authorEmail: 'nodomain@' } - ]; + action.commitData = [{ authorEmail: 'nodomain@' }]; await exec({}, action); expect(action.step.error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit author e-mails are illegal: nodomain@' - )).to.be.true; + expect(stepSpy.calledWith('The following commit author e-mails are illegal: nodomain@')).to.be + .true; }); it('should handle multiple illegal emails', async () => { @@ -157,15 +156,17 @@ describe('checkAuthorEmails', () => { action.commitData = [ { authorEmail: 'invalid1@bad.org' }, { authorEmail: 'invalid2@wrong.net' }, - { authorEmail: 'valid@example.com' } + { authorEmail: 'valid@example.com' }, ]; await exec({}, action); expect(action.step.error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit author e-mails are illegal: invalid1@bad.org,invalid2@wrong.net' - )).to.be.true; + expect( + stepSpy.calledWith( + 'The following commit author e-mails are illegal: invalid1@bad.org,invalid2@wrong.net', + ), + ).to.be.true; }); }); }); diff --git a/test/processors/checkCommitMessages.test.js b/test/processors/checkCommitMessages.test.js index 75156e0ae..141b5bf52 100644 --- a/test/processors/checkCommitMessages.test.js +++ b/test/processors/checkCommitMessages.test.js @@ -19,16 +19,19 @@ describe('checkCommitMessages', () => { message: { block: { literals: ['secret', 'password'], - patterns: ['\\b\\d{4}-\\d{4}-\\d{4}-\\d{4}\\b'] // Credit card pattern - } - } + patterns: ['\\b\\d{4}-\\d{4}-\\d{4}-\\d{4}\\b'], // Credit card pattern + }, + }, }; getCommitConfigStub = sinon.stub().returns(commitConfig); - const checkCommitMessages = proxyquire('../../src/proxy/processors/push-action/checkCommitMessages', { - '../../../config': { getCommitConfig: getCommitConfigStub } - }); + const checkCommitMessages = proxyquire( + '../../src/proxy/processors/push-action/checkCommitMessages', + { + '../../../config': { getCommitConfig: getCommitConfigStub }, + }, + ); exec = checkCommitMessages.exec; }); @@ -44,16 +47,10 @@ describe('checkCommitMessages', () => { beforeEach(() => { req = {}; - action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.commitData = [ { message: 'Fix bug', author: 'test@example.com' }, - { message: 'Update docs', author: 'test@example.com' } + { message: 'Update docs', author: 'test@example.com' }, ]; stepSpy = sinon.spy(Step.prototype, 'log'); }); @@ -63,7 +60,8 @@ describe('checkCommitMessages', () => { expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.false; - expect(logStub.calledWith('The following commit messages are legal: Fix bug,Update docs')).to.be.true; + expect(logStub.calledWith('The following commit messages are legal: Fix bug,Update docs')).to + .be.true; }); it('should block commit with illegal messages', async () => { @@ -73,32 +71,32 @@ describe('checkCommitMessages', () => { expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit messages are illegal: secret password here' - )).to.be.true; + expect(stepSpy.calledWith('The following commit messages are illegal: secret password here')) + .to.be.true; expect(result.steps[0].errorMessage).to.include('Your push has been blocked'); - expect(logStub.calledWith('The following commit messages are illegal: secret password here')).to.be.true; + expect(logStub.calledWith('The following commit messages are illegal: secret password here')) + .to.be.true; }); it('should handle duplicate messages only once', async () => { action.commitData = [ { message: 'secret', author: 'test@example.com' }, { message: 'secret', author: 'test@example.com' }, - { message: 'password', author: 'test@example.com' } + { message: 'password', author: 'test@example.com' }, ]; const result = await exec(req, action); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit messages are illegal: secret,password' - )).to.be.true; - expect(logStub.calledWith('The following commit messages are illegal: secret,password')).to.be.true; + expect(stepSpy.calledWith('The following commit messages are illegal: secret,password')).to.be + .true; + expect(logStub.calledWith('The following commit messages are illegal: secret,password')).to.be + .true; }); it('should not error when commit data is empty', async () => { // Empty commit data is a valid scenario that happens when making a branch from an unapproved commit - // This is remedied in the getMissingData.exec action + // This is remedied in the getMissingData.exec action action.commitData = []; const result = await exec(req, action); @@ -110,7 +108,7 @@ describe('checkCommitMessages', () => { it('should handle commit data with null values', async () => { action.commitData = [ { message: null, author: 'test@example.com' }, - { message: undefined, author: 'test@example.com' } + { message: undefined, author: 'test@example.com' }, ]; const result = await exec(req, action); @@ -122,33 +120,33 @@ describe('checkCommitMessages', () => { it('should handle commit messages of incorrect type', async () => { action.commitData = [ { message: 123, author: 'test@example.com' }, - { message: {}, author: 'test@example.com' } + { message: {}, author: 'test@example.com' }, ]; const result = await exec(req, action); expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit messages are illegal: 123,[object Object]' - )).to.be.true; - expect(logStub.calledWith('The following commit messages are illegal: 123,[object Object]')).to.be.true; + expect(stepSpy.calledWith('The following commit messages are illegal: 123,[object Object]')) + .to.be.true; + expect(logStub.calledWith('The following commit messages are illegal: 123,[object Object]')) + .to.be.true; }); it('should handle a mix of valid and invalid messages', async () => { action.commitData = [ { message: 'Fix bug', author: 'test@example.com' }, - { message: 'secret password here', author: 'test@example.com' } + { message: 'secret password here', author: 'test@example.com' }, ]; const result = await exec(req, action); expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith( - 'The following commit messages are illegal: secret password here' - )).to.be.true; - expect(logStub.calledWith('The following commit messages are illegal: secret password here')).to.be.true; + expect(stepSpy.calledWith('The following commit messages are illegal: secret password here')) + .to.be.true; + expect(logStub.calledWith('The following commit messages are illegal: secret password here')) + .to.be.true; }); }); }); diff --git a/test/processors/checkIfWaitingAuth.test.js b/test/processors/checkIfWaitingAuth.test.js index f9a66a3a6..5306f8da4 100644 --- a/test/processors/checkIfWaitingAuth.test.js +++ b/test/processors/checkIfWaitingAuth.test.js @@ -13,9 +13,12 @@ describe('checkIfWaitingAuth', () => { beforeEach(() => { getPushStub = sinon.stub(); - const checkIfWaitingAuth = proxyquire('../../src/proxy/processors/push-action/checkIfWaitingAuth', { - '../../../db': { getPush: getPushStub } - }); + const checkIfWaitingAuth = proxyquire( + '../../src/proxy/processors/push-action/checkIfWaitingAuth', + { + '../../../db': { getPush: getPushStub }, + }, + ); exec = checkIfWaitingAuth.exec; }); @@ -30,23 +33,11 @@ describe('checkIfWaitingAuth', () => { beforeEach(() => { req = {}; - action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); }); it('should set allowPush when action exists and is authorized', async () => { - const authorizedAction = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + const authorizedAction = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); authorizedAction.authorised = true; getPushStub.resolves(authorizedAction); @@ -59,13 +50,7 @@ describe('checkIfWaitingAuth', () => { }); it('should not set allowPush when action exists but not authorized', async () => { - const unauthorizedAction = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + const unauthorizedAction = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); unauthorizedAction.authorised = false; getPushStub.resolves(unauthorizedAction); @@ -88,13 +73,7 @@ describe('checkIfWaitingAuth', () => { it('should not modify action when it has an error', async () => { action.error = true; - const authorizedAction = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + const authorizedAction = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); authorizedAction.authorised = true; getPushStub.resolves(authorizedAction); diff --git a/test/processors/getDiff.test.js b/test/processors/getDiff.test.js index 5acbc83e2..5caed5eae 100644 --- a/test/processors/getDiff.test.js +++ b/test/processors/getDiff.test.js @@ -10,13 +10,13 @@ const expect = chai.expect; describe('getDiff', () => { let tempDir; let git; - + before(async () => { // Create a temp repo to avoid mocking simple-git tempDir = path.join(__dirname, 'temp-test-repo'); await fs.mkdir(tempDir, { recursive: true }); git = simpleGit(tempDir); - + await git.init(); await git.addConfig('user.name', 'test'); await git.addConfig('user.email', 'test@test.com'); @@ -25,53 +25,37 @@ describe('getDiff', () => { await git.add('.'); await git.commit('initial commit'); }); - + after(async () => { await fs.rmdir(tempDir, { recursive: true }); }); - + it('should get diff between commits', async () => { await fs.writeFile(path.join(tempDir, 'test.txt'), 'modified content'); await git.add('.'); await git.commit('second commit'); - - const action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + + const action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = __dirname; // Temp dir parent path action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; action.commitTo = 'HEAD'; - action.commitData = [ - { parent: '0000000000000000000000000000000000000000' } - ]; - + action.commitData = [{ parent: '0000000000000000000000000000000000000000' }]; + const result = await exec({}, action); - + expect(result.steps[0].error).to.be.false; expect(result.steps[0].content).to.include('modified content'); expect(result.steps[0].content).to.include('initial content'); }); it('should get diff between commits with no changes', async () => { - const action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + const action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = __dirname; // Temp dir parent path action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; action.commitTo = 'HEAD'; - action.commitData = [ - { parent: '0000000000000000000000000000000000000000' } - ]; + action.commitData = [{ parent: '0000000000000000000000000000000000000000' }]; const result = await exec({}, action); @@ -80,13 +64,7 @@ describe('getDiff', () => { }); it('should throw an error if no commit data is provided', async () => { - const action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + const action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = __dirname; // Temp dir parent path action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; @@ -99,13 +77,7 @@ describe('getDiff', () => { }); it('should throw an error if no commit data is provided', async () => { - const action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + const action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = __dirname; // Temp dir parent path action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; @@ -126,21 +98,13 @@ describe('getDiff', () => { const parentCommit = log.all[1].hash; const headCommit = log.all[0].hash; - const action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + const action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = path.dirname(tempDir); action.repoName = path.basename(tempDir); action.commitFrom = '0000000000000000000000000000000000000000'; action.commitTo = headCommit; - action.commitData = [ - { parent: parentCommit } - ]; + action.commitData = [{ parent: parentCommit }]; const result = await exec({}, action); diff --git a/test/processors/gitLeaks.test.js b/test/processors/gitLeaks.test.js index eeed7f8e2..9157cf301 100644 --- a/test/processors/gitLeaks.test.js +++ b/test/processors/gitLeaks.test.js @@ -22,9 +22,9 @@ describe('gitleaks', () => { fs: { stat: sinon.stub(), access: sinon.stub(), - constants: { R_OK: 0 } + constants: { R_OK: 0 }, }, - spawn: sinon.stub() + spawn: sinon.stub(), }; logStub = sinon.stub(console, 'log'); @@ -33,19 +33,13 @@ describe('gitleaks', () => { const gitleaksModule = proxyquire('../../src/proxy/processors/push-action/gitleaks', { '../../../config': { getAPIs: stubs.getAPIs }, 'node:fs/promises': stubs.fs, - 'node:child_process': { spawn: stubs.spawn } + 'node:child_process': { spawn: stubs.spawn }, }); exec = gitleaksModule.exec; req = {}; - action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = '/tmp'; action.repoName = 'test-repo'; action.commitFrom = 'abc123'; @@ -66,8 +60,10 @@ describe('gitleaks', () => { expect(result.error).to.be.true; expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith('failed setup gitleaks, please contact an administrator\n')).to.be.true; - expect(errorStub.calledWith('failed to get gitleaks config, please fix the error:')).to.be.true; + expect(stepSpy.calledWith('failed setup gitleaks, please contact an administrator\n')).to.be + .true; + expect(errorStub.calledWith('failed to get gitleaks config, please fix the error:')).to.be + .true; }); it('should skip scanning when plugin is disabled', async () => { @@ -87,31 +83,33 @@ describe('gitleaks', () => { const gitRootCommitMock = { exitCode: 0, stdout: 'rootcommit123\n', - stderr: '' + stderr: '', }; const gitleaksMock = { exitCode: 0, stdout: '', - stderr: 'No leaks found' + stderr: 'No leaks found', }; stubs.spawn - .onFirstCall().returns({ + .onFirstCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitRootCommitMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitRootCommitMock.stdout) }, - stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) } + stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) }, }) - .onSecondCall().returns({ + .onSecondCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitleaksMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitleaksMock.stdout) }, - stderr: { on: (_, cb) => cb(gitleaksMock.stderr) } + stderr: { on: (_, cb) => cb(gitleaksMock.stderr) }, }); const result = await exec(req, action); @@ -129,31 +127,33 @@ describe('gitleaks', () => { const gitRootCommitMock = { exitCode: 0, stdout: 'rootcommit123\n', - stderr: '' + stderr: '', }; const gitleaksMock = { exitCode: 99, stdout: 'Found secret in file.txt\n', - stderr: 'Warning: potential leak' + stderr: 'Warning: potential leak', }; stubs.spawn - .onFirstCall().returns({ + .onFirstCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitRootCommitMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitRootCommitMock.stdout) }, - stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) } + stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) }, }) - .onSecondCall().returns({ + .onSecondCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitleaksMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitleaksMock.stdout) }, - stderr: { on: (_, cb) => cb(gitleaksMock.stderr) } + stderr: { on: (_, cb) => cb(gitleaksMock.stderr) }, }); const result = await exec(req, action); @@ -170,31 +170,33 @@ describe('gitleaks', () => { const gitRootCommitMock = { exitCode: 0, stdout: 'rootcommit123\n', - stderr: '' + stderr: '', }; const gitleaksMock = { exitCode: 1, stdout: '', - stderr: 'Command failed' + stderr: 'Command failed', }; stubs.spawn - .onFirstCall().returns({ + .onFirstCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitRootCommitMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitRootCommitMock.stdout) }, - stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) } + stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) }, }) - .onSecondCall().returns({ + .onSecondCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitleaksMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitleaksMock.stdout) }, - stderr: { on: (_, cb) => cb(gitleaksMock.stderr) } + stderr: { on: (_, cb) => cb(gitleaksMock.stderr) }, }); const result = await exec(req, action); @@ -202,7 +204,8 @@ describe('gitleaks', () => { expect(result.error).to.be.true; expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith('failed to run gitleaks, please contact an administrator\n')).to.be.true; + expect(stepSpy.calledWith('failed to run gitleaks, please contact an administrator\n')).to.be + .true; }); it('should handle gitleaks spawn failure', async () => { @@ -214,7 +217,8 @@ describe('gitleaks', () => { expect(result.error).to.be.true; expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(stepSpy.calledWith('failed to spawn gitleaks, please contact an administrator\n')).to.be.true; + expect(stepSpy.calledWith('failed to spawn gitleaks, please contact an administrator\n')).to + .be.true; }); it('should handle empty gitleaks entry in proxy.config.json', async () => { @@ -233,7 +237,7 @@ describe('gitleaks', () => { return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb('') }, - stderr: { on: (_, cb) => cb('') } + stderr: { on: (_, cb) => cb('') }, }); const result = await exec(req, action); @@ -244,11 +248,11 @@ describe('gitleaks', () => { }); it('should handle custom config path', async () => { - stubs.getAPIs.returns({ - gitleaks: { + stubs.getAPIs.returns({ + gitleaks: { enabled: true, - configPath: `../fixtures/gitleaks-config.toml` - } + configPath: `../fixtures/gitleaks-config.toml`, + }, }); stubs.fs.stat.resolves({ isFile: () => true }); @@ -257,46 +261,50 @@ describe('gitleaks', () => { const gitRootCommitMock = { exitCode: 0, stdout: 'rootcommit123\n', - stderr: '' + stderr: '', }; const gitleaksMock = { exitCode: 0, stdout: '', - stderr: 'No leaks found' + stderr: 'No leaks found', }; stubs.spawn - .onFirstCall().returns({ + .onFirstCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitRootCommitMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitRootCommitMock.stdout) }, - stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) } + stderr: { on: (_, cb) => cb(gitRootCommitMock.stderr) }, }) - .onSecondCall().returns({ + .onSecondCall() + .returns({ on: (event, cb) => { if (event === 'close') cb(gitleaksMock.exitCode); return { stdout: { on: () => {} }, stderr: { on: () => {} } }; }, stdout: { on: (_, cb) => cb(gitleaksMock.stdout) }, - stderr: { on: (_, cb) => cb(gitleaksMock.stderr) } + stderr: { on: (_, cb) => cb(gitleaksMock.stderr) }, }); const result = await exec(req, action); expect(result.error).to.be.false; expect(result.steps[0].error).to.be.false; - expect(stubs.spawn.secondCall.args[1]).to.include('--config=../fixtures/gitleaks-config.toml'); + expect(stubs.spawn.secondCall.args[1]).to.include( + '--config=../fixtures/gitleaks-config.toml', + ); }); it('should handle invalid custom config path', async () => { - stubs.getAPIs.returns({ - gitleaks: { + stubs.getAPIs.returns({ + gitleaks: { enabled: true, - configPath: '/invalid/path.toml' - } + configPath: '/invalid/path.toml', + }, }); stubs.fs.stat.rejects(new Error('File not found')); @@ -306,7 +314,11 @@ describe('gitleaks', () => { expect(result.error).to.be.true; expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(errorStub.calledWith('could not read file at the config path provided, will not be fed to gitleaks')).to.be.true; + expect( + errorStub.calledWith( + 'could not read file at the config path provided, will not be fed to gitleaks', + ), + ).to.be.true; }); }); }); diff --git a/test/processors/writePack.test.js b/test/processors/writePack.test.js index a7580caa6..f61488171 100644 --- a/test/processors/writePack.test.js +++ b/test/processors/writePack.test.js @@ -17,7 +17,7 @@ describe('writePack', () => { spawnSyncStub = sinon.stub().returns({ stdout: 'git receive-pack output', stderr: '', - status: 0 + status: 0, }); stepLogSpy = sinon.spy(Step.prototype, 'log'); @@ -25,7 +25,7 @@ describe('writePack', () => { stepSetErrorSpy = sinon.spy(Step.prototype, 'setError'); const writePack = proxyquire('../../src/proxy/processors/push-action/writePack', { - 'child_process': { spawnSync: spawnSyncStub } + child_process: { spawnSync: spawnSyncStub }, }); exec = writePack.exec; @@ -41,15 +41,9 @@ describe('writePack', () => { beforeEach(() => { req = { - body: 'pack data' + body: 'pack data', }; - action = new Action( - '1234567890', - 'push', - 'POST', - 1234567890, - 'test/repo' - ); + action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = '/path/to/repo'; }); @@ -62,7 +56,7 @@ describe('writePack', () => { expect(spawnSyncStub.firstCall.args[2]).to.deep.equal({ cwd: '/path/to/repo', input: 'pack data', - encoding: 'utf-8' + encoding: 'utf-8', }); expect(stepLogSpy.calledWith('executing git receive-pack repo')).to.be.true; diff --git a/test/testAuthMethods.test.js b/test/testAuthMethods.test.js index 013c79d8d..6dc6737d3 100644 --- a/test/testAuthMethods.test.js +++ b/test/testAuthMethods.test.js @@ -21,16 +21,16 @@ describe('auth methods', async () => { { type: 'openidconnect', enabled: false }, ], }); - + const fsStub = { existsSync: sinon.stub().returns(true), readFileSync: sinon.stub().returns(newConfig), }; - + const config = proxyquire('../src/config', { fs: fsStub, }); - + expect(() => config.getAuthMethods()).to.throw(Error, 'No authentication method enabled'); }); @@ -57,5 +57,5 @@ describe('auth methods', async () => { expect(authMethods[0].type).to.equal('local'); expect(authMethods[1].type).to.equal('ActiveDirectory'); expect(authMethods[2].type).to.equal('openidconnect'); - }) + }); }); diff --git a/test/testConfig.test.js b/test/testConfig.test.js index 5c2314231..c4247d9ee 100644 --- a/test/testConfig.test.js +++ b/test/testConfig.test.js @@ -11,7 +11,7 @@ describe('default configuration', function () { it('should use default values if no user-settings.json file exists', function () { const config = require('../src/config'); config.logConfiguration(); - const enabledMethods = defaultSettings.authentication.filter(method => method.enabled); + const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled); expect(config.getAuthMethods()).to.deep.equal(enabledMethods); expect(config.getDatabase()).to.be.eql(defaultSettings.sink[0]); @@ -57,7 +57,7 @@ describe('user configuration', function () { fs.writeFileSync(tempUserFile, JSON.stringify(user)); const config = require('../src/config'); - const enabledMethods = defaultSettings.authentication.filter(method => method.enabled); + const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled); expect(config.getAuthorisedList()).to.be.eql(user.authorisedList); expect(config.getAuthMethods()).to.deep.equal(enabledMethods); @@ -78,7 +78,7 @@ describe('user configuration', function () { const config = require('../src/config'); const authMethods = config.getAuthMethods(); - const googleAuth = authMethods.find(method => method.type === 'google'); + const googleAuth = authMethods.find((method) => method.type === 'google'); expect(googleAuth).to.not.be.undefined; expect(googleAuth.enabled).to.be.true; @@ -100,7 +100,7 @@ describe('user configuration', function () { fs.writeFileSync(tempUserFile, JSON.stringify(user)); const config = require('../src/config'); - const enabledMethods = defaultSettings.authentication.filter(method => method.enabled); + const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled); expect(config.getDatabase()).to.be.eql(user.sink[0]); expect(config.getDatabase()).to.not.be.eql(defaultSettings.sink[0]); @@ -198,7 +198,7 @@ describe('user configuration', function () { enabled: true, key: 'my-key.pem', cert: 'my-cert.pem', - } + }, }; fs.writeFileSync(tempUserFile, JSON.stringify(user)); @@ -221,7 +221,7 @@ describe('user configuration', function () { sslCertPemPath: 'bad-cert.pem', }; fs.writeFileSync(tempUserFile, JSON.stringify(user)); - + const config = require('../src/config'); expect(config.getTLSCertPemPath()).to.be.eql(user.tls.cert); @@ -252,7 +252,7 @@ describe('user configuration', function () { }, }; fs.writeFileSync(tempUserFile, JSON.stringify(user)); - + const config = require('../src/config'); expect(config.getAPIs()).to.be.eql(user.api); From a0027c2eef4073be4a634e53b5552dac8dc8d8b2 Mon Sep 17 00:00:00 2001 From: kriswest Date: Mon, 16 Jun 2025 16:51:32 +0100 Subject: [PATCH 15/23] Merge remote-tracking branch 'upstream-prod/946-associate-commits-by-email-rebase' into 946-associate-commits-by-email-rebase From 041b6c72532d1039507a2e980f902f403345c083 Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 4 Jul 2025 18:28:39 +0100 Subject: [PATCH 16/23] fix: typing issue in getCommitData --- src/proxy/processors/push-action/parsePush.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 4c7fbad22..6ca8dce01 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -112,7 +112,7 @@ const getCommitData = (contents: CommitContent[]) => { const authorEmail = author?.split(' ').reverse()[2].slice(1, -1); console.log({ authorEmail }); - const committerEmail = committer.split(' ').reverse()[2].slice(1, -1); + const committerEmail = committer?.split(' ').reverse()[2].slice(1, -1); console.log({ committerEmail }); console.log({ @@ -133,7 +133,8 @@ const getCommitData = (contents: CommitContent[]) => { !committer || !commitTimestamp || !message || - !authorEmail + !authorEmail || + !committerEmail ) { throw new Error('Invalid commit data'); } From 4274607f1be7263531d0d8c6fc41753c426e2bc0 Mon Sep 17 00:00:00 2001 From: Kris West Date: Mon, 7 Jul 2025 16:32:01 +0100 Subject: [PATCH 17/23] fix: rationalize parsePush.getCommitData in response to review --- src/proxy/processors/push-action/parsePush.ts | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 6ca8dce01..d1c9095e3 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -59,13 +59,13 @@ const getCommitData = (contents: CommitContent[]) => { .chain(contents) .filter({ type: 1 }) .map((x) => { - console.log({ x }); + console.debug({ x }); const formattedContent = x.content.split('\n'); - console.log({ formattedContent }); + console.debug({ formattedContent }); const parts = formattedContent.filter((part) => part.length > 0); - console.log({ parts }); + console.debug({ parts }); if (!parts || parts.length < 5) { throw new Error('Invalid commit data'); @@ -75,51 +75,50 @@ const getCommitData = (contents: CommitContent[]) => { .find((t) => t.split(' ')[0] === 'tree') ?.replace('tree', '') .trim(); - console.log({ tree }); + console.debug({ tree }); const parentValue = parts.find((t) => t.split(' ')[0] === 'parent'); - console.log({ parentValue }); + console.debug({ parentValue }); const parent = parentValue ? parentValue.replace('parent', '').trim() : '0000000000000000000000000000000000000000'; - console.log({ parent }); + console.debug({ parent }); - const author = parts + const authorStr = parts .find((t) => t.split(' ')[0] === 'author') ?.replace('author', '') .trim(); - console.log({ author }); - - const committer = parts + // handle email-like author string: "UserName 1746612538 +0100" + const author = authorStr?.split('<')[0].trim(); + // slice to trim start and end from `` + const authorEmail = authorStr?.split(' ').reverse()[2].slice(1, -1); + console.debug({ authorStr, author, authorEmail }); + + // handle email-like committer string: "UserName 1746612538 +0100" + const committerStr = parts .find((t) => t.split(' ')[0] === 'committer') ?.replace('committer', '') .trim(); - console.log({ committer }); + const committer = committerStr?.split('<')[0].trim(); + const committerArr = committerStr?.split(' ').reverse() ?? []; + const commitTimestamp = committerArr[1]; + // slice to trim start and end from `` + const committerEmail = committerArr[2]?.slice(1, -1); + console.debug({ committerStr, committer, committerEmail, commitTimestamp }); const indexOfMessages = formattedContent.indexOf(''); - console.log({ indexOfMessages }); - const message = formattedContent .slice(indexOfMessages + 1) .join(' ') .trim(); - console.log({ message }); - - const commitTimestamp = committer?.split(' ').reverse()[1]; - console.log({ commitTimestamp }); - - const authorEmail = author?.split(' ').reverse()[2].slice(1, -1); - console.log({ authorEmail }); - - const committerEmail = committer?.split(' ').reverse()[2].slice(1, -1); - console.log({ committerEmail }); + console.debug({ indexOfMessages, message }); console.log({ tree, parent, - author: author?.split('<')[0].trim(), - committer: committer?.split('<')[0].trim(), + author, + committer, commitTimestamp, message, authorEmail, @@ -142,8 +141,8 @@ const getCommitData = (contents: CommitContent[]) => { return { tree, parent, - author: author.split('<')[0].trim(), - committer: committer.split('<')[0].trim(), + author, + committer, commitTimestamp, message, authorEmail, From bbd7f49def279da22d5f64cc7b1e24edd9dd518e Mon Sep 17 00:00:00 2001 From: Kris West Date: Mon, 7 Jul 2025 16:34:17 +0100 Subject: [PATCH 18/23] test: enable pushParsing tests --- test/{testParsePush.js => testParsePush.test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{testParsePush.js => testParsePush.test.js} (100%) diff --git a/test/testParsePush.js b/test/testParsePush.test.js similarity index 100% rename from test/testParsePush.js rename to test/testParsePush.test.js From 47a23a76b481e2ca40abfea1a0c015a37fe969d0 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 8 Jul 2025 10:31:46 +0100 Subject: [PATCH 19/23] fix: remove debug logs in parsePush --- src/proxy/processors/push-action/parsePush.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index d1c9095e3..c9d07fb26 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -59,13 +59,8 @@ const getCommitData = (contents: CommitContent[]) => { .chain(contents) .filter({ type: 1 }) .map((x) => { - console.debug({ x }); - const formattedContent = x.content.split('\n'); - console.debug({ formattedContent }); - const parts = formattedContent.filter((part) => part.length > 0); - console.debug({ parts }); if (!parts || parts.length < 5) { throw new Error('Invalid commit data'); @@ -75,15 +70,12 @@ const getCommitData = (contents: CommitContent[]) => { .find((t) => t.split(' ')[0] === 'tree') ?.replace('tree', '') .trim(); - console.debug({ tree }); const parentValue = parts.find((t) => t.split(' ')[0] === 'parent'); - console.debug({ parentValue }); const parent = parentValue ? parentValue.replace('parent', '').trim() : '0000000000000000000000000000000000000000'; - console.debug({ parent }); const authorStr = parts .find((t) => t.split(' ')[0] === 'author') @@ -93,7 +85,6 @@ const getCommitData = (contents: CommitContent[]) => { const author = authorStr?.split('<')[0].trim(); // slice to trim start and end from `` const authorEmail = authorStr?.split(' ').reverse()[2].slice(1, -1); - console.debug({ authorStr, author, authorEmail }); // handle email-like committer string: "UserName 1746612538 +0100" const committerStr = parts @@ -105,14 +96,12 @@ const getCommitData = (contents: CommitContent[]) => { const commitTimestamp = committerArr[1]; // slice to trim start and end from `` const committerEmail = committerArr[2]?.slice(1, -1); - console.debug({ committerStr, committer, committerEmail, commitTimestamp }); const indexOfMessages = formattedContent.indexOf(''); const message = formattedContent .slice(indexOfMessages + 1) .join(' ') .trim(); - console.debug({ indexOfMessages, message }); console.log({ tree, From 5fbe156abad152d61cd845ab19e1bbbddc907e2f Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Jul 2025 10:37:27 +0100 Subject: [PATCH 20/23] chore: minor dependency updates from npm audit fix --- package-lock.json | 81 +++++++++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3aa3505fa..e94087fcc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2717,6 +2717,19 @@ "eslint-scope": "5.1.1" } }, + "node_modules/@noble/hashes": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@noble/hashes/-/hashes-1.8.0.tgz", + "integrity": "sha512-jCs9ldd7NwzpgXDIf6P3+NrHh9/sD6CQdxHyjQI+h/6rDNo88ypBxxz45UDuZHz9r3tNz7N/VInSVoVdtXEI4A==", + "dev": true, + "license": "MIT", + "engines": { + "node": "^14.21.3 || >=16" + }, + "funding": { + "url": "https://paulmillr.com/funding/" + } + }, "node_modules/@nodelib/fs.scandir": { "version": "2.1.5", "resolved": "https://registry.npmjs.org/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz", @@ -2837,9 +2850,10 @@ } }, "node_modules/@npmcli/map-workspaces/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", + "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" } @@ -2866,6 +2880,16 @@ "node": "^14.17.0 || ^16.13.0 || >=18.0.0" } }, + "node_modules/@paralleldrive/cuid2": { + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/@paralleldrive/cuid2/-/cuid2-2.2.2.tgz", + "integrity": "sha512-ZOBkgDwEdoYVlSeRbYYXs0S9MejQofiVYoTbKzy/6GQa39/q5tQU2IX46+shYnUkpEl3wc+J6wRlar7r2EK2xA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@noble/hashes": "^1.1.5" + } + }, "node_modules/@pkgjs/parseargs": { "version": "0.11.0", "resolved": "https://registry.npmjs.org/@pkgjs/parseargs/-/parseargs-0.11.0.tgz", @@ -4073,10 +4097,11 @@ } }, "node_modules/@typescript-eslint/typescript-estree/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", "dev": true, + "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" } @@ -4794,10 +4819,11 @@ "peer": true }, "node_modules/brace-expansion": { - "version": "1.1.11", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", - "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "dev": true, + "license": "MIT", "dependencies": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -7505,13 +7531,14 @@ } }, "node_modules/formidable": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/formidable/-/formidable-2.1.2.tgz", - "integrity": "sha512-CM3GuJ57US06mlpQ47YcunuUZ9jpm8Vx+P2CGt2j7HpgkKZO/DJYQ0Bobim8G6PFQmK5lOqOOdUXboU+h73A4g==", + "version": "2.1.5", + "resolved": "https://registry.npmjs.org/formidable/-/formidable-2.1.5.tgz", + "integrity": "sha512-Oz5Hwvwak/DCaXVVUtPn4oLMLLy1CdclLKO1LFgU7XzDpVMUU5UjlSLpGMocyQNNk8F6IJW9M/YdooSn2MRI+Q==", "dev": true, + "license": "MIT", "dependencies": { + "@paralleldrive/cuid2": "^2.2.2", "dezalgo": "^1.0.4", - "hexoid": "^1.0.0", "once": "^1.4.0", "qs": "^6.11.0" }, @@ -7816,9 +7843,10 @@ } }, "node_modules/glob/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", + "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" } @@ -8066,15 +8094,6 @@ "he": "bin/he" } }, - "node_modules/hexoid": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/hexoid/-/hexoid-1.0.0.tgz", - "integrity": "sha512-QFLV0taWQOZtvIRIAdBChesmogZrtuXvVWsFHZTk2SU+anspqZ2vMnoLg7IE1+Uk16N19APic1BuF8bC8c2m5g==", - "dev": true, - "engines": { - "node": ">=8" - } - }, "node_modules/highlight.js": { "version": "11.9.0", "resolved": "https://registry.npmjs.org/highlight.js/-/highlight.js-11.9.0.tgz", @@ -10240,9 +10259,9 @@ } }, "node_modules/mocha/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", "dev": true, "license": "MIT", "dependencies": { @@ -13992,9 +14011,9 @@ } }, "node_modules/vite": { - "version": "4.5.13", - "resolved": "https://registry.npmjs.org/vite/-/vite-4.5.13.tgz", - "integrity": "sha512-Hgp8IF/yZDzKsN1hQWOuQZbrKiaFsbQud+07jJ8h9m9PaHWkpvZ5u55Xw5yYjWRXwRQ4jwFlJvY7T7FUJG9MCA==", + "version": "4.5.14", + "resolved": "https://registry.npmjs.org/vite/-/vite-4.5.14.tgz", + "integrity": "sha512-+v57oAaoYNnO3hIu5Z/tJRZjq5aHM2zDve9YZ8HngVHbhk66RStobhb1sqPMIPEleV6cNKYK4eGrAbE9Ulbl2g==", "dev": true, "license": "MIT", "dependencies": { From e5c89a903bbea42e280e6b3f180f976d51f8c98c Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 31 Jul 2025 14:51:05 +0100 Subject: [PATCH 21/23] test: fixing test failure post merging main --- .vscode/settings.json | 3 +- .../push-action/checkUserPushPermission.ts | 33 ++++++++----------- src/proxy/processors/types.ts | 11 ++++--- src/ui/utils.tsx | 8 ++--- .../checkUserPushPermission.test.js | 4 +-- test/processors/getDiff.test.js | 8 +++-- test/testCheckUserPushPermission.test.js | 3 +- test/testParsePush.test.js | 5 +-- 8 files changed, 38 insertions(+), 37 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 90d173613..dba4a2d94 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -8,5 +8,6 @@ "source.fixAll.eslint": "explicit" }, "editor.defaultFormatter": "esbenp.prettier-vscode", - "editor.formatOnSave": true + "editor.formatOnSave": true, + "cSpell.words": ["Deltafied"] } diff --git a/src/proxy/processors/push-action/checkUserPushPermission.ts b/src/proxy/processors/push-action/checkUserPushPermission.ts index 6c292bc3e..11414fb81 100644 --- a/src/proxy/processors/push-action/checkUserPushPermission.ts +++ b/src/proxy/processors/push-action/checkUserPushPermission.ts @@ -5,27 +5,30 @@ import { trimTrailingDotGit } from '../../../db/helper'; // Execute if the repo is approved const exec = async (req: any, action: Action): Promise => { const step = new Step('checkUserPushPermission'); - const user = action.user; + const userEmail = action.userEmail; - if (!user) { - console.log('Action has no user set. This may be due to a fast-forward ref update. Deferring to getMissingData action.'); + if (!userEmail) { + console.log( + 'Action has no userEmail set. This may be due to a fast-forward ref update. Deferring to getMissingData action.', + ); return action; } - return await validateUser(user, action, step); + return await validateUser(userEmail, action, step); }; /** * Helper that validates the user's push permission. * This can be used by other actions that need it. For example, when the user is missing from the commit data, * validation is deferred to getMissingData, but the logic is the same. - * @param {string} user The user to validate + * @param {string} userEmail The user email to validate * @param {Action} action The action object * @param {Step} step The step object * @return {Promise} The action object */ -const validateUser = async (user: string, action: Action, step: Step): Promise => { +const validateUser = async (userEmail: string, action: Action, step: Step): Promise => { const repoSplit = trimTrailingDotGit(action.repo.toLowerCase()).split('/'); + // we expect there to be exactly one / separating org/repoName if (repoSplit.length != 2) { step.setError('Server-side issue extracting repoName'); @@ -36,13 +39,8 @@ const validateUser = async (user: string, action: Action, step: Step): Promise 1) { console.error(`Multiple users found with email address ${userEmail}, ending`); @@ -59,18 +57,15 @@ const validateUser = async (user: string, action: Action, step: Step): Promise permission on Repo ${repoName} : ${isUserAllowed}`); + console.log(`User ${userEmail} permission on Repo ${repoName} : ${isUserAllowed}`); if (!isUserAllowed) { console.log('User not allowed to Push'); step.error = true; - step.log( - `User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`, - ); + step.log(`User ${userEmail} is not allowed to push on repo ${action.repo}, ending`); step.setError( `Your push has been blocked (${action.userEmail} ` + @@ -81,7 +76,7 @@ const validateUser = async (user: string, action: Action, step: Step): Promise is allowed to push on repo ${action.repo}`); + step.log(`User ${userEmail} is allowed to push on repo ${action.repo}`); action.addStep(step); return action; }; diff --git a/src/proxy/processors/types.ts b/src/proxy/processors/types.ts index 38f989f8a..0c89b6d24 100644 --- a/src/proxy/processors/types.ts +++ b/src/proxy/processors/types.ts @@ -17,20 +17,20 @@ export type CommitContent = { deflatedSize: number; objectRef: any; content: string; -} +}; export type PersonLine = { name: string; email: string; timestamp: string; -} +}; export type CommitHeader = { tree: string; parents: string[]; author: PersonLine; committer: PersonLine; -} +}; export type CommitData = { tree: string; @@ -38,12 +38,13 @@ export type CommitData = { author: string; committer: string; authorEmail: string; + committerEmail: string; commitTimestamp: string; message: string; -} +}; export type PackMeta = { sig: string; version: number; entries: number; -} +}; diff --git a/src/ui/utils.tsx b/src/ui/utils.tsx index be15a19ef..16d08f083 100644 --- a/src/ui/utils.tsx +++ b/src/ui/utils.tsx @@ -5,13 +5,13 @@ */ export const getCookie = (name: string): string | null => { if (!document.cookie) return null; - + const cookies = document.cookie .split(';') .map((c) => c.trim()) .filter((c) => c.startsWith(name + '=')); - + if (!cookies.length) return null; - + return decodeURIComponent(cookies[0].split('=')[1]); -}; \ No newline at end of file +}; diff --git a/test/processors/checkUserPushPermission.test.js b/test/processors/checkUserPushPermission.test.js index 2f02f77a5..687b9acbe 100644 --- a/test/processors/checkUserPushPermission.test.js +++ b/test/processors/checkUserPushPermission.test.js @@ -59,10 +59,10 @@ describe('checkUserPushPermission', () => { expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.false; expect(stepSpy.lastCall.args[0]).to.equal( - 'User db-user is allowed to push on repo test/repo.git', + 'User db-user@test.com is allowed to push on repo test/repo.git', ); expect(logStub.lastCall.args[0]).to.equal( - 'User db-user permission on Repo repo : true', + 'User db-user@test.com permission on Repo repo : true', ); }); diff --git a/test/processors/getDiff.test.js b/test/processors/getDiff.test.js index d7da18068..7f5bb4cf3 100644 --- a/test/processors/getDiff.test.js +++ b/test/processors/getDiff.test.js @@ -73,7 +73,9 @@ describe('getDiff', () => { const result = await exec({}, action); expect(result.steps[0].error).to.be.true; - expect(result.steps[0].errorMessage).to.contain('Your push has been blocked because no commit data was found'); + expect(result.steps[0].errorMessage).to.contain( + 'Your push has been blocked because no commit data was found', + ); }); it('should throw an error if no commit data is provided', async () => { @@ -86,7 +88,9 @@ describe('getDiff', () => { const result = await exec({}, action); expect(result.steps[0].error).to.be.true; - expect(result.steps[0].errorMessage).to.contain('Your push has been blocked because no commit data was found'); + expect(result.steps[0].errorMessage).to.contain( + 'Your push has been blocked because no commit data was found', + ); }); it('should handle empty commit hash in commitFrom', async () => { diff --git a/test/testCheckUserPushPermission.test.js b/test/testCheckUserPushPermission.test.js index 92573ba40..7eb281d5f 100644 --- a/test/testCheckUserPushPermission.test.js +++ b/test/testCheckUserPushPermission.test.js @@ -18,7 +18,7 @@ describe('CheckUserPushPermissions...', async () => { before(async function () { await db.deleteRepo(TEST_REPO); await db.deleteUser(TEST_USERNAME_1); - + await db.deleteUser(TEST_USERNAME_2); await db.createRepo({ project: TEST_ORG, name: TEST_REPO, @@ -26,7 +26,6 @@ describe('CheckUserPushPermissions...', async () => { }); await db.createUser(TEST_USERNAME_1, 'abc', TEST_EMAIL_1, TEST_USERNAME_1, false); await db.addUserCanPush(TEST_REPO, TEST_USERNAME_1); - await db.createUser(TEST_USERNAME_2, 'abc', TEST_EMAIL_2, TEST_USERNAME_2, false); }); diff --git a/test/testParsePush.test.js b/test/testParsePush.test.js index b07ff3864..58d31549a 100644 --- a/test/testParsePush.test.js +++ b/test/testParsePush.test.js @@ -110,7 +110,7 @@ describe('parsePackFile', () => { const step = action.steps[0]; expect(step.stepName).to.equal('parsePackFile'); expect(step.error).to.be.true; - expect(step.errorMessage).to.include('No data received'); + expect(step.errorMessage).to.include('No body found in request'); }); it('should add error step if req.body is empty', async () => { @@ -121,7 +121,7 @@ describe('parsePackFile', () => { const step = action.steps[0]; expect(step.stepName).to.equal('parsePackFile'); expect(step.error).to.be.true; - expect(step.errorMessage).to.include('No data received'); + expect(step.errorMessage).to.include('No body found in request'); }); it('should add error step if no ref updates found', async () => { @@ -536,6 +536,7 @@ describe('parsePackFile', () => { parent: '456', author: 'Au Thor', committer: 'Com Itter', + committerEmail: 'c@e.com', commitTimestamp: '222', message: 'Commit message here', authorEmail: 'a@e.com', From 5d8c5892216a83a066826948f14e031b2a0597d4 Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 31 Jul 2025 17:44:32 +0100 Subject: [PATCH 22/23] fix: fix type errors in getMissingData after merging main --- src/proxy/processors/push-action/getMissingData.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/proxy/processors/push-action/getMissingData.ts b/src/proxy/processors/push-action/getMissingData.ts index e256da3f6..a7b4bf558 100644 --- a/src/proxy/processors/push-action/getMissingData.ts +++ b/src/proxy/processors/push-action/getMissingData.ts @@ -47,13 +47,14 @@ const exec = async (req: any, action: Action): Promise => { const timestamp = Math.floor(new Date(entry.date).getTime() / 1000).toString(); return { message: entry.message || '', - committer: entry.author_name || '', + committer: entry.author_name || '', // not actually the committer, but the author of one of the commits tree: entry.hash || '', parent: parent || EMPTY_COMMIT_HASH, author: entry.author_name || '', authorEmail: entry.author_email || '', + committerEmail: entry.author_email || '', commitTimestamp: timestamp, - } + }; }); console.log(`Updated commitData:`, { commitData: action.commitData }); From 5b9fe88de0a20a8966c97e71076635e3b0a65a6d Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 7 Aug 2025 08:04:17 +0100 Subject: [PATCH 23/23] test: correcting test after merging main --- test/processors/checkUserPushPermission.test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/processors/checkUserPushPermission.test.js b/test/processors/checkUserPushPermission.test.js index dbe9e8631..fee0c7a64 100644 --- a/test/processors/checkUserPushPermission.test.js +++ b/test/processors/checkUserPushPermission.test.js @@ -14,7 +14,6 @@ describe('checkUserPushPermission', () => { beforeEach(() => { logStub = sinon.stub(console, 'log'); - getUsersStub = sinon.stub(); isUserPushAllowedStub = sinon.stub(); @@ -108,10 +107,14 @@ describe('checkUserPushPermission', () => { it('should return error when no user is set in the action', async () => { action.user = null; + action.userEmail = null; + getUsersStub.resolves([]); const result = await exec(req, action); expect(result.steps).to.have.lengthOf(1); expect(result.steps[0].error).to.be.true; - expect(result.steps[0].errorMessage).to.include('Push blocked: User not found. Please contact an administrator for support.'); + expect(result.steps[0].errorMessage).to.include( + 'Push blocked: User not found. Please contact an administrator for support.', + ); }); }); });