From 29cf91541a3a9c0dd379ffe9a1f27358c2632231 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Tue, 23 Jun 2026 09:35:49 -0400 Subject: [PATCH 1/3] feat(validation): add CRUD endpoints for validation bypass rules - added put endpoint and completed controller/service/repo implementations - wired up the routes in validation-bypasses-routes - added openapi tag/path/schema coverage - added regression tests - verified regression tests passing and linting successful --- .../components/system-configuration.yml | 46 +++ app/api/definitions/openapi.yml | 8 + .../paths/system-configuration-paths.yml | 144 +++++++++ .../validation-bypasses-controller.js | 62 ++-- .../validation-bypasses-repository.js | 36 +++ app/routes/validation-bypasses-routes.js | 1 + .../system/validation-bypasses-service.js | 4 + .../validation-bypasses.spec.js | 300 ++++++++++++++++++ 8 files changed, 577 insertions(+), 24 deletions(-) create mode 100644 app/tests/api/validation-bypasses/validation-bypasses.spec.js diff --git a/app/api/definitions/components/system-configuration.yml b/app/api/definitions/components/system-configuration.yml index 87692bc1..f0e1fb74 100644 --- a/app/api/definitions/components/system-configuration.yml +++ b/app/api/definitions/components/system-configuration.yml @@ -73,3 +73,49 @@ components: prefix: type: string example: 'MYORG' + + validation-bypass-rule: + type: object + required: + - fieldPath + - errorCode + - stixType + properties: + _id: + type: string + description: Database id of the validation bypass rule + fieldPath: + type: array + description: Zod issue path to match. Path segments are compared as strings. + items: + type: string + example: ['x_mitre_modified_by_ref'] + errorCode: + type: string + description: Zod issue code to match. + example: 'invalid_value' + stixType: + type: string + description: STIX type to match, or `all` to match any STIX type. + example: 'attack-pattern' + suppressError: + type: boolean + description: Whether matching validation errors should be suppressed. + default: true + autoCreated: + type: boolean + description: Whether this rule was created automatically by system configuration. + default: false + autoCreatedReason: + type: string + nullable: true + description: System reason for an automatically-created rule. + example: 'static' + triggerEvent: + type: string + nullable: true + description: Event that created this rule, when applicable. + warningMessage: + type: string + nullable: true + description: Warning emitted when a matching validation error is bypassed. diff --git a/app/api/definitions/openapi.yml b/app/api/definitions/openapi.yml index e66e7b04..bdb42173 100644 --- a/app/api/definitions/openapi.yml +++ b/app/api/definitions/openapi.yml @@ -58,6 +58,8 @@ tags: description: 'Operations on STIX bundles' - name: 'System Configuration' description: 'Operations on the system configuration' + - name: 'Validation Bypasses' + description: 'Operations on validation bypass rules' - name: 'Session Management' description: 'Operations on the current session' - name: 'Authentication' @@ -429,6 +431,12 @@ paths: /api/config/organization-namespace: $ref: 'paths/system-configuration-paths.yml#/paths/~1api~1config~1organization-namespace' + /api/config/validation-bypasses: + $ref: 'paths/system-configuration-paths.yml#/paths/~1api~1config~1validation-bypasses' + + /api/config/validation-bypasses/{id}: + $ref: 'paths/system-configuration-paths.yml#/paths/~1api~1config~1validation-bypasses~1{id}' + # Session Management /api/session: $ref: 'paths/session-paths.yml#/paths/~1api~1session' diff --git a/app/api/definitions/paths/system-configuration-paths.yml b/app/api/definitions/paths/system-configuration-paths.yml index 8f037ee0..947ae2ec 100644 --- a/app/api/definitions/paths/system-configuration-paths.yml +++ b/app/api/definitions/paths/system-configuration-paths.yml @@ -171,3 +171,147 @@ paths: description: 'The organization namespace has been successfully set.' '400': description: 'Missing or invalid parameters were provided. The organization namespace was not set.' + + /api/config/validation-bypasses: + get: + summary: 'Get validation bypass rules' + operationId: 'config-get-validation-bypasses' + description: | + This endpoint gets the validation bypass rules used to suppress or warn on ADM validation errors. + tags: + - 'Validation Bypasses' + parameters: + - name: limit + in: query + description: | + The number of validation bypass rules to retrieve. + The default (0) will retrieve all rules. + schema: + type: number + default: 0 + - name: offset + in: query + description: | + The number of validation bypass rules to skip. + The default (0) will start with the first rule. + schema: + type: number + default: 0 + - name: includePagination + in: query + description: | + Whether to include pagination data in the returned value. + Wraps returned objects in a larger object. + schema: + type: boolean + default: false + responses: + '200': + description: 'A list of validation bypass rules.' + content: + application/json: + schema: + type: array + items: + $ref: '../components/system-configuration.yml#/components/schemas/validation-bypass-rule' + post: + summary: 'Create a validation bypass rule' + operationId: 'config-create-validation-bypass' + description: | + This endpoint creates a validation bypass rule used to suppress or warn on an ADM validation error. + tags: + - 'Validation Bypasses' + requestBody: + required: true + content: + application/json: + schema: + $ref: '../components/system-configuration.yml#/components/schemas/validation-bypass-rule' + responses: + '201': + description: 'The validation bypass rule was created.' + content: + application/json: + schema: + $ref: '../components/system-configuration.yml#/components/schemas/validation-bypass-rule' + '400': + description: 'Missing or invalid parameters were provided. The validation bypass rule was not created.' + '409': + description: 'A matching validation bypass rule already exists.' + + /api/config/validation-bypasses/{id}: + get: + summary: 'Get a validation bypass rule' + operationId: 'config-get-validation-bypass' + description: | + This endpoint gets a validation bypass rule by its database id. + tags: + - 'Validation Bypasses' + parameters: + - name: id + in: path + description: 'Database id of the validation bypass rule.' + required: true + schema: + type: string + responses: + '200': + description: 'A validation bypass rule.' + content: + application/json: + schema: + $ref: '../components/system-configuration.yml#/components/schemas/validation-bypass-rule' + '404': + description: 'A validation bypass rule with the requested id was not found.' + put: + summary: 'Update a validation bypass rule' + operationId: 'config-update-validation-bypass' + description: | + This endpoint updates a validation bypass rule by its database id. + tags: + - 'Validation Bypasses' + parameters: + - name: id + in: path + description: 'Database id of the validation bypass rule.' + required: true + schema: + type: string + requestBody: + required: true + content: + application/json: + schema: + $ref: '../components/system-configuration.yml#/components/schemas/validation-bypass-rule' + responses: + '200': + description: 'The validation bypass rule was updated.' + content: + application/json: + schema: + $ref: '../components/system-configuration.yml#/components/schemas/validation-bypass-rule' + '400': + description: 'Missing or invalid parameters were provided. The validation bypass rule was not updated.' + '404': + description: 'A validation bypass rule with the requested id was not found.' + '409': + description: 'A matching validation bypass rule already exists.' + delete: + summary: 'Delete a validation bypass rule' + operationId: 'config-delete-validation-bypass' + description: | + This endpoint deletes a validation bypass rule by its database id. + tags: + - 'Validation Bypasses' + parameters: + - name: id + in: path + description: 'Database id of the validation bypass rule.' + required: true + schema: + type: string + responses: + '204': + description: 'The validation bypass rule was deleted.' + '404': + description: 'A validation bypass rule with the requested id was not found.' diff --git a/app/controllers/validation-bypasses-controller.js b/app/controllers/validation-bypasses-controller.js index 459adea9..1a40b83b 100644 --- a/app/controllers/validation-bypasses-controller.js +++ b/app/controllers/validation-bypasses-controller.js @@ -2,9 +2,15 @@ const validationBypassesService = require('../services/system/validation-bypasses-service'); const logger = require('../lib/logger'); -const { DuplicateIdError } = require('../exceptions'); -exports.retrieveAll = async function (req, res) { +function validateRuleData(data) { + if (!data || !Array.isArray(data.fieldPath) || !data.errorCode || !data.stixType) { + return 'Unable to save validation bypass rule. Missing required properties (fieldPath, errorCode, stixType).'; + } + return null; +} + +exports.retrieveAll = async function (req, res, next) { const options = { offset: req.query.offset || 0, limit: req.query.limit || 0, @@ -22,20 +28,16 @@ exports.retrieveAll = async function (req, res) { } return res.status(200).send(results); } catch (err) { - logger.error('Failed with error: ' + err); - return res.status(500).send('Unable to get validation bypass rules. Server error.'); + return next(err); } }; -exports.create = async function (req, res) { +exports.create = async function (req, res, next) { const data = req.body; + const validationError = validateRuleData(data); - if (!data.fieldPath || !data.errorCode || !data.stixType) { - return res - .status(400) - .send( - 'Unable to create validation bypass rule. Missing required properties (fieldPath, errorCode, stixType).', - ); + if (validationError) { + return res.status(400).send(validationError); } try { @@ -43,17 +45,11 @@ exports.create = async function (req, res) { logger.debug('Success: Created validation bypass rule with id ' + rule._id); return res.status(201).send(rule); } catch (err) { - if (err instanceof DuplicateIdError) { - logger.warn('Duplicate validation bypass rule'); - return res.status(409).send('Unable to create validation bypass rule. Duplicate rule.'); - } else { - logger.error('Failed with error: ' + err); - return res.status(500).send('Unable to create validation bypass rule. Server error.'); - } + return next(err); } }; -exports.retrieveById = async function (req, res) { +exports.retrieveById = async function (req, res, next) { try { const rule = await validationBypassesService.retrieveById(req.params.id); if (!rule) { @@ -62,12 +58,31 @@ exports.retrieveById = async function (req, res) { logger.debug('Success: Retrieved validation bypass rule with id ' + req.params.id); return res.status(200).send(rule); } catch (err) { - logger.error('Failed with error: ' + err); - return res.status(500).send('Unable to get validation bypass rule. Server error.'); + return next(err); + } +}; + +exports.updateById = async function (req, res, next) { + const data = req.body; + const validationError = validateRuleData(data); + + if (validationError) { + return res.status(400).send(validationError); + } + + try { + const rule = await validationBypassesService.updateById(req.params.id, data); + if (!rule) { + return res.status(404).send('Validation bypass rule not found.'); + } + logger.debug('Success: Updated validation bypass rule with id ' + req.params.id); + return res.status(200).send(rule); + } catch (err) { + return next(err); } }; -exports.deleteById = async function (req, res) { +exports.deleteById = async function (req, res, next) { try { const rule = await validationBypassesService.deleteById(req.params.id); if (!rule) { @@ -76,7 +91,6 @@ exports.deleteById = async function (req, res) { logger.debug('Success: Deleted validation bypass rule with id ' + req.params.id); return res.status(204).end(); } catch (err) { - logger.error('Delete validation bypass rule failed. ' + err); - return res.status(500).send('Unable to delete validation bypass rule. Server error.'); + return next(err); } }; diff --git a/app/repository/validation-bypasses-repository.js b/app/repository/validation-bypasses-repository.js index a2a4ab47..60416bbd 100644 --- a/app/repository/validation-bypasses-repository.js +++ b/app/repository/validation-bypasses-repository.js @@ -1,5 +1,7 @@ 'use strict'; +const mongoose = require('mongoose'); + const ValidationBypassRule = require('../models/validation-bypass-rule-model'); const { DuplicateIdError, DatabaseError } = require('../exceptions'); @@ -50,6 +52,10 @@ class ValidationBypassesRepository { } async retrieveById(id) { + if (!mongoose.Types.ObjectId.isValid(id)) { + return null; + } + try { return await this.model.findById(id).lean().exec(); } catch (err) { @@ -57,7 +63,37 @@ class ValidationBypassesRepository { } } + async updateById(id, data) { + if (!mongoose.Types.ObjectId.isValid(id)) { + return null; + } + + const updateData = { ...data }; + delete updateData._id; + delete updateData.__v; + + try { + return await this.model + .findByIdAndUpdate(id, { $set: updateData }, { new: true, runValidators: true }) + .lean() + .exec(); + } catch (err) { + if (err.name === 'MongoServerError' && err.code === 11000) { + throw new DuplicateIdError({ + details: + 'A validation bypass rule with this fieldPath, errorCode, and stixType already exists.', + }); + } else { + throw new DatabaseError(err); + } + } + } + async deleteById(id) { + if (!mongoose.Types.ObjectId.isValid(id)) { + return null; + } + try { return await this.model.findByIdAndDelete(id).exec(); } catch (err) { diff --git a/app/routes/validation-bypasses-routes.js b/app/routes/validation-bypasses-routes.js index 56fd44b4..42f1e88a 100644 --- a/app/routes/validation-bypasses-routes.js +++ b/app/routes/validation-bypasses-routes.js @@ -24,6 +24,7 @@ router authz.requireRole(authz.visitorOrHigher, authz.readOnlyService), validationBypassesController.retrieveById, ) + .put(authn.authenticate, authz.requireRole(authz.admin), validationBypassesController.updateById) .delete( authn.authenticate, authz.requireRole(authz.admin), diff --git a/app/services/system/validation-bypasses-service.js b/app/services/system/validation-bypasses-service.js index dbf34132..028338cf 100644 --- a/app/services/system/validation-bypasses-service.js +++ b/app/services/system/validation-bypasses-service.js @@ -94,6 +94,10 @@ class ValidationBypassesService { return await this.repository.retrieveById(id); } + async updateById(id, data) { + return await this.repository.updateById(id, data); + } + async deleteById(id) { return await this.repository.deleteById(id); } diff --git a/app/tests/api/validation-bypasses/validation-bypasses.spec.js b/app/tests/api/validation-bypasses/validation-bypasses.spec.js new file mode 100644 index 00000000..a0110e53 --- /dev/null +++ b/app/tests/api/validation-bypasses/validation-bypasses.spec.js @@ -0,0 +1,300 @@ +const request = require('supertest'); +const { expect } = require('expect'); + +const config = require('../../../config/config'); +const database = require('../../../lib/database-in-memory'); +const databaseConfiguration = require('../../../lib/database-configuration'); +const login = require('../../shared/login'); +const ValidationBypassRule = require('../../../models/validation-bypass-rule-model'); + +const logger = require('../../../lib/logger'); +logger.level = 'debug'; + +const initialRuleData = { + fieldPath: ['x_test_field'], + errorCode: 'custom', + stixType: 'x-test-validation-bypass', + suppressError: false, + warningMessage: 'Test warning.', +}; + +const secondaryRuleData = { + fieldPath: ['x_test_other_field'], + errorCode: 'invalid_type', + stixType: 'x-test-validation-bypass', + suppressError: true, +}; + +const runtimeBypassRuleData = { + fieldPath: ['x_mitre_platforms', '0'], + errorCode: 'invalid_value', + stixType: 'attack-pattern', + suppressError: true, +}; + +function buildTechniqueWithInvalidPlatform() { + const timestamp = new Date().toISOString(); + return { + workspace: { + workflow: { + state: 'work-in-progress', + }, + }, + stix: { + name: 'technique with invalid platform', + spec_version: '2.1', + type: 'attack-pattern', + description: 'This technique intentionally uses a non-ADM platform.', + object_marking_refs: ['marking-definition--fa42a846-8d90-4e51-bc29-71d5b4802168'], + created_by_ref: 'identity--c78cb6e5-0c4b-4611-8297-d1b8b55e40b5', + kill_chain_phases: [{ kill_chain_name: 'mitre-attack', phase_name: 'impact' }], + x_mitre_modified_by_ref: 'identity--c78cb6e5-0c4b-4611-8297-d1b8b55e40b5', + x_mitre_detection: 'detection text', + x_mitre_is_subtechnique: false, + x_mitre_impact_type: ['Availability'], + x_mitre_platforms: ['BogusOS'], + x_mitre_network_requirements: true, + created: timestamp, + modified: timestamp, + }, + }; +} + +describe('Validation Bypasses API', function () { + let app; + let passportCookie; + + before(async function () { + await database.initializeConnection(); + await databaseConfiguration.checkSystemConfiguration(); + await ValidationBypassRule.init(); + + config.validateRequests.withAttackDataModel = true; + config.validateRequests.withOpenApi = true; + + app = await require('../../../index').initializeApp(); + passportCookie = await login.loginAnonymous(app); + }); + + it('GET /api/config/validation-bypasses returns validation bypass rules', async function () { + const res = await request(app) + .get('/api/config/validation-bypasses') + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(200) + .expect('Content-Type', /json/); + + const rules = res.body; + expect(rules).toBeDefined(); + expect(Array.isArray(rules)).toBe(true); + expect(rules.length).toBeGreaterThan(0); + }); + + it('POST /api/config/validation-bypasses does not create an empty rule', async function () { + await request(app) + .post('/api/config/validation-bypasses') + .send({}) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + }); + + let rule1; + it('POST /api/config/validation-bypasses creates a rule', async function () { + const res = await request(app) + .post('/api/config/validation-bypasses') + .send(initialRuleData) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(201) + .expect('Content-Type', /json/); + + rule1 = res.body; + expect(rule1).toBeDefined(); + expect(rule1._id).toBeDefined(); + expect(rule1.fieldPath).toEqual(initialRuleData.fieldPath); + expect(rule1.errorCode).toBe(initialRuleData.errorCode); + expect(rule1.stixType).toBe(initialRuleData.stixType); + expect(rule1.suppressError).toBe(false); + expect(rule1.warningMessage).toBe(initialRuleData.warningMessage); + expect(rule1.autoCreated).toBe(false); + }); + + it('GET /api/config/validation-bypasses/:id returns the created rule', async function () { + const res = await request(app) + .get('/api/config/validation-bypasses/' + rule1._id) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(200) + .expect('Content-Type', /json/); + + const rule = res.body; + expect(rule).toBeDefined(); + expect(rule._id).toBe(rule1._id); + expect(rule.fieldPath).toEqual(rule1.fieldPath); + expect(rule.errorCode).toBe(rule1.errorCode); + expect(rule.stixType).toBe(rule1.stixType); + }); + + it('GET /api/config/validation-bypasses returns paginated validation bypass rules', async function () { + const res = await request(app) + .get('/api/config/validation-bypasses?includePagination=true&limit=1') + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(200) + .expect('Content-Type', /json/); + + expect(res.body).toBeDefined(); + expect(res.body.pagination).toBeDefined(); + expect(res.body.pagination.total).toBeGreaterThan(0); + expect(res.body.data).toBeDefined(); + expect(Array.isArray(res.body.data)).toBe(true); + expect(res.body.data.length).toBe(1); + }); + + it('PUT /api/config/validation-bypasses/:id does not update a rule when the body is missing required properties', async function () { + await request(app) + .put('/api/config/validation-bypasses/' + rule1._id) + .send({ warningMessage: 'No matching criteria.' }) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + }); + + it('PUT /api/config/validation-bypasses/:id does not update a rule when the id is not found', async function () { + await request(app) + .put('/api/config/validation-bypasses/000000000000000000000000') + .send(initialRuleData) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(404); + }); + + it('PUT /api/config/validation-bypasses/:id updates a rule', async function () { + const updatedRule = { + ...rule1, + fieldPath: ['x_test_field', 'nested'], + errorCode: 'invalid_type', + suppressError: true, + warningMessage: 'Updated test warning.', + }; + + const res = await request(app) + .put('/api/config/validation-bypasses/' + rule1._id) + .send(updatedRule) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(200) + .expect('Content-Type', /json/); + + rule1 = res.body; + expect(rule1).toBeDefined(); + expect(rule1.fieldPath).toEqual(updatedRule.fieldPath); + expect(rule1.errorCode).toBe(updatedRule.errorCode); + expect(rule1.suppressError).toBe(true); + expect(rule1.warningMessage).toBe(updatedRule.warningMessage); + }); + + it('POST /api/config/validation-bypasses does not create a duplicate rule', async function () { + await request(app) + .post('/api/config/validation-bypasses') + .send(rule1) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(409); + }); + + let rule2; + it('POST /api/config/validation-bypasses creates a second rule', async function () { + const res = await request(app) + .post('/api/config/validation-bypasses') + .send(secondaryRuleData) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(201) + .expect('Content-Type', /json/); + + rule2 = res.body; + expect(rule2).toBeDefined(); + expect(rule2._id).toBeDefined(); + }); + + it('PUT /api/config/validation-bypasses/:id does not update a rule to duplicate another rule', async function () { + await request(app) + .put('/api/config/validation-bypasses/' + rule2._id) + .send({ + fieldPath: rule1.fieldPath, + errorCode: rule1.errorCode, + stixType: rule1.stixType, + suppressError: true, + }) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(409); + }); + + it('POST /api/techniques rejects an ADM validation error before a runtime bypass rule exists', async function () { + await request(app) + .post('/api/techniques') + .send(buildTechniqueWithInvalidPlatform()) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + }); + + let runtimeRule; + it('POST /api/config/validation-bypasses creates a rule that is immediately honored by ADM validation', async function () { + const bypassRes = await request(app) + .post('/api/config/validation-bypasses') + .send(runtimeBypassRuleData) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(201) + .expect('Content-Type', /json/); + + runtimeRule = bypassRes.body; + + const techniqueRes = await request(app) + .post('/api/techniques') + .send(buildTechniqueWithInvalidPlatform()) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(201) + .expect('Content-Type', /json/); + + expect(techniqueRes.body).toBeDefined(); + expect(techniqueRes.body.stix.x_mitre_platforms).toEqual(['BogusOS']); + }); + + it('DELETE /api/config/validation-bypasses/:id does not delete a rule when the id is not found', async function () { + await request(app) + .delete('/api/config/validation-bypasses/000000000000000000000000') + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(404); + }); + + it('DELETE /api/config/validation-bypasses/:id deletes a rule', async function () { + await request(app) + .delete('/api/config/validation-bypasses/' + rule1._id) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(204); + + await request(app) + .get('/api/config/validation-bypasses/' + rule1._id) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(404); + }); + + after(async function () { + if (rule2?._id) { + await ValidationBypassRule.findByIdAndDelete(rule2._id).exec(); + } + if (runtimeRule?._id) { + await ValidationBypassRule.findByIdAndDelete(runtimeRule._id).exec(); + } + await database.closeConnection(); + }); +}); From ee70ed2127b7e4962c8d6f708d0a8d4dd224c081 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Wed, 24 Jun 2026 09:29:11 -0400 Subject: [PATCH 2/3] feat(api): add protected identity controls Add dynamic MITRE identity write configuration and protected identity safeguards. Expose identity allowed values and cover the new behavior in API tests. --- .../components/system-configuration.yml | 10 ++ app/api/definitions/openapi.yml | 3 + .../paths/system-configuration-paths.yml | 34 +++++ app/config/allowed-values.json | 65 +++++++++ app/config/config.js | 6 + app/controllers/identities-controller.js | 10 +- .../system-configuration-controller.js | 27 ++++ app/exceptions/index.js | 20 +++ app/lib/error-handler.js | 6 +- app/models/system-configuration-model.js | 1 + app/repository/attack-objects-repository.js | 30 ++++ app/routes/system-configuration-routes.js | 13 ++ app/services/stix/identities-service.js | 58 +++++++- .../system/system-configuration-service.js | 37 +++++ app/tests/api/identities/identities.spec.js | 135 ++++++++++++++++++ .../system-configuration.spec.js | 56 ++++++++ 16 files changed, 503 insertions(+), 8 deletions(-) diff --git a/app/api/definitions/components/system-configuration.yml b/app/api/definitions/components/system-configuration.yml index f0e1fb74..118eff14 100644 --- a/app/api/definitions/components/system-configuration.yml +++ b/app/api/definitions/components/system-configuration.yml @@ -74,6 +74,16 @@ components: type: string example: 'MYORG' + mitre-identity-writes: + type: object + required: + - enabled + properties: + enabled: + type: boolean + description: Whether create and update requests for the protected MITRE identity are enabled. + example: false + validation-bypass-rule: type: object required: diff --git a/app/api/definitions/openapi.yml b/app/api/definitions/openapi.yml index bdb42173..4b1b041b 100644 --- a/app/api/definitions/openapi.yml +++ b/app/api/definitions/openapi.yml @@ -431,6 +431,9 @@ paths: /api/config/organization-namespace: $ref: 'paths/system-configuration-paths.yml#/paths/~1api~1config~1organization-namespace' + /api/config/mitre-identity-writes: + $ref: 'paths/system-configuration-paths.yml#/paths/~1api~1config~1mitre-identity-writes' + /api/config/validation-bypasses: $ref: 'paths/system-configuration-paths.yml#/paths/~1api~1config~1validation-bypasses' diff --git a/app/api/definitions/paths/system-configuration-paths.yml b/app/api/definitions/paths/system-configuration-paths.yml index 947ae2ec..cbdc29b9 100644 --- a/app/api/definitions/paths/system-configuration-paths.yml +++ b/app/api/definitions/paths/system-configuration-paths.yml @@ -172,6 +172,40 @@ paths: '400': description: 'Missing or invalid parameters were provided. The organization namespace was not set.' + /api/config/mitre-identity-writes: + get: + summary: 'Get the MITRE identity write protection setting' + operationId: 'config-get-mitre-identity-writes' + description: | + This endpoint gets whether create and update requests for the protected MITRE identity are enabled. + tags: + - 'System Configuration' + responses: + '200': + description: 'The MITRE identity write protection setting.' + content: + application/json: + schema: + $ref: '../components/system-configuration.yml#/components/schemas/mitre-identity-writes' + post: + summary: 'Set the MITRE identity write protection setting' + operationId: 'config-set-mitre-identity-writes' + description: | + This endpoint sets whether create and update requests for the protected MITRE identity are enabled. + tags: + - 'System Configuration' + requestBody: + required: true + content: + application/json: + schema: + $ref: '../components/system-configuration.yml#/components/schemas/mitre-identity-writes' + responses: + '204': + description: 'The MITRE identity write protection setting has been successfully set.' + '400': + description: 'Missing or invalid parameters were provided. The MITRE identity write protection setting was not set.' + /api/config/validation-bypasses: get: summary: 'Get validation bypass rules' diff --git a/app/config/allowed-values.json b/app/config/allowed-values.json index 22bb412c..a731a2fc 100644 --- a/app/config/allowed-values.json +++ b/app/config/allowed-values.json @@ -308,5 +308,70 @@ ] } ] + }, + { + "objectType": "identity", + "properties": [ + { + "propertyName": "identity_class", + "domains": [ + { + "domainName": "stix", + "allowedValues": [ + "individual", + "group", + "system", + "organization", + "class", + "unspecified" + ] + } + ] + }, + { + "propertyName": "sectors", + "domains": [ + { + "domainName": "stix", + "allowedValues": [ + "agriculture", + "aerospace", + "automotive", + "chemical", + "commercial", + "communications", + "construction", + "defense", + "education", + "energy", + "entertainment", + "financial-services", + "government", + "government-emergency-services", + "government-local", + "government-national", + "government-public-services", + "government-regional", + "healthcare", + "hospitality-leisure", + "infrastructure", + "infrastructure-dams", + "infrastructure-nuclear", + "infrastructure-water", + "insurance", + "manufacturing", + "mining", + "non-profit", + "pharmaceuticals", + "retail", + "technology", + "telecommunications", + "transportation", + "utilities" + ] + } + ] + } + ] } ] diff --git a/app/config/config.js b/app/config/config.js index 74383761..ded334f6 100644 --- a/app/config/config.js +++ b/app/config/config.js @@ -173,6 +173,12 @@ function loadConfig() { attackSpecVersion: { default: packageJson.attackSpecVersion, }, + allowMitreIdentityWrites: { + doc: 'Allow create and update requests for the protected MITRE Corporation identity object', + format: Boolean, + default: false, + env: 'WB_REST_ALLOW_MITRE_IDENTITY_WRITES', + }, }, database: { url: { diff --git a/app/controllers/identities-controller.js b/app/controllers/identities-controller.js index ce9be48f..f5500244 100644 --- a/app/controllers/identities-controller.js +++ b/app/controllers/identities-controller.js @@ -137,7 +137,7 @@ exports.updateFull = async function (req, res, next) { } }; -exports.deleteVersionById = async function (req, res) { +exports.deleteVersionById = async function (req, res, next) { try { const identity = await identitiesService.deleteVersionById( req.params.stixId, @@ -150,12 +150,11 @@ exports.deleteVersionById = async function (req, res) { return res.status(204).end(); } } catch (err) { - logger.error('Delete identity failed. ' + err); - return res.status(500).send('Unable to delete identity. Server error.'); + return next(err); } }; -exports.deleteById = async function (req, res) { +exports.deleteById = async function (req, res, next) { try { const identities = await identitiesService.deleteById(req.params.stixId); if (identities.deletedCount === 0) { @@ -165,7 +164,6 @@ exports.deleteById = async function (req, res) { return res.status(204).end(); } } catch (err) { - logger.error('Delete identity failed. ' + err); - return res.status(500).send('Unable to identity identity. Server error.'); + return next(err); } }; diff --git a/app/controllers/system-configuration-controller.js b/app/controllers/system-configuration-controller.js index 2b171dcd..d968fe7e 100644 --- a/app/controllers/system-configuration-controller.js +++ b/app/controllers/system-configuration-controller.js @@ -114,3 +114,30 @@ exports.setOrganizationNamespace = async function (req, res, next) { return next(err); } }; + +exports.retrieveMitreIdentityWrites = async function (req, res, next) { + try { + const mitreIdentityWrites = await systemConfigurationService.retrieveMitreIdentityWrites(); + logger.debug('Success: Retrieved MITRE identity writes configuration.'); + return res.status(200).send(mitreIdentityWrites); + } catch (err) { + return next(err); + } +}; + +exports.setMitreIdentityWrites = async function (req, res, next) { + const mitreIdentityWrites = req.body; + + if (typeof mitreIdentityWrites?.enabled !== 'boolean') { + logger.warn('MITRE identity writes enabled value must be boolean'); + return res.status(400).send('MITRE identity writes enabled value must be boolean'); + } + + try { + await systemConfigurationService.setMitreIdentityWrites(mitreIdentityWrites.enabled); + logger.debug(`Success: Set MITRE identity writes to: ${mitreIdentityWrites.enabled}`); + return res.status(204).send(); + } catch (err) { + return next(err); + } +}; diff --git a/app/exceptions/index.js b/app/exceptions/index.js index 4ad32b55..a296adb6 100644 --- a/app/exceptions/index.js +++ b/app/exceptions/index.js @@ -201,6 +201,24 @@ class OrganizationIdentityNotFoundError extends CustomError { } } +class ActiveOrganizationIdentityDeleteError extends CustomError { + constructor(identityRef, options) { + super( + `Cannot delete active organization identity ${identityRef}. Select a different organization identity before deleting this identity.`, + options, + ); + } +} + +class MitreIdentityWriteError extends CustomError { + constructor(identityRef, options) { + super( + `Cannot create or update protected MITRE identity ${identityRef}. Enable MITRE identity writes to modify this identity.`, + options, + ); + } +} + class AnonymousUserAccountNotSetError extends CustomError { constructor(options) { super(`Anonymous user account not set`, options); @@ -373,6 +391,8 @@ module.exports = { DefaultMarkingDefinitionsNotFoundError, OrganizationIdentityNotSetError, OrganizationIdentityNotFoundError, + ActiveOrganizationIdentityDeleteError, + MitreIdentityWriteError, AnonymousUserAccountNotSetError, AnonymousUserAccountNotFoundError, }; diff --git a/app/lib/error-handler.js b/app/lib/error-handler.js index db79a262..1fc1625f 100644 --- a/app/lib/error-handler.js +++ b/app/lib/error-handler.js @@ -24,6 +24,8 @@ const { SystemConfigurationNotFound, OrganizationIdentityNotSetError, OrganizationIdentityNotFoundError, + ActiveOrganizationIdentityDeleteError, + MitreIdentityWriteError, AnonymousUserAccountNotSetError, AnonymousUserAccountNotFoundError, InvalidTypeError, @@ -98,6 +100,7 @@ exports.serviceExceptions = function (err, req, res, next) { err instanceof SelfRevocationError || err instanceof BadRequestError || err instanceof ValidationError || + err instanceof MitreIdentityWriteError || err instanceof InvalidVersionError || err instanceof NoTaggedSnapshotsError || err instanceof InvalidComponentTypeError @@ -127,7 +130,8 @@ exports.serviceExceptions = function (err, req, res, next) { err instanceof AlreadyRevokedError || err instanceof AlreadyReleasedError || err instanceof ReleaseConflictError || - err instanceof ObjectHasValidationIssuesError + err instanceof ObjectHasValidationIssuesError || + err instanceof ActiveOrganizationIdentityDeleteError ) { logger.warn('Conflict: %s', JSON.stringify(buildErrorResponse(err))); return res.status(409).send(buildErrorResponse(err)); diff --git a/app/models/system-configuration-model.js b/app/models/system-configuration-model.js index 2eb5001f..2192e2c1 100644 --- a/app/models/system-configuration-model.js +++ b/app/models/system-configuration-model.js @@ -11,6 +11,7 @@ const systemConfigurationDefinition = { range_start: { type: Number, default: null }, prefix: { type: String, default: null }, }, + mitre_identity_writes_enabled: { type: Boolean, default: false }, created_at: { type: Date, default: Date.now }, }; diff --git a/app/repository/attack-objects-repository.js b/app/repository/attack-objects-repository.js index c0e816ca..9eeb4a3e 100644 --- a/app/repository/attack-objects-repository.js +++ b/app/repository/attack-objects-repository.js @@ -160,6 +160,36 @@ class AttackObjectsRepository extends BaseRepository { } } + /** + * Retrieve latest, active objects whose created_by_ref or x_mitre_modified_by_ref matches + * the provided identity ref. + * @param {string} identityRef - Identity STIX ID + * @returns {Promise} Array of matching latest active objects + */ + async retrieveAllLatestActiveByIdentityRef(identityRef) { + try { + const aggregation = [ + { $sort: { 'stix.id': 1, 'stix.modified': -1 } }, + { $group: { _id: '$stix.id', document: { $first: '$$ROOT' } } }, + { $replaceRoot: { newRoot: '$document' } }, + { + $match: { + 'stix.revoked': { $in: [null, false] }, + 'stix.x_mitre_deprecated': { $in: [null, false] }, + $or: [ + { 'stix.created_by_ref': identityRef }, + { 'stix.x_mitre_modified_by_ref': identityRef }, + ], + }, + }, + { $project: { _id: 0, __v: 0, __t: 0 } }, + ]; + return await this.model.aggregate(aggregation).exec(); + } catch (err) { + throw new DatabaseError(err); + } + } + async findByIdAndDelete(documentId) { try { return await this.model.findByIdAndDelete(documentId).exec(); diff --git a/app/routes/system-configuration-routes.js b/app/routes/system-configuration-routes.js index d75613a1..77cbe575 100644 --- a/app/routes/system-configuration-routes.js +++ b/app/routes/system-configuration-routes.js @@ -68,4 +68,17 @@ router systemConfigurationController.setOrganizationNamespace, ); +router + .route('/config/mitre-identity-writes') + .get( + authn.authenticate, + authz.requireRole(authz.visitorOrHigher, authz.readOnlyService), + systemConfigurationController.retrieveMitreIdentityWrites, + ) + .post( + authn.authenticate, + authz.requireRole(authz.admin), + systemConfigurationController.setMitreIdentityWrites, + ); + module.exports = router; diff --git a/app/services/stix/identities-service.js b/app/services/stix/identities-service.js index d2a02a00..8993e374 100644 --- a/app/services/stix/identities-service.js +++ b/app/services/stix/identities-service.js @@ -1,13 +1,50 @@ 'use strict'; const uuid = require('uuid'); +const { xMitreIdentity } = require('@mitre-attack/attack-data-model'); const config = require('../../config/config'); +const attackObjectsRepository = require('../../repository/attack-objects-repository'); const identitiesRepository = require('../../repository/identities-repository'); +const systemConfigurationsRepository = require('../../repository/system-configurations-repository'); const { BaseService } = require('../meta-classes'); -const { InvalidTypeError } = require('../../exceptions'); +const { + ActiveOrganizationIdentityDeleteError, + InvalidTypeError, + MitreIdentityWriteError, +} = require('../../exceptions'); const { Identity: IdentityType } = require('../../lib/types'); class IdentitiesService extends BaseService { + async assertMitreIdentityWritable(stixId, options = {}) { + if (options.import || stixId !== xMitreIdentity) { + return; + } + + const systemConfig = await systemConfigurationsRepository.retrieveOne({ lean: true }); + const mitreIdentityWritesEnabled = + systemConfig?.mitre_identity_writes_enabled ?? config.app.allowMitreIdentityWrites; + + if (mitreIdentityWritesEnabled) { + return; + } + + throw new MitreIdentityWriteError(stixId); + } + + async assertIdentityCanBeDeleted(stixId) { + const systemConfig = await systemConfigurationsRepository.retrieveOne({ lean: true }); + if (systemConfig?.organization_identity_ref !== stixId) { + return; + } + + const referencedObjects = + await attackObjectsRepository.retrieveAllLatestActiveByIdentityRef(stixId); + throw new ActiveOrganizationIdentityDeleteError(stixId, { + referencedObjectCount: referencedObjects.length, + referencedObjectIds: referencedObjects.map((object) => object.stix.id), + }); + } + /** * @public * CRUD Operation: Create @@ -24,6 +61,8 @@ class IdentitiesService extends BaseService { } options = options || {}; + await this.assertMitreIdentityWritable(data.stix.id, options); + if (!options.import) { // Set the ATT&CK Spec Version data.stix.x_mitre_attack_spec_version = @@ -44,6 +83,23 @@ class IdentitiesService extends BaseService { // Save the document in the database return await this.repository.save(data); } + + async updateFull(stixId, stixModified, data, options) { + options = options || {}; + await this.assertMitreIdentityWritable(stixId, options); + + return await super.updateFull(stixId, stixModified, data, options); + } + + async deleteVersionById(stixId, stixModified) { + await this.assertIdentityCanBeDeleted(stixId); + return await super.deleteVersionById(stixId, stixModified); + } + + async deleteById(stixId) { + await this.assertIdentityCanBeDeleted(stixId); + return await super.deleteById(stixId); + } } //Default export diff --git a/app/services/system/system-configuration-service.js b/app/services/system/system-configuration-service.js index f52ebc7c..00ca8002 100644 --- a/app/services/system/system-configuration-service.js +++ b/app/services/system/system-configuration-service.js @@ -144,6 +144,7 @@ class SystemConfigurationService extends BaseService { // First-time setup: create initial config document const newConfig = this.repository.createNewDocument({ organization_identity_ref: stixId, + mitre_identity_writes_enabled: config.app.allowMitreIdentityWrites, }); await this.repository.constructor.saveDocument(newConfig); @@ -293,6 +294,40 @@ class SystemConfigurationService extends BaseService { }); } + /** + * @public + * CRUD Operation: Read + * Returns whether protected MITRE identity writes are enabled. + */ + async retrieveMitreIdentityWrites() { + const systemConfig = await this.repository.retrieveOne({ lean: true }); + + if (!systemConfig) { + throw new SystemConfigurationNotFound(); + } + + return { + enabled: systemConfig.mitre_identity_writes_enabled ?? config.app.allowMitreIdentityWrites, + }; + } + + /** + * @public + * CRUD Operation: Update + * Sets whether protected MITRE identity writes are enabled. + */ + async setMitreIdentityWrites(enabled) { + const currentConfig = await this.repository.retrieveOne({ lean: true }); + + if (!currentConfig) { + throw new SystemConfigurationNotFound(); + } + + await this._createNewConfigVersion(currentConfig, { + mitre_identity_writes_enabled: enabled, + }); + } + /** * @public * CRUD Operation: Read @@ -319,6 +354,8 @@ class SystemConfigurationService extends BaseService { anonymous_user_account_id: currentConfig.anonymous_user_account_id, default_marking_definitions: currentConfig.default_marking_definitions, organization_namespace: currentConfig.organization_namespace, + mitre_identity_writes_enabled: + currentConfig.mitre_identity_writes_enabled ?? config.app.allowMitreIdentityWrites, ...overrides, }; const newConfig = this.repository.createNewDocument(configData); diff --git a/app/tests/api/identities/identities.spec.js b/app/tests/api/identities/identities.spec.js index 8c51c12e..eaeed0e1 100644 --- a/app/tests/api/identities/identities.spec.js +++ b/app/tests/api/identities/identities.spec.js @@ -1,8 +1,10 @@ const request = require('supertest'); const { expect } = require('expect'); +const { xMitreIdentity } = require('@mitre-attack/attack-data-model'); const database = require('../../../lib/database-in-memory'); const databaseConfiguration = require('../../../lib/database-configuration'); +const identitiesService = require('../../../services/stix/identities-service'); const config = require('../../../config/config'); const login = require('../../shared/login'); @@ -30,6 +32,53 @@ const initialObjectData = { }, }; +function createMitigationData() { + const timestamp = new Date().toISOString(); + return { + workspace: { + workflow: { + state: 'work-in-progress', + }, + }, + stix: { + created: timestamp, + modified: timestamp, + name: 'course-of-action-1', + spec_version: '2.1', + type: 'course-of-action', + description: 'This is a mitigation.', + external_references: [{ source_name: 'source-1', external_id: 's1' }], + object_marking_refs: ['marking-definition--fa42a846-8d90-4e51-bc29-71d5b4802168'], + created_by_ref: xMitreIdentity, + labels: ['label1', 'label2'], + x_mitre_version: '1.1', + }, + }; +} + +function createMitreIdentityData() { + const timestamp = new Date().toISOString(); + return { + workspace: { + workflow: { + state: 'work-in-progress', + }, + }, + stix: { + id: xMitreIdentity, + created: timestamp, + modified: timestamp, + name: 'The MITRE Corporation', + identity_class: 'organization', + spec_version: '2.1', + type: 'identity', + description: 'The MITRE Corporation identity.', + external_references: [{ source_name: 'source-1', external_id: 's1' }], + object_marking_refs: ['marking-definition--fa42a846-8d90-4e51-bc29-71d5b4802168'], + }, + }; +} + describe('Identity API', function () { let app; let passportCookie; @@ -45,6 +94,7 @@ describe('Identity API', function () { // Enable ADM validation; the request payloads in this spec are ADM-compliant config.validateRequests.withAttackDataModel = true; config.validateRequests.withOpenApi = true; + config.app.allowMitreIdentityWrites = false; // Initialize the express app app = await require('../../../index').initializeApp(); @@ -68,6 +118,91 @@ describe('Identity API', function () { expect(identities.length).toBe(1); }); + it('DELETE /api/identities/:id rejects active organization identity deletion when active objects reference it', async function () { + const orgIdentityRes = await request(app) + .get('/api/config/organization-identity') + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(200) + .expect('Content-Type', /json/); + + await request(app) + .post('/api/mitigations') + .send(createMitigationData()) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(201) + .expect('Content-Type', /json/); + + const res = await request(app) + .delete('/api/identities/' + orgIdentityRes.body.stix.id) + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(409) + .expect('Content-Type', /json/); + + expect(res.body.message).toContain('Cannot delete active organization identity'); + expect(res.body.referencedObjectCount).toBeGreaterThan(0); + }); + + it('POST /api/identities rejects protected MITRE identity writes', async function () { + const res = await request(app) + .post('/api/identities') + .send(createMitreIdentityData()) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + + expect(res.text).toContain('Cannot create or update protected MITRE identity'); + }); + + it('POST /api/identities allows protected MITRE identity writes when system configuration enables them', async function () { + await request(app) + .post('/api/config/mitre-identity-writes') + .send({ enabled: true }) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(204); + + const res = await request(app) + .post('/api/identities') + .send(createMitreIdentityData()) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(201) + .expect('Content-Type', /json/); + + expect(res.body.stix.id).toBe(xMitreIdentity); + + await identitiesService.deleteById(xMitreIdentity); + + await request(app) + .post('/api/config/mitre-identity-writes') + .send({ enabled: false }) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(204); + }); + + it('PUT /api/identities rejects protected MITRE identity updates', async function () { + const mitreIdentity = await identitiesService.create(createMitreIdentityData(), { + import: true, + }); + const modified = new Date(mitreIdentity.stix.modified).toISOString(); + const body = mitreIdentity.toObject(); + body.stix.description = 'Updated MITRE identity description.'; + body.stix.modified = new Date(Date.now() + 1000).toISOString(); + + const res = await request(app) + .put('/api/identities/' + xMitreIdentity + '/modified/' + modified) + .send(body) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + + expect(res.text).toContain('Cannot create or update protected MITRE identity'); + await identitiesService.deleteById(xMitreIdentity); + }); + it('POST /api/identities does not create an empty identity', async function () { const body = {}; await request(app) diff --git a/app/tests/api/system-configuration/system-configuration.spec.js b/app/tests/api/system-configuration/system-configuration.spec.js index 99be4fdf..62f899f1 100644 --- a/app/tests/api/system-configuration/system-configuration.spec.js +++ b/app/tests/api/system-configuration/system-configuration.spec.js @@ -297,6 +297,62 @@ describe('System Configuration API', function () { expect(namespace.prefix).toBe(testNamespace.prefix); }); + it('GET /api/config/mitre-identity-writes returns the default setting', async function () { + const res = await request(app) + .get('/api/config/mitre-identity-writes') + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(200) + .expect('Content-Type', /json/); + + const mitreIdentityWrites = res.body; + expect(mitreIdentityWrites).toBeDefined(); + expect(mitreIdentityWrites.enabled).toBe(false); + }); + + it('POST /api/config/mitre-identity-writes sets the setting', async function () { + const res = await request(app) + .post('/api/config/mitre-identity-writes') + .send({ enabled: true }) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(204); + + expect(res.body).toBeDefined(); + expect(Object.getOwnPropertyNames(res.body)).toHaveLength(0); + }); + + it('GET /api/config/mitre-identity-writes returns the updated setting', async function () { + const res = await request(app) + .get('/api/config/mitre-identity-writes') + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(200) + .expect('Content-Type', /json/); + + const mitreIdentityWrites = res.body; + expect(mitreIdentityWrites).toBeDefined(); + expect(mitreIdentityWrites.enabled).toBe(true); + }); + + it('POST /api/config/mitre-identity-writes rejects invalid settings', async function () { + await request(app) + .post('/api/config/mitre-identity-writes') + .send({ enabled: 'true' }) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + }); + + it('POST /api/config/mitre-identity-writes disables the setting', async function () { + await request(app) + .post('/api/config/mitre-identity-writes') + .send({ enabled: false }) + .set('Accept', 'application/json') + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(204); + }); + after(async function () { await database.closeConnection(); }); From ffe610efadf9fe067c638ac9866442a81b9f21b5 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Wed, 24 Jun 2026 10:17:02 -0400 Subject: [PATCH 3/3] fix(identities): enforce MITRE protection via lifecycle hooks Refactor identity create, update, and delete behavior into lifecycle hooks. Wire delete lifecycle hooks through BaseService so protected MITRE identity checks apply to DELETE requests. --- app/exceptions/index.js | 2 +- app/lib/database-configuration.js | 3 + app/services/meta-classes/base.service.js | 36 ++++++++++-- app/services/meta-classes/hooks.service.js | 31 ++++++++-- app/services/stix/identities-service.js | 63 +++++++-------------- app/tests/api/identities/identities.spec.js | 42 ++++++++++++-- 6 files changed, 117 insertions(+), 60 deletions(-) diff --git a/app/exceptions/index.js b/app/exceptions/index.js index a296adb6..03df60f3 100644 --- a/app/exceptions/index.js +++ b/app/exceptions/index.js @@ -213,7 +213,7 @@ class ActiveOrganizationIdentityDeleteError extends CustomError { class MitreIdentityWriteError extends CustomError { constructor(identityRef, options) { super( - `Cannot create or update protected MITRE identity ${identityRef}. Enable MITRE identity writes to modify this identity.`, + `Cannot create, update, or delete protected MITRE identity ${identityRef}. Enable MITRE identity writes to modify this identity.`, options, ); } diff --git a/app/lib/database-configuration.js b/app/lib/database-configuration.js index 8b7a0191..78b611fe 100644 --- a/app/lib/database-configuration.js +++ b/app/lib/database-configuration.js @@ -18,6 +18,8 @@ const { AnonymousUserAccountNotSetError, } = require('../exceptions'); +const TLP_WHITE_MARKING_DEFINITION = 'marking-definition--613f2e26-407d-48c7-9eca-b8e91df99dc9'; + // Ensure event listeners are registered before checkSystemConfiguration() emits events. // ValidationBypassesService registers its listeners at module load time, so requiring it // here guarantees they are in place before the SYSTEM_CONFIGURATION_IDENTITY_CHANGED event fires. @@ -41,6 +43,7 @@ async function createPlaceholderOrganizationIdentity() { type: 'identity', description: 'This is a placeholder organization identity. Please edit it or replace it with another identity.', + object_marking_refs: [TLP_WHITE_MARKING_DEFINITION], }, }; diff --git a/app/services/meta-classes/base.service.js b/app/services/meta-classes/base.service.js index ee174b25..62aa8cba 100644 --- a/app/services/meta-classes/base.service.js +++ b/app/services/meta-classes/base.service.js @@ -81,6 +81,18 @@ class BaseService extends ServiceWithHooks { } } + /** + * Whether create() should stamp created_by_ref and x_mitre_modified_by_ref + * with the active organization identity. + * + * Most ATT&CK objects are attributed to an organization identity. Identity + * objects themselves opt out so first-time setup can create the placeholder + * organization identity before a system configuration exists. + */ + shouldSetOrganizationIdentityRefs() { + return true; + } + async setDefaultMarkingDefinitionsForObject(attackObject) { const systemConfig = await systemConfigurationRepository.retrieveOne({ lean: true }); if (!systemConfig) return; @@ -604,7 +616,9 @@ class BaseService extends ServiceWithHooks { data.stix.x_mitre_attack_spec_version = config.app.attackSpecVersion; // TODO: data.stix.modified = new Date().toISOString() (when server controls timestamps) - const organizationIdentityRef = await this.retrieveOrganizationIdentityRef(); + const organizationIdentityRef = this.shouldSetOrganizationIdentityRefs() + ? await this.retrieveOrganizationIdentityRef() + : null; // Check for an existing object (may differ from existingVersion if stix.id was just generated) let existingObject; @@ -615,7 +629,9 @@ class BaseService extends ServiceWithHooks { if (existingObject) { // New version of an existing object — carry forward revoked status, set modified_by data.stix.revoked = existingObject.stix.revoked ?? false; - data.stix.x_mitre_modified_by_ref = organizationIdentityRef; + if (organizationIdentityRef) { + data.stix.x_mitre_modified_by_ref = organizationIdentityRef; + } } else { // Brand-new object — set ID, created_by, modified_by, revoked if (!data.stix.id) { @@ -625,8 +641,10 @@ class BaseService extends ServiceWithHooks { data.stix.created = new Date().toISOString(); } data.stix.revoked = false; - data.stix.created_by_ref = organizationIdentityRef; - data.stix.x_mitre_modified_by_ref = organizationIdentityRef; + if (organizationIdentityRef) { + data.stix.created_by_ref = organizationIdentityRef; + data.stix.x_mitre_modified_by_ref = organizationIdentityRef; + } } // Set modified timestamp if not set by client — set for both new and existing objects @@ -958,12 +976,15 @@ class BaseService extends ServiceWithHooks { throw new MissingParameterError('modified'); } + await this.beforeDeleteVersionById(stixId, stixModified); + const document = await this.repository.findOneAndDelete(stixId, stixModified); if (!document) { //Note: document is null if not found return null; } + await this.afterDeleteVersionById(document); return document; } @@ -1248,7 +1269,12 @@ class BaseService extends ServiceWithHooks { if (!stixId) { throw new MissingParameterError('stixId'); } - return await this.repository.deleteMany(stixId); + await this.beforeDeleteById(stixId); + const result = await this.repository.deleteMany(stixId); + if (result.deletedCount > 0) { + await this.afterDeleteById(stixId, result); + } + return result; } } diff --git a/app/services/meta-classes/hooks.service.js b/app/services/meta-classes/hooks.service.js index d3999433..7d239a92 100644 --- a/app/services/meta-classes/hooks.service.js +++ b/app/services/meta-classes/hooks.service.js @@ -122,22 +122,41 @@ class ServiceWithHooks { // TODO there are multiple delete methods (e.g., deleteById, deleteVersionById): it is unclear whether they can/should share lifecycle hooks or have their own; if their own, then can the delete methods be consolidated? /** - * Lifecycle hook: Called before deleteVersionById() saves the document - * Subclasses can override to prepare data + * Lifecycle hook: Called before deleteVersionById() deletes the document. + * Subclasses can override to validate the delete request. * @param {string} _stixId - The STIX ID * @param {string} _stixModified - The modified timestamp - * @param {object} _data - The update data - * @param {object} _existingDocument - The existing document */ - - async beforeDeleteVersionById(_stixId, _stixModified, _data, _existingDocument) { + async beforeDeleteVersionById(_stixId, _stixModified) { // Default: no-op } + /** + * Lifecycle hook: Called after deleteVersionById() deletes the document. + * @param {object} _deletedDocument - The deleted document + */ async afterDeleteVersionById(_deletedDocument) { /// Default: no-op } + /** + * Lifecycle hook: Called before deleteById() deletes all versions of an object. + * Subclasses can override to validate the delete request. + * @param {string} _stixId - The STIX ID + */ + async beforeDeleteById(_stixId) { + // Default: no-op + } + + /** + * Lifecycle hook: Called after deleteById() deletes all versions of an object. + * @param {string} _stixId - The STIX ID + * @param {object} _result - The repository delete result + */ + async afterDeleteById(_stixId, _result) { + // Default: no-op + } + /** ******************************** REVOKE ******************************** */ /** diff --git a/app/services/stix/identities-service.js b/app/services/stix/identities-service.js index 8993e374..3c3e9de1 100644 --- a/app/services/stix/identities-service.js +++ b/app/services/stix/identities-service.js @@ -1,6 +1,5 @@ 'use strict'; -const uuid = require('uuid'); const { xMitreIdentity } = require('@mitre-attack/attack-data-model'); const config = require('../../config/config'); const attackObjectsRepository = require('../../repository/attack-objects-repository'); @@ -9,12 +8,15 @@ const systemConfigurationsRepository = require('../../repository/system-configur const { BaseService } = require('../meta-classes'); const { ActiveOrganizationIdentityDeleteError, - InvalidTypeError, MitreIdentityWriteError, } = require('../../exceptions'); const { Identity: IdentityType } = require('../../lib/types'); class IdentitiesService extends BaseService { + shouldSetOrganizationIdentityRefs() { + return false; + } + async assertMitreIdentityWritable(stixId, options = {}) { if (options.import || stixId !== xMitreIdentity) { return; @@ -45,60 +47,33 @@ class IdentitiesService extends BaseService { }); } - /** - * @public - * CRUD Operation: Create - * - * Creates a new identity object - * - * Override of base class create() because: - * 1. Does not set created_by_ref or x_mitre_modified_by_ref - * 2. Does not check for existing identity object - */ - async create(data, options) { - if (data?.stix?.type !== IdentityType) { - throw new InvalidTypeError(); + stripIdentityAttributionRefs(data, options = {}) { + if (options.import || !data?.stix) { + return; } - options = options || {}; - await this.assertMitreIdentityWritable(data.stix.id, options); - - if (!options.import) { - // Set the ATT&CK Spec Version - data.stix.x_mitre_attack_spec_version = - data.stix.x_mitre_attack_spec_version ?? config.app.attackSpecVersion; - - // Record the user account that created the object - if (options.userAccountId) { - data.workspace.workflow.created_by_user_account = options.userAccountId; - } - - // Set the default marking definitions - await this.setDefaultMarkingDefinitionsForObject(data); - - // Assign a new STIX id if not already provided - data.stix.id = data.stix.id || `identity--${uuid.v4()}`; - } + delete data.stix.created_by_ref; + delete data.stix.x_mitre_modified_by_ref; + } - // Save the document in the database - return await this.repository.save(data); + async beforeCreate(data, options = {}) { + await this.assertMitreIdentityWritable(data?.stix?.id, options); + this.stripIdentityAttributionRefs(data, options); } - async updateFull(stixId, stixModified, data, options) { - options = options || {}; + async beforeUpdate(stixId, _stixModified, data, _existingDocument, options = {}) { await this.assertMitreIdentityWritable(stixId, options); - - return await super.updateFull(stixId, stixModified, data, options); + this.stripIdentityAttributionRefs(data, options); } - async deleteVersionById(stixId, stixModified) { + async beforeDeleteVersionById(stixId) { + await this.assertMitreIdentityWritable(stixId); await this.assertIdentityCanBeDeleted(stixId); - return await super.deleteVersionById(stixId, stixModified); } - async deleteById(stixId) { + async beforeDeleteById(stixId) { + await this.assertMitreIdentityWritable(stixId); await this.assertIdentityCanBeDeleted(stixId); - return await super.deleteById(stixId); } } diff --git a/app/tests/api/identities/identities.spec.js b/app/tests/api/identities/identities.spec.js index eaeed0e1..0c3e6add 100644 --- a/app/tests/api/identities/identities.spec.js +++ b/app/tests/api/identities/identities.spec.js @@ -79,6 +79,10 @@ function createMitreIdentityData() { }; } +async function deleteMitreIdentityForTest() { + await identitiesService.repository.deleteMany(xMitreIdentity); +} + describe('Identity API', function () { let app; let passportCookie; @@ -152,7 +156,7 @@ describe('Identity API', function () { .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) .expect(400); - expect(res.text).toContain('Cannot create or update protected MITRE identity'); + expect(res.text).toContain('Cannot create, update, or delete protected MITRE identity'); }); it('POST /api/identities allows protected MITRE identity writes when system configuration enables them', async function () { @@ -188,7 +192,8 @@ describe('Identity API', function () { import: true, }); const modified = new Date(mitreIdentity.stix.modified).toISOString(); - const body = mitreIdentity.toObject(); + const body = JSON.parse(JSON.stringify(mitreIdentity)); + delete body.warnings; body.stix.description = 'Updated MITRE identity description.'; body.stix.modified = new Date(Date.now() + 1000).toISOString(); @@ -199,8 +204,37 @@ describe('Identity API', function () { .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) .expect(400); - expect(res.text).toContain('Cannot create or update protected MITRE identity'); - await identitiesService.deleteById(xMitreIdentity); + expect(res.text).toContain('Cannot create, update, or delete protected MITRE identity'); + await deleteMitreIdentityForTest(); + }); + + it('DELETE /api/identities/:id rejects protected MITRE identity deletes', async function () { + await identitiesService.create(createMitreIdentityData(), { + import: true, + }); + + const res = await request(app) + .delete('/api/identities/' + xMitreIdentity) + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + + expect(res.text).toContain('Cannot create, update, or delete protected MITRE identity'); + await deleteMitreIdentityForTest(); + }); + + it('DELETE /api/identities/:id/modified/:modified rejects protected MITRE identity version deletes', async function () { + const mitreIdentity = await identitiesService.create(createMitreIdentityData(), { + import: true, + }); + const modified = new Date(mitreIdentity.stix.modified).toISOString(); + + const res = await request(app) + .delete('/api/identities/' + xMitreIdentity + '/modified/' + modified) + .set('Cookie', `${passportCookie.name}=${passportCookie.value}`) + .expect(400); + + expect(res.text).toContain('Cannot create, update, or delete protected MITRE identity'); + await deleteMitreIdentityForTest(); }); it('POST /api/identities does not create an empty identity', async function () {