From af6b886bc575d27d20fa63c1dbb19a477ecbdb62 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 13:03:44 +0545 Subject: [PATCH 01/11] chore: PHP 7.2 with GitHub workflows --- .github/workflows/ci.yml | 62 ++++++++++++++++++++++++++++++++++++++++ .gitignore | 3 +- .php-cs-fixer.dist.php | 29 +++++++++++++++++++ composer.json | 5 ++-- phpstan.neon | 5 ++++ 5 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .php-cs-fixer.dist.php create mode 100644 phpstan.neon diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..6500a5c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,62 @@ +name: CI + +on: [push, pull_request] + +jobs: + php-cs-fixer: + name: PHP-CS-Fixer + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: PHP-CS-Fixer + uses: docker://oskarstark/php-cs-fixer-ga + with: + args: --diff --dry-run + + phpstan: + name: 'PHPStan' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Composer + uses: php-actions/composer@v5 + with: + php_version: 8.0 + composer_version: 2 + - name: PHPStan + uses: php-actions/phpstan@v2 + with: + php_version: 8.0 + + phpunit: + name: 'PHPUnit' + runs-on: ubuntu-latest + strategy: + # Making sure all versions get tested even if one fails + fail-fast: false + matrix: + phpunit-version: + - '9.5.0' + php-version: + - '7.3' + - '7.4' + - '8.0' + include: + - phpunit-version: '8.5.13' + php-version: '7.2' + + steps: + - uses: actions/checkout@v2 + - name: Composer + uses: php-actions/composer@v5 + with: + php_version: ${{ matrix.php-version }} + + - name: PHPUnit Tests + uses: php-actions/phpunit@v2 + with: + version: ${{ matrix.phpunit-version }} + php_version: ${{ matrix.php-version }} + #bootstrap: vendor/autoload.php + #configuration: test/phpunit.xml + #args: --coverage-text diff --git a/.gitignore b/.gitignore index 0f64e29..d265536 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,5 @@ /.settings /composer.lock /vendor/ -/.idea \ No newline at end of file +/.idea +.php-cs-fixer.cache diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php new file mode 100644 index 0000000..51c0584 --- /dev/null +++ b/.php-cs-fixer.dist.php @@ -0,0 +1,29 @@ +setRiskyAllowed(true) + ->setRules([ + '@PSR2' => true, + '@PSR1' => true, + 'concat_space' => true, + 'declare_strict_types' => true, + 'logical_operators' => true, + 'native_function_casing' => true, + 'native_function_invocation' => true, + 'native_function_type_declaration_casing' => true, + 'no_alternative_syntax' => true, + 'no_leading_namespace_whitespace' => true, + 'no_superfluous_phpdoc_tags' => true, + 'no_trailing_comma_in_singleline_array' => true, + 'no_unused_imports' => true, + 'no_whitespace_before_comma_in_array' => true, + 'yoda_style' => true, + ]) + ->setFinder(PhpCsFixer\Finder::create() + ->exclude('vendor') + ->in(__DIR__) + ) +; diff --git a/composer.json b/composer.json index d49f1a9..01ad940 100644 --- a/composer.json +++ b/composer.json @@ -14,13 +14,14 @@ } ], "require": { - "php": ">=5.6.0", + "php": "^7.2 || ^8.0", "paragonie/constant_time_encoding": "^1|^2", "paragonie/random_compat": ">=1", "symfony/polyfill-php56": "^1" }, "require-dev": { - "phpunit/phpunit": "^5.7.11 || ^6.0.5" + "friendsofphp/php-cs-fixer": "^3.51", + "phpunit/phpunit": "^8 || ^9.6" }, "autoload": { "psr-4": { diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..59ae674 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,5 @@ +parameters: + level: 8 + paths: + - src + - tests From fa2d4d58103f8a8dae1674f0df59a8bace8872fb Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 13:05:05 +0545 Subject: [PATCH 02/11] chore: apply cs-fixer 3.51 suggested changes --- example/index.php | 50 ++-- src/GoogleAuthenticator.php | 62 ++--- src/Otp.php | 45 ++- src/OtpInterface.php | 12 +- tests/GoogleAuthenticatorTest.php | 182 ++++++------ tests/OtpTest.php | 444 +++++++++++++++--------------- tests/bootstrap.php | 6 +- 7 files changed, 399 insertions(+), 402 deletions(-) diff --git a/example/index.php b/example/index.php index efdc12a..af0f930 100644 --- a/example/index.php +++ b/example/index.php @@ -2,7 +2,7 @@ session_start(); // using it as storage temporary -require_once __DIR__ . '/../vendor/autoload.php'; +require_once __DIR__.'/../vendor/autoload.php'; use Otp\Otp; use Otp\GoogleAuthenticator; @@ -13,12 +13,12 @@ $secret = 0; if (isset($_SESSION['otpsecret'])) { - $secret = $_SESSION['otpsecret']; + $secret = $_SESSION['otpsecret']; } -if (strlen($secret) != 16) { - $secret = GoogleAuthenticator::generateRandom(); - $_SESSION['otpsecret'] = $secret; +if (16 != \strlen($secret)) { + $secret = GoogleAuthenticator::generateRandom(); + $_SESSION['otpsecret'] = $secret; } // The secret is now an easy stored Base32 string. @@ -72,26 +72,26 @@ checkTotp(Encoding::base32DecodeUpper($secret), $key)) { - echo 'Key correct!'; - // Add here something that makes note of this key and will not allow - // the use of it, for this user for the next 2 minutes. This way you - // prevent a replay attack. Otherwise your OTP is missing one of the - // key features it can bring in security to your application! - } else { - echo 'Wrong key!'; - } - - } else { - echo 'Key not the correct size'; - } + // Sanitizing, this should take care of it + $key = preg_replace('/[^0-9]/', '', $_POST['otpkey']); + + // Standard is 6 for keys, but can be changed with setDigits on $otp + if (6 == \strlen($key)) { + // Remember that the secret is a base32 string that needs decoding + // to use it here! + if ($otp->checkTotp(Encoding::base32DecodeUpper($secret), $key)) { + echo 'Key correct!'; + // Add here something that makes note of this key and will not allow + // the use of it, for this user for the next 2 minutes. This way you + // prevent a replay attack. Otherwise your OTP is missing one of the + // key features it can bring in security to your application! + } else { + echo 'Wrong key!'; + } + + } else { + echo 'Key not the correct size'; + } } ?> diff --git a/src/GoogleAuthenticator.php b/src/GoogleAuthenticator.php index 8b909a9..17f0664 100644 --- a/src/GoogleAuthenticator.php +++ b/src/GoogleAuthenticator.php @@ -1,9 +1,7 @@ - 2) { - throw new \InvalidArgumentException('Account name contains illegal colon characters'); + throw new \InvalidArgumentException('Account name contains illegal colon characters'); } // Secret needs to be here - if (strlen($secret) < 1) { + if (\strlen($secret) < 1) { throw new \InvalidArgumentException('No secret present'); } // check for counter on hotp - if ($type == 'hotp' && is_null($counter)) { + if ('hotp' == $type && \is_null($counter)) { throw new \InvalidArgumentException('Counter required for hotp'); } // This is the base, these are at least required - $otpauth = 'otpauth://' . $type . '/' . rawurlencode($label) . '?secret=' . rawurlencode($secret); + $otpauth = 'otpauth://'.$type.'/'.rawurlencode($label).'?secret='.rawurlencode($secret); - if ($type == 'hotp' && !is_null($counter)) { - $otpauth .= '&counter=' . intval($counter); + if ('hotp' == $type && !\is_null($counter)) { + $otpauth .= '&counter='.\intval($counter); } // Now check the options array // algorithm (currently ignored by Authenticator) // Defaults to SHA1 - if (array_key_exists('algorithm', $options)) { - $otpauth .= '&algorithm=' . rawurlencode($options['algorithm']); + if (\array_key_exists('algorithm', $options)) { + $otpauth .= '&algorithm='.rawurlencode($options['algorithm']); } // digits (currently ignored by Authenticator) // Defaults to 6 - if (array_key_exists('digits', $options) && intval($options['digits']) !== 6 && intval($options['digits']) !== 8) { - throw new \InvalidArgumentException('Digits can only have the values 6 or 8, ' . $options['digits'] . ' given'); - } elseif (array_key_exists('digits', $options)) { - $otpauth .= '&digits=' . intval($options['digits']); + if (\array_key_exists('digits', $options) && 6 !== \intval($options['digits']) && 8 !== \intval($options['digits'])) { + throw new \InvalidArgumentException('Digits can only have the values 6 or 8, '.$options['digits'].' given'); + } elseif (\array_key_exists('digits', $options)) { + $otpauth .= '&digits='.\intval($options['digits']); } // period, only for totp (currently ignored by Authenticator) // Defaults to 30 - if ($type == 'totp' && array_key_exists('period', $options)) { - $otpauth .= '&period=' . rawurlencode($options['period']); + if ('totp' == $type && \array_key_exists('period', $options)) { + $otpauth .= '&period='.rawurlencode($options['period']); } // issuer // Defaults to none - if (array_key_exists('issuer', $options)) { - $otpauth .= '&issuer=' . rawurlencode($options['issuer']); + if (\array_key_exists('issuer', $options)) { + $otpauth .= '&issuer='.rawurlencode($options['issuer']); } // image (to accept images for freeotp) - if (array_key_exists('image', $options)) { - $otpauth .= '&image=' . $options['image']; + if (\array_key_exists('image', $options)) { + $otpauth .= '&image='.$options['image']; } return $otpauth; @@ -128,20 +126,20 @@ public static function getQrCodeUrl($type, $label, $secret, $counter = null, $op // Width and height can be overwritten $width = self::$width; - if (array_key_exists('width', $options) && is_numeric($options['width'])) { + if (\array_key_exists('width', $options) && is_numeric($options['width'])) { $width = $options['width']; } $height = self::$height; - if (array_key_exists('height', $options) && is_numeric($options['height'])) { + if (\array_key_exists('height', $options) && is_numeric($options['height'])) { $height = $options['height']; } $otpauth = self::getKeyUri($type, $label, $secret, $counter, $options); - $url = 'https://chart.googleapis.com/chart?chs=' . $width . 'x' - . $height . '&cht=qr&chld=M|0&chl=' . urlencode($otpauth); + $url = 'https://chart.googleapis.com/chart?chs='.$width.'x' + .$height.'&cht=qr&chld=M|0&chl='.urlencode($otpauth); return $url; } @@ -158,7 +156,7 @@ public static function getQrCodeUrl($type, $label, $secret, $counter = null, $op */ public static function generateRandom($length = 16) { - $keys = array_merge(range('A','Z'), range(2,7)); // No padding char + $keys = array_merge(range('A', 'Z'), range(2, 7)); // No padding char $string = ''; @@ -181,8 +179,8 @@ public static function generateRandom($length = 16) */ public static function generateRecoveryCodes($count = 1, $length = 9) { - $count = intval($count); - $length = intval($length); + $count = \intval($count); + $length = \intval($length); $codes = []; do { @@ -193,10 +191,10 @@ public static function generateRecoveryCodes($count = 1, $length = 9) } // To make sure no duplicates get in - if (!in_array($code, $codes)) { + if (!\in_array($code, $codes)) { $codes[] = $code; } - } while (count($codes) < $count); + } while (\count($codes) < $count); return $codes; } diff --git a/src/Otp.php b/src/Otp.php index 2373186..6f369f7 100644 --- a/src/Otp.php +++ b/src/Otp.php @@ -1,4 +1,4 @@ -= 5.6.3! * * Can be easy used with Google Authenticator @@ -74,10 +74,10 @@ public function hotp($secret, $counter) } $hash = hash_hmac( - $this->algorithm, - $this->getBinaryCounter($counter), - $secret, - true + $this->algorithm, + $this->getBinaryCounter($counter), + $secret, + true ); return str_pad($this->truncate($hash), $this->digits, '0', STR_PAD_LEFT); @@ -88,7 +88,7 @@ public function hotp($secret, $counter) */ public function totp($secret, $timecounter = null) { - if (is_null($timecounter)) { + if (\is_null($timecounter)) { $timecounter = $this->getTimecounter(); } @@ -113,12 +113,12 @@ public function checkHotpResync($secret, $counter, $key, $counterwindow = 2) throw new \InvalidArgumentException('Invalid counter supplied'); } - if(!is_numeric($counterwindow) || $counterwindow < 0){ + if(!is_numeric($counterwindow) || $counterwindow < 0) { throw new \InvalidArgumentException('Invalid counterwindow supplied'); } for($c = 0; $c <= $counterwindow; $c = $c + 1) { - if(hash_equals($this->hotp($secret, $counter + $c), $key)){ + if(hash_equals($this->hotp($secret, $counter + $c), $key)) { return $counter + $c; } } @@ -144,7 +144,7 @@ public function checkTotp($secret, $key, $timedrift = 1) // We first try the current, as it is the most likely to work if (hash_equals($this->totp($secret, $timecounter), $key)) { return true; - } elseif ($timedrift == 0) { + } elseif (0 == $timedrift) { // When timedrift is 0, this is the end of the checks return false; } @@ -176,8 +176,8 @@ public function checkTotp($secret, $key, $timedrift = 1) */ public function setAlgorithm($algorithm) { - if (!in_array($algorithm, $this->allowedAlgorithms)) { - throw new \InvalidArgumentException('Not an allowed algorithm: ' . $algorithm); + if (!\in_array($algorithm, $this->allowedAlgorithms)) { + throw new \InvalidArgumentException('Not an allowed algorithm: '.$algorithm); } $this->algorithm = $algorithm; @@ -204,7 +204,7 @@ public function getAlgorithm() */ public function setPeriod($period) { - if (!is_int($period)) { + if (!\is_int($period)) { throw new \InvalidArgumentException('Period must be an integer'); } @@ -232,7 +232,7 @@ public function getPeriod() */ public function setDigits($digits) { - if (!in_array($digits, array(6, 8))) { + if (!\in_array($digits, array(6, 8))) { throw new \InvalidArgumentException('Digits must be 6 or 8'); } @@ -260,7 +260,7 @@ public function getDigits() */ public function setTotpOffset($offset) { - if (!is_int($offset)) { + if (!\is_int($offset)) { throw new \InvalidArgumentException('Offset must be an integer'); } @@ -295,7 +295,7 @@ private function getBinaryCounter($counter) } // keep old behavior for 32 bit PHP or PHP < 5.6.3 - return pack('N*', 0) . pack('N*', $counter); + return pack('N*', 0).pack('N*', $counter); } /** @@ -321,14 +321,13 @@ private function getTimecounter() */ private function truncate($hash) { - $offset = ord($hash[strlen($hash)-1]) & 0xf; + $offset = \ord($hash[\strlen($hash)-1]) & 0xf; return ( - ((ord($hash[$offset+0]) & 0x7f) << 24 ) | - ((ord($hash[$offset+1]) & 0xff) << 16 ) | - ((ord($hash[$offset+2]) & 0xff) << 8 ) | - (ord($hash[$offset+3]) & 0xff) - ) % pow(10, $this->digits); + ((\ord($hash[$offset+0]) & 0x7f) << 24) | + ((\ord($hash[$offset+1]) & 0xff) << 16) | + ((\ord($hash[$offset+2]) & 0xff) << 8) | + (\ord($hash[$offset+3]) & 0xff) + ) % pow(10, $this->digits); } } - diff --git a/src/OtpInterface.php b/src/OtpInterface.php index 0db2bff..ea294ba 100644 --- a/src/OtpInterface.php +++ b/src/OtpInterface.php @@ -1,4 +1,4 @@ -assertEquals( - 'https://chart.googleapis.com/chart?chs=200x200&cht=qr&chld=M|0&chl=otpauth%3A%2F%2Ftotp%2Fuser%2540host.com%3Fsecret%3DMEP3EYVA6XNFNVNM', - GoogleAuthenticator::getQrCodeUrl('totp', 'user@host.com', $secret) - ); - - // hotp (include a counter) - $this->assertEquals( - 'https://chart.googleapis.com/chart?chs=200x200&cht=qr&chld=M|0&chl=otpauth%3A%2F%2Fhotp%2Fuser%2540host.com%3Fsecret%3DMEP3EYVA6XNFNVNM%26counter%3D1234', - GoogleAuthenticator::getQrCodeUrl('hotp', 'user@host.com', $secret, 1234) - ); - - // totp, this time with a parameter for changing the size of the QR - $this->assertEquals( - 'https://chart.googleapis.com/chart?chs=300x300&cht=qr&chld=M|0&chl=otpauth%3A%2F%2Ftotp%2Fuser%2540host.com%3Fsecret%3DMEP3EYVA6XNFNVNM', - GoogleAuthenticator::getQrCodeUrl('totp', 'user@host.com', $secret, null, array('height' => 300, 'width' => 300)) - ); - - } + /** + * Tests getQrCodeUrl + */ + public function testGetQrCodeUrl() + { + $secret = 'MEP3EYVA6XNFNVNM'; // testing secret + + // Standard totp case + $this->assertEquals( + 'https://chart.googleapis.com/chart?chs=200x200&cht=qr&chld=M|0&chl=otpauth%3A%2F%2Ftotp%2Fuser%2540host.com%3Fsecret%3DMEP3EYVA6XNFNVNM', + GoogleAuthenticator::getQrCodeUrl('totp', 'user@host.com', $secret) + ); + + // hotp (include a counter) + $this->assertEquals( + 'https://chart.googleapis.com/chart?chs=200x200&cht=qr&chld=M|0&chl=otpauth%3A%2F%2Fhotp%2Fuser%2540host.com%3Fsecret%3DMEP3EYVA6XNFNVNM%26counter%3D1234', + GoogleAuthenticator::getQrCodeUrl('hotp', 'user@host.com', $secret, 1234) + ); + + // totp, this time with a parameter for changing the size of the QR + $this->assertEquals( + 'https://chart.googleapis.com/chart?chs=300x300&cht=qr&chld=M|0&chl=otpauth%3A%2F%2Ftotp%2Fuser%2540host.com%3Fsecret%3DMEP3EYVA6XNFNVNM', + GoogleAuthenticator::getQrCodeUrl('totp', 'user@host.com', $secret, null, array('height' => 300, 'width' => 300)) + ); + + } - /** - * Tests getKeyUri - */ - public function testGetKeyUri() - { - $secret = 'MEP3EYVA6XNFNVNM'; // testing secret - - // Standard totp case - $this->assertEquals( - 'otpauth://totp/user%40host.com?secret=MEP3EYVA6XNFNVNM', - GoogleAuthenticator::getKeyUri('totp', 'user@host.com', $secret) - ); + /** + * Tests getKeyUri + */ + public function testGetKeyUri() + { + $secret = 'MEP3EYVA6XNFNVNM'; // testing secret + + // Standard totp case + $this->assertEquals( + 'otpauth://totp/user%40host.com?secret=MEP3EYVA6XNFNVNM', + GoogleAuthenticator::getKeyUri('totp', 'user@host.com', $secret) + ); - // hotp (include a counter) - $this->assertEquals( - 'otpauth://hotp/user%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234', - GoogleAuthenticator::getKeyUri('hotp', 'user@host.com', $secret, 1234) - ); + // hotp (include a counter) + $this->assertEquals( + 'otpauth://hotp/user%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234', + GoogleAuthenticator::getKeyUri('hotp', 'user@host.com', $secret, 1234) + ); - // totp/hotp with an issuer in the label - $this->assertEquals( - 'otpauth://hotp/issuer%3Auser%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234', - GoogleAuthenticator::getKeyUri('hotp', 'issuer:user@host.com', $secret, 1234) - ); + // totp/hotp with an issuer in the label + $this->assertEquals( + 'otpauth://hotp/issuer%3Auser%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234', + GoogleAuthenticator::getKeyUri('hotp', 'issuer:user@host.com', $secret, 1234) + ); - // totp/hotp with an issuer and spaces in the label - $this->assertEquals( - 'otpauth://hotp/an%20issuer%3A%20user%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234', - GoogleAuthenticator::getKeyUri('hotp', 'an issuer: user@host.com', $secret, 1234) - ); + // totp/hotp with an issuer and spaces in the label + $this->assertEquals( + 'otpauth://hotp/an%20issuer%3A%20user%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234', + GoogleAuthenticator::getKeyUri('hotp', 'an issuer: user@host.com', $secret, 1234) + ); - // totp/hotp with an issuer as option - $this->assertEquals( - 'otpauth://hotp/an%20issuer%3Auser%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234&issuer=an%20issuer', - GoogleAuthenticator::getKeyUri('hotp', 'an issuer:user@host.com', $secret, 1234, array('issuer' => 'an issuer')) - ); - } - - /** - * Tests generateRandom - */ - public function testGenerateRandom() - { - // contains numbers 2-7 and letters A-Z in large letters, 16 chars long - $this->assertRegExp('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); - - // Can be told to make a longer secret - $this->assertRegExp('/[2-7A-Z]{18}/', GoogleAuthenticator::generateRandom(18)); - } + // totp/hotp with an issuer as option + $this->assertEquals( + 'otpauth://hotp/an%20issuer%3Auser%40host.com?secret=MEP3EYVA6XNFNVNM&counter=1234&issuer=an%20issuer', + GoogleAuthenticator::getKeyUri('hotp', 'an issuer:user@host.com', $secret, 1234, array('issuer' => 'an issuer')) + ); + } + + /** + * Tests generateRandom + */ + public function testGenerateRandom() + { + // contains numbers 2-7 and letters A-Z in large letters, 16 chars long + $this->assertRegExp('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); + + // Can be told to make a longer secret + $this->assertRegExp('/[2-7A-Z]{18}/', GoogleAuthenticator::generateRandom(18)); + } - /** - * Test generateRecoveryCodes - */ - public function testGenerateRecoveryCodes() - { - // Default settings - $codes = GoogleAuthenticator::generateRecoveryCodes(); + /** + * Test generateRecoveryCodes + */ + public function testGenerateRecoveryCodes() + { + // Default settings + $codes = GoogleAuthenticator::generateRecoveryCodes(); - $this->assertCount(1, $codes); - $this->assertRegExp('/[0-9]{9}/', $codes[0]); + $this->assertCount(1, $codes); + $this->assertRegExp('/[0-9]{9}/', $codes[0]); - // More codes, longer - $codes = GoogleAuthenticator::generateRecoveryCodes(4, 20); - $this->assertCount(4, $codes); - $this->assertRegExp('/[0-9]{9}/', $codes[0]); - $this->assertRegExp('/[0-9]{9}/', $codes[1]); - $this->assertRegExp('/[0-9]{9}/', $codes[2]); - $this->assertRegExp('/[0-9]{9}/', $codes[3]); + // More codes, longer + $codes = GoogleAuthenticator::generateRecoveryCodes(4, 20); + $this->assertCount(4, $codes); + $this->assertRegExp('/[0-9]{9}/', $codes[0]); + $this->assertRegExp('/[0-9]{9}/', $codes[1]); + $this->assertRegExp('/[0-9]{9}/', $codes[2]); + $this->assertRegExp('/[0-9]{9}/', $codes[3]); - // To check for uniqueness - $this->assertSame($codes, array_unique($codes)); - } + // To check for uniqueness + $this->assertSame($codes, array_unique($codes)); + } } diff --git a/tests/OtpTest.php b/tests/OtpTest.php index 599febb..58e25d2 100644 --- a/tests/OtpTest.php +++ b/tests/OtpTest.php @@ -1,4 +1,4 @@ -Otp = new Otp(); - - } - - /** - * Cleans up the environment after running a test. - */ - protected function tearDown() - { - $this->Otp = null; - - parent::tearDown(); - } - - /** - * Invalid counter values for tests - */ - public function hotpTestValues() - { - return [ - ['755224', 0], ['287082', 1], ['359152', 2], - ['969429', 3], ['338314', 4], ['254676', 5], - ['287922', 6], ['162583', 7], ['399871', 8], - ['520489', 9] - ]; - } - - /** - * Invalid counter values for tests - */ - public function totpTestValues() - { - // https://www.rfc-editor.org/errata_search.php?rfc=6238 - $secretSha1 = '12345678901234567890'; - $secretSha256 = '12345678901234567890123456789012'; - $secretSha512 = '1234567890123456789012345678901234567890123456789012345678901234'; - - return [ - ['sha1', $secretSha1, '94287082', 59], - ['sha1', $secretSha1, '07081804', 1111111109], - ['sha1', $secretSha1, '14050471', 1111111111], - ['sha1', $secretSha1, '89005924', 1234567890], - ['sha1', $secretSha1, '69279037', 2000000000], - ['sha1', $secretSha1, '65353130', 20000000000], - ['sha256', $secretSha256, '46119246', 59], - ['sha256', $secretSha256, '68084774', 1111111109], - ['sha256', $secretSha256, '67062674', 1111111111], - ['sha256', $secretSha256, '91819424', 1234567890], - ['sha256', $secretSha256, '90698825', 2000000000], - ['sha256', $secretSha256, '77737706', 20000000000], - ['sha512', $secretSha512, '90693936', 59], - ['sha512', $secretSha512, '25091201', 1111111109], - ['sha512', $secretSha512, '99943326', 1111111111], - ['sha512', $secretSha512, '93441116', 1234567890], - ['sha512', $secretSha512, '38618901', 2000000000], - ['sha512', $secretSha512, '47863826', 20000000000], - ]; - } - - /** - * Invalid counter values for tests - */ - public function invalidCounterValues() - { - return [ - ['a'], [-1] - ]; - } - - /** - * Invalid counter values for tests - */ - public function hotpResyncDefaultTestValues() - { - return [ - ['755224', 0], ['287082', 1], ['359152', 2] - ]; - } - - /** - * Invalid counter values for tests - */ - public function hotpResyncWindowTestValues() - { - return [ - ['969429', 0, 3], ['338314', 0, 4], - ['287922', 3, 3], ['162583', 3, 4] - ]; - } - - /** - * Invalid counter values for tests - */ - public function hotpResyncFailureTestValues() - { - return [ - ['287922', 7], ['162583', 8], ['399871', 9] - ]; - } - - /** - * Tests Otp->hotp() - * - * Using test vectors from RFC - * https://tools.ietf.org/html/rfc4226 - * - * @dataProvider hotpTestValues - */ - public function testHotpRfc($key, $counter) - { - $secret = $this->secret; - - $this->assertEquals($key, $this->Otp->hotp($secret, $counter)); - } - - /** - * Tests TOTP general construction - * - * Still uses the hotp function, but since totp is a bit more special, has - * its own tests - * Using test vectors from RFC - * https://tools.ietf.org/html/rfc6238 - * - * @dataProvider totpTestValues - */ - public function testTotpRfc($algo, $secret, $key, $time) - { - // Test vectors are in 8 digits - $this->Otp->setDigits(8); - - // The time presented in the test vector has to be first divided through 30 - // to count as the key - - $this->Otp->setAlgorithm($algo); - $this->assertEquals($key, $this->Otp->hotp($secret, floor($time/30)), "$algo with $time"); - } - - /** - * @dataProvider invalidCounterValues - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Invalid counter supplied - */ - public function testHotpInvalidCounter($counter) - { - $this->Otp->hotp($this->secret, $counter); - } - - /** - * Tests Otp->checkHotpResync() with default counter window - * - * @dataProvider hotpResyncDefaultTestValues - */ - public function testHotpResyncDefault($key, $counter) - { - $secret = $this->secret; - - // test with default counter window - $this->assertSame($counter, $this->Otp->checkHotpResync($secret, $counter, $key)); - } - - - /** - * Tests Otp->checkHotpResync() with a provided counter window - * - * @dataProvider hotpResyncWindowTestValues - */ - public function testHotpResyncWindow($key, $counter, $counterwindow) - { - $secret = $this->secret; - - // test with provided counter window - $this->assertSame(($counter + $counterwindow), $this->Otp->checkHotpResync($secret, $counter, $key, $counterwindow)); - } - - /** - * Tests Otp->checkHotpResync() with mismatching key and counter - * - * @dataProvider hotpResyncFailureTestValues - */ - public function testHotpResyncFailures($key, $counter) - { - $secret = $this->secret; - - // test failures - $this->assertFalse($this->Otp->checkHotpResync($secret, $counter, $key)); - } - - /** - * @dataProvider invalidCounterValues - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Invalid counter supplied - */ - public function testHotpResyncInvalidCounter($counter) - { - $this->Otp->checkHotpResync($this->secret, $counter, '755224'); - } - - /** - * @dataProvider invalidCounterValues - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Invalid counterwindow supplied - */ - public function testHotpResyncInvalidCounterWindow($counterwindow) - { - $this->Otp->checkHotpResync($this->secret, 0, '755224', $counterwindow); - } + /** + * + * @var Otp + */ + private $Otp; + + private $secret = "12345678901234567890"; + + /** + * Prepares the environment before running a test. + */ + protected function setUp() + { + parent::setUp(); + + $this->Otp = new Otp(); + + } + + /** + * Cleans up the environment after running a test. + */ + protected function tearDown() + { + $this->Otp = null; + + parent::tearDown(); + } + + /** + * Invalid counter values for tests + */ + public function hotpTestValues() + { + return [ + ['755224', 0], ['287082', 1], ['359152', 2], + ['969429', 3], ['338314', 4], ['254676', 5], + ['287922', 6], ['162583', 7], ['399871', 8], + ['520489', 9] + ]; + } + + /** + * Invalid counter values for tests + */ + public function totpTestValues() + { + // https://www.rfc-editor.org/errata_search.php?rfc=6238 + $secretSha1 = '12345678901234567890'; + $secretSha256 = '12345678901234567890123456789012'; + $secretSha512 = '1234567890123456789012345678901234567890123456789012345678901234'; + + return [ + ['sha1', $secretSha1, '94287082', 59], + ['sha1', $secretSha1, '07081804', 1111111109], + ['sha1', $secretSha1, '14050471', 1111111111], + ['sha1', $secretSha1, '89005924', 1234567890], + ['sha1', $secretSha1, '69279037', 2000000000], + ['sha1', $secretSha1, '65353130', 20000000000], + ['sha256', $secretSha256, '46119246', 59], + ['sha256', $secretSha256, '68084774', 1111111109], + ['sha256', $secretSha256, '67062674', 1111111111], + ['sha256', $secretSha256, '91819424', 1234567890], + ['sha256', $secretSha256, '90698825', 2000000000], + ['sha256', $secretSha256, '77737706', 20000000000], + ['sha512', $secretSha512, '90693936', 59], + ['sha512', $secretSha512, '25091201', 1111111109], + ['sha512', $secretSha512, '99943326', 1111111111], + ['sha512', $secretSha512, '93441116', 1234567890], + ['sha512', $secretSha512, '38618901', 2000000000], + ['sha512', $secretSha512, '47863826', 20000000000], + ]; + } + + /** + * Invalid counter values for tests + */ + public function invalidCounterValues() + { + return [ + ['a'], [-1] + ]; + } + + /** + * Invalid counter values for tests + */ + public function hotpResyncDefaultTestValues() + { + return [ + ['755224', 0], ['287082', 1], ['359152', 2] + ]; + } + + /** + * Invalid counter values for tests + */ + public function hotpResyncWindowTestValues() + { + return [ + ['969429', 0, 3], ['338314', 0, 4], + ['287922', 3, 3], ['162583', 3, 4] + ]; + } + + /** + * Invalid counter values for tests + */ + public function hotpResyncFailureTestValues() + { + return [ + ['287922', 7], ['162583', 8], ['399871', 9] + ]; + } + + /** + * Tests Otp->hotp() + * + * Using test vectors from RFC + * https://tools.ietf.org/html/rfc4226 + * + * @dataProvider hotpTestValues + */ + public function testHotpRfc($key, $counter) + { + $secret = $this->secret; + + $this->assertEquals($key, $this->Otp->hotp($secret, $counter)); + } + + /** + * Tests TOTP general construction + * + * Still uses the hotp function, but since totp is a bit more special, has + * its own tests + * Using test vectors from RFC + * https://tools.ietf.org/html/rfc6238 + * + * @dataProvider totpTestValues + */ + public function testTotpRfc($algo, $secret, $key, $time) + { + // Test vectors are in 8 digits + $this->Otp->setDigits(8); + + // The time presented in the test vector has to be first divided through 30 + // to count as the key + + $this->Otp->setAlgorithm($algo); + $this->assertEquals($key, $this->Otp->hotp($secret, floor($time/30)), "$algo with $time"); + } + + /** + * @dataProvider invalidCounterValues + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Invalid counter supplied + */ + public function testHotpInvalidCounter($counter) + { + $this->Otp->hotp($this->secret, $counter); + } + + /** + * Tests Otp->checkHotpResync() with default counter window + * + * @dataProvider hotpResyncDefaultTestValues + */ + public function testHotpResyncDefault($key, $counter) + { + $secret = $this->secret; + + // test with default counter window + $this->assertSame($counter, $this->Otp->checkHotpResync($secret, $counter, $key)); + } + + + /** + * Tests Otp->checkHotpResync() with a provided counter window + * + * @dataProvider hotpResyncWindowTestValues + */ + public function testHotpResyncWindow($key, $counter, $counterwindow) + { + $secret = $this->secret; + + // test with provided counter window + $this->assertSame(($counter + $counterwindow), $this->Otp->checkHotpResync($secret, $counter, $key, $counterwindow)); + } + + /** + * Tests Otp->checkHotpResync() with mismatching key and counter + * + * @dataProvider hotpResyncFailureTestValues + */ + public function testHotpResyncFailures($key, $counter) + { + $secret = $this->secret; + + // test failures + $this->assertFalse($this->Otp->checkHotpResync($secret, $counter, $key)); + } + + /** + * @dataProvider invalidCounterValues + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Invalid counter supplied + */ + public function testHotpResyncInvalidCounter($counter) + { + $this->Otp->checkHotpResync($this->secret, $counter, '755224'); + } + + /** + * @dataProvider invalidCounterValues + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Invalid counterwindow supplied + */ + public function testHotpResyncInvalidCounterWindow($counterwindow) + { + $this->Otp->checkHotpResync($this->secret, 0, '755224', $counterwindow); + } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 353cb7a..d9df6dd 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,5 +1,5 @@ -add("Otp", __DIR__); -$loader->register(); \ No newline at end of file +$loader->register(); From e19acc311e9c3311e2a1c98e5ba05d4a89518c4c Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 13:54:51 +0545 Subject: [PATCH 03/11] chore: make phpunit pass --- .gitignore | 2 ++ phpunit.xml.dist | 15 --------------- src/Otp.php | 2 +- tests/GoogleAuthenticatorTest.php | 14 +++++++------- tests/OtpTest.php | 17 +++++++++-------- 5 files changed, 19 insertions(+), 31 deletions(-) diff --git a/.gitignore b/.gitignore index d265536..d636b1c 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ /vendor/ /.idea .php-cs-fixer.cache +.phpunit.result.cache +build diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 3d6cd8c..66be9cf 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -7,7 +7,6 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="false" - syntaxCheck="false" bootstrap="tests/bootstrap.php" colors="true"> @@ -15,19 +14,5 @@ tests/ - - - - src/ - - - - - - - - diff --git a/src/Otp.php b/src/Otp.php index 6f369f7..8ef2296 100644 --- a/src/Otp.php +++ b/src/Otp.php @@ -80,7 +80,7 @@ public function hotp($secret, $counter) true ); - return str_pad($this->truncate($hash), $this->digits, '0', STR_PAD_LEFT); + return str_pad((string) $this->truncate($hash), $this->digits, '0', STR_PAD_LEFT); } /* (non-PHPdoc) diff --git a/tests/GoogleAuthenticatorTest.php b/tests/GoogleAuthenticatorTest.php index 10eefec..f44a91b 100644 --- a/tests/GoogleAuthenticatorTest.php +++ b/tests/GoogleAuthenticatorTest.php @@ -81,10 +81,10 @@ public function testGetKeyUri() public function testGenerateRandom() { // contains numbers 2-7 and letters A-Z in large letters, 16 chars long - $this->assertRegExp('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); + $this->assertMatchesRegularExpression('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); // Can be told to make a longer secret - $this->assertRegExp('/[2-7A-Z]{18}/', GoogleAuthenticator::generateRandom(18)); + $this->assertMatchesRegularExpression('/[2-7A-Z]{18}/', GoogleAuthenticator::generateRandom(18)); } /** @@ -96,15 +96,15 @@ public function testGenerateRecoveryCodes() $codes = GoogleAuthenticator::generateRecoveryCodes(); $this->assertCount(1, $codes); - $this->assertRegExp('/[0-9]{9}/', $codes[0]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[0]); // More codes, longer $codes = GoogleAuthenticator::generateRecoveryCodes(4, 20); $this->assertCount(4, $codes); - $this->assertRegExp('/[0-9]{9}/', $codes[0]); - $this->assertRegExp('/[0-9]{9}/', $codes[1]); - $this->assertRegExp('/[0-9]{9}/', $codes[2]); - $this->assertRegExp('/[0-9]{9}/', $codes[3]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[0]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[1]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[2]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[3]); // To check for uniqueness $this->assertSame($codes, array_unique($codes)); diff --git a/tests/OtpTest.php b/tests/OtpTest.php index 58e25d2..7a5d709 100644 --- a/tests/OtpTest.php +++ b/tests/OtpTest.php @@ -2,6 +2,7 @@ namespace Otp\Tests; +use InvalidArgumentException; use Otp\Otp; use PHPUnit\Framework\TestCase; @@ -21,7 +22,7 @@ class OtpTest extends TestCase /** * Prepares the environment before running a test. */ - protected function setUp() + protected function setUp(): void { parent::setUp(); @@ -32,7 +33,7 @@ protected function setUp() /** * Cleans up the environment after running a test. */ - protected function tearDown() + protected function tearDown(): void { $this->Otp = null; @@ -164,11 +165,11 @@ public function testTotpRfc($algo, $secret, $key, $time) /** * @dataProvider invalidCounterValues - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Invalid counter supplied */ public function testHotpInvalidCounter($counter) { + $this->expectExceptionMessage("Invalid counter supplied"); + $this->expectException(InvalidArgumentException::class); $this->Otp->hotp($this->secret, $counter); } @@ -214,21 +215,21 @@ public function testHotpResyncFailures($key, $counter) /** * @dataProvider invalidCounterValues - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Invalid counter supplied */ public function testHotpResyncInvalidCounter($counter) { + $this->expectExceptionMessage("Invalid counter supplied"); + $this->expectException(InvalidArgumentException::class); $this->Otp->checkHotpResync($this->secret, $counter, '755224'); } /** * @dataProvider invalidCounterValues - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Invalid counterwindow supplied */ public function testHotpResyncInvalidCounterWindow($counterwindow) { + $this->expectExceptionMessage("Invalid counterwindow supplied"); + $this->expectException(InvalidArgumentException::class); $this->Otp->checkHotpResync($this->secret, 0, '755224', $counterwindow); } From 8024fc88d8a5bc8b57f2613a1ebfeeb4a5939af7 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 13:58:54 +0545 Subject: [PATCH 04/11] chore: delete old travis config --- .travis.yml | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 6a24efd..0000000 --- a/.travis.yml +++ /dev/null @@ -1,22 +0,0 @@ -language: php - -sudo: false - -php: - - 5.6 - - 7.0 - - 7.1 - - 7.2 - - hhvm - - nightly - -matrix: - allow_failures: - - nightly - -before_script: - - composer install --no-interaction - - mkdir -p build/logs - -script: - - vendor/bin/phpunit --coverage-text From f30a4157bfb9979103afe4ff39410c464be7858e Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 14:31:28 +0545 Subject: [PATCH 05/11] chore: make phpstan 1.10 pass on test code --- composer.json | 1 + tests/GoogleAuthenticatorTest.php | 8 ++--- tests/OtpTest.php | 52 ++++++++++++++++++++----------- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/composer.json b/composer.json index 01ad940..0f3d24e 100644 --- a/composer.json +++ b/composer.json @@ -21,6 +21,7 @@ }, "require-dev": { "friendsofphp/php-cs-fixer": "^3.51", + "phpstan/phpstan": "^1.10", "phpunit/phpunit": "^8 || ^9.6" }, "autoload": { diff --git a/tests/GoogleAuthenticatorTest.php b/tests/GoogleAuthenticatorTest.php index f44a91b..796976c 100644 --- a/tests/GoogleAuthenticatorTest.php +++ b/tests/GoogleAuthenticatorTest.php @@ -13,7 +13,7 @@ class GoogleAuthenticatorTest extends TestCase /** * Tests getQrCodeUrl */ - public function testGetQrCodeUrl() + public function testGetQrCodeUrl(): void { $secret = 'MEP3EYVA6XNFNVNM'; // testing secret @@ -40,7 +40,7 @@ public function testGetQrCodeUrl() /** * Tests getKeyUri */ - public function testGetKeyUri() + public function testGetKeyUri(): void { $secret = 'MEP3EYVA6XNFNVNM'; // testing secret @@ -78,7 +78,7 @@ public function testGetKeyUri() /** * Tests generateRandom */ - public function testGenerateRandom() + public function testGenerateRandom(): void { // contains numbers 2-7 and letters A-Z in large letters, 16 chars long $this->assertMatchesRegularExpression('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); @@ -90,7 +90,7 @@ public function testGenerateRandom() /** * Test generateRecoveryCodes */ - public function testGenerateRecoveryCodes() + public function testGenerateRecoveryCodes(): void { // Default settings $codes = GoogleAuthenticator::generateRecoveryCodes(); diff --git a/tests/OtpTest.php b/tests/OtpTest.php index 7a5d709..9212941 100644 --- a/tests/OtpTest.php +++ b/tests/OtpTest.php @@ -12,11 +12,13 @@ class OtpTest extends TestCase { /** - * * @var Otp */ private $Otp; - + + /** + * @var string + */ private $secret = "12345678901234567890"; /** @@ -35,15 +37,15 @@ protected function setUp(): void */ protected function tearDown(): void { - $this->Otp = null; - parent::tearDown(); } /** * Invalid counter values for tests + * + * @return array */ - public function hotpTestValues() + public function hotpTestValues(): array { return [ ['755224', 0], ['287082', 1], ['359152', 2], @@ -55,8 +57,9 @@ public function hotpTestValues() /** * Invalid counter values for tests + * @return array */ - public function totpTestValues() + public function totpTestValues(): array { // https://www.rfc-editor.org/errata_search.php?rfc=6238 $secretSha1 = '12345678901234567890'; @@ -87,8 +90,10 @@ public function totpTestValues() /** * Invalid counter values for tests + * + * @return array */ - public function invalidCounterValues() + public function invalidCounterValues(): array { return [ ['a'], [-1] @@ -97,8 +102,10 @@ public function invalidCounterValues() /** * Invalid counter values for tests + * + * @return array */ - public function hotpResyncDefaultTestValues() + public function hotpResyncDefaultTestValues(): array { return [ ['755224', 0], ['287082', 1], ['359152', 2] @@ -107,8 +114,10 @@ public function hotpResyncDefaultTestValues() /** * Invalid counter values for tests + * + * @return array */ - public function hotpResyncWindowTestValues() + public function hotpResyncWindowTestValues(): array { return [ ['969429', 0, 3], ['338314', 0, 4], @@ -118,8 +127,10 @@ public function hotpResyncWindowTestValues() /** * Invalid counter values for tests + * + * @return array */ - public function hotpResyncFailureTestValues() + public function hotpResyncFailureTestValues(): array { return [ ['287922', 7], ['162583', 8], ['399871', 9] @@ -134,7 +145,7 @@ public function hotpResyncFailureTestValues() * * @dataProvider hotpTestValues */ - public function testHotpRfc($key, $counter) + public function testHotpRfc(string $key, int $counter): void { $secret = $this->secret; @@ -151,7 +162,7 @@ public function testHotpRfc($key, $counter) * * @dataProvider totpTestValues */ - public function testTotpRfc($algo, $secret, $key, $time) + public function testTotpRfc(string $algo, string $secret, string $key, int $time): void { // Test vectors are in 8 digits $this->Otp->setDigits(8); @@ -160,13 +171,14 @@ public function testTotpRfc($algo, $secret, $key, $time) // to count as the key $this->Otp->setAlgorithm($algo); - $this->assertEquals($key, $this->Otp->hotp($secret, floor($time/30)), "$algo with $time"); + $this->assertEquals($key, $this->Otp->hotp($secret, intval(floor($time/30))), "$algo with $time"); } /** * @dataProvider invalidCounterValues + * @param mixed $counter various invalid values of different types that should cause an InvalidArgumentException */ - public function testHotpInvalidCounter($counter) + public function testHotpInvalidCounter($counter): void { $this->expectExceptionMessage("Invalid counter supplied"); $this->expectException(InvalidArgumentException::class); @@ -178,7 +190,7 @@ public function testHotpInvalidCounter($counter) * * @dataProvider hotpResyncDefaultTestValues */ - public function testHotpResyncDefault($key, $counter) + public function testHotpResyncDefault(string $key, int $counter): void { $secret = $this->secret; @@ -192,7 +204,7 @@ public function testHotpResyncDefault($key, $counter) * * @dataProvider hotpResyncWindowTestValues */ - public function testHotpResyncWindow($key, $counter, $counterwindow) + public function testHotpResyncWindow(string $key, int $counter, int $counterwindow): void { $secret = $this->secret; @@ -205,7 +217,7 @@ public function testHotpResyncWindow($key, $counter, $counterwindow) * * @dataProvider hotpResyncFailureTestValues */ - public function testHotpResyncFailures($key, $counter) + public function testHotpResyncFailures(string $key, int $counter): void { $secret = $this->secret; @@ -215,8 +227,9 @@ public function testHotpResyncFailures($key, $counter) /** * @dataProvider invalidCounterValues + * @param mixed $counter various invalid values of different types that should cause an InvalidArgumentException */ - public function testHotpResyncInvalidCounter($counter) + public function testHotpResyncInvalidCounter($counter): void { $this->expectExceptionMessage("Invalid counter supplied"); $this->expectException(InvalidArgumentException::class); @@ -225,8 +238,9 @@ public function testHotpResyncInvalidCounter($counter) /** * @dataProvider invalidCounterValues + * @param mixed $counterwindow various invalid values of different types that should cause an InvalidArgumentException */ - public function testHotpResyncInvalidCounterWindow($counterwindow) + public function testHotpResyncInvalidCounterWindow($counterwindow): void { $this->expectExceptionMessage("Invalid counterwindow supplied"); $this->expectException(InvalidArgumentException::class); From 22eff722143b69aab1afb3a6a788728190266e9b Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 14:44:50 +0545 Subject: [PATCH 06/11] chore: adjust OtpTest to pass php-cs-fixer --- tests/OtpTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/OtpTest.php b/tests/OtpTest.php index 9212941..3a1359a 100644 --- a/tests/OtpTest.php +++ b/tests/OtpTest.php @@ -171,7 +171,7 @@ public function testTotpRfc(string $algo, string $secret, string $key, int $time // to count as the key $this->Otp->setAlgorithm($algo); - $this->assertEquals($key, $this->Otp->hotp($secret, intval(floor($time/30))), "$algo with $time"); + $this->assertEquals($key, $this->Otp->hotp($secret, \intval(floor($time/30))), "$algo with $time"); } /** From 8b2c6040a058820f88a715fa69ed4f37dd55ea94 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 14:45:26 +0545 Subject: [PATCH 07/11] chore: make phpstan 1.10 pass on src code --- src/GoogleAuthenticator.php | 18 ++++++++++++++---- src/Otp.php | 6 +++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/GoogleAuthenticator.php b/src/GoogleAuthenticator.php index 17f0664..f8d04d4 100644 --- a/src/GoogleAuthenticator.php +++ b/src/GoogleAuthenticator.php @@ -15,9 +15,19 @@ class GoogleAuthenticator { + /** + * @var string[] + */ protected static $allowedTypes = array('hotp', 'totp'); + /** + * @var int + */ protected static $height = 200; + + /** + * @var int + */ protected static $width = 200; /** @@ -31,7 +41,7 @@ class GoogleAuthenticator * @param string $label Label to display this as to the user * @param string $secret Base32 encoded secret * @param integer $counter Required by hotp, otherwise ignored - * @param array $options Optional fields that will be set if present + * @param array $options Optional fields that will be set if present * * @return string Key URI */ @@ -66,7 +76,7 @@ public static function getKeyUri($type, $label, $secret, $counter = null, $optio // This is the base, these are at least required $otpauth = 'otpauth://'.$type.'/'.rawurlencode($label).'?secret='.rawurlencode($secret); - if ('hotp' == $type && !\is_null($counter)) { + if ('hotp' == $type) { $otpauth .= '&counter='.\intval($counter); } @@ -117,7 +127,7 @@ public static function getKeyUri($type, $label, $secret, $counter = null, $optio * @param string $label Label to display this as to the user * @param string $secret Base32 encoded secret * @param integer $counter Required by hotp, otherwise ignored - * @param array $options Optional fields that will be set if present + * @param array $options Optional fields that will be set if present * * @return string URL to the QR code */ @@ -175,7 +185,7 @@ public static function generateRandom($length = 16) * @param integer $count How many codes to return * @param integer $length How long each code should be * - * @return array Array of codes + * @return array Array of codes */ public static function generateRecoveryCodes($count = 1, $length = 9) { diff --git a/src/Otp.php b/src/Otp.php index 8ef2296..4880258 100644 --- a/src/Otp.php +++ b/src/Otp.php @@ -46,7 +46,7 @@ class Otp implements OtpInterface /** * Possible algorithms * - * @var array + * @var array */ protected $allowedAlgorithms = array('sha1', 'sha256', 'sha512'); @@ -307,7 +307,7 @@ private function getBinaryCounter($counter) */ private function getTimecounter() { - return floor((time() + $this->totpOffset) / $this->period); + return \intval(floor((time() + $this->totpOffset) / $this->period)); } /** @@ -317,7 +317,7 @@ private function getTimecounter() * calling function. * * @param string $hash hmac hash - * @return number + * @return int */ private function truncate($hash) { From e308545662e06557c260a72e022aacb5ebe1da2b Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 17:18:16 +0545 Subject: [PATCH 08/11] chore: allow php-cs-fixer 3.4 for PHP 7.2 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0f3d24e..a0045b0 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "symfony/polyfill-php56": "^1" }, "require-dev": { - "friendsofphp/php-cs-fixer": "^3.51", + "friendsofphp/php-cs-fixer": "^3.4", "phpstan/phpstan": "^1.10", "phpunit/phpunit": "^8 || ^9.6" }, From 9906dcb5bc55029cf14c20c57017ed84d2e0eb96 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 17:27:38 +0545 Subject: [PATCH 09/11] chore: add PHP 8.1 8.2 8.3 to CI --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6500a5c..24cdb9d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,6 +41,9 @@ jobs: - '7.3' - '7.4' - '8.0' + - '8.1' + - '8.2' + - '8.3' include: - phpunit-version: '8.5.13' php-version: '7.2' From 610b10ea90bc8b3f2abc31354b176e7e2c955de3 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 17:30:04 +0545 Subject: [PATCH 10/11] chore: use phpunit 9.6.17 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24cdb9d..8a79c07 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: fail-fast: false matrix: phpunit-version: - - '9.5.0' + - '9.6.17' php-version: - '7.3' - '7.4' From 82ff477a04decc1ac64763c1cdff58742946836f Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 4 Mar 2024 17:46:55 +0545 Subject: [PATCH 11/11] chore: adjust tests to work with phpunit 8 --- tests/GoogleAuthenticatorTest.php | 42 +++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/tests/GoogleAuthenticatorTest.php b/tests/GoogleAuthenticatorTest.php index 796976c..9039484 100644 --- a/tests/GoogleAuthenticatorTest.php +++ b/tests/GoogleAuthenticatorTest.php @@ -80,11 +80,20 @@ public function testGetKeyUri(): void */ public function testGenerateRandom(): void { - // contains numbers 2-7 and letters A-Z in large letters, 16 chars long - $this->assertMatchesRegularExpression('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); - - // Can be told to make a longer secret - $this->assertMatchesRegularExpression('/[2-7A-Z]{18}/', GoogleAuthenticator::generateRandom(18)); + if (method_exists($this, "assertMatchesRegularExpression")) { + // contains numbers 2-7 and letters A-Z in large letters, 16 chars long + $this->assertMatchesRegularExpression('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); + + // Can be told to make a longer secret + $this->assertMatchesRegularExpression('/[2-7A-Z]{18}/', GoogleAuthenticator::generateRandom(18)); + } else { + // for phpunit v8 with PHP 7.2 + // contains numbers 2-7 and letters A-Z in large letters, 16 chars long + $this->assertRegExp('/[2-7A-Z]{16}/', GoogleAuthenticator::generateRandom()); + + // Can be told to make a longer secret + $this->assertRegExp('/[2-7A-Z]{18}/', GoogleAuthenticator::generateRandom(18)); + } } /** @@ -96,15 +105,28 @@ public function testGenerateRecoveryCodes(): void $codes = GoogleAuthenticator::generateRecoveryCodes(); $this->assertCount(1, $codes); - $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[0]); + if (method_exists($this, "assertMatchesRegularExpression")) { + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[0]); + } else { + // for phpunit v8 with PHP 7.2 + $this->assertRegExp('/[0-9]{9}/', $codes[0]); + } // More codes, longer $codes = GoogleAuthenticator::generateRecoveryCodes(4, 20); $this->assertCount(4, $codes); - $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[0]); - $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[1]); - $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[2]); - $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[3]); + if (method_exists($this, "assertMatchesRegularExpression")) { + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[0]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[1]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[2]); + $this->assertMatchesRegularExpression('/[0-9]{9}/', $codes[3]); + } else { + // for phpunit v8 with PHP 7.2 + $this->assertRegExp('/[0-9]{9}/', $codes[0]); + $this->assertRegExp('/[0-9]{9}/', $codes[1]); + $this->assertRegExp('/[0-9]{9}/', $codes[2]); + $this->assertRegExp('/[0-9]{9}/', $codes[3]); + } // To check for uniqueness $this->assertSame($codes, array_unique($codes));