bug: don't try to csv-parse encrypted client audit logs in workers.

getodk · Oct 15, 2021 · 1601047 · 1601047
1 parent 9e41033
commit 1601047
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 16 deletions.
diff --git a/lib/model/query/submissions.js b/lib/model/query/submissions.js
@@ -131,6 +131,12 @@ where submissions."instanceId"=${instanceId} and current=true
 limit 1`)
   .then(map(construct(Submission.Def)));
 
+const getDefById = (submissionDefId) => ({ maybeOne }) => maybeOne(sql`
+select submission_defs.* from submission_defs
+inner join submissions on submissions.id = submission_defs."submissionId" and submissions."deletedAt" is null
+where submission_defs.id=${submissionDefId}`)
+  .then(map(construct(Submission.Def)));
+
 const _getDef = extender(Submission.Def)(Actor.into('submitter'))((fields, extend, options, formId, draft) => sql`
 select ${fields} from submission_defs
 inner join
@@ -219,7 +225,7 @@ module.exports = {
   createNew, createVersion,
   update, clearDraftSubmissions,
   getByIds, getAllForFormByIds, countByFormId, verifyVersion,
-  getCurrentDefByIds, getAnyDefByFormAndInstanceId, getDefsByFormAndLogicalId, getRootForInstanceId,
+  getDefById, getCurrentDefByIds, getAnyDefByFormAndInstanceId, getDefsByFormAndLogicalId, getRootForInstanceId,
   streamForExport, getForExport
 };
 
diff --git a/lib/worker/submission.attachment.update.js b/lib/worker/submission.attachment.update.js
@@ -9,22 +9,27 @@
 
 const { parseClientAudits } = require('../data/client-audits');
 
-const worker = ({ ClientAudits, Blobs, SubmissionAttachments }, event) =>
-  SubmissionAttachments.getBySubmissionDefIdAndName(event.details.submissionDefId, event.details.name)
-    .then((maybeAttachment) => maybeAttachment
-      .map((attachment) => (((attachment.blobId == null) || (attachment.isClientAudit !== true))
+const worker = ({ ClientAudits, Blobs, Submissions, SubmissionAttachments }, event) =>
+  Promise.all([
+    Submissions.getDefById(event.details.submissionDefId),
+    SubmissionAttachments.getBySubmissionDefIdAndName(event.details.submissionDefId, event.details.name)
+  ])
+    .then(([ maybeSubmission, maybeAttachment ]) =>
+      (maybeSubmission.map((s) => s.localKey != null).orElse(false)
         ? null
-        : ClientAudits.existsForBlob(attachment.blobId)
-          .then((exists) => ((exists === true)
-            ? null // do nothing
-            : Blobs.getById(attachment.blobId)
-              .then((maybeBlob) => maybeBlob.get()) // blobs are immutable
-              .then((blob) => parseClientAudits(blob.content))
-              .then((audits) => {
-                const withBlobIds = audits.map((audit) => audit.with({ blobId: attachment.blobId }));
-                return ClientAudits.createMany(withBlobIds);
-              })))))
-      .orNull());
+        : maybeAttachment.map((attachment) => (((attachment.blobId == null) || (attachment.isClientAudit !== true))
+          ? null
+          : ClientAudits.existsForBlob(attachment.blobId)
+            .then((exists) => ((exists === true)
+              ? null // do nothing
+              : Blobs.getById(attachment.blobId)
+                .then((maybeBlob) => maybeBlob.get()) // blobs are immutable
+                .then((blob) => parseClientAudits(blob.content))
+                .then((audits) => {
+                  const withBlobIds = audits.map((audit) => audit.with({ blobId: attachment.blobId }));
+                  return ClientAudits.createMany(withBlobIds);
+                })))))
+          .orNull()));
 
 module.exports = worker;
 
diff --git a/test/integration/other/encryption.js b/test/integration/other/encryption.js
@@ -8,6 +8,7 @@ const testData = require(appRoot + '/test/data/xml');
 const { zipStreamToFiles } = require(appRoot + '/test/util/zip');
 const { Form, Key, Submission } = require(appRoot + '/lib/model/frames');
 const { mapSequential } = require(appRoot + '/test/util/util');
+const { exhaust } = require(appRoot + '/lib/worker/worker');
 
 describe('managed encryption', () => {
   describe('lock management', () => {
@@ -357,6 +358,27 @@ describe('managed encryption', () => {
               done();
             })))))));
 
+    it('should decrypt client audit log attachments', testService((service, container) =>
+      service.login('alice', (asAlice) =>
+        asAlice.post('/v1/projects/1/key')
+          .send({ passphrase: 'supersecret', hint: 'it is a secret' })
+          .expect(200)
+          .then(() => asAlice.post('/v1/projects/1/forms?publish=true')
+            .set('Content-Type', 'application/xml')
+            .send(testData.forms.clientAudits)
+            .expect(200))
+          .then(() => asAlice.get('/v1/projects/1/forms/audits.xml')
+            .expect(200)
+            .then(({ text }) => sendEncrypted(asAlice, extractVersion(text), extractPubkey(text)))
+            .then((send) => send(testData.instances.clientAudits.one, { 'audit.csv.enc': readFileSync(appRoot + '/test/data/audit.csv') })
+              .then(() => send(testData.instances.clientAudits.two, { 'audit.csv.enc': readFileSync(appRoot + '/test/data/audit2.csv') }))))
+          .then(() => exhaust(container))
+          .then(() => container.oneFirst(sql`select count(*) from client_audits`)
+            .then((count) => { count.should.equal(0); }))
+          .then(() => container.oneFirst(sql`select count(*) from audits
+            where action='submission.attachment.update' and processed is not null and failures = 0`)
+            .then((count) => { count.should.equal(4); })))));
+
     it('should decrypt client audit log attachments', testService((service, container) =>
       service.login('alice', (asAlice) =>
         asAlice.post('/v1/projects/1/key')