feat: support direct querying of AD group membership via LDAP

.husky/commit-msg

@@ -1,4 +1 @@
-#!/usr/bin/env sh
-. "$(dirname -- "$0")/_/husky.sh"
-
 npx --no -- commitlint --edit ${1} && npm run lint

config.schema.json

@@ -10,7 +10,32 @@
     "sessionMaxAgeHours": { "type": "number" },
     "api": {
       "description": "Third party APIs",
-      "type": "object"
+      "type": "object",
+      "properties": {
+        "ls": {
+          "type": "object",
+          "description": "Configuration used in conjunction with ActiveDirectory auth, which relates to a REST API used to check user group membership, as opposed to direct querying via LDAP.<br />If this configuration is set direct querying of group membership via LDAP will be disabled.",
+          "properties": {
+            "userInADGroup": {
+              "type": "string",
+              "description": "URL template for a GET request that confirms a user's membership of a specific group. Should respond with a non-empty 200 status if the user is a member of the group, an empty response or non-200 status indicates that the user is not a group member. If set, this URL will be queried and direct queries via LDAP will be disabled. The template should contain the following string placeholders, which will be replaced to produce the final URL:<ul><li>\"&lt;domain&gt;\": AD domain,</li><li>\"&lt;name&gt;\": The group name to check membership of.</li><li>\"&lt;id&gt;\": The username to check group membership for.</li></ul>",
+              "examples": [
+                "https://somedomain.com/some/path/checkUserGroups?domain=<domain>&name=<name>&id=<id>"
+              ]
+            }
+          }
+        },
+        "github": {
+          "type": "object",
+          "properties": {
+            "baseUrl": {
+              "type": "string",
+              "format": "uri",
+              "examples": ["https://api.github.com"]
+            }
+          }
+        }
+      }
     },
     "commitConfig": {
       "description": "Enforce rules and patterns on commits including e-mail and message",
@@ -113,10 +138,39 @@
     },
     "authentication": {
       "type": "object",
+      "description": "Configuration for an authentication source",
       "properties": {
-        "type": { "type": "string" },
+        "type": { "type": "string", "enum": ["local", "ActiveDirectory", "OpenIdConnect"] },
         "enabled": { "type": "boolean" },
-        "options": { "type": "object" }
+        "adminGroup": {
+          "type": "string",
+          "description": "Group that indicates that a user is an admin"
+        },
+        "userGroup": {
+          "type": "string",
+          "description": "Group that indicates that a user should be able to login to the Git Proxy UI and can work as a reviewer"
+        },
+        "domain": { "type": "string", "description": "Active Directory domain" },
+        "adConfig": {
+          "type": "object",
+          "description": "Additional Active Directory configuration supporting LDAP connection which can be used to confirm group membership. For the full set of available options see the activedirectory 2 NPM module docs at https://www.npmjs.com/package/activedirectory2#activedirectoryoptions <br /><br />Please note that if the Third Party APIs config `api.ls.userInADGroup` is set then the REST API it represents is used in preference to direct querying of group memebership via LDAP.",
+          "properties": {
+            "url": {
+              "type": "string",
+              "description": "Active Directory server to connect to, e.g. `ldap://ad.example.com`."
+            },
+            "baseDN": {
+              "type": "string",
+              "description": "The root DN from which all searches will be performed, e.g. `dc=example,dc=com`."
+            },
+            "username": {
+              "type": "string",
+              "description": "An account name capable of performing the operations desired."
+            },
+            "password": { "type": "string", "description": "Password for the given `username`." }
+          },
+          "required": ["url", "baseDN", "username", "password"]
+        }
       },
       "required": ["type", "enabled"]
     }

proxy.config.json

@@ -47,7 +47,9 @@
       "adConfig": {
         "url": "",
         "baseDN": "",
-        "searchBase": ""
+        "searchBase": "",
+        "username": "",
+        "password": ""
       }
     }
   ],

src/config/index.ts

@@ -22,8 +22,8 @@
 let _api: Record<string, unknown> = defaultSettings.api;
 let _cookieSecret: string = defaultSettings.cookieSecret;
 let _sessionMaxAgeHours: number = defaultSettings.sessionMaxAgeHours;
 let _plugins: any[] = defaultSettings.plugins;
 let _commitConfig: Record<string, any> = defaultSettings.commitConfig;
 let _attestationConfig: Record<string, unknown> = defaultSettings.attestationConfig;
 let _privateOrganizations: string[] = defaultSettings.privateOrganizations;
 let _urlShortener: string = defaultSettings.urlShortener;
@@ -75,7 +75,7 @@
     }
   }
 
-  throw Error('No database cofigured!');
+  throw Error('No database configured!');
 };
 
 // Gets the configured authentication method, defaults to local
@@ -91,7 +91,7 @@
     }
   }
 
