From 2f13de824533c5e1d9b332a65be7ffde7e76e442 Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Mon, 15 Apr 2024 09:58:20 +0200 Subject: [PATCH] Add assertions and modify annotations for better code clarity Several assertions were added to verify the integrity of the timestamp and counter values. Also, param annotations were modified and added for better understanding of the input parameters and return values. Furthermore, the test parameters were changed from an Iterator to an iterable for a wider compatibility with different data types. --- phpstan-baseline.neon | 10 ---------- src/HOTP.php | 6 ++++++ src/OTP.php | 5 +++++ src/OTPInterface.php | 4 ++++ src/TOTP.php | 20 ++++++++++++++++++-- tests/TOTPTest.php | 16 +++++++++------- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ad68a32..1f84799 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -39,13 +39,3 @@ parameters: message: "#^Parameter \\#1 \\$dateTime of method OTPHP\\\\Test\\\\ClockMock\\:\\:setDateTime\\(\\) expects DateTimeImmutable\\|null, DateTimeImmutable\\|false given\\.$#" count: 5 path: tests/TOTPTest.php - - - - message: "#^Parameter \\#1 \\$otp of method OTPHP\\\\TOTP\\:\\:verify\\(\\) expects non\\-empty\\-string, string given\\.$#" - count: 2 - path: tests/TOTPTest.php - - - - message: "#^Parameter \\#3 \\$leeway of method OTPHP\\\\TOTP\\:\\:verify\\(\\) expects int\\<0, max\\>\\|null, int given\\.$#" - count: 2 - path: tests/TOTPTest.php diff --git a/src/HOTP.php b/src/HOTP.php index e6bea73..835de35 100644 --- a/src/HOTP.php +++ b/src/HOTP.php @@ -46,6 +46,9 @@ public static function generate(): self return self::createFromSecret(self::generateSecret()); } + /** + * @return 0|positive-int + */ public function getCounter(): int { $value = $this->getParameter('counter'); @@ -63,6 +66,8 @@ public function getProvisioningUri(): string /** * If the counter is not provided, the OTP is verified at the actual counter. + * + * @param null|0|positive-int $counter */ public function verify(string $otp, null|int $counter = null, null|int $window = null): bool { @@ -112,6 +117,7 @@ private function getWindow(null|int $window): int /** * @param non-empty-string $otp + * @param 0|positive-int $counter * @param null|0|positive-int $window */ private function verifyOtpWithWindow(string $otp, int $counter, null|int $window): bool diff --git a/src/OTP.php b/src/OTP.php index 9c6aca3..a6a53f8 100644 --- a/src/OTP.php +++ b/src/OTP.php @@ -35,6 +35,9 @@ public function getQrCodeUri(string $uri, string $placeholder): string return str_replace($placeholder, $provisioning_uri, $uri); } + /** + * @param 0|positive-int $input + */ public function at(int $input): string { return $this->generateOTP($input); @@ -51,6 +54,8 @@ final protected static function generateSecret(): string /** * The OTP at the specified input. * + * @param 0|positive-int $input + * * @return non-empty-string */ protected function generateOTP(int $input): string diff --git a/src/OTPInterface.php b/src/OTPInterface.php index 5ce1467..39ce4ac 100644 --- a/src/OTPInterface.php +++ b/src/OTPInterface.php @@ -35,6 +35,10 @@ public function setDigits(int $digits): void; public function setDigest(string $digest): void; /** + * Generate the OTP at the specified input. + * + * @param 0|positive-int $input + * * @return non-empty-string Return the OTP at the specified timestamp */ public function at(int $input): string; diff --git a/src/TOTP.php b/src/TOTP.php index d154969..035e04f 100644 --- a/src/TOTP.php +++ b/src/TOTP.php @@ -90,6 +90,11 @@ public function expiresIn(): int return $period - ($this->clock->now()->getTimestamp() % $this->getPeriod()); } + /** + * The OTP at the specified input. + * + * @param 0|positive-int $input + */ public function at(int $input): string { return $this->generateOTP($this->timecode($input)); @@ -97,12 +102,19 @@ public function at(int $input): string public function now(): string { - return $this->at($this->clock->now()->getTimestamp()); + $timestamp = $this->clock->now() + ->getTimestamp(); + assert($timestamp >= 0, 'The timestamp must return a positive integer.'); + + return $this->at($timestamp); } /** * If no timestamp is provided, the OTP is verified at the actual timestamp. When used, the leeway parameter will * allow time drift. The passed value is in seconds. + * + * @param 0|positive-int $timestamp + * @param null|0|positive-int $leeway */ public function verify(string $otp, null|int $timestamp = null, null|int $leeway = null): bool { @@ -118,8 +130,12 @@ public function verify(string $otp, null|int $timestamp = null, null|int $leeway $leeway < $this->getPeriod() || throw new InvalidArgumentException( 'The leeway must be lower than the TOTP period' ); + $timestampMinusLeeway = $timestamp - $leeway; + $timestampMinusLeeway >= 0 || throw new InvalidArgumentException( + 'The timestamp must be greater than or equal to the leeway.' + ); - return $this->compareOTP($this->at($timestamp - $leeway), $otp) + return $this->compareOTP($this->at($timestampMinusLeeway), $otp) || $this->compareOTP($this->at($timestamp), $otp) || $this->compareOTP($this->at($timestamp + $leeway), $otp); } diff --git a/tests/TOTPTest.php b/tests/TOTPTest.php index afc3046..1b8a2df 100644 --- a/tests/TOTPTest.php +++ b/tests/TOTPTest.php @@ -6,7 +6,6 @@ use DateTimeImmutable; use InvalidArgumentException; -use Iterator; use OTPHP\InternalClock; use OTPHP\TOTP; use OTPHP\TOTPInterface; @@ -240,9 +239,9 @@ public function vectors($totp, $timestamp, $expected_value): void * @see https://tools.ietf.org/html/rfc6238#appendix-B * @see http://www.rfc-editor.org/errata_search.php?rfc=6238 * - * @return array + * @return iterable */ - public static function dataVectors(): Iterator + public static function dataVectors(): iterable { $sha1key = Base32::encodeUpper('12345678901234567890'); assert($sha1key !== ''); @@ -318,7 +317,10 @@ public function verifyOtpWithEpochInWindow( static::assertSame($expectedResult, $otp->verify($input, null, $leeway)); } - public static function dataLeewayWithEpoch(): Iterator + /** + * @return iterable[] + */ + public static function dataLeewayWithEpoch(): iterable { yield 'Leeway of 10 seconds, **out** the period of 11sec (11 second before)' => [ 319_690_889, @@ -377,7 +379,7 @@ public function qRCodeUri(): void /** * @return int[][] */ - public static function dataRemainingTimeBeforeExpiration(): Iterator + public static function dataRemainingTimeBeforeExpiration(): iterable { yield [1_644_926_810, 90, 40]; yield [1_644_926_810, 30, 10]; @@ -394,9 +396,9 @@ public static function dataRemainingTimeBeforeExpiration(): Iterator } /** - * @return array[] + * @return iterable[] */ - public static function dataLeeway(): Iterator + public static function dataLeeway(): iterable { yield 'Leeway of 10 seconds, **out** the period of 11sec (11 second before)' => [ 319_690_789,