Skip to content

Commit 6cdac59

Browse files
authored
Merge pull request #906 from jescalada/oidc-implementation
feat: Preliminary OIDC implementation
2 parents e1a9bd2 + 8017b10 commit 6cdac59

File tree

13 files changed

+269
-35
lines changed

13 files changed

+269
-35
lines changed

cypress.config.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
const { defineConfig } = require('cypress')
1+
const { defineConfig } = require('cypress');
22

33
module.exports = defineConfig({
44
e2e: {
5-
baseUrl: process.env.CYPRESS_BASE_URL ||'http://localhost:3000',
5+
baseUrl: process.env.CYPRESS_BASE_URL || 'http://localhost:3000',
6+
chromeWebSecurity: false, // Required for OIDC testing
67
},
7-
})
8+
});

cypress/e2e/login.cy.js

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,33 @@
1-
describe("Display finos UI",()=>{
2-
3-
beforeEach(() =>{
4-
cy.visit('/login')
5-
})
6-
it('shoud find git proxy logo',() =>{
7-
cy.get('[data-test="git-proxy-logo"]').should('exist')
8-
})
9-
it('shoud find username',() =>{
10-
cy.get('[data-test="username"]').should('exist')
11-
})
1+
describe('Login page', () => {
2+
beforeEach(() => {
3+
cy.visit('/login');
4+
});
125

13-
it('shoud find passsword',() =>{
14-
cy.get('[data-test="password"]').should('exist')
15-
})
16-
it('shoud find login button',() =>{
17-
cy.get('[data-test="login"]').should('exist')
18-
})
19-
})
6+
it('should have git proxy logo', () => {
7+
cy.get('[data-test="git-proxy-logo"]').should('exist');
8+
});
9+
10+
it('should have username input', () => {
11+
cy.get('[data-test="username"]').should('exist');
12+
});
13+
14+
it('should have passsword input', () => {
15+
cy.get('[data-test="password"]').should('exist');
16+
});
17+
18+
it('should have login button', () => {
19+
cy.get('[data-test="login"]').should('exist');
20+
});
21+
22+
describe('OIDC login button', () => {
23+
it('should exist', () => {
24+
cy.get('[data-test="oidc-login"]').should('exist');
25+
});
26+
27+
// Validates that OIDC is configured correctly
28+
it('should redirect to /oidc', () => {
29+
cy.get('[data-test="oidc-login"]').click();
30+
cy.url().should('include', '/oidc');
31+
});
32+
});
33+
});

package-lock.json

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

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
"moment": "^2.29.4",
6060
"mongodb": "^5.0.0",
6161
"nodemailer": "^6.6.1",
62+
"openid-client": "^6.2.0",
6263
"parse-diff": "^0.11.1",
6364
"passport": "^0.7.0",
6465
"passport-activedirectory": "^1.0.4",

src/db/file/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ module.exports.canUserCancelPush = pushes.canUserCancelPush;
1212
module.exports.canUserApproveRejectPush = pushes.canUserApproveRejectPush;
1313

1414
module.exports.findUser = users.findUser;
15+
module.exports.findUserByOIDC = users.findUserByOIDC;
1516
module.exports.getUsers = users.getUsers;
1617
module.exports.createUser = users.createUser;
1718
module.exports.deleteUser = users.deleteUser;

src/db/file/users.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,22 @@ exports.findUser = function (username) {
2222
});
2323
};
2424