-  throw Error('No authentication cofigured!');
+  throw Error('No authentication configured!');
 };
 
 // Log configuration to console

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

@@ -70,7 +70,7 @@ const exec = async (req: any, action: Action): Promise<Action> => {
     step.error = true;
     step.log(`The following commit messages are illegal: ${illegalMessages}`);
     step.setError(
-      '\n\n\n\nYour push has been blocked.\nPlease ensure your commit message(s) does not contain sensitive information or URLs.\n\n\n',
+      `\n\n\nYour push has been blocked.\nPlease ensure your commit message(s) does not contain sensitive information or URLs.\n\nThe following commit messages are illegal: ${illegalMessages}\n\n`,
     );
 
     action.addStep(step);

src/service/passport/activeDirectory.js

@@ -29,16 +29,27 @@
             profile._json.userPrincipalName
           }, profile=${JSON.stringify(profile)}`,
         );
-        // First check to see if the user is in the usergroups
-        const isUser = await ldaphelper.isUserInAdGroup(profile.username, domain, userGroup);
-
-        if (!isUser) {
-          const message = `User it not a member of ${userGroup}`;
+        // First check to see if the user is in the AD user group
+        try {
+          const isUser = await ldaphelper.isUserInAdGroup(req, profile, ad, domain, userGroup);
+          if (!isUser) {
+            const message = `User it not a member of ${userGroup}`;
+            return done(message, null);
+          }
+        } catch (e) {
+          const message = `An error occurred while checking if the user is a member of the user group: ${JSON.stringify(e)}`;
           return done(message, null);
         }
-
+        
         // Now check if the user is an admin
-        const isAdmin = await ldaphelper.isUserInAdGroup(profile.username, domain, adminGroup);
+        let isAdmin = false;
+        try {
+          isAdmin = await ldaphelper.isUserInAdGroup(req, profile, ad, domain, adminGroup);
+
+        } catch (e) {
+          const message = `An error occurred while checking if the user is a member of the admin group: ${JSON.stringify(e)}`;
+          console.error(message, e);
+        }
 
         profile.admin = isAdmin;
         console.log(`passport.activeDirectory: ${profile.username} admin=${isAdmin}`);
@@ -65,6 +76,7 @@
   passport.deserializeUser(function (user, done) {
     done(null, user);
   });
+  passport.type = "ActiveDirectory";
 
   return passport;
 };

src/service/passport/index.js

@@ -19,7 +19,7 @@
       _passport = await oidc.configure();
       break;
     default:
-      throw Error(`uknown authentication type ${type}`);
+      throw Error(`unknown authentication type ${type}`);
   }
   if (!_passport.type) {
     _passport.type = type;

src/service/passport/ldaphelper.js

@@ -1,18 +1,48 @@
-const axios = require('axios');
+const config = require('../../config').getAuthentication();
 const thirdpartyApiConfig = require('../../config').getAPIs();
-const client = axios.create({
-  responseType: 'json',
-  headers: {
-    'content-type': 'application/json',
-  },
-});
+const axios = require('axios');
+
+const isUserInAdGroup = (req, profile, ad, domain, name) => {
+  // determine, via config, if we're using HTTP or AD directly
+  if (thirdpartyApiConfig?.ls?.userInADGroup) {
+    return isUserInAdGroupViaHttp(profile.username, domain, name);
+  } else if (config.adConfig) {
+    return isUserInAdGroupViaAD(req, profile, ad, domain, name);
+  } else {
+    console.error('Unable to check user groups as config is incomplete or unreadable');
+  }
+};
+
+const isUserInAdGroupViaAD = (req, profile, ad, domain, name) => {
+  // TODO: Added for debugging , needs to be removed
+  console.log(`checking if id: ${profile.username}, on domain: ${domain}, is a member of group name: ${name}`);
 
-const isUserInAdGroup = (id, domain, name) => {
+  return new Promise((resolve, reject) => {
+    ad.isUserMemberOf(profile.username, name, function (err, isMember) {
+      if (err) {
+        const msg = 'ERROR isUserMemberOf: ' + JSON.stringify(err);
+        reject(msg);
+      } else {
+        console.log(profile.username + ' isMemberOf ' + name + ': ' + isMember);
+        resolve(isMember);
+      }
+    });
+  });
+};
+
+const isUserInAdGroupViaHttp = (id, domain, name) => {
   const url = String(thirdpartyApiConfig.ls.userInADGroup)
     .replace('<domain>', domain)
     .replace('<name>', name)
     .replace('<id>', id);
 
+  const client = axios.create({
+    responseType: 'json',
+    headers: {
+      'content-type': 'application/json',
+    },
+  });
+
   console.log(`checking if user is in group ${url}`);
   return client
     .get(url)

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', '[email protected]', '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,10 @@ describe('add new repo', async () => {
 
   after(async function () {
     await service.httpServer.close();
+
+    // 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');
   });
 });

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) {