- Status changed to Postponed: needs info
almost 2 years ago 2:30pm 20 January 2023 - Issue was unassigned.
- Status changed to Active
over 1 year ago 8:02am 12 July 2023 - πΊπΈUnited States cmlara
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.
- π¨π¦Canada ambient.impact Toronto
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 thatcatch (EncryptException $e) {
is doing nothing with the exception that would have told me what the problem was (I did adpm()
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.