CVE data is to be in the JSON 5.2 CVE Record format. Details of the JSON 5.2 schema are located here.
Contact the CVE Services team", "contact": { diff --git a/package.json b/package.json index 8bb790cfc..16da323ab 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "cve-services", "author": "Automation Working Group", - "version": "2.7.3", + "version": "2.7.4", "license": "(CC0)", "devDependencies": { "@faker-js/faker": "^7.6.0", diff --git a/src/controller/registry-user.controller/registry-user.controller.js b/src/controller/registry-user.controller/registry-user.controller.js index fdfa630a8..3133c197e 100644 --- a/src/controller/registry-user.controller/registry-user.controller.js +++ b/src/controller/registry-user.controller/registry-user.controller.js @@ -213,7 +213,7 @@ async function updateUser (req, res, next) { We need to make sure that either way we convert to one or the other. For now, I am going shortname / username */ - const session = await mongoose.startSession() + const session = await mongoose.startSession({ causalConsistency: false }) // Check to see if identifier is set const identifier = req.ctx.params.identifier @@ -227,6 +227,11 @@ async function updateUser (req, res, next) { const body = req.ctx.body + if ('secret' in body) { + logger.info({ uuid: req.ctx.uuid, message: 'User attempted to update the secret.' }) + return res.status(400).json(error.secretUpdateNotAllowed()) + } + const requestingUserParameters = { org: req.ctx.org, username: req.ctx.user @@ -248,12 +253,24 @@ async function updateUser (req, res, next) { : await userRepo.findOneByUsernameAndOrgShortname(userToEditParameters.username, userToEditParameters.org, { session }) const org = await orgRepo.findOneByShortName(userToEditParameters.org) + if (!org) { + logger.info({ uuid: req.ctx.uuid, message: `Target organization ${userToEditParameters.org} does not exist.` }) + return res.status(404).json(error.orgDnePathParam(userToEditParameters.org)) + } if (body.org_short_name && !isSecretariat) { logger.info({ uuid: req.ctx.uuid, message: 'Only Secretariat can reassign user organization.' }) return res.status(403).json(error.notAllowedToChangeOrganization()) } + if (body.org_short_name) { + const targetOrg = await orgRepo.findOneByShortName(body.org_short_name) + if (!targetOrg) { + logger.info({ uuid: req.ctx.uuid, message: `Target organization ${body.org_short_name} does not exist.` }) + return res.status(404).json(error.orgDnePathParam(body.org_short_name)) + } + } + if (body.org_short_name && isSecretariat && userToEditParameters.org === org.short_name && body.org_short_name === org.short_name) { logger.info({ uuid: req.ctx.uuid, message: `User ${userToEditParameters.username} is already in organization ${userToEditParameters.org}.` }) return res.status(403).json(error.alreadyInOrg(org.short_name, userToEditParameters.username)) @@ -298,6 +315,7 @@ async function updateUser (req, res, next) { let result let updatedUser + let updatedUserUUID try { session.startTransaction() try { @@ -332,6 +350,8 @@ async function updateUser (req, res, next) { } } + // UUID of the user will not change, lets get it before we write to avoid read after write issues. + updatedUserUUID = await userRepo.getUserUUID(req.ctx.user, org.UUID) updatedUser = await userRepo.updateUserFull(userToEdit.UUID, body, { session }) await session.commitTransaction() } catch (error) { @@ -346,9 +366,9 @@ async function updateUser (req, res, next) { change: userToEditParameters.username + ' was successfully updated.', req_UUID: req.ctx.uuid, org_UUID: org.UUID, - user: updatedUser + user: updatedUser, + user_UUID: updatedUserUUID } - payload.user_UUID = await userRepo.getUserUUID(req.ctx.user, payload.org_UUID) logger.info(JSON.stringify(payload)) return res.status(200).json( diff --git a/src/controller/user.controller/error.js b/src/controller/user.controller/error.js index 11ab679b2..7e0f7006f 100644 --- a/src/controller/user.controller/error.js +++ b/src/controller/user.controller/error.js @@ -64,6 +64,13 @@ class UserControllerError extends idrErr.IDRError { err.message = 'This information can only be viewed or modified by the Secretariat, an Org Admin or if the requester is the user.' return err } + + secretUpdateNotAllowed () { + const err = {} + err.error = 'SECRET_UPDATE_NOT_ALLOWED' + err.message = 'The secret field must be updated through the reset_secret endpoint' + return err + } } module.exports = { diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index 03aee444c..67e47e7b8 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -61,7 +61,7 @@ async function optionallyValidateUser (req, res, next) { authenticated = false } else { result = await userRepo.findOneByUserNameAndOrgUUID(user, orgUUID) - if (!result || !result.active) { + if (!result || result.status === 'inactive') { authenticated = false } else { const isPwd = await argon2.verify(result.secret, key) @@ -133,7 +133,7 @@ async function validateUser (req, res, next) { return res.status(401).json(error.unauthorized()) } - if (result.active === false || result.status === 'inactive') { + if (result.status === 'inactive') { logger.warn(JSON.stringify({ uuid: req.ctx.uuid, message: 'User deactivated. Authentication failed for ' + user })) return res.status(401).json(error.unauthorized()) } diff --git a/src/repositories/baseUserRepository.js b/src/repositories/baseUserRepository.js index 903f6d58a..98a0f5289 100644 --- a/src/repositories/baseUserRepository.js +++ b/src/repositories/baseUserRepository.js @@ -7,8 +7,15 @@ const BaseOrgModel = require('../model/baseorg') const RegistryUser = require('../model/registryuser') const cryptoRandomString = require('crypto-random-string') const UserRepository = require('./userRepository') -const _ = require('lodash') const getConstants = require('../constants').getConstants +const _ = require('lodash') + +const skipNulls = (objValue, srcValue) => { + if (_.isArray(objValue)) { + return srcValue + } + return undefined +} /** * @function setAggregateUserObj @@ -514,19 +521,23 @@ class BaseUserRepository extends BaseRepository { throw new Error('Legacy user not found') } + const { ...incomingUserBody } = incomingUser let legacyObjectRaw let registryObjectRaw if (!isRegistryObject) { - legacyObjectRaw = incomingUser - registryObjectRaw = this.convertLegacyToRegistry(incomingUser) + legacyObjectRaw = incomingUserBody + registryObjectRaw = this.convertLegacyToRegistry(incomingUserBody) } else { - registryObjectRaw = incomingUser - legacyObjectRaw = this.convertRegistryToLegacy(incomingUser) + registryObjectRaw = incomingUserBody + legacyObjectRaw = this.convertRegistryToLegacy(incomingUserBody) } - const updatedLegacyUser = _.merge(legacyUser, legacyObjectRaw) - const updatedRegistryUser = _.merge(registryUser, registryObjectRaw) + const protectedFieldsRegistry = ['_id', 'UUID', '__v', 'secret', 'created', 'last_updated'] + const protectedFieldsLegacy = ['_id', 'UUID', '__v', 'secret', 'time', 'org_UUID'] + + const updatedRegistryUser = registryUser.overwrite(_.mergeWith(_.pick(registryUser.toObject(), protectedFieldsRegistry), registryObjectRaw, skipNulls)) + const updatedLegacyUser = legacyUser.overwrite(_.mergeWith(_.pick(legacyUser.toObject(), protectedFieldsLegacy), legacyObjectRaw, skipNulls)) try { if (incomingUser.org_short_name) { diff --git a/src/swagger.js b/src/swagger.js index 8df39f4d7..4fc21c77e 100644 --- a/src/swagger.js +++ b/src/swagger.js @@ -22,7 +22,7 @@ const fullCnaContainerRequest = require('../schemas/cve/create-cve-record-cna-re /* eslint-disable no-multi-str */ const doc = { info: { - version: '2.7.3', + version: '2.7.4', title: 'CVE Services API', description: "The CVE Services API supports automation tooling for the CVE Program. Credentials are \ required for most service endpoints. Representatives of \ diff --git a/test/integration-tests/cve-id/getCveIdTest.js b/test/integration-tests/cve-id/getCveIdTest.js index 12ed01c0b..81ff1cc6d 100644 --- a/test/integration-tests/cve-id/getCveIdTest.js +++ b/test/integration-tests/cve-id/getCveIdTest.js @@ -188,8 +188,92 @@ describe('Testing Get CVE-ID endpoint', () => { expect(cveIdObject.requested_by.user).to.equal(constants.nonSecretariatUserHeaders['CVE-API-USER']) }) }) + it('For Secretariat users, should return full information when getting a single RESERVED CVE ID', async () => { + const cveId = await helpers.cveIdReserveHelper(1, '2023', constants.nonSecretariatUserHeaders['CVE-API-ORG'], 'non-sequential') + + await chai.request(app) + .get(`/api/cve-id/${cveId}`) + .set(constants.headers) + .then(async (res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(200) + expect(res.body.cve_id).to.equal(cveId) + expect(res.body.state).to.equal('RESERVED') + // Secretariat user should see owning org details + expect(res.body.owning_cna).to.equal(constants.nonSecretariatUserHeaders['CVE-API-ORG']) + }) + }) + + it('For owning CNA users, should return full information when getting a single RESERVED CVE ID', async () => { + const cveId = await helpers.cveIdReserveHelper(1, '2023', constants.nonSecretariatUserHeaders['CVE-API-ORG'], 'non-sequential') + + await chai.request(app) + .get(`/api/cve-id/${cveId}`) + .set(constants.nonSecretariatUserHeaders) + .then(async (res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(200) + expect(res.body.cve_id).to.equal(cveId) + expect(res.body.state).to.equal('RESERVED') + // Non-secretariat user from owning org should see owning org details + expect(res.body.owning_cna).to.equal(constants.nonSecretariatUserHeaders['CVE-API-ORG']) + }) + }) + + it('For non-owning CNA users, should return partial information and redacted owning_cna when getting a single RESERVED CVE ID', async function () { + const cveId = await helpers.cveIdReserveHelper(1, '2023', constants.nonSecretariatUserHeaders['CVE-API-ORG'], 'non-sequential') + + await chai.request(app) + .get(`/api/cve-id/${cveId}`) + .set(constants.nonSecretariatUserHeaders3) // evidence_15 + .then(async (res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(200) + expect(res.body.cve_id).to.equal(cveId) + expect(res.body.state).to.equal('RESERVED') + expect(res.body.owning_cna).to.equal('[REDACTED]') + expect(res.body).to.not.have.property('requested_by') + }) + }) }) context('negative tests', () => { + it('An inactive user should be treated as unauthenticated for optionallyValidateUser endpoints (GET /api/cve-id/:id)', async function () { + const cveId = await helpers.cveIdReserveHelper(1, '2023', constants.nonSecretariatUserHeaders['CVE-API-ORG'], 'non-sequential') + + // Deactivate user + await helpers.userDeactivateAsSecHelper(constants.nonSecretariatUserHeaders['CVE-API-USER'], constants.nonSecretariatUserHeaders['CVE-API-ORG']) + + await chai.request(app) + .get(`/api/cve-id/${cveId}`) + .set(constants.nonSecretariatUserHeaders) + .then(async (res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(200) + expect(res.body.cve_id).to.equal(cveId) + expect(res.body.owning_cna).to.equal('[REDACTED]') // Should be redacted because user is treated as unauthenticated + + // Reactivate user for other tests + await helpers.userReactivateAsSecHelper(constants.nonSecretariatUserHeaders['CVE-API-USER'], constants.nonSecretariatUserHeaders['CVE-API-ORG']) + }) + }) + + it('An inactive user should be denied access for validateUser endpoints (GET /api/cve-id)', async function () { + // Deactivate user + await helpers.userDeactivateAsSecHelper(constants.nonSecretariatUserHeaders['CVE-API-USER'], constants.nonSecretariatUserHeaders['CVE-API-ORG']) + + await chai.request(app) + .get('/api/cve-id') + .set(constants.nonSecretariatUserHeaders) + .then(async (res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(401) + expect(res.body.error).to.equal('UNAUTHORIZED') + + // Reactivate user for other tests + await helpers.userReactivateAsSecHelper(constants.nonSecretariatUserHeaders['CVE-API-USER'], constants.nonSecretariatUserHeaders['CVE-API-ORG']) + }) + }) + it('Feb 29 2100 should not be valid', async () => { await chai.request(app) .get('/api/cve-id?time_modified.gt=2100-02-29T00:00:00Z') diff --git a/test/integration-tests/helpers.js b/test/integration-tests/helpers.js index 4fd5cce6f..2b8685d4e 100644 --- a/test/integration-tests/helpers.js +++ b/test/integration-tests/helpers.js @@ -116,6 +116,40 @@ async function updateOwningOrgAsSecHelper (cveId, newOrgShortName) { }) } +async function userDeactivateAsSecHelper (userName, orgShortName) { + const user = await chai.request(app) + .get(`/api/registry/org/${orgShortName}/user/${userName}`) + .set(constants.headers) + .then(res => res.body) + + await chai.request(app) + .put(`/api/registry/org/${orgShortName}/user/${userName}`) + .set(constants.headers) + .send({ ...user, status: 'inactive' }) + .then((res, err) => { + // Safety Expect + expect(res).to.have.status(200) + }) +} + +async function userReactivateAsSecHelper (userName, orgShortName) { + const user = await chai.request(app) + .get(`/api/registry/org/${orgShortName}/user/${userName}`) + .set(constants.headers) + .then(res => res.body) + + user.status = 'active' + + await chai.request(app) + .put(`/api/registry/org/${orgShortName}/user/${userName}`) + .set(constants.headers) + .send(user) + .then((res, err) => { + // Safety Expect + expect(res).to.have.status(200) + }) +} + module.exports = { cveIdReserveHelper, cveIdBulkReserveHelper, @@ -126,5 +160,7 @@ module.exports = { cveUpdateAsSecHelper, cveUpdateAsCnaHelperWithAdpContainer, userOrgUpdateAsSecHelper, - updateOwningOrgAsSecHelper + updateOwningOrgAsSecHelper, + userDeactivateAsSecHelper, + userReactivateAsSecHelper } diff --git a/test/integration-tests/user/updateUserTest.js b/test/integration-tests/user/updateUserTest.js index d885536af..610438687 100644 --- a/test/integration-tests/user/updateUserTest.js +++ b/test/integration-tests/user/updateUserTest.js @@ -189,5 +189,52 @@ describe('Testing Edit user endpoint', () => { expect(res.body.error).to.contain('USER_ALREADY_IN_ORG') }) }) + it('Should return an error when attempting to update secret with registry enabled', async () => { + let user + await chai.request(app).get('/api/registry/org/win_5/user/jasminesmith@win_5.com').set(constants.nonSecretariatUserHeaders).then((res) => { user = res.body }) + await chai.request(app) + .put('/api/registry/org/win_5/user/jasminesmith@win_5.com') + .set(constants.nonSecretariatUserHeaders) + .send({ + ...user, + secret: 'some_new_secret_hash' + }) + .then((res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(400) + expect(res.body.error).to.equal('SECRET_UPDATE_NOT_ALLOWED') + }) + }) + it('Should return 404 when target organization in path does not exist', async () => { + const user = constants.headers['CVE-API-USER'] + await chai.request(app) + .put(`/api/registry/org/non_existent_org/user/${user}`) + .set(constants.headers) + .send({ + name: { + first: 'NewFirst', + last: 'NewLast' + } + }) + .then((res) => { + expect(res).to.have.status(404) + expect(res.body.error).to.contain('ORG_DNE_PARAM') + }) + }) + + it('Should return 404 when target organization in body does not exist', async () => { + const user = constants.headers['CVE-API-USER'] + const org = constants.headers['CVE-API-ORG'] + await chai.request(app) + .put(`/api/registry/org/${org}/user/${user}`) + .set(constants.headers) + .send({ + org_short_name: 'non_existent_org' + }) + .then((res) => { + expect(res).to.have.status(404) + expect(res.body.error).to.contain('ORG_DNE_PARAM') + }) + }) }) }) diff --git a/test/unit-tests/middleware/mockObjects.middleware.js b/test/unit-tests/middleware/mockObjects.middleware.js index 81fdfd9ce..a353d8444 100644 --- a/test/unit-tests/middleware/mockObjects.middleware.js +++ b/test/unit-tests/middleware/mockObjects.middleware.js @@ -34,7 +34,8 @@ const existentUser = { }, secret: '$argon2i$v=19$m=4096,t=3,p=1$+qGHEfH5h4/tk404iWBxFw$xV96/b4NvQVvlZIq57wTS8s7gfKzsfMXRiOyf3ffgcw', username: 'cpadro', - active: true + active: true, + status: 'active' } const deactivatedUser = { @@ -49,7 +50,8 @@ const deactivatedUser = { }, secret: '$argon2i$v=19$m=4096,t=3,p=1$+qGHEfH5h4/tk404iWBxFw$xV96/b4NvQVvlZIq57wTS8s7gfKzsfMXRiOyf3ffgcw', username: 'flast', - active: false + active: false, + status: 'inactive' } module.exports = {