From 74fcc23a07a9de6a2168511c0b1381931ed2eaa2 Mon Sep 17 00:00:00 2001 From: "C. Fuhrman" Date: Thu, 6 Mar 2025 21:19:57 -0500 Subject: [PATCH] Revert "Merge pull request #280 from ets-cfuhrman-pfe/bug/sso-consent" This reverts commit 7418f53e31e482db6e7eeabe6d1b14cf6098ab96, reversing changes made to 9d24507f418b14dd76172ad06c6e1546847008e6. --- server/__tests__/oidc.test.js | 234 ------------------ server/auth/auth-manager.js | 6 +- .../auth/modules/passport-providers/oidc.js | 118 +++++---- 3 files changed, 69 insertions(+), 289 deletions(-) delete mode 100644 server/__tests__/oidc.test.js diff --git a/server/__tests__/oidc.test.js b/server/__tests__/oidc.test.js deleted file mode 100644 index 576849a..0000000 --- a/server/__tests__/oidc.test.js +++ /dev/null @@ -1,234 +0,0 @@ -let PassportOpenIDConnect = require('../auth/modules/passport-providers/oidc'); -const AppError = require('../middleware/AppError'); -const authUserAssoc = require('../models/authUserAssociation'); -const userModel = require('../models/users'); - -global.fetch = jest.fn(); - -// Mock the authUserAssoc methods -jest.mock('../models/authUserAssociation', () => ({ - find_user_association: jest.fn(), - link: jest.fn(), - unlink: jest.fn(), -})); - - // Mock userModel methods -jest.mock('../models/users', () => ({ - getById: jest.fn(), - getId: jest.fn(), - generatePassword: jest.fn(), - register: jest.fn(), - editUser: jest.fn(), -})); - - // Mock db connection methods -jest.mock('../config/db', () => ({ - connect: jest.fn(), - getConnection: jest.fn(() => ({ - collection: jest.fn(() => ({ - findOne: jest.fn(), - insertOne: jest.fn(), - deleteOne: jest.fn(), - })), - })), -})); - -const getProfileMock = (id = 'test-auth-id', email = 'test@example.com', name = 'Test User', roles = ['teacher']) => ({ - id, - emails: [{ value: email }], - name: { displayName: name }, - roles, -}); - -const setupMocks = (authAssocReturn = null, userReturn = null, fetchReturn = {}) => { - // Mock authUserAssoc and userModel - authUserAssoc.find_user_association.mockResolvedValue(authAssocReturn); - userModel.getId.mockResolvedValue(userReturn); - userModel.getById.mockResolvedValue(userReturn); - - // Mock fetch to return a fake configuration - fetch.mockResolvedValue({ - json: jest.fn().mockResolvedValue(fetchReturn), - }); -}; - -const createOidcConfig = () => ({ - OIDC_CONFIG_URL: 'https://example.com/.well-known/openid-configuration', - OIDC_CLIENT_ID: 'test-client-id', - OIDC_CLIENT_SECRET: 'test-client-secret', - OIDC_ADD_SCOPE: '', - OIDC_ROLE_TEACHER_VALUE: 'teacher', - OIDC_ROLE_STUDENT_VALUE: 'student', - tokenURL: 'https://example.com/token', -}); - -describe('PassportOpenIDConnect Class', () => { - let passportMock; - let appMock; - let oidcInstance; - - beforeEach(() => { - passportMock = { use: jest.fn(), register: jest.fn(), authenticate: jest.fn()}; - appMock = { get: jest.fn() }; - - // Mock configuration object - const oidcConfig = { - issuer: 'exom', // Ensure the issuer is correctly set - authorization_endpoint: 'https://example.com/auth', - token_endpoint: 'https://example.com/token', - tokenURL: 'https://example.com/token', - userInfoURL: 'https://example.com/userinfo', - clientID: 'test-client-id', - clientSecret: 'test-client-secret', - callbackURL: 'https://example.com/callback', - passReqToCallback: true, - scope: 'openid profile email', // Set required scopes - }; - // Mock passport's use method to simulate adding the strategy - passportMock.use.mockImplementation((strategy, callback) => { - passportMock._callback = callback; - }); - passportMock.register = jest.fn().mockResolvedValue(getProfileMock()); - - // Instantiate the PassportOpenIDConnect class with the mock - oidcInstance = new PassportOpenIDConnect(passportMock, oidcConfig); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('should fetch OIDC configuration', async () => { - const fakeConfig = { issuer: 'exom', authorization_endpoint: 'https://example.com/auth' }; - fetch.mockResolvedValue({ json: jest.fn().mockResolvedValue(fakeConfig) }); - - const result = await oidcInstance.getConfigFromConfigURL('test-provider', { OIDC_CONFIG_URL: 'https://example.com/.well-known/openid-configuration' }); - - expect(fetch).toHaveBeenCalledWith('https://example.com/.well-known/openid-configuration'); - expect(result).toEqual(fakeConfig); - }); - - it('should throw AppError if fetching OIDC config fails', async () => { - fetch.mockRejectedValue(new Error('Network error')); - - await expect(oidcInstance.getConfigFromConfigURL('test-provider', { OIDC_CONFIG_URL: 'https://example.com' })) - .rejects.toThrow(AppError); - }); - - it('should register passport strategy and process new users consent flag true', async () => { - setupMocks(null, null, { - issuer: 'https://example.com', - authorization_endpoint: 'https://example.com/auth', - token_endpoint: 'https://example.com/token' - }); - const profileMock = getProfileMock(); - - - const reqMock = { session: {} }; - const doneMock = jest.fn((error, user) => { - console.log('doneMock was called:', error, user); // Log to check if doneMock is called - }); - - // Updated configuration with tokenURL - const config = createOidcConfig(); - - // Ensure `passportMock.use` is being called correctly with the expected config - await oidcInstance.register(appMock, passportMock, '/auth', 'oidc-test', config, userModel); - - - // Check that passport.use has been called, which registers the strategy - expect(passportMock.use).toHaveBeenCalledWith('oidc-test', expect.objectContaining({ - _issuer: 'https://example.com', // Assert that tokenURL is present in the strategy config - })); - - // Ensure routes are defined - expect(appMock.get).toHaveBeenCalledWith('/auth/oidc-test', expect.any(Function)); - expect(appMock.get).toHaveBeenCalledWith('/auth/oidc-test/callback', expect.any(Function), expect.any(Function)); - - // Access the _verify method inside the passportMock._callback (which is a Strategy instance) - const strategyCallback = passportMock._callback._verify; - - // Ensure the callback is correctly registered and is a function - expect(typeof strategyCallback).toBe('function'); - - doneMock.mockImplementation((error, user) => { - // Check if session.requiresConsent is set - console.log("In doneMock, error:", error, "user:", user); - }); - - // Simulate the strategy callback being invoked - await strategyCallback(reqMock, null, profileMock, null, null, doneMock, null, userModel); - - // Ensure the doneMock was called - expect(doneMock).toHaveBeenCalled(); - // Validate that consent flag is set for new users - expect(reqMock.session.requiresConsent).toBe(true); - }); - - - - it('should correctly process new users and set consent flag', async () => { - setupMocks(null, { _id: 'mocked-user-id', email: 'test@example.com' }, { - issuer: 'https://example.com', - authorization_endpoint: 'https://example.com/auth', - token_endpoint: 'https://example.com/token' - }); - - const config = createOidcConfig(); - const profileMock = getProfileMock(); - - const reqMock = { session: {} }; - const doneMock = jest.fn((error, user) => { - console.log('doneMock was called:', error, user); - }); - passportMock.register.mockResolvedValue({ _id: 'new-user-id' }); - - - await oidcInstance.register(appMock, passportMock, '/auth', 'oidc-test', config, userModel); - - const strategyCallback = passportMock._callback._verify; - expect(typeof strategyCallback).toBe('function'); - - doneMock.mockImplementation((error, user) => { - console.log("In doneMock, error:", error, "user:", user); - }); - - await strategyCallback(reqMock, null, profileMock, null, null, doneMock, null, userModel); - - expect(doneMock).toHaveBeenCalled(); - expect(reqMock.session.requiresConsent).toBe(true); - }); - - it('should correctly process existing users without setting consent flag', async () => { - let linkData = { - user_id: 'existing-user-id', - provider_id: 'test-auth-id', - provider: 'oidc', - email: 'test@example.com', - role: 'teacher', - }; - - setupMocks(linkData, { _id: 'mocked-user-id', email: 'test@example.com' }, { - issuer: 'https://example.com', - authorization_endpoint: 'https://example.com/auth', - token_endpoint: 'https://example.com/token' - }); - - const config = createOidcConfig(); - const profileMock = getProfileMock(); - const reqMock = { session: {} }; - const doneMock = jest.fn((error, user) => { - console.log('doneMock was called:', error, user); - }); - - await oidcInstance.register(appMock, passportMock, '/auth', 'oidc-test', config, userModel); - - const strategyCallback = passportMock._callback._verify; - expect(typeof strategyCallback).toBe('function'); - - await strategyCallback(reqMock, null, profileMock, null, null, doneMock, null, userModel); - - expect(doneMock).toHaveBeenCalled(); - expect(reqMock.session.requiresConsent).toBe(false); - }); -}); \ No newline at end of file diff --git a/server/auth/auth-manager.js b/server/auth/auth-manager.js index 6a7dcba..0233672 100644 --- a/server/auth/auth-manager.js +++ b/server/auth/auth-manager.js @@ -7,12 +7,16 @@ const AppError = require('../middleware/AppError.js'); class AuthManager{ constructor(expressapp,configs=null,userModel){ + console.log(`AuthManager: constructor: configs: ${JSON.stringify(configs)}`); + console.log(`AuthManager: constructor: userModel: ${JSON.stringify(userModel)}`); this.modules = [] this.app = expressapp - this.simpleregister = userModel; + this.configs = configs ?? (new AuthConfig()).loadConfig() this.addModules() + this.simpleregister = userModel; this.registerAuths() + console.log(`AuthManager: constructor: this.configs: ${JSON.stringify(this.configs)}`); } getUserModel(){ diff --git a/server/auth/modules/passport-providers/oidc.js b/server/auth/modules/passport-providers/oidc.js index 8b9fa41..03da065 100644 --- a/server/auth/modules/passport-providers/oidc.js +++ b/server/auth/modules/passport-providers/oidc.js @@ -3,6 +3,7 @@ var authUserAssoc = require('../../../models/authUserAssociation'); var { hasNestedValue } = require('../../../utils'); const { MISSING_OIDC_PARAMETER } = require('../../../constants/errorCodes.js'); const AppError = require('../../../middleware/AppError.js'); +const expressListEndpoints = require('express-list-endpoints'); class PassportOpenIDConnect { constructor(passportjs, auth_name) { @@ -20,55 +21,22 @@ class PassportOpenIDConnect { } } - async processOIDC(req, issuer, profile, times, tok, done, provider, userModel, self) { - try { - const received_user = { - auth_id: profile.id, - email: profile.emails[0].value, - name: profile.name.givenName, - roles: [] - }; - - if (hasNestedValue(profile, provider.OIDC_ROLE_TEACHER_VALUE)) received_user.roles.push('teacher'); - if (hasNestedValue(profile, provider.OIDC_ROLE_STUDENT_VALUE)) received_user.roles.push('student'); - - const user_association = await authUserAssoc.find_user_association(self.auth_name, received_user.auth_id); - - let user_account; - let newUser = true; - - if (user_association) { - newUser = false; - user_account = await userModel.getById(user_association.user_id); - } else { - let user_id = await userModel.getId(received_user.email); - if (user_id) { - user_account = await userModel.getById(user_id); - } else { - received_user.password = userModel.generatePassword(); - user_account = await self.passportjs.register(received_user); - } - await authUserAssoc.link(self.auth_name, received_user.auth_id, user_account._id); - } - - user_account.name = received_user.name; - user_account.roles = received_user.roles; - await userModel.editUser(user_account); - req.session.requiresConsent = newUser; - - return done(null, user_account); - } catch (error) { - console.error(`Error: ${error} `); - } - } - async register(app, passport, endpoint, name, provider, userModel) { + console.log(`oidc.js: register: endpoint: ${endpoint}`); + console.log(`oidc.js: register: name: ${name}`); + console.log(`oidc.js: register: provider: ${JSON.stringify(provider)}`); + console.log(`oidc.js: register: userModel: ${JSON.stringify(userModel)}`); + const config = await this.getConfigFromConfigURL(name, provider); const cb_url = `${process.env['OIDC_URL']}${endpoint}/${name}/callback`; const self = this; const scope = 'openid profile email ' + `${provider.OIDC_ADD_SCOPE}`; + console.log(`oidc.js: register: config: ${JSON.stringify(config)}`); + console.log(`oidc.js: register: cb_url: ${cb_url}`); + console.log(`oidc.js: register: scope: ${scope}`); + passport.use(name, new OpenIDConnectStrategy({ issuer: config.issuer, authorizationURL: config.authorization_endpoint, @@ -80,22 +48,63 @@ class PassportOpenIDConnect { passReqToCallback: true, scope: scope, }, + // patch pour la librairie permet d'obtenir les groupes, PR en cours mais "morte" : https://github.com/jaredhanson/passport-openidconnect/pull/101 + async function (req, issuer, profile, times, tok, done) { + console.log(`oidc.js: register: issuer: ${JSON.stringify(issuer)}`); + console.log(`oidc.js: register: profile: ${JSON.stringify(profile)}`); + try { + const received_user = { + auth_id: profile.id, + email: profile.emails[0].value.toLowerCase(), + name: profile.displayName, + roles: [] + }; - // patch pour la librairie permet d'obtenir les groupes, PR en cours mais "morte" : https://github.com/jaredhanson/passport-openidconnect/pull/101 - (req, issuer, profile, times, tok, done) => this.processOIDC(req, issuer, profile, times, tok, done, provider, userModel, self))); + if (hasNestedValue(profile, provider.OIDC_ROLE_TEACHER_VALUE)) received_user.roles.push('teacher') + if (hasNestedValue(profile, provider.OIDC_ROLE_STUDENT_VALUE)) received_user.roles.push('student') + + console.log(`oidc.js: register: received_user: ${JSON.stringify(received_user)}`); + const user_association = await authUserAssoc.find_user_association(self.auth_name, received_user.auth_id); + console.log(`oidc.js: register: user_association: ${JSON.stringify(user_association)}`); + + let user_account + if (user_association) { + console.log(`oidc.js: register: user_association: ${JSON.stringify(user_association)}`); + user_account = await userModel.getById(user_association.user_id) + console.log(`oidc.js: register: user_account: ${JSON.stringify(user_account)}`); + } + else { + console.log(`oidc.js: register: user_association: ${JSON.stringify(user_association)}`); + let user_id = await userModel.getId(received_user.email) + console.log(`oidc.js: register: user_id: ${JSON.stringify(user_id)}`); + if (user_id) { + user_account = await userModel.getById(user_id); + console.log(`oidc.js: register: user_account: ${JSON.stringify(user_account)}`); + } else { + received_user.password = userModel.generatePassword() + user_account = await self.passportjs.register(received_user) + console.log(`oidc.js: register: user_account: ${JSON.stringify(user_account)}`); + } + console.log(`oidc.js: register: authUserAssoc.ling.`); + await authUserAssoc.link(self.auth_name, received_user.auth_id, user_account._id) + } + + user_account.name = received_user.name + user_account.roles = received_user.roles + console.log(`oidc.js: register: calling userModel.editUser: ${JSON.stringify(user_account)}`); + await userModel.editUser(user_account); + + return done(null, user_account); + } catch (error) { + console.error(`Error: ${error} `); + } + })); app.get(`${endpoint}/${name}`, (req, res, next) => { - let promptConsent = req.session.requiresConsent || true; // Default to false - let authObj = { + passport.authenticate(name, { scope: scope, - prompt: 'none' - }; - - if(promptConsent){ - authObj.prompt = 'consent'; - } - - passport.authenticate(name, authObj)(req, res, next); + prompt: 'consent' + })(req, res, next); }); app.get(`${endpoint}/${name}/callback`, @@ -111,6 +120,7 @@ class PassportOpenIDConnect { } ); console.info(`Ajout de la connexion : ${name}(OIDC)`); + console.log(expressListEndpoints(app)); } }