Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 51 additions & 7 deletions src/RecoveryCodes.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,21 @@ public function generate(
throw new InvalidArgumentException('Invalid recovery code configuration.');
}

$characters = $this->characterSet($characterSet);
if (!$this->canGenerateUniqueCodes($count, $length, count($characters))) {
throw new InvalidArgumentException('Recovery code configuration cannot produce the requested number of unique codes.');
}

$plainCodes = [];
$hashedCodes = [];
for ($i = 0; $i < $count; $i++) {
$code = $this->randomCode($length, $characterSet);
$generatedCodes = [];
while (count($plainCodes) < $count) {
$code = $this->randomCode($length, $characters);
if (isset($generatedCodes[$code])) {
continue;
}

$generatedCodes[$code] = true;
$plainCodes[] = $groupSize > 0 ? trim(chunk_split($code, $groupSize, '-'), '-') : $code;
$hashedCodes[] = $this->hash($code);
}
Expand All @@ -60,21 +71,54 @@ public function generate(
return new RecoveryCodeGenerationResult($plainCodes, $metadata['total'], $metadata['remaining'], $metadata['lastUsedAt']);
}

private function hash(string $code): string
private function canGenerateUniqueCodes(int $count, int $length, int $characterCount): bool
{
return hash_hmac($this->hashAlgorithm, $code, $this->hashKey ?? 'otp-recovery-codes');
$capacity = 1;
for ($i = 0; $i < $length; $i++) {
if ($capacity >= $count || $capacity > intdiv($count - 1, $characterCount)) {
return true;
}

$capacity *= $characterCount;
}

return $capacity >= $count;
}

private function randomCode(int $length, string $characterSet): string
/**
* @param $characterSet Candidate recovery-code characters.
* @return array Unique recovery-code characters.
* @phpstan-return non-empty-list<string>
*/
private function characterSet(string $characterSet): array
{
$characters = array_values(array_unique(str_split($characterSet)));
$characters = [];
foreach (str_split($characterSet) as $character) {
$characters[$character] = true;
}
if ($characters === []) {
throw new InvalidArgumentException('Recovery code character set cannot be empty.');
}

return array_keys($characters);
}

private function hash(string $code): string
{
return hash_hmac($this->hashAlgorithm, $code, $this->hashKey ?? 'otp-recovery-codes');
}

/**
* @param $length Recovery-code length.
* @param $characterSet Unique recovery-code characters.
* @phpstan-param non-empty-list<string> $characterSet
*/
private function randomCode(int $length, array $characterSet): string
{
$code = '';
$characterCount = count($characterSet);
for ($i = 0; $i < $length; $i++) {
$code .= $characters[random_int(0, count($characters) - 1)];
$code .= $characterSet[random_int(0, $characterCount - 1)];
}

return $code;
Expand Down
2 changes: 1 addition & 1 deletion src/Stores/InMemoryRecoveryCodeStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function replace(string $binding, array $hashedCodes, DateTimeImmutable $

$this->storage[$binding] = [
'codes' => $codes,
'total' => count($hashedCodes),
'total' => count($codes),
'lastUsedAt' => null,
];
}
Expand Down
8 changes: 4 additions & 4 deletions src/Support/AlgorithmValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ final class AlgorithmValidator
public static function normalize(string $algorithm): string
{
$algorithm = strtolower(trim($algorithm));
if (!in_array($algorithm, self::supported(), true)) {
throw new InvalidArgumentException('Unsupported OTP algorithm.');
}

return $algorithm;
return match ($algorithm) {
'sha1', 'sha256', 'sha512' => $algorithm,
default => throw new InvalidArgumentException('Unsupported OTP algorithm.'),
};
}

/**
Expand Down
45 changes: 42 additions & 3 deletions src/Support/ProvisioningUriParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ public static function parse(string $uri): ParsedOtpAuthUri
$labelParts = LabelHelper::parseLabel(ltrim((string) ($parts['path'] ?? ''), '/'), $issuer);
$algorithmValue = self::optionalStringQueryValue($query, 'algorithm');
$algorithm = $algorithmValue !== null ? AlgorithmValidator::normalize($algorithmValue) : 'sha1';
$digits = isset($query['digits']) ? (int) $query['digits'] : 6;
$digits = self::optionalNonNegativeIntQueryValue($query, 'digits') ?? 6;
if (($type === 'hotp' || $type === 'totp') && ($digits < 4 || $digits > 10)) {
throw new InvalidArgumentException('HOTP and TOTP digit counts must be between 4 and 10.');
}

$period = self::optionalPositiveIntQueryValue($query, 'period');
$counter = self::optionalNonNegativeIntQueryValue($query, 'counter');

return new ParsedOtpAuthUri(
$type,
Expand All @@ -37,12 +43,45 @@ public static function parse(string $uri): ParsedOtpAuthUri
$labelParts['issuer'],
$algorithm,
$digits,
isset($query['period']) ? (int) $query['period'] : null,
isset($query['counter']) ? (int) $query['counter'] : null,
$period,
$counter,
self::optionalStringQueryValue($query, 'ocraSuite'),
);
}

/**
* @param $query Parsed URI query values.
* @param $key Query parameter name.
* @phpstan-param array<array-key, mixed> $query
*/
private static function optionalNonNegativeIntQueryValue(array $query, string $key): ?int
{
$value = self::optionalStringQueryValue($query, $key);
if ($value === null) {
return null;
}
if (!ctype_digit($value)) {
throw new InvalidArgumentException(sprintf('Invalid non-negative integer otpauth query parameter "%s".', $key));
}

return (int) $value;
}

/**
* @param $query Parsed URI query values.
* @param $key Query parameter name.
* @phpstan-param array<array-key, mixed> $query
*/
private static function optionalPositiveIntQueryValue(array $query, string $key): ?int
{
$value = self::optionalNonNegativeIntQueryValue($query, $key);
if ($value !== null && $value < 1) {
throw new InvalidArgumentException(sprintf('Otpauth query parameter "%s" must be greater than zero.', $key));
}

return $value;
}

/**
* @param $query Parsed URI query values.
* @param $key Query parameter name.
Expand Down
5 changes: 3 additions & 2 deletions src/Support/StepUp.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ public static function assess(?DateTimeImmutable $verifiedAt, int $seconds, ?Dat
{
self::assertWindow($seconds);
$now ??= new DateTimeImmutable();
$age = $verifiedAt !== null ? self::ageInSeconds($verifiedAt, $now) : null;

return new StepUpResult(
self::requiresFreshOtp($verifiedAt, $seconds, $now),
$verifiedAt === null || $age > $seconds,
$verifiedAt,
$verifiedAt !== null ? self::ageInSeconds($verifiedAt, $now) : null,
$age,
$seconds,
$verifiedAt?->modify(sprintf('+%d seconds', $seconds)),
);
Expand Down
11 changes: 4 additions & 7 deletions src/TOTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,6 @@ public function planSecretRotation(
bool $withQrSvg = false,
int $imageSize = 200,
): SecretRotation {
if ($gracePeriodInSeconds !== null && $gracePeriodInSeconds < 0) {
throw new \InvalidArgumentException('Grace period must be non-negative.');
}

$rotation = $this->rotateSecret($newSecret, $gracePeriodInSeconds, $now);
$next = new self($rotation['next'], $this->digitCount, $this->period);
$next->setAlgorithm($this->algorithm);
Expand All @@ -215,6 +211,10 @@ public function rotateSecret(
?int $gracePeriodInSeconds = null,
?int $now = null,
): array {
if ($gracePeriodInSeconds !== null && $gracePeriodInSeconds < 0) {
throw new \InvalidArgumentException('Grace period must be non-negative.');
}

$now ??= time();

return [
Expand Down Expand Up @@ -254,9 +254,6 @@ public function verifyWithWindow(
): VerificationResult {
$this->assertOtp($otp, $this->digitCount);
$window ??= new VerificationWindow();
if ($window->past < 0 || $window->future < 0) {
throw new \InvalidArgumentException('Verification windows must be non-negative.');
}

$baseTimestamp = $timestamp ?? time();
$currentStep = $this->getTimeStepFromTimestamp($baseTimestamp);
Expand Down
4 changes: 0 additions & 4 deletions src/ValueObjects/DeviceEnrollment.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ public static function create(
string $secretReference,
?DateTimeImmutable $createdAt = null,
): self {
self::assertNonEmpty('deviceId', $deviceId);
self::assertNonEmpty('label', $label);
self::assertNonEmpty('secretReference', $secretReference);

return new self($deviceId, $label, $secretReference, $createdAt ?? new DateTimeImmutable());
}

Expand Down
8 changes: 7 additions & 1 deletion src/ValueObjects/VerificationWindow.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@

namespace Infocyph\OTP\ValueObjects;

use InvalidArgumentException;

final readonly class VerificationWindow
{
public function __construct(
public int $past = 0,
public int $future = 0,
) {}
) {
if ($past < 0 || $future < 0) {
throw new InvalidArgumentException('Verification windows must be non-negative.');
}
}

public static function symmetric(int $window): self
{
Expand Down
21 changes: 21 additions & 0 deletions tests/AdvancedOTPTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@
->and($regenerated->remainingCount)->toBe(10);
});

test('Recovery code generation rejects insufficient unique code space', function () {
$codes = new RecoveryCodes(new InMemoryRecoveryCodeStore());

expect(fn () => $codes->generate('user-1', count: 2, length: 6, characterSet: 'A'))
->toThrow(InvalidArgumentException::class);
});

test('otpauth URIs round-trip through parser with issuer-safe labels', function () {
$secret = TOTP::generateSecret();
$totp = (new TOTP($secret))->setAlgorithm('sha256');
Expand All @@ -85,3 +92,17 @@
->and($parsed->algorithm)->toBe('sha256')
->and($parsed->period)->toBe(30);
});

test('otpauth URI parser rejects malformed numeric parameters', function () {
$baseUri = 'otpauth://totp/Example:user?secret=JBSWY3DPEHPK3PXP';

foreach (['&digits=invalid', '&digits=3', '&period=0', '&counter=-1'] as $parameter) {
expect(fn () => ProvisioningUriParser::parse($baseUri . $parameter))
->toThrow(InvalidArgumentException::class);
}
});

test('verification windows reject negative bounds when constructed', function () {
expect(fn () => new VerificationWindow(-1, 0))
->toThrow(InvalidArgumentException::class);
});
9 changes: 9 additions & 0 deletions tests/WorkflowHelpersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@
->and($rotation->nextEnrollment?->qrSvg)->toContain('<svg');
});

test('totp secret rotation rejects negative grace periods through every public entry point', function () {
$totp = new TOTP(TOTP::generateSecret());

expect(fn () => $totp->rotateSecret(TOTP::generateSecret(), -1))
->toThrow(InvalidArgumentException::class)
->and(fn () => $totp->planSecretRotation(TOTP::generateSecret(), 'alice@example.com', 'Example App', -1))
->toThrow(InvalidArgumentException::class);
});

test('hotp and ocra secret rotation can prepare replacement enrollment payloads', function () {
$hotp = (new HOTP(HOTP::generateSecret()))->setAlgorithm('sha512')->setCounter(5);
$hotpRotation = $hotp->planSecretRotation(
Expand Down