From 791e03d72e26975fcc34fd9a2814c10e79109840 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Mon, 15 Sep 2025 14:31:18 -0400 Subject: [PATCH 1/2] Fixed tests for resetting secret --- src/controller/org.controller/index.js | 2 - test/unit-tests/user/mockObjects.user.js | 2 +- test/unit-tests/user/userResetSecretTest.js | 146 +++++++++++--------- 3 files changed, 78 insertions(+), 72 deletions(-) diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js index 3acef38c5..1aeae4c6e 100644 --- a/src/controller/org.controller/index.js +++ b/src/controller/org.controller/index.js @@ -1004,8 +1004,6 @@ router.put('/org/:shortname/user/:username/reset_secret', } } */ - param(['registry']).optional().isBoolean(), - mw.handleRegistryParameter, mw.validateUser, mw.onlyOrgWithPartnerRole, param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), diff --git a/test/unit-tests/user/mockObjects.user.js b/test/unit-tests/user/mockObjects.user.js index 427415625..8c07dbabe 100644 --- a/test/unit-tests/user/mockObjects.user.js +++ b/test/unit-tests/user/mockObjects.user.js @@ -267,7 +267,7 @@ const userC = { // same org_UUID as userA but different username active_roles: [] }, org_UUID: existentOrgDummy.UUID, - UUID: '33394284-4acf-423b-b199-9e57656ee451', + UUID: '13394284-4acf-423b-b199-9e57656ee451', secret: '$argon2i$v=19$m=4096,t=3,p=1$meXeqZas6Ba2eQrIb3xbiA$x8KRFqYVuvlvsyMiUA2/hSaFbd2mxaKhEM5rXUfx9sw' } diff --git a/test/unit-tests/user/userResetSecretTest.js b/test/unit-tests/user/userResetSecretTest.js index 2a224771e..66cd7ab08 100644 --- a/test/unit-tests/user/userResetSecretTest.js +++ b/test/unit-tests/user/userResetSecretTest.js @@ -10,6 +10,10 @@ const OrgRepository = require('../../../src/repositories/orgRepository.js') const UserRepository = require('../../../src/repositories/userRepository.js') const RegistryOrgRepository = require('../../../src/repositories/registryOrgRepository.js') const RegistryUserRepository = require('../../../src/repositories/registryUserRepository.js') + +const BaseOrgRepository = require('../../../src/repositories/baseOrgRepository.js') +const BaseUserRepository = require('../../../src/repositories/baseUserRepository.js') + const orgController = require('../../../src/controller/org.controller/org.controller.js') // Mocks for error messages and fixtures @@ -17,11 +21,11 @@ const { OrgControllerError } = require('../../../src/controller/org.controller/e const error = new OrgControllerError() const userFixtures = require('./mockObjects.user.js') -describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint', () => { - let status, json, res, next, getOrgRepository, orgRepo, regOrgRepo, getUserRepository, getRegistryOrgRepository, - userRepo, userRegistryRepo, mockSession, orgUUIDStub, regOrgUUIDStub, userUUIDStub, regUserUUIDStub, - isSecretariatStub, isAdminStub, findOneUserStub, findOneRegUserStub, updateUserStub, updateRegUserStub, - isRegSecretariatStub, isRegAdminStub, getRegistryUserRepository +describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint', () => { + let status, json, res, next, getOrgRepository, orgRepo, getUserRepository, + userRepo, mockSession, orgUUIDStub, regOrgUUIDStub, userUUIDStub, regUserUUIDStub, + isSecretariatStub, isAdminStub, findOneUserStub, updateUserStub, + isRegSecretariatStub, isRegAdminStub, baseOrgRepo, getBaseOrgRepository, baseUserRepo, getBaseUserRepository, isSecretariatByShortName beforeEach(() => { // Mock Express response objects @@ -45,10 +49,12 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo getOrgRepository = sinon.stub().returns(orgRepo) userRepo = new UserRepository() getUserRepository = sinon.stub().returns(userRepo) - regOrgRepo = new RegistryOrgRepository() - getRegistryOrgRepository = sinon.stub().returns(regOrgRepo) - userRegistryRepo = new RegistryUserRepository() - getRegistryUserRepository = sinon.stub().returns(userRegistryRepo) + + baseOrgRepo = new BaseOrgRepository() + getBaseOrgRepository = sinon.stub().returns(baseOrgRepo) + + baseUserRepo = new BaseUserRepository() + getBaseUserRepository = sinon.stub().returns(baseUserRepo) // Set up stubs for all repository methods that will be called isSecretariatStub = sinon.stub(orgRepo, 'isSecretariat') @@ -59,12 +65,11 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo userUUIDStub = sinon.stub(userRepo, 'getUserUUID') // Stubs for registry repositories - isRegSecretariatStub = sinon.stub(regOrgRepo, 'isSecretariat') - isRegAdminStub = sinon.stub(userRegistryRepo, 'isAdmin') - regOrgUUIDStub = sinon.stub(regOrgRepo, 'getOrgUUID') - findOneRegUserStub = sinon.stub(userRegistryRepo, 'findOneByUserNameAndOrgUUID') - updateRegUserStub = sinon.stub(userRegistryRepo, 'updateByUserNameAndOrgUUID') - regUserUUIDStub = sinon.stub(userRegistryRepo, 'getUserUUID') + isRegSecretariatStub = sinon.stub(baseOrgRepo, 'isSecretariat') + isSecretariatByShortName = sinon.stub(baseOrgRepo, 'isSecretariatByShortName') + isRegAdminStub = sinon.stub(baseUserRepo, 'isAdmin') + regOrgUUIDStub = sinon.stub(baseOrgRepo, 'getOrgUUID') + regUserUUIDStub = sinon.stub(baseUserRepo, 'getUserUUID') }) afterEach(() => { @@ -81,11 +86,11 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo uuid: faker.datatype.uuid(), org: 'secretariat_org', user: 'secretariat_user', - repositories: { getOrgRepository, getUserRepository, getRegistryOrgRepository, getRegistryUserRepository } - }, - params: { - shortname: userFixtures.nonExistentOrg.short_name, - username: userFixtures.existentUser.username + repositories: { getOrgRepository, getUserRepository, getBaseUserRepository, getBaseOrgRepository }, + params: { + shortname: userFixtures.nonExistentOrg.short_name, + username: userFixtures.existentUser.username + } } } @@ -94,7 +99,7 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo const errObj = error.orgDnePathParam(userFixtures.nonExistentOrg.short_name) expect(status.calledWith(404)).to.be.true expect(json.calledWithMatch({ error: errObj.error, message: errObj.message })).to.be.true - expect(mockSession.abortTransaction.called).to.be.false + expect(mockSession.abortTransaction.called).to.be.true expect(mockSession.endSession.calledOnce).to.be.true }) @@ -104,18 +109,17 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo isSecretariatStub.resolves(true) isRegSecretariatStub.resolves(true) findOneUserStub.resolves(null) - findOneRegUserStub.resolves(null) const req = { ctx: { uuid: faker.datatype.uuid(), org: 'secretariat_org', user: 'secretariat_user', - repositories: { getOrgRepository, getUserRepository, getRegistryOrgRepository, getRegistryUserRepository } - }, - params: { - shortname: userFixtures.existentOrg.short_name, - username: userFixtures.nonExistentUser.username + repositories: { getOrgRepository, getUserRepository, getBaseUserRepository, getBaseOrgRepository }, + params: { + shortname: userFixtures.existentOrg.short_name, + username: userFixtures.nonExistentUser.username + } } } @@ -124,27 +128,31 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo const errObj = error.userDne(userFixtures.nonExistentUser.username) expect(status.calledWith(404)).to.be.true expect(json.calledWithMatch({ error: errObj.error, message: errObj.message })).to.be.true - expect(mockSession.abortTransaction.called).to.be.false + expect(mockSession.abortTransaction.called).to.be.true expect(mockSession.endSession.calledOnce).to.be.true }) it('Should fail if a non-Secretariat user tries to access a different organization', async () => { orgUUIDStub.resolves(userFixtures.existentOrg.UUID) regOrgUUIDStub.resolves(userFixtures.existentOrg.UUID) - isSecretariatStub.resolves(false) + isSecretariatByShortName.resolves(false) isRegSecretariatStub.resolves(false) + isRegAdminStub.resolves(false) + regUserUUIDStub.onFirstCall().resolves(userFixtures.existentUser.UUID) + regUserUUIDStub.onSecondCall().resolves('FakeUUID') const req = { ctx: { uuid: faker.datatype.uuid(), org: userFixtures.owningOrg.short_name, user: 'some_user', - repositories: { getOrgRepository, getUserRepository, getRegistryOrgRepository, getRegistryUserRepository } - }, - params: { - shortname: userFixtures.existentOrg.short_name, - username: userFixtures.existentUser.username + repositories: { getOrgRepository, getUserRepository, getBaseUserRepository, getBaseOrgRepository }, + params: { + shortname: userFixtures.existentOrg.short_name, + username: userFixtures.existentUser.username + } } + } await orgController.USER_RESET_SECRET(req, res, next) @@ -152,7 +160,7 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo const errObj = error.notSameOrgOrSecretariat() expect(status.calledWith(403)).to.be.true expect(json.calledWithMatch({ error: errObj.error, message: errObj.message })).to.be.true - expect(mockSession.abortTransaction.called).to.be.false + expect(mockSession.abortTransaction.called).to.be.true expect(mockSession.endSession.calledOnce).to.be.true }) @@ -164,18 +172,19 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo isAdminStub.resolves(false) isRegAdminStub.resolves(false) findOneUserStub.resolves(userFixtures.userC) - findOneRegUserStub.resolves(userFixtures.userC) + regUserUUIDStub.onFirstCall().resolves(userFixtures.userC.UUID) + regUserUUIDStub.onSecondCall().resolves(userFixtures.userA.UUID) const req = { ctx: { uuid: faker.datatype.uuid(), org: userFixtures.existentOrgDummy.short_name, user: userFixtures.userA.username, - repositories: { getOrgRepository, getUserRepository, getRegistryOrgRepository, getRegistryUserRepository } - }, - params: { - shortname: userFixtures.existentOrgDummy.short_name, - username: userFixtures.userC.username + repositories: { getOrgRepository, getUserRepository, getBaseOrgRepository, getBaseUserRepository }, + params: { + shortname: userFixtures.existentOrgDummy.short_name, + username: userFixtures.userC.username + } } } @@ -184,7 +193,7 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo const errObj = error.notSameUserOrSecretariat() expect(status.calledWith(403)).to.be.true expect(json.calledWithMatch({ error: errObj.error, message: errObj.message })).to.be.true - expect(mockSession.abortTransaction.called).to.be.false + expect(mockSession.abortTransaction.called).to.be.true expect(mockSession.endSession.calledOnce).to.be.true }) }) @@ -195,30 +204,28 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo orgUUIDStub.resolves(userFixtures.existentOrgDummy.UUID) regOrgUUIDStub.resolves(userFixtures.existentOrgDummy.UUID) updateUserStub.resolves({ matchedCount: 1, modifiedCount: 1 }) - updateRegUserStub.resolves({ matchedCount: 1, modifiedCount: 1 }) userUUIDStub.resolves(userFixtures.userA.UUID) regUserUUIDStub.resolves(userFixtures.userA.UUID) }) it('Should reset the secret if the requester is the user themselves', async () => { - isSecretariatStub.resolves(false) - isRegSecretariatStub.resolves(false) - isAdminStub.resolves(false) + isSecretariatByShortName.resolves(false) isRegAdminStub.resolves(false) findOneUserStub.resolves(userFixtures.userA) - findOneRegUserStub.resolves(userFixtures.userA) + sinon.stub(baseUserRepo, 'resetSecret').resolves('ANEWUUID') const req = { ctx: { uuid: faker.datatype.uuid(), org: userFixtures.existentOrgDummy.short_name, user: userFixtures.userA.username, - repositories: { getOrgRepository, getUserRepository, getRegistryOrgRepository, getRegistryUserRepository } - }, - params: { - shortname: userFixtures.existentOrgDummy.short_name, - username: userFixtures.userA.username + repositories: { getOrgRepository, getUserRepository, getBaseOrgRepository, getBaseUserRepository }, + params: { + shortname: userFixtures.existentOrgDummy.short_name, + username: userFixtures.userA.username + } } + } await orgController.USER_RESET_SECRET(req, res, next) @@ -230,25 +237,25 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo }) it('Should reset the secret if the requester is a Secretariat', async () => { - isSecretariatStub.resolves(true) - isRegSecretariatStub.resolves(true) + isSecretariatByShortName.resolves(true) isAdminStub.resolves(false) isRegAdminStub.resolves(false) - findOneUserStub.resolves(userFixtures.existentUser) - findOneRegUserStub.resolves(userFixtures.existentUser) + regUserUUIDStub.onFirstCall().resolves(userFixtures.userC.UUID) + regUserUUIDStub.onSecondCall().resolves(userFixtures.userA.UUID) orgUUIDStub.withArgs(userFixtures.existentOrg.short_name).resolves(userFixtures.existentOrg.UUID) regOrgUUIDStub.withArgs(userFixtures.existentOrg.short_name).resolves(userFixtures.existentOrg.UUID) + sinon.stub(baseUserRepo, 'resetSecret').resolves('ANEWUUID') const req = { ctx: { uuid: faker.datatype.uuid(), org: 'secretariat_org', user: 'secretariat_user', - repositories: { getOrgRepository, getUserRepository, getRegistryOrgRepository, getRegistryUserRepository } - }, - params: { - shortname: userFixtures.existentOrg.short_name, - username: userFixtures.existentUser.username + repositories: { getBaseOrgRepository, getBaseUserRepository }, + params: { + shortname: userFixtures.existentOrg.short_name, + username: userFixtures.existentUser.username + } } } @@ -260,23 +267,24 @@ describe.skip('Testing the PUT /org/:shortname/user/:username/reset_secret endpo }) it('Should reset the secret if the requester is an admin of the target user\'s org', async () => { - isSecretariatStub.resolves(false) + isSecretariatByShortName.resolves(false) isRegSecretariatStub.resolves(false) isAdminStub.resolves(true) isRegAdminStub.resolves(true) - findOneUserStub.resolves(userFixtures.userC) - findOneRegUserStub.resolves(userFixtures.userC) + regUserUUIDStub.onFirstCall().resolves(userFixtures.userC.UUID) + regUserUUIDStub.onSecondCall().resolves(userFixtures.userA.UUID) + sinon.stub(baseUserRepo, 'resetSecret').resolves('ANEWUUID') const req = { ctx: { uuid: faker.datatype.uuid(), org: userFixtures.existentOrgDummy.short_name, user: userFixtures.userA.username, - repositories: { getOrgRepository, getUserRepository, getRegistryOrgRepository, getRegistryUserRepository } - }, - params: { - shortname: userFixtures.existentOrgDummy.short_name, - username: userFixtures.userC.username + repositories: { getBaseOrgRepository, getBaseUserRepository }, + params: { + shortname: userFixtures.existentOrgDummy.short_name, + username: userFixtures.userC.username + } } } From 7e83cdb70480d5695a6cc5122ec5c33fcda7ab8a Mon Sep 17 00:00:00 2001 From: david-rocca Date: Mon, 15 Sep 2025 14:37:25 -0400 Subject: [PATCH 2/2] Linting issues --- test/unit-tests/user/userResetSecretTest.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/unit-tests/user/userResetSecretTest.js b/test/unit-tests/user/userResetSecretTest.js index 66cd7ab08..49d66b02b 100644 --- a/test/unit-tests/user/userResetSecretTest.js +++ b/test/unit-tests/user/userResetSecretTest.js @@ -8,9 +8,6 @@ const mongoose = require('mongoose') // Mock Repositories and Controller const OrgRepository = require('../../../src/repositories/orgRepository.js') const UserRepository = require('../../../src/repositories/userRepository.js') -const RegistryOrgRepository = require('../../../src/repositories/registryOrgRepository.js') -const RegistryUserRepository = require('../../../src/repositories/registryUserRepository.js') - const BaseOrgRepository = require('../../../src/repositories/baseOrgRepository.js') const BaseUserRepository = require('../../../src/repositories/baseUserRepository.js')