Merge pull request #963 from jescalada/enable-multiple-auth-methods

feat: enable multiple auth methods

Author: JamieSlome

cypress/e2e/login.cy.js

@@ -38,8 +38,10 @@ describe('Login page', () => {
 
     // Validates that OIDC is configured correctly
     it('should redirect to /oidc', () => {
+      // Set intercept first, since redirect on click can be quick
+      cy.intercept('GET', '/api/auth/oidc').as('oidcRedirect');
       cy.get('[data-test="oidc-login"]').click();
-      cy.url().should('include', '/oidc');
+      cy.wait('@oidcRedirect');
     });
   });
 });

package-lock.json

@@ -109,6 +109,7 @@
     "mocha": "^10.8.2",
     "nyc": "^17.0.0",
     "prettier": "^3.0.0",
+    "proxyquire": "^2.1.3",
     "sinon": "^19.0.2",
     "ts-mocha": "^11.1.0",
     "ts-node": "^10.9.2",

package.json

@@ -53,6 +53,17 @@
         "baseDN": "",
         "searchBase": ""
       }
+    },
+    {
+      "type": "openidconnect",
+      "enabled": false,
+      "oidcConfig": {
+        "issuer": "",
+        "clientID": "",
+        "clientSecret": "",
+        "callbackURL": "",
+        "scope": ""
+      }
     }
   ],
   "api": {

proxy.config.json

@@ -89,27 +89,29 @@ export const getDatabase = () => {
   throw Error('No database cofigured!');
 };
 
-// Gets the configured authentication method, defaults to local
-export const getAuthentication = () => {
+/**
+ * Get the list of enabled authentication methods
+ * @return {Array} List of enabled authentication methods
+ */
+export const getAuthMethods = () => {
   if (_userSettings !== null && _userSettings.authentication) {
     _authentication = _userSettings.authentication;
   }
-  for (const ix in _authentication) {
-    if (!ix) continue;
-    const auth = _authentication[ix];
-    if (auth.enabled) {
-      return auth;
-    }
+
+  const enabledAuthMethods = _authentication.filter((auth) => auth.enabled);
+
+  if (enabledAuthMethods.length === 0) {
+    throw new Error("No authentication method enabled");
   }
 
-  throw Error('No authentication cofigured!');
+  return enabledAuthMethods;
 };
 
 // Log configuration to console
 export const logConfiguration = () => {
   console.log(`authorisedList = ${JSON.stringify(getAuthorisedList())}`);
   console.log(`data sink = ${JSON.stringify(getDatabase())}`);
-  console.log(`authentication = ${JSON.stringify(getAuthentication())}`);
+  console.log(`authentication = ${JSON.stringify(getAuthMethods())}`);
   console.log(`rateLimit = ${JSON.stringify(getRateLimit())}`);
 };
 

src/config/index.ts

@@ -1,16 +1,19 @@
-const configure = () => {
-  const passport = require('passport');
-  const ActiveDirectoryStrategy = require('passport-activedirectory');
-  const config = require('../../config').getAuthentication();
-  const adConfig = config.adConfig;
+const ActiveDirectoryStrategy = require('passport-activedirectory');
+const ldaphelper = require('./ldaphelper');
+
+const configure = (passport) => {
   const db = require('../../db');
-  const userGroup = config.userGroup;
-  const adminGroup = config.adminGroup;
-  const domain = config.domain;
+
+  // We can refactor this by normalizing auth strategy config and pass it directly into the configure() function,
+  // ideally when we convert this to TS.
+  const authMethods = require('../../config').getAuthMethods();
+  const config = authMethods.find((method) => method.type.toLowerCase() === "activeDirectory");
+  const adConfig = config.adConfig;
+
+  const { userGroup, adminGroup, domain } = config;
 
   console.log(`AD User Group: ${userGroup}, AD Admin Group: ${adminGroup}`);
 
-  const ldaphelper = require('./ldaphelper');
   passport.use(
     new ActiveDirectoryStrategy(
       {
@@ -19,42 +22,47 @@ const configure = () => {
         ldap: adConfig,
       },
       async function (req, profile, ad, done) {
-        profile.username = profile._json.sAMAccountName.toLowerCase();
-        profile.email = profile._json.mail;
-        profile.id = profile.username;
-        req.user = profile;
-
-        console.log(
-          `passport.activeDirectory: resolved login ${
-            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}`;
-          return done(message, null);
-        }
+        try {
+          profile.username = profile._json.sAMAccountName?.toLowerCase();
+          profile.email = profile._json.mail;
+          profile.id = profile.username;
+          req.user = profile;
 
-        // Now check if the user is an admin
-        const isAdmin = await ldaphelper.isUserInAdGroup(profile.username, domain, adminGroup);
+          console.log(
+            `passport.activeDirectory: resolved login ${
+              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);
 
-        profile.admin = isAdmin;
-        console.log(`passport.activeDirectory: ${profile.username} admin=${isAdmin}`);
+          if (!isUser) {
+            const message = `User it not a member of ${userGroup}`;
+            return done(message, null);
+          }
 
-        const user = {
-          username: profile.username,
-          admin: isAdmin,
-          email: profile._json.mail,
-          displayName: profile.displayName,
-          title: profile._json.title,
-        };
+          // Now check if the user is an admin
+          const isAdmin = await ldaphelper.isUserInAdGroup(profile.username, domain, adminGroup);
 
-        await db.updateUser(user);
+          profile.admin = isAdmin;
+          console.log(`passport.activeDirectory: ${profile.username} admin=${isAdmin}`);
 
-        return done(null, user);
-      },
+          const user = {
+            username: profile.username,
+            admin: isAdmin,
+            email: profile._json.mail,
+            displayName: profile.displayName,
+            title: profile._json.title,
+          };
+
+          await db.updateUser(user);
+
+          return done(null, user);
+        } catch (err) {
+          console.log(`Error authenticating AD user: ${err.message}`);
+          return done(err, null);
+        }
+      }
     ),
   );
 
@@ -69,4 +77,4 @@ const configure = () => {
   return passport;
 };
 
-module.exports.configure = configure;
+module.exports = { configure };

src/service/passport/activeDirectory.js

@@ -1,33 +1,36 @@
+const passport = require("passport");
 const local = require('./local');
 const activeDirectory = require('./activeDirectory');
 const oidc = require('./oidc');
 const config = require('../../config');
-const authenticationConfig = config.getAuthentication();
-let _passport;
+
+// Allows obtaining strategy config function and type
+// Keep in mind to add AuthStrategy enum when refactoring this to TS
+const authStrategies = {
+  local: local,
+  activedirectory: activeDirectory,
+  openidconnect: oidc,
+};
 
 const configure = async () => {
-  const type = authenticationConfig.type.toLowerCase();
-
-  switch (type) {
-    case 'activedirectory':
-      _passport = await activeDirectory.configure();
-      break;
-    case 'local':
-      _passport = await local.configure();
-      break;
-    case 'openidconnect':
-      _passport = await oidc.configure();
-      break;
-    default:
-      throw Error(`uknown authentication type ${type}`);
+  passport.initialize();
+
+  const authMethods = config.getAuthMethods();
+
+  for (const auth of authMethods) {
+    const strategy = authStrategies[auth.type.toLowerCase()];
+    if (strategy && typeof strategy.configure === "function") {
+      await strategy.configure(passport);
+    }
   }
-  if (!_passport.type) {
-    _passport.type = type;
+
+  if (authMethods.some(auth => auth.type.toLowerCase() === "local")) {
+    await local.createDefaultAdmin();
   }
-  return _passport;
-};
 
-module.exports.configure = configure;
-module.exports.getPassport = () => {
-  return _passport;
+  return passport;
 };
+
+const getPassport = () => passport;
+
+module.exports = { authStrategies, configure, getPassport };