25+
exports.findUserByOIDC = function (oidcId) {
26+
return new Promise((resolve, reject) => {
27+
db.findOne({ oidcId: oidcId }, (err, doc) => {
28+
if (err) {
29+
reject(err);
30+
} else {
31+
if (!doc) {
32+
resolve(null);
33+
} else {
34+
resolve(doc);
35+
}
36+
}
37+
});
38+
});
39+
};
40+
2541
exports.createUser = function (data) {
2642
return new Promise((resolve, reject) => {
2743
db.insert(data, (err) => {

src/db/index.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,30 @@ if (config.getDatabase().type === 'mongo') {
77
sink = require('../db/file');
88
}
99

10-
module.exports.createUser = async (username, password, email, gitAccount, admin = false) => {
10+
module.exports.createUser = async (
11+
username,
12+
password,
13+
email,
14+
gitAccount,
15+
admin = false,
16+
oidcId = null,
17+
) => {
1118
console.log(
1219
`creating user
1320
user=${username},
1421
gitAccount=${gitAccount}
1522
email=${email},
16-
admin=${admin}`,
23+
admin=${admin}
24+
oidcId=${oidcId}`,
1725
);
1826

1927
const data = {
2028
username: username,
21-
password: await bcrypt.hash(password, 10),
29+
password: oidcId ? null : await bcrypt.hash(password, 10),
2230
gitAccount: gitAccount,
2331
email: email,
2432
admin: admin,
33+
oidcId: oidcId,
2534
};
2635

2736
if (username === undefined || username === null || username === '') {
@@ -56,6 +65,7 @@ module.exports.getPushes = sink.getPushes;
5665
module.exports.writeAudit = sink.writeAudit;
5766
module.exports.getPush = sink.getPush;
5867
module.exports.findUser = sink.findUser;
68+
module.exports.findUserByOIDC = sink.findUserByOIDC;
5969
module.exports.getUsers = sink.getUsers;
6070
module.exports.deleteUser = sink.deleteUser;
6171
module.exports.updateUser = sink.updateUser;

src/service/passport/index.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const local = require('./local');
22
const activeDirectory = require('./activeDirectory');
3+
const oidc = require('./oidc');
34
const config = require('../../config');
45
const authenticationConfig = config.getAuthentication();
56
let _passport;
@@ -14,10 +15,15 @@ const configure = async () => {
1415
case 'local':
1516
_passport = await local.configure();
1617
break;
18+
case 'openidconnect':
19+
_passport = await oidc.configure();
20+
break;
1721
default:
1822
throw Error(`uknown authentication type ${type}`);
1923
}
20-
_passport.type = authenticationConfig.type;
24+
if (!_passport.type) {
25+
_passport.type = type;
26+
}
2127
return _passport;
2228
};
2329

src/service/passport/local.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ const configure = async () => {
4343
const admin = await db.findUser('admin');
4444

4545
if (!admin) {
46-
await db.createUser('admin', 'admin', '[email protected]', 'none', true, true, true, true);
46+
await db.createUser('admin', 'admin', '[email protected]', 'none', true);
4747
}
4848

4949
passport.type = 'local';

src/service/passport/oidc.js

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
const passport = require('passport');
2+
const db = require('../../db');
3+
4+
const configure = async () => {
5+
// Temp fix for ERR_REQUIRE_ESM, will be changed when we refactor to ESM
6+
const { discovery, fetchUserInfo } = await import('openid-client');
7+
const { Strategy } = await import('openid-client/passport');
8+
const config = require('../../config').getAuthentication();
9+
const { oidcConfig } = config;
10+
const { issuer, clientID, clientSecret, callbackURL, scope } = oidcConfig;
11+
12+
if (!oidcConfig || !oidcConfig.issuer) {
13+
throw new Error('Missing OIDC issuer in configuration')
14+
}
15+
16+
const server = new URL(issuer);
17+
18+
try {
19+
const config = await discovery(server, clientID, clientSecret);
20+
21+
const strategy = new Strategy({ callbackURL, config, scope }, async (tokenSet, done) => {
22+
// Validate token sub for added security
23+
const idTokenClaims = tokenSet.claims();
24+
const expectedSub = idTokenClaims.sub;
25+
const userInfo = await fetchUserInfo(config, tokenSet.access_token, expectedSub);
26+
handleUserAuthentication(userInfo, done);
27+
});
28+
29+
// currentUrl must be overridden to match the callback URL
30+
strategy.currentUrl = function (request) {
31+
const callbackUrl = new URL(callbackURL);
32+
const currentUrl = Strategy.prototype.currentUrl.call(this, request);
33+
currentUrl.host = callbackUrl.host;
34+
currentUrl.protocol = callbackUrl.protocol;
35+
return currentUrl;
36+
};
37+
38+
passport.use(strategy);
39+
40+
passport.serializeUser((user, done) => {
41+
done(null, user.oidcId || user.username);
42+
})
43+
44+
passport.deserializeUser(async (id, done) => {
45+
try {
46+
const user = await db.findUserByOIDC(id);
47+
done(null, user);
48+
} catch (err) {
49+
done(err);
50+
}
51+
})
52+
passport.type = server.host;
53+
54+
return passport;
55+
} catch (error) {
56+
console.error('OIDC configuration failed:', error);
57+
throw error;
58+
}
59+
}
60+
61+
62+
module.exports.configure = configure;
63+
64+
/**
65+
* Handles user authentication with OIDC.
66+
* @param {Object} userInfo the OIDC user info object
67+
* @param {Function} done the callback function
68+
* @return {Promise} a promise with the authenticated user or an error
69+
*/
70+
const handleUserAuthentication = async (userInfo, done) => {
71+
try {
72+
const user = await db.findUserByOIDC(userInfo.sub);
73+
74+
if (!user) {
75+
const email = safelyExtractEmail(userInfo);
76+
if (!email) return done(new Error('No email found in OIDC profile'));
77+
78+
const newUser = {
79+
username: getUsername(email),
80+
email,
81+
oidcId: userInfo.sub,
82+
};
83+
84+
await db.createUser(newUser.username, null, newUser.email, 'Edit me', false, newUser.oidcId);
85+
return done(null, newUser);
86+
}
87+
88+
return done(null, user);
89+
} catch (err) {
90+
return done(err);
91+
}
92+
};
93+
94+
/**
95+
* Extracts email from OIDC profile.
96+
* This function is necessary because OIDC providers have different ways of storing emails.
97+
* @param {object} profile the profile object from OIDC provider
98+
* @return {string | null} the email address
99+
*/
100+
const safelyExtractEmail = (profile) => {
101+
return profile.email || (profile.emails && profile.emails.length > 0 ? profile.emails[0].value : null);
102+
};
103+
104+
/**
105+
* Generates a username from email address.
106+
* This helps differentiate users within the specific OIDC provider.
107+
* Note: This is incompatible with multiple providers. Ideally, users are identified by
108+
* OIDC ID (requires refactoring the database).
109+
* @param {string} email the email address
110+
* @return {string} the username
111+
*/
112+
const getUsername = (email) => {
113+
return email ? email.split('@')[0] : '';
114+
};

0 commit comments

Comments
 (0)