Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify that encryption + entities doesn't break things #668

Closed
ktuite opened this issue Nov 14, 2022 · 4 comments · May be fixed by #1348
Closed

Verify that encryption + entities doesn't break things #668

ktuite opened this issue Nov 14, 2022 · 4 comments · May be fixed by #1348
Assignees
Labels
entities Multiple Encounter workflows

Comments

@ktuite
Copy link
Member

ktuite commented Nov 14, 2022

From Matt:

Maybe this is done already, but we also talked about verifying that using encryption + entities together won't cause Backend to crash (even if it's not an amazing user experience):

  • It should be possible to upload a form that both uses encryption and creates entities: there should be no error.
  • If a submission is created for such a form, that shouldn't result in an entity, since Backend can't read the contents of the submission. However, Backend should be able to handle the submission reasonably well and not crash.

There are different ways that a form can use encryption. If a project as a whole uses managed encryption, then forms within the project will be encrypted. A form can also specify its own encryption even if it's within a project that doesn't use managed encryption.

@ktuite
Copy link
Member Author

ktuite commented Nov 14, 2022

I verified that with the "current" entity worker code, it reports an error creating an entity but it doesn't break. But we're going to change up the entity worker (#669) so we ought to re-verify this again after that change (or as part of that change).

@ktuite ktuite added the entities Multiple Encounter workflows label Nov 14, 2022
@matthew-white
Copy link
Member

@ktuite, now that #669 is merged, would you mind taking another look at this? That could happen after QA begins though.

@ktuite ktuite self-assigned this Nov 21, 2022
@ktuite
Copy link
Member Author

ktuite commented Nov 22, 2022

Just checked this. It doesn't break but it does note a problem creating the entity that says: "There was a problem with entity processing: Dataset empty or missing."

In the future, we could check for encryption and either not process it or leave a different kind of note that says something like "Entities on encrypted submissions are not supported."

Here's a test (not included in the code) that sets up this encryption scenario.

it.only('should do something reasonable when processing an encrypted submission', testService(async (service, container) => {
  // eslint-disable-next-line import/no-dynamic-require
  const { extractPubkey, extractVersion, sendEncrypted } = require(appRoot + '/test/util/crypto-odk');

  const asAlice = await service.login('alice', identity);

  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/key')
    .send({ passphrase: 'supersecret', hint: 'it is a secret' })
    .expect(200);

  await asAlice.get('/v1/projects/1/forms/simpleEntity.xml')
    .expect(200)
    .then(({ text }) => sendEncrypted(asAlice, extractVersion(text), extractPubkey(text)))
    .then((send) => send(testData.instances.simpleEntity.one));

  await asAlice.patch('/v1/projects/1/forms/simpleEntity/submissions/one')
    .send({ reviewState: 'approved' })
    .expect(200);

  await exhaust(container);

  const createEvent = await container.Audits.getLatestByAction('entity.create');
  const errorEvent = await container.Audits.getLatestByAction('entity.create.error');
  // eslint-disable-next-line no-console
  console.log(errorEvent);
  createEvent.isEmpty().should.equal(true);
  errorEvent.isEmpty().should.equal(true); // currently this assertion fails - there IS an error event
}));

@ktuite
Copy link
Member Author

ktuite commented Nov 22, 2022

Putting this in a future issue #698 and closing this one.

@ktuite ktuite closed this as completed Nov 22, 2022
sadiqkhoja added a commit to sadiqkhoja/central-backend that referenced this issue Dec 23, 2024
sadiqkhoja added a commit to sadiqkhoja/central-backend that referenced this issue Jan 3, 2025
sadiqkhoja added a commit to sadiqkhoja/central-backend that referenced this issue Jan 6, 2025
sadiqkhoja added a commit to sadiqkhoja/central-backend that referenced this issue Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
entities Multiple Encounter workflows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants