fix: 946 associate commits by email

packages/git-proxy-cli/index.js

@@ -176,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:
@@ -223,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:
@@ -270,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:

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 should 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,12 +494,17 @@ 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.removeGitPushFromDb(pushId);
+      await helper.removeUserFromDb(TEST_USER);
+      await helper.removeRepoFromDb(TEST_REPO_CONFIG.name);
     });
 
     after(async function () {

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

src/proxy/actions/Action.ts

@@ -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;

src/proxy/processors/push-action/checkUserPushPermission.ts

@@ -7,37 +7,53 @@ const exec = async (req: any, action: Action): Promise<Action> => {
 
   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 });
+  // 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';
 
-  console.log(`Users for this git account: ${JSON.stringify(list)}`);
+  // Find the user associated with this email address
+  const list = await getUsers({ email: action.userEmail });
 
-  if (list.length == 1) {
-    user = list[0].username;
-    isUserAllowed = await isUserPushAllowed(repoName, user);
+  if (list.length > 1) {
+    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 {
+    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`);
-
-    console.log('setting error');
+    step.log(
+      `User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`,
+    );
 
     step.setError(
-      `Rejecting push as user ${action.user} ` +
+      `Your push has been blocked (${action.userEmail} ` +
         `is not allowed to push on repo ` +
-        `${action.repo}`,
+        `${action.repo})`,
     );
     action.addStep(step);
     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;
 };

src/proxy/processors/push-action/parsePush.ts

@@ -35,9 +35,10 @@
       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];
+    console.log(`Push Request received from user ${committer} with email ${committerEmail}`);
+    action.user = committer;
+    action.userEmail = committerEmail;
 
     step.content = {
       meta: meta,
@@ -111,6 +112,9 @@
       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,
@@ -119,6 +123,7 @@
         commitTimestamp,
         message,
         authorEmail,
+        committerEmail,
       });
 
       if (
@@ -128,7 +133,8 @@
         !committer ||
         !commitTimestamp ||
         !message ||
-        !authorEmail
+        !authorEmail ||
+        !committerEmail
       ) {
         throw new Error('Invalid commit data');
       }
@@ -140,7 +146,8 @@
         committer: committer.split('<')[0].trim(),
         commitTimestamp,
         message,
-        authorEmail: authorEmail,
+        authorEmail,
+        committerEmail,
       };
     })
     .value();

src/proxy/processors/push-action/pullRemote.ts

@@ -22,7 +22,7 @@ const exec = async (req: any, action: Action): Promise<Action> => {
     }
 
     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')