Merge pull request #1090 from getodk/entity-delete-verb

Add entity.delete verb

lib/model/migrations/20240215-01-entity-delete-verb.js

@@ -0,0 +1,20 @@
+// Copyright 2024 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 = (db) => db.raw(`
+UPDATE roles
+SET verbs = verbs || '["entity.delete"]'::jsonb
+WHERE system IN ('admin', 'manager')`);
+
+const down = (db) => db.raw(`
+UPDATE roles
+SET verbs = verbs - 'entity.delete'
+WHERE system IN ('admin', 'manager')`);
+
+module.exports = { up, down };

lib/model/migrations/20240215-02-dedupe-verbs.js

@@ -0,0 +1,17 @@
+// Copyright 2024 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 = (db) => db.raw(`
+UPDATE roles
+SET verbs = (verbs - 'submission.update') || '["submission.update"]'::jsonb
+WHERE system IN ('admin', 'manager')`);
+
+const down = () => {};
+
+module.exports = { up, down };

lib/resources/entities.js

@@ -140,7 +140,7 @@ module.exports = (service, endpoint) => {
 
     const dataset = await Datasets.get(params.projectId, params.name, true).then(getOrNotFound);
 
-    await auth.canOrReject('entity.update', dataset);
+    await auth.canOrReject('entity.delete', dataset);
 
     const entity = await Entities.getById(dataset.id, params.uuid, queryOptions).then(getOrNotFound);
 

test/integration/api/entities.js

@@ -2039,7 +2039,7 @@ describe('Entities API', () => {
         .expect(404);
     }));
 
-    it('should reject if the user cannot read', testEntities(async (service) => {
+    it('should reject if the user cannot delete', testEntities(async (service) => {
       const asChelsea = await service.login('chelsea');
 
       await asChelsea.delete('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')

test/integration/api/projects.js

@@ -62,19 +62,20 @@ describe('api: /projects', () => {
                 body[0].name.should.equal('Default Project');
               }))))));
 
-    it('should return the correct project verbs when multiply assigned', testService((service) =>
-      service.login('alice', (asAlice) =>
-        asAlice.get('/v1/users/current').expect(200).then(({ body }) => body.id)
-          .then((aliceId) => asAlice.post('/v1/projects/1/assignments/manager/' + aliceId)
-            .expect(200)
-            .then(() => asAlice.get('/v1/projects/1')
-              .set('X-Extended-Metadata', 'true')
-              .expect(200)
-              .then(({ body }) => {
-                body.verbs.length.should.equal(47);
-                body.should.be.a.Project();
-                body.name.should.equal('Default Project');
-              }))))));
+    it('should return the correct project verbs when multiply assigned', testService(async (service) => {
+      const asAlice = await service.login('alice');
+      const { body: alice } = await asAlice.get('/v1/users/current')
+        .expect(200);
+      await asAlice.post('/v1/projects/1/assignments/manager/' + alice.id)
+        .expect(200);
+      const { body: project } = await asAlice.get('/v1/projects/1')
+        .set('X-Extended-Metadata', 'true')
+        .expect(200);
+      const { body: admin } = await asAlice.get('/v1/roles/admin').expect(200);
+      project.verbs.should.eqlInAnyOrder(admin.verbs);
+      project.should.be.a.Project();
+      project.name.should.equal('Default Project');
+    }));
 
     it('should order projects appropriately', testService((service) =>
       service.login('alice', (asAlice) =>
@@ -383,28 +384,29 @@ describe('api: /projects', () => {
           .expect(200)
           .then(({ body }) => { should.not.exist(body.verbs); }))));
 
-    it('should return verb information with extended metadata (alice)', testService((service) =>
-      service.login('alice', (asAlice) =>
-        asAlice.get('/v1/projects/1')
-          .set('X-Extended-Metadata', 'true')
-          .expect(200)
-          .then(({ body }) => {
-            body.verbs.should.be.an.Array();
-            body.verbs.length.should.equal(47);
-            body.verbs.should.containDeep([ 'user.password.invalidate', 'project.delete' ]);
-          }))));
+    it('should return verb information with extended metadata (alice)', testService(async (service) => {
+      const asAlice = await service.login('alice');
+      const { body: project } = await asAlice.get('/v1/projects/1')
+        .set('X-Extended-Metadata', 'true')
+        .expect(200);
+      project.verbs.should.be.an.Array();
+      const { body: admin } = await asAlice.get('/v1/roles/admin').expect(200);
+      project.verbs.should.eql(admin.verbs);
+      project.verbs.should.containDeep([ 'user.password.invalidate', 'project.delete' ]);
+    }));
 
-    it('should return verb information with extended metadata (bob)', testService((service) =>
-      service.login('bob', (asBob) =>
-        asBob.get('/v1/projects/1')
-          .set('X-Extended-Metadata', 'true')
-          .expect(200)
-          .then(({ body }) => {
-            body.verbs.should.be.an.Array();
-            body.verbs.length.should.equal(32);
-            body.verbs.should.containDeep([ 'assignment.create', 'project.delete', 'dataset.list' ]);
-            body.verbs.should.not.containDeep([ 'project.create' ]);
-          }))));
+    it('should return verb information with extended metadata (bob)', testService(async (service) => {
+      const asBob = await service.login('bob');
+      const { body: project } = await asBob.get('/v1/projects/1')
+        .set('X-Extended-Metadata', 'true')
+        .expect(200);
+      project.verbs.should.be.an.Array();
+      const { body: manager } = await asBob.get('/v1/roles/manager')
+        .expect(200);
+      project.verbs.should.eql(manager.verbs);
+      project.verbs.should.containDeep([ 'assignment.create', 'project.delete', 'dataset.list' ]);
+      project.verbs.should.not.containDeep([ 'project.create' ]);
+    }));
 
     it('should return verb information with extended metadata (data collector only)', testService((service) =>
       service.login('alice', (asAlice) =>
@@ -547,7 +549,7 @@ describe('api: /projects', () => {
     it('should return notfound if the project does not exist', testService((service) =>
       service.delete('/v1/projects/99').expect(404)));
 
-    it('should reject unless the user can update', testService((service) =>
+    it('should reject unless the user can delete', testService((service) =>
       service.login('chelsea', (asChelsea) =>
         asChelsea.delete('/v1/projects/1').expect(403))));
 
@@ -1403,20 +1405,21 @@ describe('api: /projects', () => {
 // Nested extended forms
 describe('api: /projects?forms=true', () => {
   describe('GET', () => {
-    it('should return projects with verbs and nested extended forms', testService((service) =>
-      service.login('alice', (asAlice) => asAlice.get('/v1/projects?forms=true')
-        .expect(200)
-        .then(({ body }) => {
-          body.length.should.equal(1);
-          body[0].should.be.a.Project();
-          const { formList, verbs } = body[0];
-          verbs.length.should.equal(47);
-          formList.length.should.equal(2);
-          const form = formList[0];
-          form.should.be.a.ExtendedForm();
-          form.name.should.equal('Simple');
-          form.reviewStates.received.should.equal(0);
-        }))));
+    it('should return projects with verbs and nested extended forms', testService(async (service) => {
+      const asAlice = await service.login('alice');
+      const { body: projects } = await asAlice.get('/v1/projects?forms=true')
+        .expect(200);
+      projects.length.should.equal(1);
+      projects[0].should.be.a.Project();
+      const { body: admin } = await asAlice.get('/v1/roles/admin').expect(200);
+      projects[0].verbs.should.eqlInAnyOrder(admin.verbs);
+      const { formList } = projects[0];
+      formList.length.should.equal(2);
+      const form = formList[0];
+      form.should.be.a.ExtendedForm();
+      form.name.should.equal('Simple');
+      form.reviewStates.received.should.equal(0);
+    }));
 
     it('should return projects with datasets', testService(async (service) => {
       const asAlice = await service.login('alice');
@@ -1536,18 +1539,22 @@ describe('api: /projects?forms=true', () => {
             .expect(200))
           .then(() => Users.getByEmail('[email protected]').then((o) => o.get()))
           .then((bob) => asAlice.post(`/v1/projects/${body.id}/assignments/formfill/${bob.actorId}`)))
-        .then(() => service.login('bob', (asBob) => asBob.get('/v1/projects?forms=true')
-          .expect(200)
-          .then(({ body }) => {
-            body.length.should.equal(2);
-            // First project
-            body[0].formList.length.should.equal(2);
-            body[0].verbs.length.should.equal(32);
-            // Second project
-            body[1].formList.length.should.equal(1);
-            body[1].verbs.length.should.be.lessThan(5); // 4 for data collector
-            body[1].formList[0].name.should.equal('Simple 2');
-          }))))));
+        .then(() => service.login('bob', (asBob) =>
+          Promise.all([
+            asBob.get('/v1/projects?forms=true').expect(200),
+            asBob.get('/v1/roles/manager').expect(200),
+            asBob.get('/v1/roles/formfill').expect(200)
+          ])
+            .then(([{ body }, { body: manager }, { body: formfill }]) => {
+              body.length.should.equal(2);
+              // First project
+              body[0].formList.length.should.equal(2);
+              body[0].verbs.should.eqlInAnyOrder(manager.verbs);
+              // Second project
+              body[1].formList.length.should.equal(1);
+              body[1].verbs.should.eqlInAnyOrder(formfill.verbs);
+              body[1].formList[0].name.should.equal('Simple 2');
+            }))))));
 
     it('should set project data from formList even on non-extended projects', testService((service) =>
       service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms/simple/submissions')

test/integration/other/migrations.js

@@ -11,7 +11,6 @@ const populateUsers = require('../fixtures/01-users');
 const populateForms = require('../fixtures/02-forms');
 const { getFormFields } = require('../../../lib/data/schema');
 
-
 const withTestDatabase = withDatabase(config.get('test.database'));
 const migrationsDir = appRoot + '/lib/model/migrations';
 const upToMigration = (toName, inclusive = true) => withTestDatabase(async (migrator) => {
@@ -33,6 +32,25 @@ const up = () => withTestDatabase((migrator) =>
 const down = () => withTestDatabase((migrator) =>
   migrator.migrate.down({ directory: migrationsDir }));
 
+const testMigration = (filename, tests, options = {}) => {
+  const { only = false, skip = false } = options;
+  const f = only
+    ? describe.only.bind(describe)
+    : (skip ? describe.skip.bind(describe) : describe);
+  // eslint-disable-next-line func-names, space-before-function-paren
+  f(`database migrations: ${filename}`, function() {
+    this.timeout(20000);
+
+    beforeEach(() => upToMigration(filename, false));
+
+    tests.call(this);
+  });
+};
+testMigration.only = (filename, tests) =>
+  testMigration(filename, tests, { only: true });
+testMigration.skip = (filename, tests) =>
+  testMigration(filename, tests, { skip: true });
+
 // NOTE/TODO: figure out something else here D:
 // Skipping these migrations because after adding a new description
 // column to projects and forms, it is not possible to migrate part way
@@ -909,3 +927,65 @@ describe.skip('database migration: 20231002-01-add-conflict-details.js', functio
     defs[1].should.have.property('baseVersion').which.is.eql(1);
   }));
 });
+
+testMigration('20240215-01-entity-delete-verb.js', () => {
+  it('should add entity.delete verb to correct roles', testServiceFullTrx(async (service) => {
+    const verbsByRole = async () => {
+      const { body: roles } = await service.get('/v1/roles').expect(200);
+      const bySystem = {};
+      for (const role of roles) bySystem[role.system] = role.verbs;
+      return bySystem;
+    };
+
+    const before = await verbsByRole();
+    before.admin.length.should.equal(48);
+    before.admin.should.not.containEql('entity.delete');
+    before.manager.length.should.equal(33);
+    before.manager.should.not.containEql('entity.delete');
+    before.viewer.length.should.equal(9);
+    before.viewer.should.not.containEql('entity.delete');
+
+    await up();
+
+    const after = await verbsByRole();
+    after.admin.length.should.equal(49);
+    after.admin.should.containEql('entity.delete');
+    after.manager.length.should.equal(34);
+    after.manager.should.containEql('entity.delete');
+    after.viewer.length.should.equal(9);
+    after.viewer.should.not.containEql('entity.delete');
+  }));
+});
+
+testMigration('20240215-02-dedupe-verbs.js', () => {
+  it('should remove duplicate submission.update verb', testServiceFullTrx(async (service) => {
+    const verbsByRole = async () => {
+      const { body: roles } = await service.get('/v1/roles').expect(200);
+      const bySystem = {};
+      for (const role of roles) bySystem[role.system] = role.verbs;
+      return bySystem;
+    };
+
+    const before = await verbsByRole();
+    before.admin.length.should.equal(49);
+    before.admin.filter(verb => verb === 'submission.update').length.should.equal(2);
+    before.manager.length.should.equal(34);
+    before.manager.filter(verb => verb === 'submission.update').length.should.equal(2);
+    before.viewer.length.should.equal(9);
+
+    await up();
+
+    const after = await verbsByRole();
+    after.admin.length.should.equal(48);
+    after.admin.should.eqlInAnyOrder([...new Set(before.admin)]);
+    after.manager.length.should.equal(33);
+    after.manager.should.eqlInAnyOrder([...new Set(before.manager)]);
+    after.viewer.length.should.equal(9);
+  }));
+
+  it('should result in unique verbs for all roles', testServiceFullTrx(async (service) => {
+    await up();
+    const { body: roles } = await service.get('/v1/roles').expect(200);
+    for (const { verbs } of roles) verbs.should.eql([...new Set(verbs)]);
+  }));
+});