Allow encryption for more elements

I've tested #6 extensively and it works well. We are using it on several sites.

Currently encrypted data provided in doPostSave() method. This causes incorrect behavior like: encrypted data sent in webform submission emails etc.

Data should be decrypted for further PostSave processing, as handlers and emails. Solution is to override doPostSave() method in WebformEncryptSubmissionStorage and decrypt data before calling parent method.

@jribeiro & @l0ke thanks for this patch!

Not sure if this is worth mentioning but just reporting anyways.

I applied the patch 10, added an email field with encryption (no default value) to an existing webform (that I had made a prior submission).
Loading the form again results in the following undefined index notice messages

Notice: Undefined index: encrypted_mail in Drupal\webform_encrypt\WebformEncryptSubmissionStorage->getDecryptedData() (line 64 of modules/contrib/webform_encrypt/src/WebformEncryptSubmissionStorage.php).
Drupal\webform_encrypt\WebformEncryptSubmissionStorage->getDecryptedData(Object) (Line: 97)
Drupal\webform_encrypt\WebformEncryptSubmissionStorage->loadData(Array) (Line: 114)
Drupal\webform\WebformSubmissionStorage->loadMultiple(Array) (Line: 212)
Drupal\Core\Entity\EntityStorageBase->load('5') (Line: 283)
Drupal\webform\WebformSubmissionStorage->getTerminusSubmission(Object, NULL, Object, 'DESC') (Line: 239)
Drupal\webform\WebformSubmissionStorage->getLastSubmission(Object, NULL, Object) (Line: 568)
Drupal\webform\WebformSubmissionForm->displayMessages(Array, Object) (Line: 233)

Notice: Undefined index: encrypted_mail in Drupal\webform_encrypt\WebformEncryptSubmissionStorage->getDecryptedData() (line 73 of modules/contrib/webform_encrypt/src/WebformEncryptSubmissionStorage.php).
Drupal\webform_encrypt\WebformEncryptSubmissionStorage->getDecryptedData(Object) (Line: 97)
Drupal\webform_encrypt\WebformEncryptSubmissionStorage->loadData(Array) (Line: 114)
Drupal\webform\WebformSubmissionStorage->loadMultiple(Array) (Line: 212)
Drupal\Core\Entity\EntityStorageBase->load('5') (Line: 283)
Drupal\webform\WebformSubmissionStorage->getTerminusSubmission(Object, NULL, Object, 'DESC') (Line: 239)
Drupal\webform\WebformSubmissionStorage->getLastSubmission(Object, NULL, Object) (Line: 568)
Drupal\webform\WebformSubmissionForm->displayMessages(Array, Object) (Line: 233)
Drupal\webform\WebformSubmissionForm->buildForm(Array, Object)

@optimusprime619 Did you try following these same steps without the patch? This could be a pre-existing issue and not related specifically with the patch in which case a new issue should be created.

@badjava not sure if I can do this without the patch since the ability to encrypt non textfield & textarea fields was available only through the patch along with the WebformEncryptSubmissionStorage.php. However, I haven't been able to reproduce the error message since that one occasion. In case if I am able to do so I will revert with details on the same as a new issue.

I have an another finding.
Solution I've proposed in #10 to decrypt data at the beginning of doPostSave is not good enough.

Thing is when form is submitted by user who doesn't have a permission view encrypted values email have placeholder [Value Encrypted] instead of actual value.

Added parameter that controls permission check in decrypt method and run getDecryptedData with this parameter set to FALSE, so post save actions will always have an actual decrypted values.

Tested and running #16 on several sites - RTBC.

I also agree that this is RTBC as it makes the module actually useful.

With such large changes and given the sensitive nature of this module, we definitely need tests here.

I've just manually tested and this seems like a really good improvement for the module. The only regression I can see currently is that for some reason the submission view pages are no longer being treated as admin pages (i.e they are themed with the site's frontend theme rather than admin theme). EDIT: Strangely enough once I debugged into it fixed itself, not sure what was happening there.

Other than that it fixes a lot of problems with encrypted values being shown to users that have access to decrypt.

I may have a look at writing some tests for this today.

Marking #2884812: Fix fatal error on individual submission page and #2884810: Fix various php notices and warings as duplicates as this solves both.

Here's some pretty low level tests for checking that the correct values are displayed to the end user.

Add a test for the admin form changes too.

The tests added in #23 and #24 looks to cover things nicely. Given the re-architecture has been RTBC'd before I'm marking as such again.

Tests don't seem to be running (these are the first the d8 branch has). I think the module maintainer @th_tushar needs to enable qa on the 'Automated testing' tab of the module?

Back to needs work, we've found a bug where a user with permission to view webform submission, but without the permission to see unencrypted values is able to view the unencrypted value via the submission route.

EDIT: To clarify, I believe this is a caching issue of some sort. Working on steps to reproduce now.
EDIT2: Even stranger, the bug only occurs with the destination param in the url (i.e when clicking the view button from the results page)

I've tracked this down to the $_SERVER['argc'] variable being set on our environments. For some reason this is set to 1 when viewing a submission via the UI but only when the destination query parameter is set.

EDIT: This is caused by the ini setting register_argc_argv = On

Updated patch removing the check for argc.

After chatting about this more, I think we should have the same behaviour on CLI and UI. If you want to load submission values via drush you can use the -u parameter.

Had some more discussion on this. I agree that #29 is the right approach.

#29 changes the cli/drush behaviour, normally drush does not actively check any permissions. So the changes is actually a break from standard drush practice. However given the sensitive nature of the encrypted data then it seems like a sensible change. As @acbramley noted this could be overcome by running drush --user=1 ...

Tests still look like they need to be turned on for the d8 branch.

this is working for me as well

Thanks all. Commited to 8.x-1.x

Added jribeiro as a co-maintainer.