- Issue created by @cmlara
- πΊπΈUnited States cmlara
Sample Log:
Time: 00:55.922, Memory: 10.00 MB There was 1 failure: 1) Drupal\Tests\tfa\Unit\Plugin\TfaValidation\TfaTotpTest::testTotpCodeValidation Code from previous window accepted Failed asserting that false is true.
https://git.drupalcode.org/project/tfa/-/jobs/567976
$ /var/www/html/vendor/phpunit/phpunit/phpunit --repeat=60000 --bootstrap /var/www/html/web/core/tests/bootstrap.php --configuration /var/www/html/phpunit.xml /var/www/html/web/modules/custom/tfa/tests/src/Unit/Plugin/TfaValidation/TfaTotpTest.php HTML output directory /var/www/html/simpletest/browser_output is not a writable directory. PHPUnit 9.6.15 by Sebastian Bergmann and contributors. Warning: Your XML configuration validates against a deprecated schema. Suggestion: Migrate your XML configuration using "--migrate-configuration"! Testing ........................................................... 59 / 60000 ( 0%) ... ........................................................ 60000 / 60000 (100%) Time: 02:38.191, Memory: 124.01 MB OK (60000 tests, 660000 assertions)
- πΊπΈUnited States cmlara
Another failure.
https://git.drupalcode.org/project/tfa/-/jobs/583531There was 1 failure:
1) Drupal\Tests\tfa\Unit\Plugin\TfaValidation\TfaTotpTest::testTotpCodeValidation
Valid token accepted
Failed asserting that false is true.I'm starting to suspect this perhaps is some weirdness around float to int conversion in the tests, or its a fault in string->int->string conversion.
- πΊπΈUnited States cmlara
Was able to capture this
3300) Drupal\Tests\tfa\Unit\Plugin\TfaValidation\TfaTotpTest::testTotpCodeValidation Code from next window accepted 014656 Failed asserting that false is true.
In 2.x we have already converted this method to only accept a string instead of integer so the flaw will not exist in future versions.
We can't really change the API in 1.x since its committed.I'm thinking inside of validateRequest() for the TOTP and HOTP plugin left padding the submitted int with zeros may be our solution.
Re-titling issue now that a root cause has been found
- Status changed to Needs review
over 1 year ago 1:36am 9 January 2024 - Merge request !72Issue #3412200 by cmlara: validateRequest() does not convert integers to zero padded strings before calling validate() β (Merged) created by cmlara
- Status changed to Fixed
about 1 year ago 6:25am 10 January 2024 -
cmlara β
committed 4b9dc842 on 8.x-1.x
Issue #3412200 by cmlara: validateRequest() does not convert integers to...
-
cmlara β
committed 4b9dc842 on 8.x-1.x
Automatically closed - issue fixed for 2 weeks with no activity.