skip secret length check in OTP client setting (#5)

Some providers don't follow the RFCs to the letter and only use 10 bytes for the shared secret. Therefore, the generate() method now skips the secret length validation.

The verify() method continues to throw if the secret is too short. This is to enforce the security recommendations from the RFCs when you're implementing an OTP provider.

Author: jiripudil

src/Algorithm.php

@@ -14,9 +14,9 @@ enum Algorithm: string
 	case Sha256 = 'sha256';
 	case Sha512 = 'sha512';
 
-	public function hash(string $value, Secret $secret): string
+	public function hash(string $value, Secret $secret, bool $skipLengthCheck): string
 	{
-		if (strlen($secret->asBinary()) < $this->minSecretLength()) {
+		if (!$skipLengthCheck && strlen($secret->asBinary()) < $this->minSecretLength()) {
 			throw new InvalidArgumentException(sprintf(
 				"Secret is too short (%d bytes), at least %d bytes should be provided for algorithm %s.",
 				strlen($secret->asBinary()),

src/OTP.php

@@ -82,7 +82,8 @@ public function generate(
 		$value = $generator->current();
 		$generator->send(true);
 
-		return $this->generateOneTimePassword($value, $account->getSecret(), $digits);
+		// skip length check because an OTP client does not have control over the secret length and some providers only use 10 bytes
+		return $this->generateOneTimePassword($value, $account->getSecret(), $digits, skipLengthCheck: true);
 	}
 
 	/**
@@ -131,10 +132,11 @@ private function generateOneTimePassword(
 		int $value,
 		Secret $secret,
 		int $digits,
+		bool $skipLengthCheck = false,
 	): string
 	{
 		$packedValue = pack('J*', $value);
-		$hash = $this->algorithm->hash($packedValue, $secret);
+		$hash = $this->algorithm->hash($packedValue, $secret, $skipLengthCheck);
 		$offset = ord($hash[strlen($hash) - 1]) & 0xf;
 
 		$code = (

tests/TOTPIntegrationTest.php

@@ -30,8 +30,11 @@ public function getTime(): float
 		);
 
 		$account = new SimpleAccountDescriptor('AccountName', Secret::fromBinary(str_repeat("\x42", 20)));
-
 		Assert::same('930232', $authenticator->generate($account));
+
+		// short secret is ok here
+		$account = new SimpleAccountDescriptor('AccountName', Secret::fromBinary(str_repeat("\x42", 10)));
+		Assert::noError(fn() => $authenticator->generate($account));
 	}
 
 	public function testVerify(): void
@@ -47,12 +50,19 @@ public function getTime(): float
 		);
 
 		$account = new SimpleAccountDescriptor('AccountName', Secret::fromBinary(str_repeat("\x42", 20)));
-
 		Assert::false($authenticator->verify($account, '027585'));
 		Assert::true($authenticator->verify($account, '714222'));
 		Assert::true($authenticator->verify($account, '930232'));
 		Assert::true($authenticator->verify($account, '605509'));
 		Assert::false($authenticator->verify($account, '570599'));
+
+		// short secret means error
+		$account = new SimpleAccountDescriptor('AccountName', Secret::fromBinary(str_repeat("\x42", 10)));
+		Assert::throws(
+			fn() => $authenticator->verify($account, '111111'),
+			\InvalidArgumentException::class,
+			'Secret is too short (10 bytes), at least 20 bytes should be provided for algorithm sha1.',
+		);
 	}
 }