Skip to content

Commit cd1dd89

Browse files
authored
Merge pull request #1061 from 06kellyjac/fix_multi_auth
fix: allow for auth with activedirectory again
2 parents 41dbc46 + e85b3bd commit cd1dd89

File tree

3 files changed

+58
-19
lines changed

3 files changed

+58
-19
lines changed

src/config/ConfigLoader.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ export class ConfigLoader extends EventEmitter {
316316
const execOptions = {
317317
cwd: process.cwd(),
318318
env: {
319+
// dont wait for credentials; the command should be sufficiently authed
320+
GIT_TERMINAL_PROMPT: 0,
319321
...process.env,
320322
...(source.auth?.type === 'ssh'
321323
? {

src/service/routes/auth.js

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
const express = require('express');
22
const router = new express.Router();
33
const passport = require('../passport').getPassport();
4+
const { getAuthMethods } = require('../../config');
5+
const passportLocal = require('../passport/local');
6+
const passportAD = require('../passport/activeDirectory');
47
const authStrategies = require('../passport').authStrategies;
58
const db = require('../../db');
69
const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 3000 } =
@@ -23,25 +26,59 @@ router.get('/', (req, res) => {
2326
});
2427
});
2528

26-
router.post('/login', passport.authenticate(authStrategies['local'].type), async (req, res) => {
27-
try {
28-
const currentUser = { ...req.user };
29-
delete currentUser.password;
30-
console.log(
31-
`serivce.routes.auth.login: user logged in, username=${
32-
currentUser.username
33-
} profile=${JSON.stringify(currentUser)}`,
34-
);
35-
res.send({
36-
message: 'success',
37-
user: currentUser,
38-
});
39-
} catch (e) {
40-
console.log(`service.routes.auth.login: Error logging user in ${JSON.stringify(e)}`);
41-
res.status(500).send('Failed to login').end();
42-
return;
29+
// login strategies that will work with /login e.g. take username and password
30+
const appropriateLoginStrategies = [passportLocal.type, passportAD.type];
31+
// getLoginStrategy fetches the enabled auth methods and identifies if there's an appropriate
32+
// auth method for username and password login. If there isn't it returns null, if there is it
33+
// returns the first.
34+
const getLoginStrategy = () => {
35+
// returns only enabled auth methods
36+
// returns at least one enabled auth method
37+
const enabledAppropriateLoginStrategies = getAuthMethods().filter((am) =>
38+
appropriateLoginStrategies.includes(am.type.toLowerCase()),
39+
);
40+
// for where no login strategies which work for /login are enabled
41+
// just return null
42+
if (enabledAppropriateLoginStrategies.length === 0) {
43+
return null;
4344
}
44-
});
45+
// return the first enabled auth method
46+
return enabledAppropriateLoginStrategies[0].type.toLowerCase();
47+
};
48+
49+
// TODO: provide separate auth endpoints for each auth strategy or chain compatibile auth strategies
50+
// TODO: if providing separate auth methods, inform the frontend so it has relevant UI elements and appropriate client-side behavior
51+
router.post(
52+
'/login',
53+
(req, res, next) => {
54+
const authType = getLoginStrategy();
55+
if (authType === null) {
56+
res.status(403).send('Username and Password based Login is not enabled at this time').end();
57+
return;
58+
}
59+
console.log('going to auth with', authType);
60+
return passport.authenticate(authType)(req, res, next);
61+
},
62+
async (req, res) => {
63+
try {
64+
const currentUser = { ...req.user };
65+
delete currentUser.password;
66+
console.log(
67+
`serivce.routes.auth.login: user logged in, username=${
68+
currentUser.username
69+
} profile=${JSON.stringify(currentUser)}`,
70+
);
71+
res.send({
72+
message: 'success',
73+
user: currentUser,
74+
});
75+
} catch (e) {
76+
console.log(`service.routes.auth.login: Error logging user in ${JSON.stringify(e)}`);
77+
res.status(500).send('Failed to login').end();
78+
return;
79+
}
80+
},
81+
);
4582

4683
router.get('/oidc', passport.authenticate(authStrategies['openidconnect'].type));
4784

test/ConfigLoader.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ describe('ConfigLoader', () => {
437437
it('should throw error if repository is a valid URL but not a git repository', async function () {
438438
const source = {
439439
type: 'git',
440-
repository: 'https://github.com/test-org/test-repo.git',
440+
repository: 'https://github.com/finos/made-up-test-repo.git',
441441
path: 'proxy.config.json',
442442
branch: 'main',
443443
enabled: true,

0 commit comments

Comments
 (0)