Fix PHPStan errors

rollerworks · Nov 6, 2023 · 8015b98 · 8015b98
1 parent 79f0ea9
commit 8015b98
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 42 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -0,0 +1,16 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Parameter \\#1 \\$hash of method Rollerworks\\\\Component\\\\SplitToken\\\\SplitToken\\:\\:verifyHash\\(\\) expects string, string\\|null given\\.$#"
+			count: 1
+			path: src/SplitToken.php
+
+		-
+			message: "#^Parameter \\#1 \\$selector of class Rollerworks\\\\Component\\\\SplitToken\\\\SplitTokenValueHolder constructor expects string, string\\|null given\\.$#"
+			count: 1
+			path: src/SplitTokenValueHolder.php
+
+		-
+			message: "#^Parameter \\#2 \\$verifierHash of class Rollerworks\\\\Component\\\\SplitToken\\\\SplitTokenValueHolder constructor expects string, string\\|null given\\.$#"
+			count: 1
+			path: src/SplitTokenValueHolder.php
diff --git a/phpstan.neon b/phpstan.neon
@@ -1,6 +1,6 @@
 includes:
     - vendor/rollerscapes/standards/phpstan.neon
-    #- phpstan-baseline.neon
+    - phpstan-baseline.neon
 
 parameters:
     #reportUnmatchedIgnoredErrors: false
@@ -13,4 +13,5 @@ parameters:
         - templates/
         - translations/
 
