diff --git a/package.json b/package.json index 6f435519..a3323d2b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openhim-core", "description": "The OpenHIM core application that provides logging and routing of http requests", - "version": "7.2.0", + "version": "7.2.1", "main": "./lib/server.js", "bin": { "openhim-core": "./bin/openhim-core.js" diff --git a/src/api/metadata.js b/src/api/metadata.js index 06a1c4c3..83c4e78c 100644 --- a/src/api/metadata.js +++ b/src/api/metadata.js @@ -10,6 +10,7 @@ import {ContactGroupModelAPI} from '../model/contactGroups' import {KeystoreModelAPI} from '../model/keystore' import {MediatorModelAPI} from '../model/mediators' import {UserModelAPI} from '../model/users' +import {PassportModelAPI} from '../model/passport' import * as polling from '../polling' // Map string parameters to collections @@ -18,6 +19,7 @@ const collections = { Clients: ClientModelAPI, Mediators: MediatorModelAPI, Users: UserModelAPI, + Passports: PassportModelAPI, ContactGroups: ContactGroupModelAPI, KeystoreModelAPI } @@ -39,28 +41,27 @@ function removeProperties(obj) { // Function to return unique identifier key and value for a collection function getUniqueIdentifierForCollection(collection, doc) { - let uid - let uidKey + const returnObj = {} + switch (collection) { case 'Channels': - uidKey = 'name' - uid = doc.name + returnObj['name'] = doc.name break case 'Clients': - uidKey = 'clientID' - uid = doc.clientID + returnObj['clientID'] = doc.clientID break case 'Mediators': - uidKey = 'urn' - uid = doc.urn + returnObj['urn'] = doc.urn break case 'Users': - uidKey = 'email' - uid = doc.email + returnObj['email'] = doc.email + break + case 'Passports': + returnObj['protocol'] = doc.protocol + returnObj['email'] = doc.email break case 'ContactGroups': - uidKey = 'groups' - uid = doc.groups + returnObj['groups'] = doc.groups break default: logger.debug( @@ -68,8 +69,7 @@ function getUniqueIdentifierForCollection(collection, doc) { ) break } - const returnObj = {} - returnObj[uidKey] = uid + return returnObj } diff --git a/src/api/users.js b/src/api/users.js index ee86de89..f087f003 100644 --- a/src/api/users.js +++ b/src/api/users.js @@ -87,7 +87,7 @@ export async function authenticateToken(ctx, email) { } else { const passport = await PassportModelAPI.findOne({ protocol: 'token', - user: user.id + email: user.email }) if (!passport) { diff --git a/src/model/passport.js b/src/model/passport.js index 106e70b3..b81135f4 100644 --- a/src/model/passport.js +++ b/src/model/passport.js @@ -68,8 +68,11 @@ const PassportSchema = new Schema({ // Associations // - // Associate every passport with one, and only one, user. - user: {type: Schema.Types.ObjectId, ref: 'User'} + // Associate every passport with a user. + email: { + type: String, + required: true + } }) // compile the Passport Schema into a Model @@ -90,7 +93,7 @@ export const createPassport = async function (user, passwordInfo) { let result = {error: null, user: null} return await PassportModelAPI.create({ protocol: passwordInfo.password ? 'local' : 'token', - user: user.id, + email: user.email, ...passwordInfo }) .then(async function () { diff --git a/src/model/users.js b/src/model/users.js index 7d4575ef..4371c914 100644 --- a/src/model/users.js +++ b/src/model/users.js @@ -117,7 +117,7 @@ export const updateUser = async function (newUserData) { if (password) { await PassportModelAPI.findOne({ protocol: 'local', - user: user.id + email: user.email }).then(async function (passport) { if (passport) { passport.password = password @@ -174,7 +174,7 @@ export const updateTokenUser = async function (newUserData) { if (passwordHash && passwordAlgorithm && passwordSalt) { await PassportModelAPI.findOne({ protocol: 'token', - user: user.id + email: user.email }).then(async function (passport) { if (passport) { passport.passwordHash = passwordHash diff --git a/src/protocols/local.js b/src/protocols/local.js index 242b77bd..3446b84f 100644 --- a/src/protocols/local.js +++ b/src/protocols/local.js @@ -33,7 +33,7 @@ export const login = async function (email, password, next) { } return PassportModelAPI.findOne({ protocol: user.provider === 'token' ? 'token' : 'local', - user: user.id + email: user.email }) .then(function (passport) { if (passport) { diff --git a/src/protocols/openid.js b/src/protocols/openid.js index 1e9b0b82..92d3cee6 100644 --- a/src/protocols/openid.js +++ b/src/protocols/openid.js @@ -94,7 +94,8 @@ export const login = async ( tokens, issuer, identifier: identifier, - protocol: 'openid' + protocol: 'openid', + email: user.email } PassportModelAPI.findOne({ @@ -111,10 +112,7 @@ export const login = async ( return createOrUpdateUser(user) .then(function (retrievedUser) { return PassportModelAPI.create( - { - user: retrievedUser.id, - ...newPassport - }, + newPassport, function () { next(null, retrievedUser) } diff --git a/src/protocols/token.js b/src/protocols/token.js index cb8f13f4..3d32b1de 100644 --- a/src/protocols/token.js +++ b/src/protocols/token.js @@ -82,7 +82,7 @@ export const login = async function (req, next) { const passport = await PassportModelAPI.findOne({ protocol: 'token', - user: user.id + email: user.email }) if (passport == null) { diff --git a/src/upgradeDB.js b/src/upgradeDB.js index 73579642..7563a1a2 100644 --- a/src/upgradeDB.js +++ b/src/upgradeDB.js @@ -252,7 +252,7 @@ upgradeFuncs.push({ passwordHash: user.passwordHash, passwordAlgorithm: user.passwordAlgorithm, passwordSalt: user.passwordSalt, - user: user.id, + email: user.email, protocol: 'token' } diff --git a/test/integration/metadataAPITests.js b/test/integration/metadataAPITests.js index e3d8fecf..aaf0c122 100644 --- a/test/integration/metadataAPITests.js +++ b/test/integration/metadataAPITests.js @@ -16,6 +16,7 @@ import {ClientModelAPI} from '../../src/model/clients' import {ContactGroupModelAPI} from '../../src/model/contactGroups' import {MediatorModelAPI} from '../../src/model/mediators' import {UserModelAPI} from '../../src/model/users' +import {PassportModelAPI} from '../../src/model/passport' import * as polling from '../../src/polling' const sampleMetadata = { @@ -89,6 +90,12 @@ const sampleMetadata = { groups: ['admin', 'RHIE'] } ], + Passports: [ + { + email: 'r2..@jembi.org', + protocol: 'local' + } + ], ContactGroups: [ { group: 'Group 1', @@ -101,6 +108,11 @@ const sampleMetadata = { {user: 'User 6', method: 'email', maxAlerts: '1 per day'} ] } + ], + Keystore: [ + { + key: 'Key' + } ] } @@ -112,6 +124,7 @@ describe('API Integration Tests', () => { nonRootCookie = '' before(async () => { + await PassportModelAPI.deleteMany({}) await promisify(server.start)({apiPort: SERVER_PORTS.apiPort}) await testUtils.setupTestUsers() }) @@ -216,6 +229,17 @@ describe('API Integration Tests', () => { }) }) + describe('Passports', () => { + it('should fetch passports and return status 200', async () => { + const res = await request(BASE_URL) + .get('/metadata') + .set('Cookie', rootCookie) + .expect(200) + + res.body[0].Passports.length.should.equal(2) + }) + }) + describe('ContactGroups', () => { beforeEach(async () => { await new ContactGroupModelAPI(sampleMetadata.ContactGroups[0]).save() @@ -724,10 +748,103 @@ describe('API Integration Tests', () => { // POST TO VALIDATE METADATA TESTS describe('*validateMetadata', () => { beforeEach(async () => { + await PassportModelAPI.deleteMany() await testUtils.cleanupAllTestUsers() await testUtils.setupTestUsers() }) + const sampleMetadata = { + Channels: [ + { + name: 'TestChannel1', + urlPattern: 'test/sample', + allow: ['PoC', 'Test1', 'Test2'], + routes: [ + {name: 'test route', host: 'localhost', port: 9876, primary: true} + ], + txViewAcl: 'group1', + updatedBy: { + id: new ObjectId(), + name: 'Test' + } + } + ], + Clients: [ + { + clientID: 'YUIAIIIICIIAIA', + clientDomain: 'him.jembi.org', + name: 'OpenMRS Ishmael instance', + roles: ['OpenMRS_PoC', 'PoC'], + passwordHash: + '$2a$10$w8GyqInkl72LMIQNpMM/fenF6VsVukyya.c6fh/GRtrKq05C2.Zgy', + certFingerprint: + '23:37:6A:5E:A9:13:A4:8C:66:C5:BB:9F:0E:0D:68:9B:99:80:10:FC' + } + ], + Mediators: [ + { + urn: 'urn:uuid:EEA84E13-1C92-467C-B0BD-7C480462D1ED', + version: '1.0.0', + name: 'Save Encounter Mediator', + description: 'A mediator for testing', + endpoints: [ + { + name: 'Save Encounter', + host: 'localhost', + port: '8005', + type: 'http' + } + ], + defaultChannelConfig: [ + { + name: 'Save Encounter 1', + urlPattern: '/encounters', + type: 'http', + allow: [], + routes: [ + { + name: 'Save Encounter 1', + host: 'localhost', + port: '8005', + type: 'http' + } + ] + } + ] + } + ], + Users: [ + { + firstname: 'Namey', + surname: 'mcTestName', + email: 'r..@jembi.org', + passwordAlgorithm: 'sha512', + passwordHash: '796a5a8e-4e44-4d9f-9e04-c27ec6374ffa', + passwordSalt: 'bf93caba-6eec-4c0c-a1a3-d968a7533fd7', + groups: ['admin', 'RHIE'] + } + ], + Passports: [ + { + email: 'r2..@jembi.org', + protocol: 'local' + } + ], + ContactGroups: [ + { + group: 'Group 1', + users: [ + {user: 'User 1', method: 'sms', maxAlerts: 'no max'}, + {user: 'User 2', method: 'email', maxAlerts: '1 per hour'}, + {user: 'User 3', method: 'sms', maxAlerts: '1 per day'}, + {user: 'User 4', method: 'email', maxAlerts: 'no max'}, + {user: 'User 5', method: 'sms', maxAlerts: '1 per hour'}, + {user: 'User 6', method: 'email', maxAlerts: '1 per day'} + ] + } + ] + } + it('should validate metadata and return status 201', async () => { const res = await request(BASE_URL) .post('/metadata/validate') @@ -741,7 +858,7 @@ describe('API Integration Tests', () => { statusCheckObj[doc.status] += 1 } - statusCheckObj.Valid.should.equal(5) + statusCheckObj.Valid.should.equal(6) statusCheckObj.Conflict.should.equal(0) statusCheckObj.Error.should.equal(0) }) @@ -762,7 +879,7 @@ describe('API Integration Tests', () => { statusCheckObj[doc.status] += 1 } - statusCheckObj.Valid.should.equal(4) + statusCheckObj.Valid.should.equal(5) statusCheckObj.Conflict.should.equal(0) statusCheckObj.Error.should.equal(1) }) @@ -787,7 +904,7 @@ describe('API Integration Tests', () => { statusCheckObj[doc.status] += 1 } - statusCheckObj.Valid.should.equal(4) + statusCheckObj.Valid.should.equal(5) statusCheckObj.Conflict.should.equal(1) statusCheckObj.Error.should.equal(0) ChannelModelAPI.deleteMany({}) diff --git a/test/integration/usersAPITests.js b/test/integration/usersAPITests.js index 37ec4cfc..a8011a87 100644 --- a/test/integration/usersAPITests.js +++ b/test/integration/usersAPITests.js @@ -272,7 +272,8 @@ describe('API Integration Tests', () => { .send(updates) .expect(200) - const user = await UserModelAPI.findOne({email: 'jane@doe.net'}) + const email = 'jane@doe.net' + const user = await UserModelAPI.findOne({email}) user.should.have.property('firstname', 'Jane Sally') user.should.have.property('surname', 'Doe') @@ -281,7 +282,7 @@ describe('API Integration Tests', () => { user.should.have.property('locked', false) user.should.have.property('expiry', null) - const passport = await PassportModelAPI.findOne({user: user.id}) + const passport = await PassportModelAPI.findOne({email}) passport.should.have.property('password') }) @@ -920,7 +921,7 @@ describe('API Integration Tests', () => { user.should.have.property('expiry', null) const passport = await PassportModelAPI.findOne({ - user: user._id, + email: user.email, protocol: 'token' }) @@ -1144,7 +1145,7 @@ describe('API Integration Tests', () => { user.groups.should.have.length(3) const passport = await PassportModelAPI.findOne({ - user: user.id, + email: user.email, protocol: 'token' }) diff --git a/test/unit/metadataTest.js b/test/unit/metadataTest.js index a784194a..a1384f56 100644 --- a/test/unit/metadataTest.js +++ b/test/unit/metadataTest.js @@ -42,6 +42,13 @@ describe('Metadata Functions', () => { }) result.should.have.property('urn', 'mediatorUID') + result = metadata.getUniqueIdentifierForCollection('Passports', { + email: 'userEmail', + protocol: 'local' + }) + result.should.have.property('email', 'userEmail') + result.should.have.property('protocol', 'local') + result = metadata.getUniqueIdentifierForCollection('Users', { email: 'userUID' }) @@ -51,6 +58,11 @@ describe('Metadata Functions', () => { groups: 'cgUID' }) result.should.have.property('groups', 'cgUID') + + result = metadata.getUniqueIdentifierForCollection('Invalid-collection', { + id: '123333' + }) + result.should.deepEqual({}) return done() })) diff --git a/test/unit/passportTest.js b/test/unit/passportTest.js index ca20312e..d060992f 100644 --- a/test/unit/passportTest.js +++ b/test/unit/passportTest.js @@ -8,18 +8,17 @@ import should from 'should' import * as model from '../../src/model' describe('PassportModel tests', () => { - let userId + const email = 'bfm@crazy.net' const user = new model.UserModelAPI({ firstname: 'Bill', surname: 'Murray', - email: 'bfm@crazy.net', + email, groups: ['HISP', 'group2'] }) before(async () => { - const res = await user.save() - userId = res.id + await user.save() }) after(async () => { @@ -28,45 +27,35 @@ describe('PassportModel tests', () => { describe('.createPassport()', () => { it('should create a new password', async () => { - const {error, user} = await model.createPassport({id: userId}, 'password') + const {error, user} = await model.createPassport({email}, 'password') should.equal(error, null) - user.should.have.property('id', userId) + user.should.have.property('email', email) const passportResult = await model.PassportModelAPI.findOne({ - user: userId + email }) passportResult.should.have.property('password') }) - - it('should return error for non existent user', async () => { - const {error, user} = await model.createPassport( - {id: 'non_existent_id'}, - 'password' - ) - - should.equal(user, null) - error.should.have.property('message') - }) }) describe('.updatePassport()', () => { it('should update passport of a user', async () => { const passportResult = await model.PassportModelAPI.findOne({ - user: userId + email }) const {error, user} = await model.updatePassport( - {id: userId}, + {email}, {id: passportResult.id, password: 'new_password'} ) should.equal(error, null) - user.should.have.property('id', userId) + user.should.have.property('email', email) const newPassportResult = await model.PassportModelAPI.findOne({ - user: userId + email }) newPassportResult.should.have.property('password') @@ -75,7 +64,7 @@ describe('PassportModel tests', () => { it('should return error for non existent passport', async () => { const {error, user} = await model.updatePassport( - {id: userId}, + {email}, {id: 'non_existent_id'} ) diff --git a/test/unit/upgradeDBTest.js b/test/unit/upgradeDBTest.js index ebdcc661..3362cc0c 100644 --- a/test/unit/upgradeDBTest.js +++ b/test/unit/upgradeDBTest.js @@ -424,8 +424,6 @@ describe('Upgrade DB Tests', () => { describe(`updateFunction3 - Migrate password properties of token auth strategy from User collection to Passport collection`, () => { const upgradeFunc = originalUpgradeFuncs[3].func - let userId1 = '', - userId2 = '' const userObj1 = { // password: "password" @@ -471,9 +469,7 @@ describe('Upgrade DB Tests', () => { beforeEach(async () => { const res1 = await new UserModel(userObj1).save() - userId1 = res1._id const res2 = await new UserModel(userObj2).save() - userId2 = res2._id }) it('should migrate password properties of token auth strategy from User to Passport collection', async () => { @@ -492,12 +488,12 @@ describe('Upgrade DB Tests', () => { idx1.should.be.above(-1) passports[idx1].passwordHash.should.be.equal(userObj1.passwordHash) passports[idx1].passwordSalt.should.be.equal(userObj1.passwordSalt) - passports[idx1].user.should.be.deepEqual(userId1) + passports[idx1].email.should.be.equal(userObj1.email) passports[idx1].protocol.should.be.equal('token') idx2.should.be.above(-1) passports[idx2].passwordHash.should.be.equal(userObj2.passwordHash) passports[idx2].passwordSalt.should.be.equal(userObj2.passwordSalt) - passports[idx2].user.should.be.deepEqual(userId2) + passports[idx2].email.should.be.equal(userObj2.email) passports[idx2].protocol.should.be.equal('token') }) diff --git a/test/unit/usersTest.js b/test/unit/usersTest.js index 840a46fc..b816dc0f 100644 --- a/test/unit/usersTest.js +++ b/test/unit/usersTest.js @@ -35,7 +35,7 @@ describe('UserModel tests', () => { user.should.not.have.property('password') const passportResult = await model.PassportModelAPI.findOne({ - user: user.id + email: user.email }) passportResult.should.have.property('password') @@ -74,17 +74,17 @@ describe('UserModel tests', () => { let userId = null let userIdWithoutPassword = null let oldPassword = null + const email = 'bfm@crazy.net' const userToBeCreated = { firstname: 'Bill', surname: 'Murray', - email: 'bfm@crazy.net', + email, password: 'password', groups: ['HISP', 'group2'] } const userWithoutPassword = new model.UserModelAPI({ - id: userId, firstname: 'Elena', surname: 'Smith', email: 'bfm@cool.net', @@ -100,7 +100,7 @@ describe('UserModel tests', () => { userId = user.id const passportResult = await model.PassportModelAPI.findOne({ - user: userId + email }) oldPassword = passportResult.password @@ -155,7 +155,7 @@ describe('UserModel tests', () => { user.should.not.have.property('password') const passportResult = await model.PassportModelAPI.find({ - user: user.id + email: user.email }) .limit(1) .sort({$natural: -1}) @@ -182,7 +182,7 @@ describe('UserModel tests', () => { user.should.not.have.property('password') const passportResult = await model.PassportModelAPI.findOne({ - user: user.id + email: user.email }) passportResult.should.have.property('password') }) @@ -242,7 +242,7 @@ describe('UserModel tests', () => { await model.updateTokenUser({id: userId, ...userToBeCreated}) oldPassport = await model.PassportModelAPI.findOne({ - user: userId + email: userToBeCreated.email }) const res = await userWithoutPassword.save() @@ -298,7 +298,7 @@ describe('UserModel tests', () => { user.should.have.property('email', userToBeUpdated.email) const res = await model.PassportModelAPI.find({ - user: user.id, + email: user.email, protocol: 'token' }) .limit(1) @@ -337,7 +337,7 @@ describe('UserModel tests', () => { user.should.have.property('provider', 'token') const passportResult = await model.PassportModelAPI.findOne({ - user: user.id, + email: user.email, protocol: 'token' }) passportResult.should.have.property(