Skip to content

Commit

Permalink
Add assertions and modify annotations for better code clarity
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Spomky committed Apr 15, 2024
1 parent 0d35971 commit 2f13de8
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 19 deletions.
10 changes: 0 additions & 10 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions src/HOTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
{
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/OTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/OTPInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 18 additions & 2 deletions src/TOTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,31 @@ 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));
}

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
{
Expand All @@ -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);
}
Expand Down
16 changes: 9 additions & 7 deletions tests/TOTPTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use DateTimeImmutable;
use InvalidArgumentException;
use Iterator;
use OTPHP\InternalClock;
use OTPHP\TOTP;
use OTPHP\TOTPInterface;
Expand Down Expand Up @@ -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<int, mixed[]>
* @return iterable<int, mixed[]>
*/
public static function dataVectors(): Iterator
public static function dataVectors(): iterable
{
$sha1key = Base32::encodeUpper('12345678901234567890');
assert($sha1key !== '');
Expand Down Expand Up @@ -318,7 +317,10 @@ public function verifyOtpWithEpochInWindow(
static::assertSame($expectedResult, $otp->verify($input, null, $leeway));
}

public static function dataLeewayWithEpoch(): Iterator
/**
* @return iterable<array-key, int|string|bool>[]
*/
public static function dataLeewayWithEpoch(): iterable
{
yield 'Leeway of 10 seconds, **out** the period of 11sec (11 second before)' => [
319_690_889,
Expand Down Expand Up @@ -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];
Expand All @@ -394,9 +396,9 @@ public static function dataRemainingTimeBeforeExpiration(): Iterator
}

/**
* @return array<int, int|string|bool>[]
* @return iterable<int, int|string|bool>[]
*/
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,
Expand Down

0 comments on commit 2f13de8

Please sign in to comment.