diff --git a/src/service/passport/activeDirectory.js b/src/service/passport/activeDirectory.js index e5eb6f8a2..8f681c823 100644 --- a/src/service/passport/activeDirectory.js +++ b/src/service/passport/activeDirectory.js @@ -43,8 +43,9 @@ const configure = (passport) => { const message = `User it not a member of ${userGroup}`; return done(message, null); } - } catch (e) { - const message = `An error occurred while checking if the user is a member of the user group: ${JSON.stringify(e)}`; + } catch (err) { + console.log('ad test (isUser): e', err); + const message = `An error occurred while checking if the user is a member of the user group: ${err.message}`; return done(message, null); } @@ -53,9 +54,9 @@ const configure = (passport) => { try { isAdmin = await ldaphelper.isUserInAdGroup(req, profile, ad, domain, adminGroup); - } catch (e) { - const message = `An error occurred while checking if the user is a member of the admin group: ${JSON.stringify(e)}`; - console.error(message, e); // don't return an error for this case as you may still be a user + } catch (err) { + const message = `An error occurred while checking if the user is a member of the admin group: ${err.message}`; + console.error(message, err); // don't return an error for this case as you may still be a user } profile.admin = isAdmin; diff --git a/src/service/routes/auth.js b/src/service/routes/auth.js index 9544931d9..ea0080792 100644 --- a/src/service/routes/auth.js +++ b/src/service/routes/auth.js @@ -66,29 +66,6 @@ router.get('/oidc/callback', (req, res, next) => { })(req, res, next); }); -// when login is successful, retrieve user info -router.get('/success', (req, res) => { - console.log('authenticated' + JSON.stringify(req.user)); - if (req.user) { - res.json({ - success: true, - message: 'user has successfully authenticated', - user: req.user, - cookies: req.cookies, - }); - } else { - res.status(401).end(); - } -}); - -// when login failed, send failed msg -router.get('failed', (req, res) => { - res.status(401).json({ - success: false, - message: 'user failed to authenticate.', - }); -}); - router.post('/logout', (req, res, next) => { req.logout(req.user, (err) => { if (err) return next(err); @@ -110,24 +87,28 @@ router.get('/profile', async (req, res) => { router.post('/gitAccount', async (req, res) => { if (req.user) { try { - let login = + let username = req.body.username == null || req.body.username == 'undefined' ? req.body.id : req.body.username; + username = username?.split('@')[0]; - login = login.split('@')[0]; + if (!username) { + res.status(400).send('Error: Missing username. Git account not updated').end(); + return; + } - const user = await db.findUser(login); + const user = await db.findUser(username); console.log('Adding gitAccount' + req.body.gitAccount); user.gitAccount = req.body.gitAccount; db.updateUser(user); res.status(200).end(); - } catch { + } catch (e) { res .status(500) .send({ - message: 'An error occurred', + message: `Error updating git account: ${e.message}`, }) .end(); } diff --git a/src/ui/services/git-push.js b/src/ui/services/git-push.js index f9100fd02..da654aaad 100644 --- a/src/ui/services/git-push.js +++ b/src/ui/services/git-push.js @@ -9,21 +9,6 @@ const config = { withCredentials: true, }; -const getUser = async (setIsLoading, setData, setAuth, setIsError) => { - const url = new URL(`${location.origin}/api/auth/success`); - await axios(url.toString(), config) - .then((response) => { - const data = response.data; - setData(data); - setIsLoading(false); - }) - .catch((error) => { - if (error.response && error.response.status === 401) setAuth(false); - else setIsError(true); - setIsLoading(false); - }); -}; - const getPush = async (id, setIsLoading, setData, setAuth, setIsError) => { const url = `${baseUrl}/push/${id}`; await axios(url, config) @@ -128,4 +113,4 @@ const cancelPush = async (id, setAuth, setIsError) => { }); }; -export { getPush, getPushes, authorisePush, rejectPush, cancelPush, getUser }; +export { getPush, getPushes, authorisePush, rejectPush, cancelPush }; diff --git a/test/testActiveDirectoryAuth.test.js b/test/testActiveDirectoryAuth.test.js new file mode 100644 index 000000000..4d50dbb45 --- /dev/null +++ b/test/testActiveDirectoryAuth.test.js @@ -0,0 +1,150 @@ +const chai = require('chai'); +const sinon = require('sinon'); +const proxyquire = require('proxyquire'); +const expect = chai.expect; + +describe('ActiveDirectory auth method', () => { + let ldapStub; + let dbStub; + let passportStub; + let strategyCallback; + + const newConfig = JSON.stringify({ + authentication: [ + { + type: 'ActiveDirectory', + enabled: true, + adminGroup: 'test-admin-group', + userGroup: 'test-user-group', + domain: 'test.com', + adConfig: { + url: 'ldap://test-url', + baseDN: 'dc=test,dc=com', + searchBase: 'ou=users,dc=test,dc=com', + }, + }, + ], + }); + + beforeEach(() => { + ldapStub = { + isUserInAdGroup: sinon.stub(), + }; + + dbStub = { + updateUser: sinon.stub(), + }; + + passportStub = { + use: sinon.stub(), + serializeUser: sinon.stub(), + deserializeUser: sinon.stub(), + }; + + const fsStub = { + existsSync: sinon.stub().returns(true), + readFileSync: sinon.stub().returns(newConfig), + }; + + const config = proxyquire('../src/config', { + fs: fsStub, + }); + + const { configure } = proxyquire('../src/service/passport/activeDirectory', { + './ldaphelper': ldapStub, + '../../db': dbStub, + '../../config': config, + 'passport-activedirectory': function (options, callback) { + strategyCallback = callback; + return { + name: 'ActiveDirectory', + authenticate: () => {}, + }; + }, + }); + + configure(passportStub); + }); + + it('should authenticate a valid user and mark them as admin', async () => { + const mockReq = {}; + const mockProfile = { + _json: { + sAMAccountName: 'test-user', + mail: 'test@test.com', + userPrincipalName: 'test@test.com', + title: 'Test User', + }, + displayName: 'Test User', + }; + + ldapStub.isUserInAdGroup + .onCall(0).resolves(true) + .onCall(1).resolves(true); + + const done = sinon.spy(); + + await strategyCallback(mockReq, mockProfile, {}, done); + + expect(done.calledOnce).to.be.true; + const [err, user] = done.firstCall.args; + expect(err).to.be.null; + expect(user).to.have.property('username', 'test-user'); + expect(user).to.have.property('email', 'test@test.com'); + expect(user).to.have.property('displayName', 'Test User'); + expect(user).to.have.property('admin', true); + expect(user).to.have.property('title', 'Test User'); + + expect(dbStub.updateUser.calledOnce).to.be.true; + }); + + it('should fail if user is not in user group', async () => { + const mockReq = {}; + const mockProfile = { + _json: { + sAMAccountName: 'bad-user', + mail: 'bad@test.com', + userPrincipalName: 'bad@test.com', + title: 'Bad User' + }, + displayName: 'Bad User' + }; + + ldapStub.isUserInAdGroup.onCall(0).resolves(false); + + const done = sinon.spy(); + + await strategyCallback(mockReq, mockProfile, {}, done); + + expect(done.calledOnce).to.be.true; + const [err, user] = done.firstCall.args; + expect(err).to.include('not a member'); + expect(user).to.be.null; + + expect(dbStub.updateUser.notCalled).to.be.true; + }); + + it('should handle LDAP errors gracefully', async () => { + const mockReq = {}; + const mockProfile = { + _json: { + sAMAccountName: 'error-user', + mail: 'err@test.com', + userPrincipalName: 'err@test.com', + title: 'Whoops' + }, + displayName: 'Error User' + }; + + ldapStub.isUserInAdGroup.rejects(new Error('LDAP error')); + + const done = sinon.spy(); + + await strategyCallback(mockReq, mockProfile, {}, done); + + expect(done.calledOnce).to.be.true; + const [err, user] = done.firstCall.args; + expect(err).to.contain('LDAP error'); + expect(user).to.be.null; + }); +}); diff --git a/test/testLogin.test.js b/test/testLogin.test.js index 107bb7256..dea0cfc75 100644 --- a/test/testLogin.test.js +++ b/test/testLogin.test.js @@ -52,6 +52,27 @@ describe('auth', async () => { res.should.have.status(200); }); + it('should be able to set the git account', async function () { + console.log(`cookie: ${cookie}`); + const res = await chai.request(app).post('/api/auth/gitAccount') + .set('Cookie', `${cookie}`) + .send({ + username: 'admin', + gitAccount: 'new-account', + }); + res.should.have.status(200); + }); + + it('should throw an error if the username is not provided when setting the git account', async function () { + const res = await chai.request(app).post('/api/auth/gitAccount') + .set('Cookie', `${cookie}`) + .send({ + gitAccount: 'new-account', + }); + console.log(`res: ${JSON.stringify(res)}`); + res.should.have.status(400); + }); + it('should now be able to logout', async function () { const res = await chai.request(app).post('/api/auth/logout').set('Cookie', `${cookie}`); res.should.have.status(200); @@ -78,6 +99,27 @@ describe('auth', async () => { }); res.should.have.status(401); }); + + it('should fail to set the git account if the user is not logged in', async function () { + const res = await chai.request(app).post('/api/auth/gitAccount').send({ + username: 'admin', + gitAccount: 'new-account', + }); + res.should.have.status(401); + }); + + it('should fail to get the current user metadata if not logged in', async function () { + const res = await chai.request(app).get('/api/auth/me'); + res.should.have.status(401); + }); + + it('should fail to login with invalid credentials', async function () { + const res = await chai.request(app).post('/api/auth/login').send({ + username: 'admin', + password: 'invalid', + }); + res.should.have.status(401); + }); }); after(async function () {