Skip to content

Commit

Permalink
dataset verb wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@ktuite
ktuite committed May 20, 2024
1 parent 30b76db commit 225da1b
Show file tree
Hide file tree
Showing 2 changed files with 212 additions and 15 deletions.
33 changes: 30 additions & 3 deletions lib/model/query/datasets.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ SELECT "action", "id" FROM ds
// * 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
// (form fields do not need any extra IDs of the form, form def, or schema.)
const createOrMerge = (parsedDataset, form, fields) => async ({ one, Actees, Datasets, Projects }) => {
const createOrMerge = (parsedDataset, form, fields) => async ({ context, one, Actees, Datasets, Projects }) => {
const { projectId } = form;
const { id: formDefId, schemaId } = form.def;

Expand Down Expand Up @@ -126,6 +126,18 @@ const createOrMerge = (parsedDataset, form, fields) => async ({ one, Actees, Dat
throw error;
});

// TODO: some unfortunate issue here where `result` only has created/updated and doesnt
// mention if update actually changed any properties or not. every form referencing a dataset counts
// as an update.

// Verify that user has ability to create dataset
if (result.action === 'created')
await context.auth.canOrReject('dataset.create', { acteeId });

// Verify that user has ability to update dataset
if (result.action === 'updated')
await context.auth.canOrReject('dataset.update', { acteeId });

// Partial contains acteeId which is needed for auditing.
// Additional form fields and properties needed for audit logging as well.
return partial.with({ action: result.action, fields: dsPropertyFields });
Expand Down Expand Up @@ -180,7 +192,7 @@ createPublishedProperty.audit = (property, dataset) => (log) =>
// `IfExists` part is particularly helpful for `Forms.publish`,
// with that it doesn't need to ensure the existence of the Dataset
// and it can call this function blindly
const publishIfExists = (formDefId, publishedAt) => async ({ all, maybeOne }) => {
const publishIfExists = (formDefId, publishedAt) => async ({ all, context, maybeOne }) => {

// Helper sub-queries
const _publishIfExists = () => sql`
Expand Down Expand Up @@ -238,7 +250,22 @@ const publishIfExists = (formDefId, publishedAt) => async ({ all, maybeOne }) =>
const { current, provided } = d.get();
throw Problem.user.datasetNameConflict({ current, provided });
}
return all(_publishIfExists());
const properties = await all(_publishIfExists());
const datasets = groupBy(c => c.acteeId, properties);

for (const acteeId of Object.keys(datasets)) {
const event = datasets[acteeId].some(p => p.action === 'datasetCreated') ? 'dataset.create' : 'dataset.update';
if (event === 'dataset.create') {
// eslint-disable-next-line no-await-in-loop
await context.auth.canOrReject('dataset.create', { acteeId });
}

if (event === 'dataset.update' && datasets[acteeId].some(p => p.action === 'propertyAdded'))
// eslint-disable-next-line no-await-in-loop
await context.auth.canOrReject('dataset.update', { acteeId });
}

return properties;
};

publishIfExists.audit = (properties) => (log) => {
Expand Down
194 changes: 182 additions & 12 deletions test/integration/api/datasets.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { readFileSync } = require('fs');
const appRoot = require('app-root-path');
const { testService } = require('../setup');
const { testService, testServiceFullTrx } = require('../setup');
const testData = require('../../data/xml');
const config = require('config');
const { Form } = require('../../../lib/model/frames');
Expand Down Expand Up @@ -3376,17 +3376,6 @@ describe('datasets and entities', () => {

describe('parsing datasets on form upload', () => {
describe('parsing datasets at /projects/:id/forms POST', () => {
it('should allow someone without dataset.create to create a dataset through posting a form', testService(async (service, { run }) => {
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.create') WHERE system in ('manager')`);

const asBob = await service.login('bob');

await asBob.post('/v1/projects/1/forms')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);
}));

it('should return a Problem if the entity xml has the wrong version', testService((service) =>
service.login('alice', (asAlice) =>
asAlice.post('/v1/projects/1/forms')
Expand Down Expand Up @@ -3904,6 +3893,187 @@ describe('datasets and entities', () => {
}));
}));
});

describe('dataset-specific verbs', () => {
it('should NOT allow a new form that creates a dataset without user having dataset.create verb', testServiceFullTrx(async (service, { run }) => {
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.create') WHERE system in ('manager')`);

const asBob = await service.login('bob');

await asBob.post('/v1/projects/1/forms')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(403);

// shouldn't work with immediate publish, either
await asBob.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(403);
}));

it('should NOT allow a new form that updates a dataset without user having dataset.update verb', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
await asAlice.post('/v1/projects/1/datasets')
.send({ name: 'people' })
.expect(200);

await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.update') WHERE system in ('manager')`);

const asBob = await service.login('bob');

await asBob.post('/v1/projects/1/forms')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(403);

// shouldn't work with immediate publish, either
await asBob.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(403);
}));

