Merge branch 'main' into improve-auth-test-coverage

Author: JamieSlome

cypress/e2e/login.cy.js

@@ -31,6 +31,16 @@ describe('Login page', () => {
     cy.url().should('include', '/dashboard/repo');
   })
 
+  it('should show an error snackbar on invalid login', () => {
+    cy.get('[data-test="username"]').type('wronguser');
+    cy.get('[data-test="password"]').type('wrongpass');
+    cy.get('[data-test="login"]').click();
+
+    cy.get('.MuiSnackbarContent-message')
+      .should('be.visible')
+      .and('contain', 'You entered an invalid username or password...');
+  });
+
   describe('OIDC login button', () => {
     it('should exist', () => {
       cy.get('[data-test="oidc-login"]').should('exist');

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@finos/git-proxy",
-  "version": "1.17.2",
+  "version": "1.18.0",
   "description": "Deploy custom push protections and policies on top of Git.",
   "scripts": {
     "cli": "node ./packages/git-proxy-cli/index.js",
@@ -9,6 +9,7 @@
     "server": "tsx index.ts",
     "start": "concurrently \"npm run server\" \"npm run client\"",
     "build": "npm run build-ui && npm run build-lib",
+    "build-ts": "tsc",
     "build-ui": "vite build",
     "build-lib": "./scripts/build-for-publish.sh",
     "restore-lib": "./scripts/undo-build.sh",
@@ -66,7 +67,7 @@
     "moment": "^2.29.4",
     "mongodb": "^5.0.0",
     "nodemailer": "^6.6.1",
-    "openid-client": "^6.3.1",
+    "openid-client": "^6.4.2",
     "parse-diff": "^0.11.1",
     "passport": "^0.7.0",
     "passport-activedirectory": "^1.0.4",

package.json

@@ -153,8 +153,14 @@
       "type": "jwt",
       "enabled": false,
       "jwtConfig": {
+        "authorityURL": "",
         "clientID": "",
-        "authorityURL": ""
+        "expectedAudience": "",
+        "roleMapping": {
+          "admin": {
+            "": ""
+          }
+        }
       }
     }
   ],

proxy.config.json

@@ -94,9 +94,9 @@ 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
+ * @return {Authentication[]} List of enabled authentication methods
  */
-export const getAuthMethods = () => {
+export const getAuthMethods = (): Authentication[] => {
   if (_userSettings !== null && _userSettings.authentication) {
     _authentication = _userSettings.authentication;
   }
@@ -114,15 +114,19 @@ 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
+ * @return {Authentication[]} List of enabled authentication methods
  */
-export const getAPIAuthMethods = () => {
+export const getAPIAuthMethods = (): Authentication[] => {
   if (_userSettings !== null && _userSettings.apiAuthentication) {
     _apiAuthentication = _userSettings.apiAuthentication;
   }
 
   const enabledAuthMethods = _apiAuthentication.filter(auth => auth.enabled);
 
+  if (enabledAuthMethods.length === 0) {
+    console.log("Warning: No authentication method enabled for API endpoints.");
+  }
+
   return enabledAuthMethods;
 };
 

src/config/index.ts

@@ -16,7 +16,6 @@ const configure = async () => {
   passport.initialize();
 
   const authMethods = config.getAuthMethods();
-  console.log(`authMethods: ${JSON.stringify(authMethods)}`);
 
   for (const auth of authMethods) {
     const strategy = authStrategies[auth.type.toLowerCase()];

src/service/passport/index.js

@@ -1,74 +1,19 @@
-const axios = require("axios");
-const jwt = require("jsonwebtoken");
-const jwkToPem = require("jwk-to-pem");
-const config = require('../../config');
+const { assignRoles, validateJwt } = require('./jwtUtils');
 
 /**
- * Obtain the JSON Web Key Set (JWKS) from the OIDC authority.
- * @param {string} authorityUrl the OIDC authority URL. e.g. https://login.microsoftonline.com/{tenantId}
- * @return {Promise<object[]>} the JWKS keys
+ * Middleware function to handle JWT authentication.
+ * @param {*} overrideConfig optional configuration to override the default JWT configuration (e.g. for testing)
+ * @return {Function} the middleware function 
  */
-async function getJwks(authorityUrl) {
-  try {
-      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;
-  } catch (error) {
-      console.error("Error fetching JWKS:", error);
-      throw new Error("Failed to fetch JWKS");
-  }
-}
-
-/**
- * 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 {*} expectedAudience the expected audience for the token
- * @return {Promise<object>} the verified payload or an error
- */
-async function validateJwt(token, authorityUrl, clientID, expectedAudience) {
-  try {
-      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 { 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 verifiedPayload = jwt.verify(token, pubKey, {
-          algorithms: ["RS256"],
-          issuer: authorityUrl,
-          audience: expectedAudience,
-      });
-
-      if (verifiedPayload.azp !== clientID) {
-          throw new Error("JWT client ID does not match");
-      }
-
-      return { verifiedPayload };
-  } catch (error) {
-      const errorMessage = `JWT validation failed: ${error.message}\n`;
-      console.error(errorMessage);
-      return { error: errorMessage };
-  }
-}
-
-const jwtAuthHandler = () => {
+const jwtAuthHandler = (overrideConfig = null) => {
   return async (req, res, next) => {
-    const apiAuthMethods = config.getAPIAuthMethods();
+    const apiAuthMethods = 
+        overrideConfig
+          ? [{ type: "jwt", jwtConfig: overrideConfig }]
+          : require('../../config').getAPIAuthMethods();
+
     const jwtAuthMethod = apiAuthMethods.find((method) => method.type.toLowerCase() === "jwt");
-      if (!jwtAuthMethod) {
+      if (!overrideConfig && (!jwtAuthMethod || !jwtAuthMethod.enabled)) {
           return next();
       }
 
@@ -81,7 +26,7 @@ const jwtAuthHandler = () => {
           return res.status(401).send("No token provided\n");
       }
 
-      const { clientID, authorityURL, expectedAudience } = jwtAuthMethod.jwtConfig;
+      const { clientID, authorityURL, expectedAudience, roleMapping } = jwtAuthMethod.jwtConfig;
       const audience = expectedAudience || clientID;
 
       if (!authorityURL) {
@@ -99,6 +44,8 @@ const jwtAuthHandler = () => {
       }
 
       req.user = verifiedPayload;
+      assignRoles(roleMapping, verifiedPayload, req.user);
+
       return next();
   }
 }

src/service/passport/jwtAuthHandler.js

@@ -0,0 +1,93 @@
+const axios = require("axios");
+const jwt = require("jsonwebtoken");
+const jwkToPem = require("jwk-to-pem");
+
+/**
+ * Obtain the JSON Web Key Set (JWKS) from the OIDC authority.
+ * @param {string} authorityUrl the OIDC authority URL. e.g. https://login.microsoftonline.com/{tenantId}
+ * @return {Promise<object[]>} the JWKS keys
+ */
+async function getJwks(authorityUrl) {
+  try {
+      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;
+  } catch (error) {
+      console.error("Error fetching JWKS:", error);
+      throw new Error("Failed to fetch JWKS");
+  }
+}
+
+/**
+ * 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 {*} expectedAudience the expected audience for the token
+ * @param {*} getJwksInject the getJwks function to use (for dependency injection). Defaults to the built-in getJwks function.
+ * @return {Promise<object>} the verified payload or an error
+ */
+async function validateJwt(token, authorityUrl, clientID, expectedAudience, getJwksInject = getJwks) {
+  try {
+      const jwks = await getJwksInject(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 { 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 verifiedPayload = jwt.verify(token, pubKey, {
+          algorithms: ["RS256"],
+          issuer: authorityUrl,
+          audience: expectedAudience,
+      });
+
+      if (verifiedPayload.azp !== clientID) {
+          throw new Error("JWT client ID does not match");
+      }
+
+      return { verifiedPayload };
+  } catch (error) {
+      const errorMessage = `JWT validation failed: ${error.message}\n`;
+      console.error(errorMessage);
+      return { error: errorMessage };
+  }
+}
+
+/**
+ * Assign roles to the user based on the role mappings provided in the jwtConfig.
+ * 
+ * If no role mapping is provided, the user will not have any roles assigned (i.e. user.admin = false).
+ * @param {*} roleMapping the role mapping configuration
+ * @param {*} payload the JWT payload
+ * @param {*} user the req.user object to assign roles to
+ */
+function assignRoles(roleMapping, payload, user) {
+    if (roleMapping) {
+        for (const role of Object.keys(roleMapping)) {
+            const claimValuePair = roleMapping[role];
+            const claim = Object.keys(claimValuePair)[0];
+            const value = claimValuePair[claim];
+
+            if (payload[claim] && payload[claim] === value) {
+                user[role] = true;
+            } 
+        }
+    }
+}
+
+module.exports = {
+  getJwks,
+  validateJwt,
+  assignRoles,
+};

src/service/passport/jwtUtils.js

@@ -17,10 +17,16 @@ const configure = async (passport) => {
   }
 
   const server = new URL(issuer);
+  let config;
 
   try {
-    const config = await discovery(server, clientID, clientSecret);
+    config = await discovery(server, clientID, clientSecret);
+  } catch (error) {
+    console.error('Error during OIDC discovery:', error);
+    throw new Error('OIDC setup error (discovery): ' + error.message);
+  }
 
+  try {
     const strategy = new Strategy({ callbackURL, config, scope }, async (tokenSet, done) => {
       // Validate token sub for added security
       const idTokenClaims = tokenSet.claims();
@@ -56,8 +62,8 @@ const configure = async (passport) => {
 
     return passport;
   } catch (error) {
-    console.error('OIDC configuration failed:', error);
-    throw error;
+    console.error('Error during OIDC passport setup:', error);
+    throw new Error('OIDC setup error (strategy): ' + error.message);
   }
 };