Skip to content

Commit

Permalink
Check entity actions in submission against those permitted by form def
Browse files Browse the repository at this point in the history
  • Loading branch information
matthew-white committed Dec 8, 2023
1 parent 544fb6a commit f971794
Show file tree
Hide file tree
Showing 9 changed files with 235 additions and 85 deletions.
31 changes: 23 additions & 8 deletions lib/data/dataset.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

const Option = require('../util/option');
const Problem = require('../util/problem');
const { traverseXml, findOne, and, root, node, hasAttr, tree, attr } = require('../util/xml');
const { traverseXml, findOne, root, node, attr, stripNamespacesFromPath } = require('../util/xml');

const validateDatasetName = (name) => {
// Regex explanation:
Expand Down Expand Up @@ -61,21 +61,36 @@ const getDataset = (xml) => {
const metaNode = findOne(root('html'), node('head'), node('model'), node('instance'), node(), node('meta'));
return traverseXml(xml, [
findOne(root('html'), node('head'), node('model'))(attr('entities-version')),
metaNode(findOne(root(), node('entity'))(tree())),
metaNode(findOne(root(), and(node('entity'), hasAttr('dataset')))(attr('dataset')))
]).then(([ version, entityTag, datasetName ]) => {
if (entityTag.isEmpty() && datasetName.isEmpty())
metaNode(findOne(root(), node('entity'))(attr()))
]).then(([ version, entityAttrs ]) => {
if (entityAttrs.isEmpty())
return Option.none();

if (version.isEmpty())
throw Problem.user.invalidEntityForm({ reason: 'Entities specification version is missing.' });
else if (!(version.get().startsWith('2022.1.') || version.get().startsWith('2023.1.')))
throw Problem.user.invalidEntityForm({ reason: `Entities specification version [${version.get()}] is not supported.` });

const strippedAttrs = Object.create(null);
for (const [name, value] of Object.entries(entityAttrs.get()))
strippedAttrs[stripNamespacesFromPath(name)] = value;

// check that dataset name is valid
if (datasetName.isEmpty())
const datasetName = strippedAttrs.dataset;
if (datasetName == null)
throw Problem.user.invalidEntityForm({ reason: 'Dataset name is missing.' });
if (!validateDatasetName(datasetName.get()))
if (!validateDatasetName(datasetName))
throw Problem.user.invalidEntityForm({ reason: 'Invalid dataset name.' });
return Option.of(datasetName.get().trim());

// Entity actions permitted by the form def
const actions = [];
const { create, update } = strippedAttrs;
if (create != null) actions.push('create');
if (update != null) actions.push('update');
if (actions.length === 0)
throw Problem.user.invalidEntityForm({ reason: 'The form must specify at least one entity action, for example, create or update.' });

return Option.of({ name: datasetName.trim(), actions });
});
};

Expand Down
20 changes: 20 additions & 0 deletions lib/model/migrations/20231208-01-dataset-form-def-actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2023 ODK Central Developers
// See the NOTICE file at the top-level directory of this distribution and at
// https://github.com/getodk/central-backend/blob/master/NOTICE.
// This file is part of ODK Central. It is subject to the license terms in
// the LICENSE file found in the top-level directory of this distribution and at
// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central,
// including this file, may be copied, modified, propagated, or distributed
// except according to the terms contained in the LICENSE file.

const up = async (db) => {
await db.raw('ALTER TABLE dataset_form_defs ADD COLUMN actions jsonb');
// eslint-disable-next-line quotes
await db.raw(`UPDATE dataset_form_defs SET actions = '["create"]'`);
await db.raw('ALTER TABLE dataset_form_defs ALTER COLUMN actions SET NOT NULL');
};

const down = (db) =>
db.raw('ALTER TABLE dataset_form_defs DROP COLUMN actions');

module.exports = { up, down };
20 changes: 11 additions & 9 deletions lib/model/query/datasets.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const { sanitizeOdataIdentifier } = require('../../util/util');
////////////////////////////////////////////////////////////////////////////
// DATASET CREATE AND UPDATE

const _insertDatasetDef = (dataset, acteeId) => sql`
const _insertDatasetDef = (dataset, acteeId, actions) => sql`
WITH ds_ins AS (
INSERT INTO datasets (id, name, "projectId", "createdAt", "acteeId")
VALUES (nextval(pg_get_serial_sequence('datasets', 'id')), ${dataset.name}, ${dataset.projectId}, clock_timestamp(), ${acteeId})
Expand All @@ -35,7 +35,7 @@ const _insertDatasetDef = (dataset, acteeId) => sql`
),
ds_defs AS (
INSERT INTO dataset_form_defs
SELECT ds.id, ${dataset.aux.formDefId} FROM ds
SELECT ds.id, ${dataset.aux.formDefId}, ${JSON.stringify(actions)} FROM ds
ON CONFLICT ON CONSTRAINT dataset_form_defs_datasetid_formdefid_unique
DO NOTHING
)
Expand Down Expand Up @@ -69,25 +69,27 @@ insert_property_fields AS (
// should be moved to util.js or we already have similar func somewhere?
const isNilOrEmpty = either(isNil, isEmpty);

const _createOrMerge = (dataset, fields, acteeId) => sql`
${_insertDatasetDef(dataset, acteeId)}
const _createOrMerge = (dataset, acteeId, actions, fields) => sql`
${_insertDatasetDef(dataset, acteeId, actions)}
${isNilOrEmpty(fields) ? sql`` : _insertProperties(fields)}
SELECT "action", "id" FROM ds
`;

// Takes the dataset name and field->property mappings defined in a form
// and creates or updates the named dataset.
// Takes information about the dataset parsed from the form XML, as well as
// field->property mappings defined in a form, then creates or updates the named
// dataset.
// Arguments:
// * dataset name
// * information about the dataset parsed from the form XML
// * form (a Form frame or object containing projectId, formDefId, and schemaId)
// * array of field objects and property names that were parsed from the form xml
const createOrMerge = (name, form, fields) => async ({ one, Actees, Datasets, Projects }) => {
const createOrMerge = (parsedDataset, form, fields) => async ({ one, Actees, Datasets, Projects }) => {
const { projectId } = form;
const { id: formDefId, schemaId } = form.def;

// Provision acteeId if necessary.
// Need to check for existing dataset, and if not found, need to also
// fetch the project since the dataset will be an actee child of the project.
const { name } = parsedDataset;
const existingDataset = await Datasets.get(projectId, name, false);
const acteeId = existingDataset.isDefined()
? existingDataset.get().acteeId
Expand All @@ -107,7 +109,7 @@ const createOrMerge = (name, form, fields) => async ({ one, Actees, Datasets, Pr

// Insert or update: update dataset, dataset properties, and links to fields and current form
// result contains action (created or updated) and the id of the dataset.
const result = await one(_createOrMerge(partial, dsPropertyFields, acteeId))
const result = await one(_createOrMerge(partial, acteeId, parsedDataset.actions, dsPropertyFields))
.catch(error => {
if (error.constraint === 'ds_property_fields_dspropertyid_formdefid_unique') {
throw Problem.user.invalidEntityForm({ reason: 'Multiple Form Fields cannot be saved to a single property.' });
Expand Down
20 changes: 19 additions & 1 deletion lib/model/query/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,14 @@ resolveConflict.audit = (entity, dataset) => (log) => log('entity.update.resolve
/////////////////////////////////////////////////////////////////////////
// Processing submission events to create and update entities

const _getFormDefActions = async (oneFirst, datasetId, submissionDefId) => oneFirst(sql`
SELECT dataset_form_defs.actions
FROM submission_defs
JOIN dataset_form_defs ON
dataset_form_defs."datasetId" = ${datasetId} AND
dataset_form_defs."formDefId" = submission_defs."formDefId"
WHERE submission_defs.id = ${submissionDefId}`);

const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent) => async ({ Audits, Entities }) => {
// If dataset requires approval on submission to create an entity and this event is not
// an approval event, then don't create an entity
Expand Down Expand Up @@ -236,7 +244,7 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
};

// Entrypoint to where submissions (a specific version) become entities
const _processSubmissionEvent = (event, parentEvent) => async ({ Datasets, Entities, Submissions, Forms }) => {
const _processSubmissionEvent = (event, parentEvent) => async ({ Datasets, Entities, Submissions, Forms, oneFirst }) => {
const { submissionId, submissionDefId } = event.details;

const form = await Forms.getByActeeId(event.acteeId);
Expand Down Expand Up @@ -279,6 +287,16 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Datasets, Entit
const dataset = (await Datasets.get(form.get().projectId, entityData.system.dataset, true))
.orThrow(Problem.user.datasetNotFound({ datasetName: entityData.system.dataset }));

// Check that the form permits the entity action(s) specified in the
// submission.
const permittedActions = await _getFormDefActions(oneFirst, dataset.id, submissionDefId);
for (const action of ['create', 'update']) {
const submissionAction = entityData.system[action];
if ((submissionAction === '1' || submissionAction === 'true') &&
!permittedActions.includes(action))
throw Problem.user.entityActionNotAllowed({ action, permitted: permittedActions });
}

// Try update before create (if both are specified)
if (entityData.system.update === '1' || entityData.system.update === 'true')
try {
Expand Down
12 changes: 6 additions & 6 deletions lib/model/query/forms.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ const createNew = (partial, project, publish = false) => async ({ run, Actees, D
defData.keyId = await partial.aux.key.map(Keys.ensure).orElse(resolve(null));

// Parse form XML for fields and entity/dataset definitions
const [fields, datasetName] = await Promise.all([
const [fields, parsedDataset] = await Promise.all([
getFormFields(partial.xml),
(partial.aux.key.isDefined() ? resolve(Option.none()) : getDataset(partial.xml)) // Don't parse dataset schema if Form has encryption key
]);
Expand Down Expand Up @@ -163,8 +163,8 @@ const createNew = (partial, project, publish = false) => async ({ run, Actees, D
]);

// Update datasets and properties if defined
if (datasetName.isDefined()) {
await Datasets.createOrMerge(datasetName.get(), savedForm, fields);
if (parsedDataset.isDefined()) {
await Datasets.createOrMerge(parsedDataset.get(), savedForm, fields);

if (publish) {
await Datasets.publishIfExists(savedForm.def.id, savedForm.def.publishedAt.toISOString());
Expand Down Expand Up @@ -242,7 +242,7 @@ const createVersion = (partial, form, publish, duplicating = false) => async ({
const keyId = await partial.aux.key.map(Keys.ensure).orElse(resolve(null));

// Parse form fields and dataset/entity definition from form XML
const [fields, datasetName] = await Promise.all([
const [fields, parsedDataset] = await Promise.all([
getFormFields(partial.xml),
(partial.aux.key.isDefined() ? resolve(Option.none()) : getDataset(partial.xml))
]);
Expand Down Expand Up @@ -299,8 +299,8 @@ const createVersion = (partial, form, publish, duplicating = false) => async ({
]);

// Update datasets and properties if defined
if (datasetName.isDefined()) {
await Datasets.createOrMerge(datasetName.get(), new Form(form, { def: savedDef }), fieldsForInsert);
if (parsedDataset.isDefined()) {
await Datasets.createOrMerge(parsedDataset.get(), new Form(form, { def: savedDef }), fieldsForInsert);
if (publish) {
await Datasets.publishIfExists(savedDef.id, savedDef.publishedAt.toISOString());
}
Expand Down
3 changes: 3 additions & 0 deletions lib/util/problem.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ const problems = {
// TODO: should have details but not sure what yet.
insufficientRights: problem(403.1, () => 'The authentication you provided does not have rights to perform that action.'),

entityActionNotAllowed: problem(403.2, ({ action, permitted }) =>
`The submission attempts an entity ${action}, but the form does not permit that action. The form permits the following actions: ${permitted.join(',')}.`),

// no detail information as the problem should be self-evident by REST conventions.
notFound: problem(404.1, () => 'Could not find the resource you were looking for.'),

Expand Down
22 changes: 11 additions & 11 deletions test/data/xml.js
Original file line number Diff line number Diff line change
Expand Up @@ -603,23 +603,23 @@ module.exports = {
<entities:label>Alice (88)</entities:label>
</entities:entity>
<orx:instanceName>one</orx:instanceName>
</meta>
<name>Alice</name>
<age>88</age>
<hometown>Chicago</hometown>
</data>`,
</meta>
<name>Alice</name>
<age>88</age>
<hometown>Chicago</hometown>
</data>`,
two: `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:entities="http://www.opendatakit.org/xforms" id="simpleEntity" version="1.0">
<meta>
<instanceID>two</instanceID>
<entities:entity dataset="people" id="uuid:12345678-1234-4123-8234-123456789aaa" create="1">
<entities:label>Jane (30)</entities:label>
<entities:label>Jane (30)</entities:label>
</entities:entity>
<orx:instanceName>two</orx:instanceName>
</meta>
<name>Jane</name>
<age>30</age>
<hometown>Boston</hometown>
</data>`,
</meta>
<name>Jane</name>
<age>30</age>
<hometown>Boston</hometown>
</data>`,
three: `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:entities="http://www.opendatakit.org/xforms" id="simpleEntity" version="1.0">
<meta>
<instanceID>three</instanceID>
Expand Down
62 changes: 60 additions & 2 deletions test/integration/api/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,10 @@ const testEntityUpdates = (test) => testService(async (service, container) => {
})
.expect(200);

// create form and submission to update entity
// create a form that can update or create an entity
await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.updateEntity)
.send(testData.forms.updateEntity
.replace('update=""', 'update="" create=""'))
.set('Content-Type', 'application/xml')
.expect(200);

Expand Down Expand Up @@ -2809,4 +2810,61 @@ describe('Entities API', () => {
}));
});
});

describe('permitted entity actions', () => {
it('should result in an error for an update from a create form', testService(async (service, container) => {
const asAlice = await service.login('alice');
await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'application/xml')
.expect(200);
await asAlice.post('/v1/projects/1/forms/simpleEntity/submissions')
.send(testData.instances.simpleEntity.one)
.set('Content-Type', 'application/xml')
.expect(200);
await exhaust(container);
await asAlice.post('/v1/projects/1/forms/simpleEntity/submissions')
.send(testData.instances.simpleEntity.one
.replace('<instanceID>one</instanceID>', '<instanceID>another</instanceID>')
.replace('create="1"', 'update="1"'))
.set('Content-Type', 'application/xml')
.expect(200);
await exhaust(container);
const { body: audits } = await asAlice.get('/v1/projects/1/forms/simpleEntity/submissions/another/audits')
.expect(200);
const actions = audits.map(audit => audit.action);
actions.should.eql(['entity.error', 'submission.create']);
audits[0].details.should.containEql({
problem: {
problemCode: 403.2,
problemDetails: { action: 'update', permitted: ['create'] }
},
errorMessage: 'The submission attempts an entity update, but the form does not permit that action. The form permits the following actions: create.'
});
}));

it('should result in an error for a create from an update form', testService(async (service, container) => {
const asAlice = await service.login('alice');
await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.updateEntity)
.set('Content-Type', 'application/xml')
.expect(200);
await asAlice.post('/v1/projects/1/forms/updateEntity/submissions')
.send(testData.instances.updateEntity.one
.replace('update="1"', 'create="1"'))
.expect(200);
await exhaust(container);
const { body: audits } = await asAlice.get('/v1/projects/1/forms/updateEntity/submissions/one/audits')
.expect(200);
const actions = audits.map(audit => audit.action);
actions.should.eql(['entity.error', 'submission.create']);
audits[0].details.should.containEql({
problem: {
problemCode: 403.2,
problemDetails: { action: 'create', permitted: ['update'] }
},
errorMessage: 'The submission attempts an entity create, but the form does not permit that action. The form permits the following actions: update.'
});
}));
});
});
Loading

0 comments on commit f971794

Please sign in to comment.