Raise PHPStan's level from 3 to 4 (#535)

Author: dkarlovi

Form/Model/Authorize.php

@@ -52,12 +52,12 @@ class Authorize
      * @param bool  $accepted
      * @param array $query
      */
-    public function __construct($accepted, array $query = [])
+    public function __construct(bool $accepted, array $query = [])
     {
         foreach ($query as $key => $value) {
             $this->{$key} = $value;
         }
 
-        $this->accepted = (bool) $accepted;
+        $this->accepted = $accepted;
     }
 }

Makefile

@@ -6,7 +6,7 @@ ci: cs-full-check phpstan phpunit-coverage
 lint: cs-full-check phpstan
 
 phpstan:
-	sh -c "${QA_DOCKER_COMMAND} phpstan analyse --configuration phpstan.neon --level 3 ."
+	sh -c "${QA_DOCKER_COMMAND} phpstan analyse --configuration phpstan.neon --level 4 ."
 
 cs:
 	sh -c "${QA_DOCKER_COMMAND} php-cs-fixer fix -vvv --diff"

Security/Authentication/Provider/OAuthProvider.php

@@ -76,58 +76,57 @@ public function authenticate(TokenInterface $token)
             // TODO: this is nasty, create a proper interface here
             /** @var OAuthToken&TokenInterface&\OAuth2\Model\IOAuth2AccessToken $accessToken */
             $accessToken = $this->serverService->verifyAccessToken($tokenString);
-            if (null !== $accessToken) {
-                $scope = $accessToken->getScope();
-                $user = $accessToken->getUser();
-
-                if (null !== $user) {
-                    try {
-                        $this->userChecker->checkPreAuth($user);
-                    } catch (AccountStatusException $e) {
-                        throw new OAuth2AuthenticateException(
-                            OAuth2::HTTP_UNAUTHORIZED,
-                            OAuth2::TOKEN_TYPE_BEARER,
-                            $this->serverService->getVariable(OAuth2::CONFIG_WWW_REALM),
-                            'access_denied',
-                            $e->getMessage()
-                        );
-                    }
-
-                    $token->setUser($user);
+
+            $scope = $accessToken->getScope();
+            $user = $accessToken->getUser();
+
+            if (null !== $user) {
+                try {
+                    $this->userChecker->checkPreAuth($user);
+                } catch (AccountStatusException $e) {
+                    throw new OAuth2AuthenticateException(
+                        OAuth2::HTTP_UNAUTHORIZED,
+                        OAuth2::TOKEN_TYPE_BEARER,
+                        $this->serverService->getVariable(OAuth2::CONFIG_WWW_REALM),
+                        'access_denied',
+                        $e->getMessage()
+                    );
                 }
 
-                $roles = (null !== $user) ? $user->getRoles() : [];
+                $token->setUser($user);
+            }
+
+            $roles = (null !== $user) ? $user->getRoles() : [];
 
-                if (!empty($scope)) {
-                    foreach (explode(' ', $scope) as $role) {
-                        $roles[] = 'ROLE_'.mb_strtoupper($role);
-                    }
+            if (!empty($scope)) {
+                foreach (explode(' ', $scope) as $role) {
+                    $roles[] = 'ROLE_'.mb_strtoupper($role);
                 }
+            }
 
-                $roles = array_unique($roles, SORT_REGULAR);
-
-                $token = new OAuthToken($roles);
-                $token->setAuthenticated(true);
-                $token->setToken($tokenString);
-
-                if (null !== $user) {
-                    try {
-                        $this->userChecker->checkPostAuth($user);
-                    } catch (AccountStatusException $e) {
-                        throw new OAuth2AuthenticateException(
-                            OAuth2::HTTP_UNAUTHORIZED,
-                            OAuth2::TOKEN_TYPE_BEARER,
-                            $this->serverService->getVariable(OAuth2::CONFIG_WWW_REALM),
-                            'access_denied',
-                            $e->getMessage()
-                        );
-                    }
-
-                    $token->setUser($user);
+            $roles = array_unique($roles, SORT_REGULAR);
+
+            $token = new OAuthToken($roles);
+            $token->setAuthenticated(true);
+            $token->setToken($tokenString);
+
+            if (null !== $user) {
+                try {
+                    $this->userChecker->checkPostAuth($user);
+                } catch (AccountStatusException $e) {
+                    throw new OAuth2AuthenticateException(
+                        OAuth2::HTTP_UNAUTHORIZED,
+                        OAuth2::TOKEN_TYPE_BEARER,
+                        $this->serverService->getVariable(OAuth2::CONFIG_WWW_REALM),
+                        'access_denied',
+                        $e->getMessage()
+                    );
                 }
 
-                return $token;
+                $token->setUser($user);
             }
+
+            return $token;
         } catch (OAuth2ServerException $e) {
             throw new AuthenticationException('OAuth2 authentication failed', 0, $e);
         }

Storage/OAuthStorage.php

@@ -164,14 +164,11 @@ public function checkUserCredentials(IOAuth2Client $client, $username, $password
             return false;
         }
 
-        if (null !== $user) {
-            $encoder = $this->encoderFactory->getEncoder($user);
-
-            if ($encoder->isPasswordValid($user->getPassword(), $password, $user->getSalt())) {
-                return [
-                    'data' => $user,
-                ];
-            }
+        $encoder = $this->encoderFactory->getEncoder($user);
+        if ($encoder->isPasswordValid($user->getPassword(), $password, $user->getSalt())) {
+            return [
+                'data' => $user,
+            ];
         }
 
         return false;

Tests/Form/Type/AuthorizeFormTypeTest.php

@@ -44,7 +44,7 @@ protected function setUp()
 
     public function testSubmit()
     {
-        $accepted = 'true';
+        $accepted = true;
         $formData = [
             'client_id' => '1',
             'response_type' => 'code',
@@ -61,7 +61,7 @@ public function testSubmit()
 
         $this->assertTrue($form->isSynchronized());
         $this->assertSame($authorize, $form->getData());
-        $this->assertSame((bool) $accepted, $authorize->accepted);
+        $this->assertSame($accepted, $authorize->accepted);
 
         $view = $form->createView();
         $children = $view->children;

Tests/Storage/OAuthStorageTest.php

@@ -18,6 +18,7 @@
 use FOS\OAuthServerBundle\Model\Client;
 use FOS\OAuthServerBundle\Model\RefreshToken;
 use FOS\OAuthServerBundle\Storage\OAuthStorage;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\User\UserInterface;
 
 class OAuthStorageTest extends \PHPUnit\Framework\TestCase
@@ -339,6 +340,13 @@ public function testCheckUserCredentialsCatchesAuthenticationExceptions()
     {
         $client = new Client();
 
+        $this->userProvider
+            ->expects(self::once())
+            ->method('loadUserByUsername')
+            ->with('Joe')
+            ->willThrowException(new AuthenticationException('No such user'))
+        ;
+
         $result = $this->storage->checkUserCredentials($client, 'Joe', 'baz');
 
         $this->assertFalse($result);
@@ -427,7 +435,7 @@ public function testCheckUserCredentialsReturnsFalseIfUserNotExist()
         $this->userProvider->expects($this->once())
             ->method('loadUserByUsername')
             ->with('Joe')
-            ->will($this->returnValue(null))
+            ->willThrowException(new AuthenticationException('No such user'))
         ;
 
         $this->assertFalse($this->storage->checkUserCredentials($client, 'Joe', 'baz'));