-    #ignoreErrors:
+    ignoreErrors:
+        - '#Attribute class Symfony\\Contracts\\Service\\Attribute\\Required does not exist#' # Not required
diff --git a/src/Argon2SplitToken.php b/src/Argon2SplitToken.php
@@ -37,10 +37,10 @@ protected function verifyHash(string $hash, string $verifier): bool
     /** @codeCoverageIgnore */
     protected function hashVerifier(string $verifier): string
     {
-        $passwordHash = password_hash($verifier, \PASSWORD_ARGON2ID, $this->config);
-
-        if ($passwordHash === false || $passwordHash === null) {
-            throw new \RuntimeException('Unrecoverable password hashing error.');
+        try {
+            $passwordHash = password_hash($verifier, \PASSWORD_ARGON2ID, $this->config);
+        } catch (\Throwable $e) {
+            throw new \RuntimeException('Unrecoverable password hashing error.', 0, $e);
         }
 
         return $passwordHash;

diff --git a/src/Argon2SplitTokenFactory.php b/src/Argon2SplitTokenFactory.php
@@ -24,7 +24,7 @@
 final class Argon2SplitTokenFactory extends AbstractSplitTokenFactory
 {
     /** @param array<string, int> $config */
-    public function __construct(private array $config = [], \DateInterval | string | null $defaultLifeTime = null)
+    public function __construct(private array $config = [], \DateInterval | string $defaultLifeTime = null)
     {
         parent::__construct($defaultLifeTime);
     }

diff --git a/src/FakeSplitTokenFactory.php b/src/FakeSplitTokenFactory.php
@@ -23,7 +23,7 @@ final class FakeSplitTokenFactory extends AbstractSplitTokenFactory
     public const VERIFIER = '_OR6OOnV1o8Vy_rWhDoxKNIt';
     public const FULL_TOKEN = self::SELECTOR . self::VERIFIER;
 
-    private $randomValue;
+    private string $randomValue;
 
     public static function randomInstance(): self
     {
@@ -34,7 +34,7 @@ public function __construct(string $randomValue = null, \DateInterval | string $
     {
         parent::__construct($defaultLifeTime);
 
-        $this->randomValue = $randomValue ?? hex2bin('d7351e5d4bebe0b2b298034107f6cb12a88fe463ebf8f85afce47a38e9d5d68f15cbfad6843a3128d22d');
+        $this->randomValue = $randomValue ?? ((string) hex2bin('d7351e5d4bebe0b2b298034107f6cb12a88fe463ebf8f85afce47a38e9d5d68f15cbfad6843a3128d22d'));
     }
 
     public function generate(\DateTimeImmutable | \DateInterval $expiresAt = null): SplitToken

diff --git a/src/SplitToken.php b/src/SplitToken.php
@@ -98,7 +98,7 @@ abstract class SplitToken
     private ?string $verifierHash = null;
     private ?\DateTimeImmutable $expiresAt = null;
 
-    private function __construct(HiddenString $token, string $selector, string $verifier)
+    final private function __construct(HiddenString $token, string $selector, string $verifier)
     {
         $this->token = $token;
         $this->selector = $selector;
@@ -189,7 +189,7 @@ public function token(): HiddenString
      */
     final public function matches(?SplitTokenValueHolder $token): bool
     {
-        if (SplitTokenValueHolder::isEmpty($token)) {
+        if ($token === null || SplitTokenValueHolder::isEmpty($token)) {
             return false;
         }
 
@@ -235,6 +235,8 @@ public function getExpirationTime(): ?\DateTimeImmutable
     /**
      * This method is called in create() before the verifier is hashed,
      * allowing to set-up configuration for the hashing method.
+     *
+     * @param array<string, mixed> $config
      */
     protected function configureHasher(array $config): void
     {

diff --git a/src/SplitTokenValueHolder.php b/src/SplitTokenValueHolder.php
@@ -27,11 +27,13 @@
  */
 final class SplitTokenValueHolder
 {
-    private $selector;
-    private $verifierHash;
-    private $expiresAt;
-    private $metadata = [];
+    private ?string $selector = null;
+    private ?string $verifierHash = null;
+    private ?\DateTimeImmutable $expiresAt = null;
+    /** @var array<string, scalar> */
+    private array $metadata = [];
 
+    /** @param array<string, scalar> $metadata */
     public function __construct(string $selector, string $verifierHash, \DateTimeImmutable $expiresAt = null, array $metadata = [])
     {
         $this->selector = $selector;
@@ -46,18 +48,22 @@ public static function isEmpty(?self $valueHolder): bool
             return true;
         }
 
+        // It's possible these values are empty when used as Embedded, because Embedded
+        // will always produce an object.
         return $valueHolder->selector === null || $valueHolder->verifierHash === null;
     }
 
     /**
      * Returns whether the current token (if any) can be replaced with the new token.
      *
      * This methods should only to be used to prevent setting a token when a token
-     * was already set, which has not expired, and the same metadata was given (type checked!).
+     * was already set, which has not expired, and the same metadata was given (strict checked!).
+     *
+     * @param array<string, scalar> $expectedMetadata
      */
     public static function mayReplaceCurrentToken(?self $valueHolder, array $expectedMetadata = []): bool
     {
-        if (self::isEmpty($valueHolder)) {
+        if ($valueHolder === null || self::isEmpty($valueHolder)) {
             return true;
         }
 
@@ -78,23 +84,29 @@ public function verifierHash(): ?string
         return $this->verifierHash;
     }
 
+    /** @param array<string, scalar> $metadata */
     public function withMetadata(array $metadata): self
     {
+        if (self::isEmpty($this)) {
+            throw new \RuntimeException('Incomplete TokenValueHolder.');
+        }
+
         return new self($this->selector, $this->verifierHash, $this->expiresAt, $metadata);
     }
 
+    /** @return array<string, scalar> */
     public function metadata(): array
     {
         return $this->metadata ?? [];
     }
 
-    public function isExpired(\DateTimeImmutable $datetime = null): bool
+    public function isExpired(\DateTimeImmutable $now = null): bool
     {
         if ($this->expiresAt === null) {
             return false;
         }
 
-        return $this->expiresAt->getTimestamp() < ($datetime ?? new \DateTimeImmutable())->getTimestamp();
+        return $this->expiresAt->getTimestamp() < ($now ?? new \DateTimeImmutable())->getTimestamp();
     }
 
     public function expiresAt(): ?\DateTimeImmutable
@@ -106,6 +118,7 @@ public function expiresAt(): ?\DateTimeImmutable
      * Compares if both objects are the same.
      *
      * Warning this method leaks timing information and the expiration date is ignored!
+     * This method should only be used to check if a new token is provided.
      */
     public function equals(self $other): bool
     {

diff --git a/tests/Argon2SplitTokenFactoryTest.php b/tests/Argon2SplitTokenFactoryTest.php
@@ -62,6 +62,7 @@ public function it_generates_with_default_expiration_as_string(): void
 
     private static function assertExpirationEquals(string $expected, SplitToken $actual): void
     {
+        self::assertNotNull($actual->getExpirationTime());
         self::assertSame($expected, $actual->getExpirationTime()->format('Y-m-d\TH:i:s'));
     }
 

diff --git a/tests/Argon2SplitTokenTest.php b/tests/Argon2SplitTokenTest.php
@@ -24,16 +24,16 @@ final class Argon2SplitTokenTest extends TestCase
     private const FULL_TOKEN = '1zUeXUvr4LKymANBB_bLEqiP5GPr-Pha_OR6OOnV1o8Vy_rWhDoxKNIt';
     private const SELECTOR = '1zUeXUvr4LKymANBB_bLEqiP5GPr-Pha';
 
-    private static $randValue;
+    private static HiddenString $randValue;
 
     #[BeforeClass]
-    public static function createRandomBytes()
+    public static function createRandomBytes(): void
     {
-        self::$randValue = new HiddenString(hex2bin('d7351e5d4bebe0b2b298034107f6cb12a88fe463ebf8f85afce47a38e9d5d68f15cbfad6843a3128d22d'), false, true);
+        self::$randValue = new HiddenString((string) hex2bin('d7351e5d4bebe0b2b298034107f6cb12a88fe463ebf8f85afce47a38e9d5d68f15cbfad6843a3128d22d'), false, true);
     }
 
     #[Test]
-    public function it_validates_the_correct_length_less()
+    public function it_validates_the_correct_length_less(): void
     {
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('Invalid token-data provided, expected exactly 42 bytes.');
@@ -42,7 +42,7 @@ public function it_validates_the_correct_length_less()
     }
 
     #[Test]
-    public function it_validates_the_correct_length_more()
+    public function it_validates_the_correct_length_more(): void
     {
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('Invalid token-data provided, expected exactly 42 bytes.');
@@ -51,7 +51,7 @@ public function it_validates_the_correct_length_more()
     }
 
     #[Test]
-    public function it_validates_the_correct_length_from_string_less()
+    public function it_validates_the_correct_length_from_string_less(): void
     {
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('Invalid token provided.');
@@ -60,7 +60,7 @@ public function it_validates_the_correct_length_from_string_less()
     }
 
     #[Test]
-    public function it_validates_the_correct_length_from_string_more()
+    public function it_validates_the_correct_length_from_string_more(): void
     {
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('Invalid token provided.');
@@ -69,7 +69,7 @@ public function it_validates_the_correct_length_from_string_more()
     }
 
     #[Test]
-    public function it_creates_a_split_token_without_id()
+    public function it_creates_a_split_token_without_id(): void
     {
         $splitToken = SplitToken::create(self::$randValue);
 
@@ -78,7 +78,7 @@ public function it_creates_a_split_token_without_id()
     }
 
     #[Test]
-    public function it_creates_a_split_token_with_id()
+    public function it_creates_a_split_token_with_id(): void
     {
         $splitToken = SplitToken::create($fullToken = self::$randValue);
 
@@ -87,7 +87,7 @@ public function it_creates_a_split_token_with_id()
     }
 
     #[Test]
-    public function it_compares_two_split_tokens()
+    public function it_compares_two_split_tokens(): void
     {
         $splitToken1 = SplitToken::create(self::$randValue);
 
@@ -97,43 +97,46 @@ public function it_compares_two_split_tokens()
     }
 
     #[Test]
-    public function it_creates_a_split_token_with_custom_config()
+    public function it_creates_a_split_token_with_custom_config(): void
     {
         $splitToken = SplitToken::create(self::$randValue, [
             'memory_cost' => 512,
             'time_cost' => 1,
             'threads' => 1,
         ]);
 
-        self::assertMatchesRegularExpression('/^\$argon2[id]+\$v=19\$m=512,t=1,p=1/', $splitToken->toValueHolder()->verifierHash());
+        self::assertNotNull($hash = $splitToken->toValueHolder()->verifierHash());
+        self::assertMatchesRegularExpression('/^\$argon2id+\$v=19\$m=512,t=1,p=1/', $hash);
     }
 
     #[Test]
-    public function it_produces_a_split_token_value_holder()
+    public function it_produces_a_split_token_value_holder(): void
     {
         $splitToken = SplitToken::create(self::$randValue);
 
         $value = $splitToken->toValueHolder();
 
         self::assertEquals($splitToken->selector(), $value->selector());
-        self::assertStringStartsWith('$argon2i', $value->verifierHash());
+        self::assertNotNull($hash = $splitToken->toValueHolder()->verifierHash());
+        self::assertStringStartsWith('$argon2id', $hash);
         self::assertEquals([], $value->metadata());
         self::assertFalse($value->isExpired());
         self::assertFalse($value->isExpired(new \DateTimeImmutable('-5 minutes')));
     }
 
     #[Test]
-    public function it_produces_a_split_token_value_holder_with_metadata()
+    public function it_produces_a_split_token_value_holder_with_metadata(): void
     {
         $splitToken = SplitToken::create(self::$randValue);
         $value = $splitToken->toValueHolder(['he' => 'now']);
 
-        self::assertStringStartsWith('$argon2i', $value->verifierHash());
+        self::assertNotNull($hash = $splitToken->toValueHolder()->verifierHash());
+        self::assertStringStartsWith('$argon2id', $hash);
         self::assertEquals(['he' => 'now'], $value->metadata());
     }
 
     #[Test]
-    public function it_produces_a_split_token_value_holder_with_expiration()
+    public function it_produces_a_split_token_value_holder_with_expiration(): void
     {
         $date = new \DateTimeImmutable('+5 minutes');
         $splitToken = SplitToken::create($fullToken = self::$randValue)->expireAt($date);
@@ -146,7 +149,7 @@ public function it_produces_a_split_token_value_holder_with_expiration()
     }
 
     #[Test]
-    public function it_reconstructs_from_string()
+    public function it_reconstructs_from_string(): void
     {
         $splitTokenReconstituted = SplitToken::fromString(self::FULL_TOKEN);
 
@@ -155,7 +158,7 @@ public function it_reconstructs_from_string()
     }
 
     #[Test]
-    public function it_fails_when_creating_holder_with_string_constructed()
+    public function it_fails_when_creating_holder_with_string_constructed(): void
     {
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('toValueHolder() does not work with a SplitToken object when created with fromString().');
@@ -164,7 +167,7 @@ public function it_fails_when_creating_holder_with_string_constructed()
     }
 
     #[Test]
-    public function it_verifies_split_token()
+    public function it_verifies_split_token(): void
     {
         // Stored.
         $splitTokenHolder = SplitToken::create(self::$randValue)->toValueHolder();
@@ -176,15 +179,15 @@ public function it_verifies_split_token()
     }
 
     #[Test]
-    public function it_verifies_split_token_from_string_and_no_current_token_set()
+    public function it_verifies_split_token_from_string_and_no_current_token_set(): void
     {
         $fromString = SplitToken::fromString(self::FULL_TOKEN);
 
         self::assertFalse($fromString->matches(null));
     }
 
     #[Test]
-    public function it_verifies_split_token_from_string_selector()
+    public function it_verifies_split_token_from_string_selector(): void
     {
         // Stored.
         $splitTokenHolder = SplitToken::create(self::$randValue)->toValueHolder();
@@ -197,7 +200,7 @@ public function it_verifies_split_token_from_string_selector()
     }
 
     #[Test]
-    public function it_verifies_split_token_from_string_with_expiration()
+    public function it_verifies_split_token_from_string_with_expiration(): void
     {
         // Stored.
         $splitTokenHolder = SplitToken::create(self::$randValue)

diff --git a/tests/FakeSplitTokenFactoryTest.php b/tests/FakeSplitTokenFactoryTest.php
@@ -68,6 +68,7 @@ public function it_generates_with_default_expiration_date(): void
 
     private static function assertExpirationEquals(string $expected, SplitToken $actual): void
     {
+        self::assertNotNull($actual->getExpirationTime());
         self::assertSame($expected, $actual->getExpirationTime()->format('Y-m-d\TH:i:s'));
     }
-Original file line number
+Diff line change
@@ Expand Up @@
         private static function assertExpirationEquals(string $expected, SplitToken $actual): void
         {
+            self::assertNotNull($actual->getExpirationTime());
             self::assertSame($expected, $actual->getExpirationTime()->format('Y-m-d\TH:i:s'));
         }
@@ Expand Down @@