Merge pull request #973 from kriswest/946-associate-commits-by-email-rebase

fix: 946 associate commits by email

Author: jescalada

.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"]
 }

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 = '[email protected]';
+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 = '[email protected]';
-
     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', '[email protected]', 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

@@ -5,28 +5,29 @@ import { trimTrailingDotGit } from '../../../db/helper';
 // Execute if the repo is approved
 const exec = async (req: any, action: Action): Promise<Action> => {
   const step = new Step('checkUserPushPermission');
-  const user = action.user;
+  const userEmail = action.userEmail;
 
-  if (!user) {
+  if (!userEmail) {
     step.setError('Push blocked: User not found. Please contact an administrator for support.');
     action.addStep(step);
     step.error = true;
     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.
- * @param {string} user The user to validate
+ * @param {string} userEmail The user to validate
  * @param {Action} action The action object
  * @param {Step} step The step object
  * @return {Promise<Action>} The action object
  */
-const validateUser = async (user: string, action: Action, step: Step): Promise<Action> => { 
+const validateUser = async (userEmail: string, action: Action, step: Step): Promise<Action> => {
   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');
@@ -37,35 +38,44 @@ const validateUser = async (user: string, action: Action, step: Step): Promise<A
   const repoName = repoSplit[1];
   let isUserAllowed = false;
 
-  // Find the user associated with this Git Account
-  const list = await getUsers({ gitAccount: action.user });
+  // Find the user associated with this email address
+  const list = await getUsers({ email: userEmail });
 
-  console.log(`Users for this git account: ${JSON.stringify(list)}`);
+  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`,
+    );
 
-  if (list.length == 1) {
-    user = list[0].username;
-    isUserAllowed = await isUserPushAllowed(repoName, user);
+    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 {
+    isUserAllowed = await isUserPushAllowed(repoName, list[0].username);
   }
 
-  console.log(`User ${user} 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 ${user} is not allowed to push on repo ${action.repo}, ending`);
-
-    console.log('setting error');
+    step.log(`User ${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 ${userEmail} is allowed to push on repo ${action.repo}`);
   action.addStep(step);
   return action;
 };