Check entity actions in submission against those permitted by form def (

getodk#1061)

Closes getodk/central#518.

lib/data/dataset.js

@@ -8,15 +8,23 @@
 // except according to the terms contained in the LICENSE file.
 //
 // This is an extension of schema.js where we define *datasest-specific* functions
-// for dealing with the XForms XML schema. When a form us uploaded to Central,
+// for dealing with the XForms XML schema. When a form is uploaded to Central,
 // we check the XML for an <entity> block, which will contain information
 // about datasets and mappings from form fields to
 // dataset properties.
 
 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');
 
+/*
+Validates a dataset name:
+
+  - valid xml identifier
+  - does not contain `.`
+  - `__` prefix reserved
+  - case insensitive?
+*/
 const validateDatasetName = (name) => {
   // Regex explanation:
   // Check for a match with a valid string
@@ -26,11 +34,10 @@ const validateDatasetName = (name) => {
   // If there's a match, return true (valid)!
   // Note that '.' is not in the valid character set.
   // Non-letter unicode characters also not currently allowed
-  const match = /^(?!__)[\p{L}:_][\p{L}:\d_-]*$/u.exec(name.trim());
+  const match = /^(?!__)[\p{L}:_][\p{L}:\d_-]*$/u.exec(name);
   return (match !== null);
 };
 
-
 const validatePropertyName = (name) => {
   // Regex explanation
   // (similar to above dataset name check with some slight differences)
@@ -46,37 +53,53 @@ const validatePropertyName = (name) => {
   return (match !== null);
 };
 
-// Here we are looking for dataset-registrating information within the <entity> tag.
-// We mainly need the dataset name, but we also do a bit of validation:
-//   1. namespace validation:
-//       e.g. xmlns:entities="http://www.opendatakit.org/xforms/entities
-//   2. dataset name validation:
-//       - valid xml identifier
-//       - does not contain `.`
-//       - `__` prefix reserved
-//       - case insensitive?
-//
-// Like with getFormFields in schema.js, we assume the form is otherwise valid.
-const getDataset = (xml) => {
-  const metaNode = findOne(root('html'), node('head'), node('model'), node('instance'), node(), node('meta'));
-  return traverseXml(xml, [
+/*
+getDataset() parses form XML for dataset-related information on the <entity>
+tag:
+
+  - The dataset name
+  - The entity actions permitted by the form def
+
+getDataset() also does some validation, including of:
+
+  - The version of the entities spec
+  - The dataset name
+
+Like with getFormFields() in schema.js, we assume the form is otherwise valid.
+*/
+const getDataset = (xml) =>
+  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())
+    findOne(root('html'), node('head'), node('model'), node('instance'), node(), node('meta'), 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?.trim();
+    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, actions });
   });
-};
 
 module.exports = { getDataset, validateDatasetName, validatePropertyName };

lib/model/migrations/20231208-01-dataset-form-def-actions.js

@@ -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 };

lib/model/query/datasets.js

@@ -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})
@@ -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
   )
@@ -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
@@ -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.' });

lib/model/query/entities.js

@@ -148,6 +148,11 @@ resolveConflict.audit = (entity, dataset) => (log) => log('entity.update.resolve
 /////////////////////////////////////////////////////////////////////////
 // Processing submission events to create and update entities
 
+const _getFormDefActions = (oneFirst, datasetId, formDefId) => oneFirst(sql`
+SELECT actions
+FROM dataset_form_defs
+WHERE "datasetId" = ${datasetId} AND "formDefId" = ${formDefId}`);
+
 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
@@ -236,7 +241,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);
@@ -279,6 +284,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, submissionDef.formDefId);
+  for (const action of ['create', 'update']) {
+    const submissionAction = entityData.system[action];
+    if ((submissionAction === '1' || submissionAction === 'true') &&
+      !permittedActions.includes(action))
+      throw Problem.user.entityActionNotPermitted({ action, permitted: permittedActions });
+  }
+
   // Try update before create (if both are specified)
   if (entityData.system.update === '1' || entityData.system.update === 'true')
     try {

lib/model/query/forms.js

@@ -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
   ]);
@@ -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());
@@ -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))
   ]);
@@ -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());
     }

lib/util/problem.js

@@ -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.'),
 
+    entityActionNotPermitted: 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.'),
 

test/data/xml.js

@@ -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>

test/integration/api/entities.js

@@ -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);
 
@@ -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.'
+      });
+    }));
+  });
 });