it('should NOT allow update draft that creates a dataset without user having dataset.create verb', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
const asBob = await service.login('bob');
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.create') WHERE system in ('manager')`);

// Alice can upload first version of form
await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);

await asBob.post('/v1/projects/1/forms/simpleEntity/draft')
.send(testData.forms.simpleEntity.replace(/people/g, 'trees'))
.set('Content-Type', 'text/xml')
.expect(403);
}));

it('should NOT allow update draft that updates a dataset without user having dataset.update verb', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
const asBob = await service.login('bob');
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.update') WHERE system in ('manager')`);

// Alice can upload first version of form
await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);

await asBob.post('/v1/projects/1/forms/simpleEntity/draft')
.send(testData.forms.simpleEntity.replace('saveto="age"', 'saveto="birth_year"'))
.set('Content-Type', 'text/xml')
.expect(403);
}));

it('should NOT allow unpublished dataset to be published on form publish if user does not have dataset.create verb', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
const asBob = await service.login('bob');
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.create') WHERE system in ('manager')`);

// Alice can upload first version of form
await asAlice.post('/v1/projects/1/forms')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);

// Bob should not be able to publish form because it will publish the new dataset
await asBob.post('/v1/projects/1/forms/simpleEntity/draft/publish')
.expect(403);
}));

it('should NOT allow unpublished properties to be published on form publish if user does not have dataset.update verb', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
const asBob = await service.login('bob');
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.update') WHERE system in ('manager')`);

// Alice can upload and publish first version of form
await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);

// Alice can upload new version of form with new property
await asAlice.post('/v1/projects/1/forms/simpleEntity/draft')
.send(testData.forms.simpleEntity
.replace('saveto="age"', 'saveto="birth_year"')
.replace('orx:version="1.0"', 'orx:version="2.0"')
)
.set('Content-Type', 'text/xml')
.expect(200);

// Bob should not be able to publish form because it will publish the new property
await asBob.post('/v1/projects/1/forms/simpleEntity/draft/publish')
.expect(403);
}));

// TODO: bob not allowed to "update" dataset even with no substantive changes
it.skip('should ALLOW update of form draft that does not modify existing dataset', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
const asBob = await service.login('bob');
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.update') WHERE system in ('manager')`);

// Alice can upload first version of form
await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);

// Should be OK for bob to update draft if not updating dataset
await asBob.post('/v1/projects/1/forms/simpleEntity/draft')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);
}));

// TODO: bob shouldn't be allowed to create dataset but it counts as updating it
it.skip('should not allow "creating" of a dataset when the dataset exists but unpublished', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
const asBob = await service.login('bob');
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.create') WHERE system in ('manager')`);

// Alice can upload first version of form that creates unpublished "people" dataset
await asAlice.post('/v1/projects/1/forms')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);

// Should not be OK for bob to "create" people dataset
await asBob.post('/v1/projects/1/forms')
.send(testData.forms.simpleEntity
.replace('simpleEntity', 'simpleEntity2'))
.set('Content-Type', 'text/xml')
.expect(403);
}));

// TODO: bob not allowed to "update" dataset even with no substantive changes
it.skip('should ALLOW new form about existing dataset that does not update it', testServiceFullTrx(async (service, { run }) => {
const asAlice = await service.login('alice');
const asBob = await service.login('bob');
await run(sql`UPDATE roles SET verbs = (verbs - 'dataset.update') WHERE system in ('manager')`);

// Alice can create the dataset
await asAlice.post('/v1/projects/1/datasets')
.send({ name: 'people' })
.expect(200);

// And create the properties
await asAlice.post('/v1/projects/1/datasets/people/properties')
.send({ name: 'age' })
.expect(200);
await asAlice.post('/v1/projects/1/datasets/people/properties')
.send({ name: 'first_name' })
.expect(200);

// Should be OK for bob to upload form that uses existing dataset and properties
await asBob.post('/v1/projects/1/forms')
.send(testData.forms.simpleEntity)
.set('Content-Type', 'text/xml')
.expect(200);
}));
});
});

describe('dataset audit logging at /projects/:id/forms POST', () => {
Expand Down

0 comments on commit 225da1b

Please sign in to comment.