Skip to content

Commit c6ed94c

Browse files
authored
Merge branch 'main' into improve-plugin-test-coverage
2 parents 544936b + 96c4754 commit c6ed94c

File tree

24 files changed

+1518
-286
lines changed

24 files changed

+1518
-286
lines changed

cypress/e2e/login.cy.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ describe('Login page', () => {
3131
cy.url().should('include', '/dashboard/repo');
3232
})
3333

34+
it('should show an error snackbar on invalid login', () => {
35+
cy.get('[data-test="username"]').type('wronguser');
36+
cy.get('[data-test="password"]').type('wrongpass');
37+
cy.get('[data-test="login"]').click();
38+
39+
cy.get('.MuiSnackbarContent-message')
40+
.should('be.visible')
41+
.and('contain', 'You entered an invalid username or password...');
42+
});
43+
3444
describe('OIDC login button', () => {
3545
it('should exist', () => {
3646
cy.get('[data-test="oidc-login"]').should('exist');

experimental/license-inventory/package-lock.json

Lines changed: 775 additions & 125 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

experimental/license-inventory/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"devDependencies": {
3434
"@eslint/compat": "^1.2.7",
3535
"@eslint/js": "^9.21.0",
36-
"@jest/globals": "^29.7.0",
36+
"@jest/globals": "^30.0.3",
3737
"@types/cors": "^2.8.17",
3838
"@types/express": "^5.0.0",
3939
"@types/mongoose": "^5.11.97",

package-lock.json

Lines changed: 14 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@finos/git-proxy",
3-
"version": "1.17.2",
3+
"version": "1.18.0",
44
"description": "Deploy custom push protections and policies on top of Git.",
55
"scripts": {
66
"cli": "node ./packages/git-proxy-cli/index.js",
@@ -9,6 +9,7 @@
99
"server": "tsx index.ts",
1010
"start": "concurrently \"npm run server\" \"npm run client\"",
1111
"build": "npm run build-ui && npm run build-lib",
12+
"build-ts": "tsc",
1213
"build-ui": "vite build",
1314
"build-lib": "./scripts/build-for-publish.sh",
1415
"restore-lib": "./scripts/undo-build.sh",
@@ -66,7 +67,7 @@
6667
"moment": "^2.29.4",
6768
"mongodb": "^5.0.0",
6869
"nodemailer": "^6.6.1",
69-
"openid-client": "^6.3.1",
70+
"openid-client": "^6.4.2",
7071
"parse-diff": "^0.11.1",
7172
"passport": "^0.7.0",
7273
"passport-activedirectory": "^1.0.4",

proxy.config.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,14 @@
153153
"type": "jwt",
154154
"enabled": false,
155155
"jwtConfig": {
156+
"authorityURL": "",
156157
"clientID": "",
157-
"authorityURL": ""
158+
"expectedAudience": "",
159+
"roleMapping": {
160+
"admin": {
161+
"": ""
162+
}
163+
}
158164
}
159165
}
160166
],

src/config/index.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ export const getDatabase = () => {
9494
* Get the list of enabled authentication methods
9595
*
9696
* At least one authentication method must be enabled.
97-
* @return {Array} List of enabled authentication methods
97+
* @return {Authentication[]} List of enabled authentication methods
9898
*/
99-
export const getAuthMethods = () => {
99+
export const getAuthMethods = (): Authentication[] => {
100100
if (_userSettings !== null && _userSettings.authentication) {
101101
_authentication = _userSettings.authentication;
102102
}
@@ -114,15 +114,19 @@ export const getAuthMethods = () => {
114114
* Get the list of enabled authentication methods for API endpoints
115115
*
116116
* If no API authentication methods are enabled, all endpoints are public.
117-
* @return {Array} List of enabled authentication methods
117+
* @return {Authentication[]} List of enabled authentication methods
118118
*/
119-
export const getAPIAuthMethods = () => {
119+
export const getAPIAuthMethods = (): Authentication[] => {
120120
if (_userSettings !== null && _userSettings.apiAuthentication) {
121121
_apiAuthentication = _userSettings.apiAuthentication;
122122
}
123123

124124
const enabledAuthMethods = _apiAuthentication.filter(auth => auth.enabled);
125125

126+
if (enabledAuthMethods.length === 0) {
127+
console.log("Warning: No authentication method enabled for API endpoints.");
128+
}
129+
126130
return enabledAuthMethods;
127131
};
128132

src/service/passport/activeDirectory.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ const configure = (passport) => {
4343
const message = `User it not a member of ${userGroup}`;
4444
return done(message, null);
4545
}
46-
} catch (e) {
47-
const message = `An error occurred while checking if the user is a member of the user group: ${JSON.stringify(e)}`;
46+
} catch (err) {
47+
console.log('ad test (isUser): e', err);
48+
const message = `An error occurred while checking if the user is a member of the user group: ${err.message}`;
4849
return done(message, null);
4950
}
5051

@@ -53,9 +54,9 @@ const configure = (passport) => {
5354
try {
5455
isAdmin = await ldaphelper.isUserInAdGroup(req, profile, ad, domain, adminGroup);
5556

56-
} catch (e) {
57-
const message = `An error occurred while checking if the user is a member of the admin group: ${JSON.stringify(e)}`;
58-
console.error(message, e); // don't return an error for this case as you may still be a user
57+
} catch (err) {
58+
const message = `An error occurred while checking if the user is a member of the admin group: ${err.message}`;
59+
console.error(message, err); // don't return an error for this case as you may still be a user
5960
}
6061

6162
profile.admin = isAdmin;

src/service/passport/index.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ const configure = async () => {
1616
passport.initialize();
1717

1818
const authMethods = config.getAuthMethods();
19-
console.log(`authMethods: ${JSON.stringify(authMethods)}`);
2019

2120
for (const auth of authMethods) {
2221
const strategy = authStrategies[auth.type.toLowerCase()];

src/service/passport/jwtAuthHandler.js

Lines changed: 14 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,19 @@
1-
const axios = require("axios");
2-
const jwt = require("jsonwebtoken");
3-
const jwkToPem = require("jwk-to-pem");
4-
const config = require('../../config');
1+
const { assignRoles, validateJwt } = require('./jwtUtils');
52

63
/**
7-
* Obtain the JSON Web Key Set (JWKS) from the OIDC authority.
8-
* @param {string} authorityUrl the OIDC authority URL. e.g. https://login.microsoftonline.com/{tenantId}
9-
* @return {Promise<object[]>} the JWKS keys
4+
* Middleware function to handle JWT authentication.
5+
* @param {*} overrideConfig optional configuration to override the default JWT configuration (e.g. for testing)
6+
* @return {Function} the middleware function
107
*/
11-
async function getJwks(authorityUrl) {
12-
try {
13-
const { data } = await axios.get(`${authorityUrl}/.well-known/openid-configuration`);
14-
const jwksUri = data.jwks_uri;
15-
16-
const { data: jwks } = await axios.get(jwksUri);
17-
return jwks.keys;
18-
} catch (error) {
19-
console.error("Error fetching JWKS:", error);
20-
throw new Error("Failed to fetch JWKS");
21-
}
22-
}
23-
24-
/**
25-
* Validate a JWT token using the OIDC configuration.
26-
* @param {*} token the JWT token
27-
* @param {*} authorityUrl the OIDC authority URL
28-
* @param {*} clientID the OIDC client ID
29-
* @param {*} expectedAudience the expected audience for the token
30-
* @return {Promise<object>} the verified payload or an error
31-
*/
32-
async function validateJwt(token, authorityUrl, clientID, expectedAudience) {
33-
try {
34-
const jwks = await getJwks(authorityUrl);
35-
36-
const decodedHeader = await jwt.decode(token, { complete: true });
37-
if (!decodedHeader || !decodedHeader.header || !decodedHeader.header.kid) {
38-
throw new Error("Invalid JWT: Missing key ID (kid)");
39-
}
40-
41-
const { kid } = decodedHeader.header;
42-
const jwk = jwks.find((key) => key.kid === kid);
43-
if (!jwk) {
44-
throw new Error("No matching key found in JWKS");
45-
}
46-
47-
const pubKey = jwkToPem(jwk);
48-
49-
const verifiedPayload = jwt.verify(token, pubKey, {
50-
algorithms: ["RS256"],
51-
issuer: authorityUrl,
52-
audience: expectedAudience,
53-
});
54-
55-
if (verifiedPayload.azp !== clientID) {
56-
throw new Error("JWT client ID does not match");
57-
}
58-
59-
return { verifiedPayload };
60-
} catch (error) {
61-
const errorMessage = `JWT validation failed: ${error.message}\n`;
62-
console.error(errorMessage);
63-
return { error: errorMessage };
64-
}
65-
}
66-
67-
const jwtAuthHandler = () => {
8+
const jwtAuthHandler = (overrideConfig = null) => {
689
return async (req, res, next) => {
69-
const apiAuthMethods = config.getAPIAuthMethods();
10+
const apiAuthMethods =
11+
overrideConfig
12+
? [{ type: "jwt", jwtConfig: overrideConfig }]
13+
: require('../../config').getAPIAuthMethods();
14+
7015
const jwtAuthMethod = apiAuthMethods.find((method) => method.type.toLowerCase() === "jwt");
71-
if (!jwtAuthMethod) {
16+
if (!overrideConfig && (!jwtAuthMethod || !jwtAuthMethod.enabled)) {
7217
return next();
7318
}
7419

@@ -81,7 +26,7 @@ const jwtAuthHandler = () => {
8126
return res.status(401).send("No token provided\n");
8227
}
8328

84-
const { clientID, authorityURL, expectedAudience } = jwtAuthMethod.jwtConfig;
29+
const { clientID, authorityURL, expectedAudience, roleMapping } = jwtAuthMethod.jwtConfig;
8530
const audience = expectedAudience || clientID;
8631

8732
if (!authorityURL) {
@@ -99,6 +44,8 @@ const jwtAuthHandler = () => {
9944
}
10045

10146
req.user = verifiedPayload;
47+
assignRoles(roleMapping, verifiedPayload, req.user);
48+
10249
return next();
10350
}
10451
}

0 commit comments

Comments
 (0)