Gracefully handle EncryptionServiceInterface Exceptions

Can you provide more "steps to repeat" this bug? How did you get Encrypt into a state where it's throwing the exception?

i guess TFA should catch the exception and display a reasonable error message.

Updating title to match.

For sites affected by this, I suggest fixing whatever is making encrypt broken?

If there was an exception and a 500 error there should be an error either displayed to the page or in watchdog or in the apache or php error log. Can you find those errors and copy and paste them here?

This is indeed a legitimate fault that we have.

An example workflow that could cause this in 1.x is TfaTotpValidation::ready()->TfaTotpValidation::getSeed()->tfaBasePlugin::Decrypt($data) which can throw \Drupal\encrypt\Exception\EncryptionMethodCanNotDecryptException or \Drupal\encrypt\Exception\EncryptException. Similar path exists in the HOTP plugin.

Real AES is known to throw this when its key is misconfiguration in some manner, however it could be any sort of issue from any of the various encryption engines, including network faults to remote systems.

We call ready() when a user logs in to check if they have a token configured, we have no provision in our API for raising an exception, and at the same time we also can not just return FALSE from the ready() method when an exception occurs as that could allow an access bypass. Returning true() would also pose issues.

Yes fixing the root fault in Real AES is the correct solution to solve however that doesn't really absolve us of our obligation to handle exceptions.

Realistically anything that calls tfaBasePlugin::Decrypt() needs to expect a failure to occur, the big question is how we present this. WSOD's and Raised Exceptions at least prevent a login from occurring, but its not the most user friendly presentation.

Moving this to 2.x, this could involve API changes.

As a related aside (I skimmed some of this so apologies if you already addressed this) but I noticed the code referenced in the diff in #3 has this:

  public function submitSetupForm(array $form, FormStateInterface $form_state) {
    // Write seed for user.
    try {
      $this->storeSeed($this->seed);
      return TRUE;
    }
    catch (EncryptException $e) {
    }
    return FALSE;
  }

which is found in src/Plugin/Tfa/TfaHotp.php. I was trying to set up TFA on a site where I hadn't provided the key to the Encrypt module and couldn't figure out why TFA wasn't working until I dug into the code and found that catch (EncryptException $e) { is doing nothing with the exception that would have told me what the problem was (I did a dpm() call to the Devel module there to eventually figure it out). I feel like that needs to be addressed as part of this or a related issue as well.