Log actor property events in audit log

Author: ktuite

lib/model/query/actor-properties.js

@@ -5,13 +5,15 @@ const { construct } = require('../../util/util');
 
 
 // Creates a new actor property name for a project.
-const create = (projectId, name) => ({ one }) =>
+const create = (project, name) => ({ one }) =>
   one(sql`
 INSERT INTO actor_properties ("projectId", "name")
-VALUES (${projectId}, ${name})
+VALUES (${project.id}, ${name})
 RETURNING *`)
     .then(construct(ActorProperty));
 
+create.audit = (project, name) => (log) => log('actor_property.create', project, { name });
+
 // Returns all actor property names for a project.
 const getAllForProject = (projectId) => ({ all }) =>
   all(sql`
@@ -39,15 +41,15 @@ ORDER BY ap.name`)
 // one to delete rows for null-valued properties, one to upsert non-null ones.
 // properties is a plain object mapping property names to values (string or null),
 // as returned by extractActorProperties() in lib/data/actor-properties.js.
-const setValuesForActor = (projectId, actorId, properties) => async ({ run }) => {
+const setValuesForActor = (projectId, fieldKeyOrLink, properties) => async ({ run }) => {
   const entries = Object.entries(properties);
   const toDelete = entries.filter(([, v]) => v === null).map(([name]) => name);
   const toUpsert = entries.filter(([, v]) => v !== null);
 
   if (toDelete.length > 0) {
     await run(sql`
 DELETE FROM actor_property_values
-WHERE "actorId" = ${actorId}
+WHERE "actorId" = ${fieldKeyOrLink.actorId}
   AND "actorPropertyId" IN (
     SELECT id FROM actor_properties
     WHERE "projectId" = ${projectId} AND name = ANY(${sql.array(toDelete, 'text')})
@@ -57,12 +59,18 @@ WHERE "actorId" = ${actorId}
   if (toUpsert.length > 0) {
     await run(sql`
 INSERT INTO actor_property_values ("actorId", "actorPropertyId", "value")
-SELECT ${actorId}, ap.id, v.value
+SELECT ${fieldKeyOrLink.actorId}, ap.id, v.value
 FROM ${sql.unnest(toUpsert, ['text', 'text'])} AS v(name, value)
 JOIN actor_properties ap ON ap."projectId" = ${projectId} AND ap.name = v.name
 ON CONFLICT ("actorId", "actorPropertyId") DO UPDATE SET "value" = EXCLUDED."value"`);
   }
 };
 
+setValuesForActor.audit = (_, fieldKeyOrLink, properties) => (log) => {
+  // Look up `public_link` or `field_key` type from fieldKeyOrLink
+  const action = `${fieldKeyOrLink.constructor.actorType}.property.set`;
+  return log(action, fieldKeyOrLink.actor, { properties });
+};
+
 module.exports = { create, getAllForProject, getAllForProjectWithValues, setValuesForActor };
 

lib/model/query/audits.js

@@ -42,11 +42,11 @@ const actionCondition = (action) => {
   else if (action === 'user')
     return sql`action in ('user.create', 'user.update', 'user.delete', 'user.assignment.create', 'user.assignment.delete', 'user.session.create')`;
   else if (action === 'field_key')
-    return sql`action in ('field_key.create', 'field_key.assignment.create', 'field_key.assignment.delete', 'field_key.session.end', 'field_key.delete')`;
+    return sql`action in ('field_key.create', 'field_key.assignment.create', 'field_key.assignment.delete', 'field_key.session.end', 'field_key.delete', 'field_key.property.set')`;
   else if (action === 'public_link')
-    return sql`action in ('public_link.create', 'public_link.assignment.create', 'public_link.assignment.delete', 'public_link.session.end', 'public_link.delete')`;
+    return sql`action in ('public_link.create', 'public_link.assignment.create', 'public_link.assignment.delete', 'public_link.session.end', 'public_link.delete', 'public_link.property.set')`;
   else if (action === 'project')
-    return sql`action in ('project.create', 'project.update', 'project.delete')`;
+    return sql`action in ('project.create', 'project.update', 'project.delete', 'actor_property.create')`;
   else if (action === 'form')
     return sql`action in ('form.create', 'form.update', 'form.delete', 'form.restore', 'form.purge', 'form.attachment.update', 'form.submission.export', 'form.update.draft.set', 'form.update.draft.delete', 'form.update.draft.replace', 'form.update.publish')`;
   else if (action === 'submission')

lib/resources/actor-properties.js

@@ -14,7 +14,7 @@ module.exports = (service, endpoint) => {
     if (name == null || !validateActorPropertyName(name))
       throw Problem.user.unexpectedValue({ field: 'name', value: name, reason: 'This is not a valid property name.' });
 
-    await ActorProperties.create(project.id, name);
+    await ActorProperties.create(project, name);
     return success;
   }));
 

lib/resources/app-users.js

@@ -30,7 +30,7 @@ module.exports = (service, endpoint) => {
     if (body.properties != null) {
       const knownProperties = await ActorProperties.getAllForProject(project.id);
       const properties = extractActorProperties(body.properties, knownProperties.map((p) => p.name));
-      await ActorProperties.setValuesForActor(project.id, fk.actorId, properties);
+      await ActorProperties.setValuesForActor(project.id, fk, properties);
     }
 
     // Returns the raw FK from create() without new properties
@@ -64,7 +64,7 @@ module.exports = (service, endpoint) => {
     if (body.properties != null) {
       const knownProperties = await ActorProperties.getAllForProject(project.id);
       const properties = extractActorProperties(body.properties, knownProperties.map((p) => p.name));
-      await ActorProperties.setValuesForActor(project.id, fk.actorId, properties);
+      await ActorProperties.setValuesForActor(project.id, fk, properties);
     }
 
     return FieldKeys.getByProjectAndActorId(project.id, params.id, QueryOptions.extended).then(getOrNotFound);

lib/resources/public-links.js

@@ -30,7 +30,7 @@ module.exports = (service, endpoint) => {
     if (body.properties != null) {
       const knownProperties = await ActorProperties.getAllForProject(form.projectId);
       const properties = extractActorProperties(body.properties, knownProperties.map((p) => p.name));
-      await ActorProperties.setValuesForActor(form.projectId, pl.actorId, properties);
+      await ActorProperties.setValuesForActor(form.projectId, pl, properties);
     }
 
     // Returns the raw public link from create() without new properties
@@ -54,7 +54,7 @@ module.exports = (service, endpoint) => {
     if (body.properties != null) {
       const knownProperties = await ActorProperties.getAllForProject(form.projectId);
       const properties = extractActorProperties(body.properties, knownProperties.map((p) => p.name));
-      await ActorProperties.setValuesForActor(form.projectId, pl.actorId, properties);
+      await ActorProperties.setValuesForActor(form.projectId, pl, properties);
     }
 
     return PublicLinks.getByFormAndActorId(form.id, params.id, QueryOptions.extended).then(getOrNotFound);

test/integration/api/actor-properties.js

@@ -16,6 +16,21 @@ describe('api: /projects/:id/actor-properties', () => {
         .expect(200);
     }));
 
+    it('should log the property creation in the audit log', testService(async (service, container) => {
+      const asAlice = await service.login('alice');
+      await asAlice.post('/v1/projects/1/actor-properties')
+        .send({ name: 'region' })
+        .expect(200);
+
+      const project = await container.Projects.getById(1).then((o) => o.get());
+
+      const { body: audits } = await asAlice.get('/v1/audits?action=actor_property.create').expect(200);
+      audits.length.should.equal(1);
+      audits[0].actorId.should.equal(5); // alice
+      audits[0].acteeId.should.equal(project.acteeId);
+      audits[0].details.should.eql({ name: 'region' });
+    }));
+
     it('should reject if name is missing', testService(async (service) => {
       const asAlice = await service.login('alice');
       await asAlice.post('/v1/projects/1/actor-properties')

test/integration/api/app-users.js

@@ -80,6 +80,26 @@ describe('api: /projects/:id/app-users', () => {
           body.properties.should.eql({ region: 'north' });
         });
     }));
+
+    it('should log the property set in the audit log', testService(async (service, container) => {
+      const asAlice = await service.login('alice');
+      await asAlice.post('/v1/projects/1/actor-properties').send({ name: 'region' }).expect(200);
+
+      const { body: appUser } = await asAlice.post('/v1/projects/1/app-users')
+        .send({
+          displayName: 'test user',
+          properties: { region: 'north' }
+        })
+        .expect(200);
+
+      const actor = await container.Actors.getById(appUser.id).then((o) => o.get());
+
+      const { body: audits } = await asAlice.get('/v1/audits?action=field_key.property.set').expect(200);
+      audits.length.should.equal(1);
+      audits[0].actorId.should.equal(5); // alice
+      audits[0].acteeId.should.equal(actor.acteeId);
+      audits[0].details.properties.should.eql({ region: 'north' });
+    }));
   });
 
   describe('GET', () => {
@@ -432,6 +452,27 @@ describe('api: /projects/:id/app-users', () => {
     // The property-setting logic (validation, coercion, bulk upsert/delete) is shared
     // under the hood with public links. The tests above cover it fully; public-links
     // tests only verify the happy-path integration without re-testing every edge case.
+
+    it('should log the property set in the audit log', testService(async (service, container) => {
+      const asAlice = await service.login('alice');
+      await asAlice.post('/v1/projects/1/actor-properties').send({ name: 'region' }).expect(200);
+
+      const { body: appUser } = await asAlice.post('/v1/projects/1/app-users')
+        .send({ displayName: 'test user' })
+        .expect(200);
+
+      await asAlice.patch(`/v1/projects/1/app-users/${appUser.id}`)
+        .send({ properties: { region: 'north' } })
+        .expect(200);
+
+      const actor = await container.Actors.getById(appUser.id).then((o) => o.get());
+
+      const { body: audits } = await asAlice.get('/v1/audits?action=field_key.property.set').expect(200);
+      audits.length.should.equal(1);
+      audits[0].actorId.should.equal(5); // alice
+      audits[0].acteeId.should.equal(actor.acteeId);
+      audits[0].details.properties.should.eql({ region: 'north' });
+    }));
   });
 
   describe('/:id DELETE', () => {

test/integration/api/public-links.js

@@ -79,6 +79,26 @@ describe('api: /projects/:id/forms/:id/public-links', () => {
           body.properties.should.eql({ region: 'north' });
         });
     }));
+
+    it('should log the property set in the audit log', testService(async (service, container) => {
+      const asAlice = await service.login('alice');
+      await asAlice.post('/v1/projects/1/actor-properties').send({ name: 'region' }).expect(200);
+
+      const { body: pl } = await asAlice.post('/v1/projects/1/forms/simple/public-links')
+        .send({
+          displayName: 'test link',
+          properties: { region: 'north' }
+        })
+        .expect(200);
+
+      const actor = await container.Actors.getById(pl.id).then((o) => o.get());
+
+      const { body: audits } = await asAlice.get('/v1/audits?action=public_link.property.set').expect(200);
+      audits.length.should.equal(1);
+      audits[0].actorId.should.equal(5); // alice
+      audits[0].acteeId.should.equal(actor.acteeId);
+      audits[0].details.properties.should.eql({ region: 'north' });
+    }));
   });
 
   describe('GET', () => {
@@ -313,6 +333,27 @@ describe('api: /projects/:id/forms/:id/public-links', () => {
     }));
 
     // Additional behavior tested in app-users test because the machinery of setting properties is the same.
+
+    it('should log the property set in the audit log', testService(async (service, container) => {
+      const asAlice = await service.login('alice');
+      await asAlice.post('/v1/projects/1/actor-properties').send({ name: 'region' }).expect(200);
+
+      const { body: pl } = await asAlice.post('/v1/projects/1/forms/simple/public-links')
+        .send({ displayName: 'test link' })
+        .expect(200);
+
+      await asAlice.patch(`/v1/projects/1/forms/simple/public-links/${pl.id}`)
+        .send({ properties: { region: 'north' } })
+        .expect(200);
+
+      const actor = await container.Actors.getById(pl.id).then((o) => o.get());
+
+      const { body: audits } = await asAlice.get('/v1/audits?action=public_link.property.set').expect(200);
+      audits.length.should.equal(1);
+      audits[0].actorId.should.equal(5); // alice
+      audits[0].acteeId.should.equal(actor.acteeId);
+      audits[0].details.properties.should.eql({ region: 'north' });
+    }));
   });
 
   describe('/:id DELETE', () => {