From d3d04a0d6bcb03c54fd11254618a05bc6c1e8ccd Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 31 Jan 2025 18:17:25 +0900 Subject: [PATCH 01/12] feat(auth): add OIDC login button to login page --- src/ui/views/Login/Login.jsx | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/ui/views/Login/Login.jsx b/src/ui/views/Login/Login.jsx index 46c989ca2..719714ec2 100644 --- a/src/ui/views/Login/Login.jsx +++ b/src/ui/views/Login/Login.jsx @@ -33,6 +33,10 @@ export default function UserProfile() { ); } + function handleOIDCLogin() { + window.location.href = `${import.meta.env.VITE_API_URI}/api/auth/oidc`; + } + function handleSubmit(event) { setIsLoading(true); axios @@ -104,7 +108,7 @@ export default function UserProfile() { width={'150px'} src={logo} alt='logo' - data-test ="git-proxy-logo" + data-test='git-proxy-logo' /> @@ -119,7 +123,7 @@ export default function UserProfile() { value={username} onChange={(e) => setUsername(e.target.value)} autoFocus={true} - data-test ='username' + data-test='username' /> @@ -133,7 +137,7 @@ export default function UserProfile() { type='password' value={password} onChange={(e) => setPassword(e.target.value)} - data-test ='password' + data-test='password' /> @@ -141,9 +145,20 @@ export default function UserProfile() { {!isLoading ? ( - + <> + + + ) : (
From d157d965be26c83d607bc03e2b73cb5552dd2a7e Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 31 Jan 2025 18:20:04 +0900 Subject: [PATCH 02/12] feat(auth): add OIDC strategy to passport --- src/service/passport/index.js | 4 ++ src/service/passport/oidc.js | 91 +++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 src/service/passport/oidc.js diff --git a/src/service/passport/index.js b/src/service/passport/index.js index 92a1c0bd5..9a16ef2fc 100644 --- a/src/service/passport/index.js +++ b/src/service/passport/index.js @@ -1,5 +1,6 @@ const local = require('./local'); const activeDirectory = require('./activeDirectory'); +const oidc = require('./oidc'); const config = require('../../config'); const authenticationConfig = config.getAuthentication(); let _passport; @@ -14,6 +15,9 @@ const configure = async () => { case 'local': _passport = await local.configure(); break; + case 'openidconnect': + _passport = await oidc.configure(); + break; default: throw Error(`uknown authentication type ${type}`); } diff --git a/src/service/passport/oidc.js b/src/service/passport/oidc.js new file mode 100644 index 000000000..8e49866f5 --- /dev/null +++ b/src/service/passport/oidc.js @@ -0,0 +1,91 @@ +const configure = async () => { + const passport = require('passport'); + const { Strategy: OIDCStrategy } = require('passport-openidconnect'); + const db = require('../../db'); + + const config = require('../../config').getAuthentication(); + const oidcConfig = config.oidcConfig; + + passport.use( + new OIDCStrategy(oidcConfig, async function verify(issuer, profile, cb) { + try { + const user = await db.findUserByOIDC(profile.id); + + if (!user) { + const email = safelyExtractEmail(profile); + if (!email) { + return cb(new Error('No email found in OIDC profile')); + } + + const username = getUsername(email); + const newUser = { + username: username, + email: email, + oidcId: profile.id, + }; + + await db.createUser( + newUser.username, + null, + newUser.email, + 'Edit me', + false, + newUser.oidcId, + ); + + return cb(null, newUser); + } + return cb(null, user); + } catch (err) { + return cb(err); + } + }), + ); + + passport.serializeUser((user, cb) => { + cb(null, user.oidcId || user.username); + }); + + passport.deserializeUser(async (id, cb) => { + try { + const user = (await db.findUserByOIDC(id)) || (await db.findUser(id)); + cb(null, user); + } catch (err) { + cb(err); + } + }); + + passport.type = 'openidconnect'; + return passport; +}; + +module.exports.configure = configure; + +/** + * Extracts email from OIDC profile. + * This function is necessary because OIDC providers have different ways of storing emails. + * @param {object} profile the profile object from OIDC provider + * @return {string | null} the email address + */ +const safelyExtractEmail = (profile) => { + if (profile.emails && profile.emails.length > 0) { + return profile.emails[0].value; + } + + if (profile.email) { + return profile.email; + } + return null; +}; + +/** + * Generates a username from email address. + * This helps differentiate users within the specific OIDC provider. + * Note: This is incompatible with multiple providers. Ideally, users are identified by + * OIDC ID (requires refactoring the database). + * @param {string} email the email address + * @return {string} the username + */ +const getUsername = (email) => { + return email ? email.split('@')[0] : ''; +}; From 1c192640a155255d6a303c863a443013de4dfff9 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 31 Jan 2025 18:22:56 +0900 Subject: [PATCH 03/12] feat(auth): add OIDC database handler and endpoint --- package-lock.json | 81 +++++++++++++++++++++++++++++++++++++- package.json | 1 + src/db/file/index.js | 1 + src/db/file/users.js | 16 ++++++++ src/db/index.js | 16 ++++++-- src/service/routes/auth.js | 31 +++++++++++++++ 6 files changed, 141 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7140321df..1b3fcdc37 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,6 +42,7 @@ "passport": "^0.7.0", "passport-activedirectory": "^1.0.4", "passport-local": "^1.0.0", + "passport-openidconnect": "^0.1.2", "perfect-scrollbar": "^1.5.5", "prop-types": "15.8.1", "react": "^16.13.1", @@ -2191,8 +2192,63 @@ } }, "node_modules/@finos/git-proxy": { - "resolved": "", - "link": true + "version": "1.7.1", + "resolved": "file:", + "license": "Apache-2.0", + "workspaces": [ + "./packages/git-proxy-cli" + ], + "dependencies": { + "@material-ui/core": "^4.11.0", + "@material-ui/icons": "4.11.3", + "@primer/octicons-react": "^19.8.0", + "@seald-io/nedb": "^4.0.2", + "axios": "^1.6.0", + "bcryptjs": "^2.4.3", + "bit-mask": "^1.0.2", + "body-parser": "^1.20.1", + "classnames": "2.5.1", + "concurrently": "^9.0.0", + "connect-mongo": "^5.1.0", + "cors": "^2.8.5", + "diff2html": "^3.4.33", + "express": "^4.18.2", + "express-http-proxy": "^2.0.0", + "express-rate-limit": "^7.1.5", + "express-session": "^1.17.1", + "history": "5.3.0", + "isomorphic-git": "^1.27.1", + "jsonschema": "^1.4.1", + "load-plugin": "^6.0.0", + "lodash": "^4.17.21", + "lusca": "^1.7.0", + "moment": "^2.29.4", + "mongodb": "^5.0.0", + "nodemailer": "^6.6.1", + "parse-diff": "^0.11.1", + "passport": "^0.7.0", + "passport-activedirectory": "^1.0.4", + "passport-local": "^1.0.0", + "perfect-scrollbar": "^1.5.5", + "prop-types": "15.8.1", + "react": "^16.13.1", + "react-dom": "^16.13.1", + "react-html-parser": "^2.0.2", + "react-router-dom": "6.28.0", + "simple-git": "^3.25.0", + "uuid": "^11.0.0", + "yargs": "^17.7.2" + }, + "bin": { + "git-proxy": "index.js", + "git-proxy-all": "concurrently 'npm run server' 'npm run client'" + }, + "optionalDependencies": { + "@esbuild/darwin-arm64": "^0.18.20", + "@esbuild/darwin-x64": "^0.23.0", + "@esbuild/linux-x64": "0.23.0", + "@esbuild/win32-x64": "0.23.0" + } }, "node_modules/@finos/git-proxy-cli": { "resolved": "packages/git-proxy-cli", @@ -9965,6 +10021,11 @@ "node": ">=6" } }, + "node_modules/oauth": { + "version": "0.10.0", + "resolved": "https://registry.npmjs.org/oauth/-/oauth-0.10.0.tgz", + "integrity": "sha512-1orQ9MT1vHFGQxhuy7E/0gECD3fd2fCC+PIX+/jgmU/gI3EpRocXtmtvxCO5x3WZ443FLTLFWNDjl5MPJf9u+Q==" + }, "node_modules/object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", @@ -10320,6 +10381,22 @@ "node": ">= 0.4.0" } }, + "node_modules/passport-openidconnect": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/passport-openidconnect/-/passport-openidconnect-0.1.2.tgz", + "integrity": "sha512-JX3rTyW+KFZ/E9OF/IpXJPbyLO9vGzcmXB5FgSP2jfL3LGKJPdV7zUE8rWeKeeI/iueQggOeFa3onrCmhxXZTg==", + "dependencies": { + "oauth": "0.10.x", + "passport-strategy": "1.x.x" + }, + "engines": { + "node": ">= 0.6.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/jaredhanson" + } + }, "node_modules/passport-strategy": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/passport-strategy/-/passport-strategy-1.0.0.tgz", diff --git a/package.json b/package.json index 69f5833f4..d24ab62cb 100644 --- a/package.json +++ b/package.json @@ -63,6 +63,7 @@ "passport": "^0.7.0", "passport-activedirectory": "^1.0.4", "passport-local": "^1.0.0", + "passport-openidconnect": "^0.1.2", "perfect-scrollbar": "^1.5.5", "prop-types": "15.8.1", "react": "^16.13.1", diff --git a/src/db/file/index.js b/src/db/file/index.js index f77833f52..03dd8ecf0 100644 --- a/src/db/file/index.js +++ b/src/db/file/index.js @@ -12,6 +12,7 @@ module.exports.canUserCancelPush = pushes.canUserCancelPush; module.exports.canUserApproveRejectPush = pushes.canUserApproveRejectPush; module.exports.findUser = users.findUser; +module.exports.findUserByOIDC = users.findUserByOIDC; module.exports.getUsers = users.getUsers; module.exports.createUser = users.createUser; module.exports.deleteUser = users.deleteUser; diff --git a/src/db/file/users.js b/src/db/file/users.js index e29dfa4f7..9d0d122e9 100644 --- a/src/db/file/users.js +++ b/src/db/file/users.js @@ -22,6 +22,22 @@ exports.findUser = function (username) { }); }; +exports.findUserByOIDC = function (oidcId) { + return new Promise((resolve, reject) => { + db.findOne({ oidcId: oidcId }, (err, doc) => { + if (err) { + reject(err); + } else { + if (!doc) { + resolve(null); + } else { + resolve(doc); + } + } + }); + }); +}; + exports.createUser = function (data) { return new Promise((resolve, reject) => { db.insert(data, (err) => { diff --git a/src/db/index.js b/src/db/index.js index e685279f5..ed2c13524 100644 --- a/src/db/index.js +++ b/src/db/index.js @@ -7,21 +7,30 @@ if (config.getDatabase().type === 'mongo') { sink = require('../db/file'); } -module.exports.createUser = async (username, password, email, gitAccount, admin = false) => { +module.exports.createUser = async ( + username, + password, + email, + gitAccount, + admin = false, + oidcId = null, +) => { console.log( `creating user user=${username}, gitAccount=${gitAccount} email=${email}, - admin=${admin}`, + admin=${admin} + oidcId=${oidcId}`, ); const data = { username: username, - password: await bcrypt.hash(password, 10), + password: oidcId ? null : await bcrypt.hash(password, 10), gitAccount: gitAccount, email: email, admin: admin, + oidcId: oidcId, }; if (username === undefined || username === null || username === '') { @@ -56,6 +65,7 @@ module.exports.getPushes = sink.getPushes; module.exports.writeAudit = sink.writeAudit; module.exports.getPush = sink.getPush; module.exports.findUser = sink.findUser; +module.exports.findUserByOIDC = sink.findUserByOIDC; module.exports.getUsers = sink.getUsers; module.exports.deleteUser = sink.deleteUser; module.exports.updateUser = sink.updateUser; diff --git a/src/service/routes/auth.js b/src/service/routes/auth.js index d92a1a236..74fdf2c82 100644 --- a/src/service/routes/auth.js +++ b/src/service/routes/auth.js @@ -3,6 +3,13 @@ const router = new express.Router(); const passport = require('../passport').getPassport(); const db = require('../../db'); const passportType = passport.type; +const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', NODE_ENV } = process.env; + +// TODO: Refactor this through proper .env loading. This handles redirects in dev. +let uiPort = 3000; +if (NODE_ENV === 'production') { + uiPort = process.env.GIT_PROXY_UI_PORT; +} router.get('/', (req, res) => { res.status(200).json({ @@ -41,6 +48,30 @@ router.post('/login', passport.authenticate(passportType), async (req, res) => { } }); +router.get('/oidc', passport.authenticate(passportType)); + +router.get('/oidc/callback', (req, res, next) => { + passport.authenticate(passportType, (err, user, info) => { + console.log('authenticate callback executed'); + if (err) { + console.error('Authentication error:', err); + return res.status(401).end(); + } + if (!user) { + console.error('No user found:', info); + return res.status(401).end(); + } + req.logIn(user, (err) => { + if (err) { + console.error('Login error:', err); + return res.status(401).end(); + } + console.log('Logged in successfully. User:', user); + return res.redirect(`${uiHost}:${uiPort}/admin/profile`); + }); + })(req, res, next); +}); + // when login is successful, retrieve user info router.get('/success', (req, res) => { console.log('authenticated' + JSON.stringify(req.user)); From 96edf6aff2031571792deeb4a24a673151798b11 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 31 Jan 2025 18:24:33 +0900 Subject: [PATCH 04/12] fix(auth): fix null password error on OIDC login --- src/service/routes/auth.js | 4 ++-- src/service/routes/users.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/service/routes/auth.js b/src/service/routes/auth.js index 74fdf2c82..d79855905 100644 --- a/src/service/routes/auth.js +++ b/src/service/routes/auth.js @@ -145,10 +145,10 @@ router.post('/gitAccount', async (req, res) => { router.get('/userLoggedIn', async (req, res) => { if (req.user) { const user = JSON.parse(JSON.stringify(req.user)); - delete user.password; + if (user && user.password) delete user.password; const login = user.username; const userVal = await db.findUser(login); - delete userVal.password; + if (userVal && userVal.password) delete userVal.password; res.send(userVal); } else { res.status(401).end(); diff --git a/src/service/routes/users.js b/src/service/routes/users.js index d25bd84d2..118243d70 100644 --- a/src/service/routes/users.js +++ b/src/service/routes/users.js @@ -25,7 +25,7 @@ router.get('/:id', async (req, res) => { console.log(`Retrieving details for user: ${username}`); const data = await db.findUser(username); const user = JSON.parse(JSON.stringify(data)); - delete user.password; + if (user && user.password) delete user.password; res.send(user); }); From ddcacee9c0686df1557ed5e38052b640ad25a886 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 31 Jan 2025 18:25:58 +0900 Subject: [PATCH 05/12] fix(backend): serve static files only in production --- src/service/index.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/service/index.js b/src/service/index.js index 39126a37c..6a637e9f1 100644 --- a/src/service/index.js +++ b/src/service/index.js @@ -64,10 +64,16 @@ const createApp = async () => { app.use(express.json()); app.use(express.urlencoded({ extended: true })); app.use('/', routes); - app.use('/', express.static(absBuildPath)); - app.get('/*', (req, res) => { - res.sendFile(path.join(`${absBuildPath}/index.html`)); - }); + + // In production, serves the static files from the build directory + if (process.env.NODE_ENV === 'production') { + app.use('/', express.static(absBuildPath)); + app.get('/*', (req, res) => { + res.sendFile(path.join(`${absBuildPath}/index.html`)); + }); + } else { + console.log('Not serving static files'); + } return app; }; From 5c93210c3f967fea6c7b228000eaed69cf46dec3 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 2 Feb 2025 11:56:48 +0900 Subject: [PATCH 06/12] test(auth): add basic E2E test for OIDC --- cypress.config.js | 7 +++--- cypress/e2e/login.cy.js | 50 ++++++++++++++++++++++++++--------------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/cypress.config.js b/cypress.config.js index e4fb11d4f..f2d91e0a3 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -1,7 +1,8 @@ -const { defineConfig } = require('cypress') +const { defineConfig } = require('cypress'); module.exports = defineConfig({ e2e: { - baseUrl: process.env.CYPRESS_BASE_URL ||'http://localhost:3000', + baseUrl: process.env.CYPRESS_BASE_URL || 'http://localhost:3000', + chromeWebSecurity: false, // Required for OIDC testing }, -}) \ No newline at end of file +}); diff --git a/cypress/e2e/login.cy.js b/cypress/e2e/login.cy.js index 179001c75..590506f62 100644 --- a/cypress/e2e/login.cy.js +++ b/cypress/e2e/login.cy.js @@ -1,19 +1,33 @@ -describe("Display finos UI",()=>{ - - beforeEach(() =>{ - cy.visit('/login') - }) - it('shoud find git proxy logo',() =>{ - cy.get('[data-test="git-proxy-logo"]').should('exist') -}) - it('shoud find username',() =>{ - cy.get('[data-test="username"]').should('exist') - }) +describe('Login page', () => { + beforeEach(() => { + cy.visit('/login'); + }); - it('shoud find passsword',() =>{ - cy.get('[data-test="password"]').should('exist') - }) - it('shoud find login button',() =>{ - cy.get('[data-test="login"]').should('exist') - }) -}) \ No newline at end of file + it('should have git proxy logo', () => { + cy.get('[data-test="git-proxy-logo"]').should('exist'); + }); + + it('should have username input', () => { + cy.get('[data-test="username"]').should('exist'); + }); + + it('should have passsword input', () => { + cy.get('[data-test="password"]').should('exist'); + }); + + it('should have login button', () => { + cy.get('[data-test="login"]').should('exist'); + }); + + describe('OIDC login button', () => { + it('should exist', () => { + cy.get('[data-test="oidc-login"]').should('exist'); + }); + + // Validates that OIDC is configured correctly + it('should redirect to /oidc', () => { + cy.get('[data-test="oidc-login"]').click(); + cy.url().should('include', '/oidc'); + }); + }); +}); From 8d0453f0201651435fad46bc3db4bf186f5722f1 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 17 Feb 2025 21:16:13 +0900 Subject: [PATCH 07/12] chore(auth): remove unnecessary changes --- src/service/index.js | 14 ++++---------- src/service/routes/auth.js | 8 +------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/service/index.js b/src/service/index.js index 6a637e9f1..39126a37c 100644 --- a/src/service/index.js +++ b/src/service/index.js @@ -64,16 +64,10 @@ const createApp = async () => { app.use(express.json()); app.use(express.urlencoded({ extended: true })); app.use('/', routes); - - // In production, serves the static files from the build directory - if (process.env.NODE_ENV === 'production') { - app.use('/', express.static(absBuildPath)); - app.get('/*', (req, res) => { - res.sendFile(path.join(`${absBuildPath}/index.html`)); - }); - } else { - console.log('Not serving static files'); - } + app.use('/', express.static(absBuildPath)); + app.get('/*', (req, res) => { + res.sendFile(path.join(`${absBuildPath}/index.html`)); + }); return app; }; diff --git a/src/service/routes/auth.js b/src/service/routes/auth.js index d79855905..672b89262 100644 --- a/src/service/routes/auth.js +++ b/src/service/routes/auth.js @@ -3,13 +3,7 @@ const router = new express.Router(); const passport = require('../passport').getPassport(); const db = require('../../db'); const passportType = passport.type; -const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', NODE_ENV } = process.env; - -// TODO: Refactor this through proper .env loading. This handles redirects in dev. -let uiPort = 3000; -if (NODE_ENV === 'production') { - uiPort = process.env.GIT_PROXY_UI_PORT; -} +const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 3000 } = process.env; router.get('/', (req, res) => { res.status(200).json({ From 52e33d9a568825130596ec5fd80df3dc03c164f4 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 19 Feb 2025 15:20:40 +0900 Subject: [PATCH 08/12] feat(auth): refactor to use openid-client instead of passport-openidconnect --- package-lock.json | 111 ++++++++------------------ package.json | 2 +- src/service/passport/index.js | 4 +- src/service/passport/oidc.js | 144 ++++++++++++++++++++-------------- src/service/routes/auth.js | 1 - 5 files changed, 120 insertions(+), 142 deletions(-) diff --git a/package-lock.json b/package-lock.json index c2a3c6ead..65bf8342e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38,11 +38,11 @@ "moment": "^2.29.4", "mongodb": "^5.0.0", "nodemailer": "^6.6.1", + "openid-client": "^6.2.0", "parse-diff": "^0.11.1", "passport": "^0.7.0", "passport-activedirectory": "^1.0.4", "passport-local": "^1.0.0", - "passport-openidconnect": "^0.1.2", "perfect-scrollbar": "^1.5.5", "prop-types": "15.8.1", "react": "^16.13.1", @@ -2192,63 +2192,8 @@ } }, "node_modules/@finos/git-proxy": { - "version": "1.7.1", - "resolved": "file:", - "license": "Apache-2.0", - "workspaces": [ - "./packages/git-proxy-cli" - ], - "dependencies": { - "@material-ui/core": "^4.11.0", - "@material-ui/icons": "4.11.3", - "@primer/octicons-react": "^19.8.0", - "@seald-io/nedb": "^4.0.2", - "axios": "^1.6.0", - "bcryptjs": "^2.4.3", - "bit-mask": "^1.0.2", - "body-parser": "^1.20.1", - "classnames": "2.5.1", - "concurrently": "^9.0.0", - "connect-mongo": "^5.1.0", - "cors": "^2.8.5", - "diff2html": "^3.4.33", - "express": "^4.18.2", - "express-http-proxy": "^2.0.0", - "express-rate-limit": "^7.1.5", - "express-session": "^1.17.1", - "history": "5.3.0", - "isomorphic-git": "^1.27.1", - "jsonschema": "^1.4.1", - "load-plugin": "^6.0.0", - "lodash": "^4.17.21", - "lusca": "^1.7.0", - "moment": "^2.29.4", - "mongodb": "^5.0.0", - "nodemailer": "^6.6.1", - "parse-diff": "^0.11.1", - "passport": "^0.7.0", - "passport-activedirectory": "^1.0.4", - "passport-local": "^1.0.0", - "perfect-scrollbar": "^1.5.5", - "prop-types": "15.8.1", - "react": "^16.13.1", - "react-dom": "^16.13.1", - "react-html-parser": "^2.0.2", - "react-router-dom": "6.28.0", - "simple-git": "^3.25.0", - "uuid": "^11.0.0", - "yargs": "^17.7.2" - }, - "bin": { - "git-proxy": "index.js", - "git-proxy-all": "concurrently 'npm run server' 'npm run client'" - }, - "optionalDependencies": { - "@esbuild/darwin-arm64": "^0.18.20", - "@esbuild/darwin-x64": "^0.23.0", - "@esbuild/linux-x64": "0.23.0", - "@esbuild/win32-x64": "0.23.0" - } + "resolved": "", + "link": true }, "node_modules/@finos/git-proxy-cli": { "resolved": "packages/git-proxy-cli", @@ -8555,6 +8500,15 @@ "jiti": "lib/jiti-cli.mjs" } }, + "node_modules/jose": { + "version": "5.9.6", + "resolved": "https://registry.npmjs.org/jose/-/jose-5.9.6.tgz", + "integrity": "sha512-AMlnetc9+CV9asI19zHmrgS/WYsWUwCn2R7RzlbJWD7F9eWYUTGyBmU9o6PxngtLGOiDGPRu+Uc4fhKzbpteZQ==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", @@ -10022,10 +9976,14 @@ "node": ">=6" } }, - "node_modules/oauth": { - "version": "0.10.0", - "resolved": "https://registry.npmjs.org/oauth/-/oauth-0.10.0.tgz", - "integrity": "sha512-1orQ9MT1vHFGQxhuy7E/0gECD3fd2fCC+PIX+/jgmU/gI3EpRocXtmtvxCO5x3WZ443FLTLFWNDjl5MPJf9u+Q==" + "node_modules/oauth4webapi": { + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/oauth4webapi/-/oauth4webapi-3.2.0.tgz", + "integrity": "sha512-2sYwQXuuzGKOHpnM7QL9BssDrly5gKCgJKTyrhmFIHzJRj0fFsr6GVJEdesmrX6NpMg2u63V4hJwRsZE6PUSSA==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/panva" + } }, "node_modules/object-assign": { "version": "4.1.1", @@ -10173,6 +10131,19 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/openid-client": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/openid-client/-/openid-client-6.2.0.tgz", + "integrity": "sha512-pvLVkLcRWNU7YuKKTto376rgL//+rn3ca0XRqsrQVN30lVlpXBPHhSLcGoM/hPbux5p+Ha4tdoz96eEYpyguOQ==", + "license": "MIT", + "dependencies": { + "jose": "^5.9.6", + "oauth4webapi": "^3.2.0" + }, + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/optionator": { "version": "0.9.3", "resolved": "https://registry.npmjs.org/optionator/-/optionator-0.9.3.tgz", @@ -10382,22 +10353,6 @@ "node": ">= 0.4.0" } }, - "node_modules/passport-openidconnect": { - "version": "0.1.2", - "resolved": "https://registry.npmjs.org/passport-openidconnect/-/passport-openidconnect-0.1.2.tgz", - "integrity": "sha512-JX3rTyW+KFZ/E9OF/IpXJPbyLO9vGzcmXB5FgSP2jfL3LGKJPdV7zUE8rWeKeeI/iueQggOeFa3onrCmhxXZTg==", - "dependencies": { - "oauth": "0.10.x", - "passport-strategy": "1.x.x" - }, - "engines": { - "node": ">= 0.6.0" - }, - "funding": { - "type": "github", - "url": "https://github.com/sponsors/jaredhanson" - } - }, "node_modules/passport-strategy": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/passport-strategy/-/passport-strategy-1.0.0.tgz", diff --git a/package.json b/package.json index d25963f15..cb4ffa98d 100644 --- a/package.json +++ b/package.json @@ -59,11 +59,11 @@ "moment": "^2.29.4", "mongodb": "^5.0.0", "nodemailer": "^6.6.1", + "openid-client": "^6.2.0", "parse-diff": "^0.11.1", "passport": "^0.7.0", "passport-activedirectory": "^1.0.4", "passport-local": "^1.0.0", - "passport-openidconnect": "^0.1.2", "perfect-scrollbar": "^1.5.5", "prop-types": "15.8.1", "react": "^16.13.1", diff --git a/src/service/passport/index.js b/src/service/passport/index.js index 9a16ef2fc..a2d7931ef 100644 --- a/src/service/passport/index.js +++ b/src/service/passport/index.js @@ -21,7 +21,9 @@ const configure = async () => { default: throw Error(`uknown authentication type ${type}`); } - _passport.type = authenticationConfig.type; + if (!_passport.type) { + _passport.type = type; + } return _passport; }; diff --git a/src/service/passport/oidc.js b/src/service/passport/oidc.js index 8e49866f5..f9500b586 100644 --- a/src/service/passport/oidc.js +++ b/src/service/passport/oidc.js @@ -1,66 +1,95 @@ -const configure = async () => { - const passport = require('passport'); - const { Strategy: OIDCStrategy } = require('passport-openidconnect'); - const db = require('../../db'); +const openIdClient = require('openid-client'); +const { Strategy } = require('openid-client/passport'); +const passport = require('passport'); +const db = require('../../db'); +const configure = async () => { const config = require('../../config').getAuthentication(); - const oidcConfig = config.oidcConfig; + const { oidcConfig } = config; + const { issuer, clientID, clientSecret, callbackURL, scope } = oidcConfig; + + if (!oidcConfig || !oidcConfig.issuer) { + throw new Error('Missing OIDC issuer in configuration') + } + + const server = new URL(issuer); + + try { + const config = await openIdClient.discovery(server, clientID, clientSecret); + + const strategy = new Strategy({ callbackURL, config, scope }, async (tokenSet, done) => { + // Validate token sub for added security + const idTokenClaims = tokenSet.claims(); + const expectedSub = idTokenClaims.sub; + const userInfo = await openIdClient.fetchUserInfo(config, tokenSet.access_token, expectedSub); + handleUserAuthentication(userInfo, done); + }); + + // currentUrl must be overridden to match the callback URL + strategy.currentUrl = (request) => { + const callbackUrl = new URL(callbackURL); + const currentUrl = Strategy.prototype.currentUrl.call(this, request); + currentUrl.host = callbackUrl.host; + currentUrl.protocol = callbackUrl.protocol; + return currentUrl; + }; - passport.use( - new OIDCStrategy(oidcConfig, async function verify(issuer, profile, cb) { + passport.use(strategy); + + passport.serializeUser((user, done) => { + done(null, user.oidcId || user.username); + }) + + passport.deserializeUser(async (id, done) => { try { - const user = await db.findUserByOIDC(profile.id); - - if (!user) { - const email = safelyExtractEmail(profile); - if (!email) { - return cb(new Error('No email found in OIDC profile')); - } - - const username = getUsername(email); - const newUser = { - username: username, - email: email, - oidcId: profile.id, - }; - - await db.createUser( - newUser.username, - null, - newUser.email, - 'Edit me', - false, - newUser.oidcId, - ); - - return cb(null, newUser); - } - return cb(null, user); + const user = await db.findUserByOIDC(id); + done(null, user); } catch (err) { - return cb(err); + done(err); } - }), - ); - - passport.serializeUser((user, cb) => { - cb(null, user.oidcId || user.username); - }); - - passport.deserializeUser(async (id, cb) => { - try { - const user = (await db.findUserByOIDC(id)) || (await db.findUser(id)); - cb(null, user); - } catch (err) { - cb(err); - } - }); + }) + passport.type = server.host; + + return passport; + } catch (error) { + console.error('OIDC configuration failed:', error); + throw error; + } +} - passport.type = 'openidconnect'; - return passport; -}; module.exports.configure = configure; +/** + * Handles user authentication with OIDC. + * @param userInfo the OIDC user info object + * @param done the callback function + * @returns a promise with the authenticated user or an error + */ +const handleUserAuthentication = async (userInfo, done) => { + try { + let user = await db.findUserByOIDC(userInfo.sub); + + if (!user) { + const email = safelyExtractEmail(userInfo); + if (!email) return done(new Error('No email found in OIDC profile')); + + const newUser = { + username: getUsername(email), + email, + oidcId: userInfo.sub, + }; + + await db.createUser(newUser.username, null, newUser.email, 'Edit me', false, newUser.oidcId); + return done(null, newUser); + } + + return done(null, user); + } catch (err) { + return done(err); + } +}; + /** * Extracts email from OIDC profile. * This function is necessary because OIDC providers have different ways of storing emails. @@ -68,14 +97,7 @@ module.exports.configure = configure; * @return {string | null} the email address */ const safelyExtractEmail = (profile) => { - if (profile.emails && profile.emails.length > 0) { - return profile.emails[0].value; - } - - if (profile.email) { - return profile.email; - } - return null; + return profile.email || (profile.emails && profile.emails.length > 0 ? profile.emails[0].value : null); }; /** diff --git a/src/service/routes/auth.js b/src/service/routes/auth.js index 672b89262..e433d7ad8 100644 --- a/src/service/routes/auth.js +++ b/src/service/routes/auth.js @@ -46,7 +46,6 @@ router.get('/oidc', passport.authenticate(passportType)); router.get('/oidc/callback', (req, res, next) => { passport.authenticate(passportType, (err, user, info) => { - console.log('authenticate callback executed'); if (err) { console.error('Authentication error:', err); return res.status(401).end(); From 4f71e774e9aa51f119fdf196dea3da3a23dc4ecd Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 23 Feb 2025 12:46:17 +0900 Subject: [PATCH 09/12] chore(auth): fix lint warnings --- src/service/passport/oidc.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/service/passport/oidc.js b/src/service/passport/oidc.js index f9500b586..31a80f842 100644 --- a/src/service/passport/oidc.js +++ b/src/service/passport/oidc.js @@ -26,7 +26,7 @@ const configure = async () => { }); // currentUrl must be overridden to match the callback URL - strategy.currentUrl = (request) => { + strategy.currentUrl = function (request) { const callbackUrl = new URL(callbackURL); const currentUrl = Strategy.prototype.currentUrl.call(this, request); currentUrl.host = callbackUrl.host; @@ -62,13 +62,13 @@ module.exports.configure = configure; /** * Handles user authentication with OIDC. - * @param userInfo the OIDC user info object - * @param done the callback function - * @returns a promise with the authenticated user or an error + * @param {Object} userInfo the OIDC user info object + * @param {Function} done the callback function + * @return {Promise} a promise with the authenticated user or an error */ const handleUserAuthentication = async (userInfo, done) => { try { - let user = await db.findUserByOIDC(userInfo.sub); + const user = await db.findUserByOIDC(userInfo.sub); if (!user) { const email = safelyExtractEmail(userInfo); From 0a0656dd2905c096a3a63503b1927fd3aa0cbfa0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 25 Feb 2025 17:48:58 +0900 Subject: [PATCH 10/12] fix(auth): attempt to fix CI ERR_REQUIRE_ESM --- src/service/passport/oidc.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/service/passport/oidc.js b/src/service/passport/oidc.js index 31a80f842..904faff04 100644 --- a/src/service/passport/oidc.js +++ b/src/service/passport/oidc.js @@ -1,9 +1,10 @@ -const openIdClient = require('openid-client'); -const { Strategy } = require('openid-client/passport'); const passport = require('passport'); const db = require('../../db'); const configure = async () => { + // Temp fix for ERR_REQUIRE_ESM, will be changed when we refactor to ESM + const { discovery, fetchUserInfo } = await import('openid-client'); + const { Strategy } = await import('openid-client/passport'); const config = require('../../config').getAuthentication(); const { oidcConfig } = config; const { issuer, clientID, clientSecret, callbackURL, scope } = oidcConfig; @@ -15,13 +16,13 @@ const configure = async () => { const server = new URL(issuer); try { - const config = await openIdClient.discovery(server, clientID, clientSecret); + const config = await discovery(server, clientID, clientSecret); const strategy = new Strategy({ callbackURL, config, scope }, async (tokenSet, done) => { // Validate token sub for added security const idTokenClaims = tokenSet.claims(); const expectedSub = idTokenClaims.sub; - const userInfo = await openIdClient.fetchUserInfo(config, tokenSet.access_token, expectedSub); + const userInfo = await fetchUserInfo(config, tokenSet.access_token, expectedSub); handleUserAuthentication(userInfo, done); }); From 712938be36f4e6a3288128f823236051d0c8a4e8 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 10 Mar 2025 12:27:29 +0900 Subject: [PATCH 11/12] fix(auth): fix bug with default admin account --- src/service/passport/local.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/service/passport/local.js b/src/service/passport/local.js index 6bcce7e7e..c75676577 100644 --- a/src/service/passport/local.js +++ b/src/service/passport/local.js @@ -43,7 +43,7 @@ const configure = async () => { const admin = await db.findUser('admin'); if (!admin) { - await db.createUser('admin', 'admin', 'admin@place.com', 'none', true, true, true, true); + await db.createUser('admin', 'admin', 'admin@place.com', 'none', true); } passport.type = 'local'; From 6ba8021cb89d723b91b24a25404ab8b5e5b60dad Mon Sep 17 00:00:00 2001 From: Jamie Slome Date: Thu, 13 Mar 2025 11:43:15 +0000 Subject: [PATCH 12/12] chore: bump by patch to 1.8.1 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index f82320fd0..1e651c0ce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@finos/git-proxy", - "version": "1.8.0", + "version": "1.8.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@finos/git-proxy", - "version": "1.8.0", + "version": "1.8.1", "license": "Apache-2.0", "workspaces": [ "./packages/git-proxy-cli" diff --git a/package.json b/package.json index 1b26cfee0..1ca9b41ee 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@finos/git-proxy", - "version": "1.8.0", + "version": "1.8.1", "description": "Deploy custom push protections and policies on top of Git.", "scripts": { "cli": "node ./packages/git-proxy-cli/index.js",