Completed
Pull Request — master (#24)
by Christopher
08:17
created

Authentication::getMeta()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 4
Code Lines 2

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 1
eloc 2
c 0
b 0
f 0
nc 1
nop 0
dl 0
loc 4
rs 10
1
<?php
2
3
namespace TechWilk\Rota;
4
5
use DateTime;
6
use DateTimeZone;
7
use Exception;
8
use Psr\Container\ContainerInterface;
9
use Psr\Http\Message\ResponseInterface;
10
use Psr\Http\Message\ServerRequestInterface;
11
use TechWilk\Rota\AuthProvider\CallbackInterface;
12
use TechWilk\Rota\AuthProvider\UsernamePasswordInterface;
13
use TechWilk\Rota\Exception\UnknownUserException;
14
15
class Authentication
16
{
17
    protected $container;
18
    protected $authProvider;
19
    protected $routesWhitelist;
20
21
    public function __construct(ContainerInterface $container, AuthProviderInterface $authProvider, $routesWhitelist)
22
    {
23
        $this->container = $container;
24
        $this->authProvider = $authProvider;
25
        $this->routesWhitelist = $routesWhitelist;
26
    }
27
28
    public function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next)
0 ignored issues
show
Coding Style introduced by
__invoke uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
29
    {
30
        /* Skip auth if uri is whitelisted. */
31
        if ($this->uriInWhitelist($request)) {
32
            $response = $next($request, $response);
33
34
            return $response;
35
        }
36
37
        if ($this->isUserLoggedIn()) {
38
            $response = $next($request, $response);
39
        } else {
40
            $_SESSION['urlRedirect'] = strval($request->getUri());
41
            $router = $this->container->get('router');
42
43
            return $response->withStatus(302)->withHeader('Location', $router->pathFor('login'));
44
        }
45
46
        return $response;
47
    }
48
49
    private function uriInWhitelist(ServerRequestInterface $request)
50
    {
51
        $route = $request->getAttribute('route');
52
        if (!isset($route)) {
53
            return false;
54
        }
55
56
        return in_array($route->getName(), $this->routesWhitelist);
57
    }
58
59
    public function isUserLoggedIn()
0 ignored issues
show
Coding Style introduced by
isUserLoggedIn uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
60
    {
61
        return isset($_SESSION['userId']);
62
    }
63
64
    public function loginAttempt(EmailAddress $email, $password)
65
    {
66
        if (!$this->numberOfLoginAttemptsIsOk($email)) {
67
            throw new Exception('Too many attempts.');
68
            return false;
0 ignored issues
show
Unused Code introduced by
return false; does not seem to be reachable.

This check looks for unreachable code. It uses sophisticated control flow analysis techniques to find statements which will never be executed.

Unreachable code is most often the result of return, die or exit statements that have been added for debug purposes.

function fx() {
    try {
        doSomething();
        return true;
    }
    catch (\Exception $e) {
        return false;
    }

    return false;
}

In the above example, the last return false will never be executed, because a return statement has already been met in every possible execution path.

Loading history...
69
        }
70
71
        if ($this->authProvider->checkCredentials($email, $password) !== true) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method checkCredentials() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth, TechWilk\Rota\AuthProvid...rd\UsernamePasswordAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
72
            $this->logFailedLoginAttempt($email);
73
74
            return false;
75
        }
76
77
        switch ($this->authProvider->getAuthProviderSlug()) {
78
            case 'onebody':
79
                if (is_null($this->authProvider->getUserId())) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getUserId() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth, TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
80
                    return false;
81
                }
82
                $socialAuth = SocialAuthQuery::create()
83
                    ->filterByPlatform($this->authProvider->getAuthProviderSlug())
84
                    ->filterBySocialId($this->authProvider->getUserId())
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getUserId() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth, TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
85
                    ->filterByRevoked(false)
86
                    ->findOne();
87
                if (is_null($socialAuth)) {
88
                    $user = UserQuery::create()->filterByEmail($email)->findOne();
89
                    if (!is_null($user)) {
90
                        $socialAuth = new SocialAuth();
91
                        $socialAuth->setUser($user);
92
                        $socialAuth->setPlatform('onebody');
93
                        $socialAuth->setSocialId($this->authProvider->getUserId());
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getUserId() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth, TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
94
                    }
95
                }
96
                if (!is_null($socialAuth)) {
97
                    $socialAuth->setMeta($this->authProvider->getMeta());
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getMeta() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth, TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
98
                    $socialAuth->save();
99
                    $user = $socialAuth->getUser();
100
                }
101
            break;
102
            default:
103
                $user = UserQuery::create()->filterByEmail($email)->findOne();
104
            break;
105
        }
106
107
        return $this->loginSuccess($user);
0 ignored issues
show
Bug introduced by
The variable $user does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
108
    }
109
110
    private function loginSuccess(User $user)
0 ignored issues
show
Coding Style introduced by
loginSuccess uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
111
    {
112
        if (is_null($user)) {
113
            throw new UnknownUserException('User not found in the database.');
114
        }
115
116
        $_SESSION['userId'] = $user->getId();
117
        $user->setLastLogin(new DateTime());
118
        $user->save();
119
120
        return true;
121
    }
122
123
    private function numberOfLoginAttemptsIsOk($username)
0 ignored issues
show
Coding Style introduced by
numberOfLoginAttemptsIsOk uses the super-global variable $_SERVER which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
124
    {
125
        $numberOfAllowedAttempts = 8;
126
        $lockOutInterval = 15; // mins
127
128
        $date = new DateTime("-$lockOutInterval minutes");
129
        $date->setTimezone(new DateTimeZone('UTC'));
130
131
        // check ip address
132
        if (isset($_SERVER['REMOTE_ADDR'])) {
133
            $loginFailures = LoginFailureQuery::create()
134
                ->filterByIpAddress($_SERVER['REMOTE_ADDR'])
135
                ->filterByTimestamp(['min' => $date])
136
                ->count();
137
138
            if ($loginFailures >= $numberOfAllowedAttempts) {
139
                $this->logFailedLoginAttempt($username);
140
141
                return false;
142
            }
143
        }
144
145
        // check user account
146
        $loginFailures = LoginFailureQuery::create()
147
            ->filterByUsername($username)
148
            ->filterByTimestamp(['min' => $date])
149
            ->count();
150
151
        if ($loginFailures >= $numberOfAllowedAttempts) {
152
            $this->logFailedLoginAttempt($username);
153
154
            return false;
155
        }
156
157
        return true;
158
    }
159
160
    private function logFailedLoginAttempt($username)
0 ignored issues
show
Coding Style introduced by
logFailedLoginAttempt uses the super-global variable $_SERVER which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
161
    {
162
        $f = new LoginFailure();
163
        $f->setUsername($username);
164
        $ip = isset($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : '0.0.0.0';
165
        $f->setIpAddress($ip);
166
        $f->save();
167
    }
168
169
    public function currentUser()
0 ignored issues
show
Coding Style introduced by
currentUser uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
170
    {
171
        $userId = $_SESSION['userId'];
172
173
        return UserQuery::create()->findPK($userId);
174
    }
175
176
    public function getResetPasswordUrl()
177
    {
178
        if ($this->authProvider instanceof UsernamePasswordInterface) {
179
            return $this->authProvider->getResetPasswordUrl();
180
        }
181
182
        return '';
183
    }
184
185
    public function getCallbackUrl()
186
    {
187
        return $this->authProvider->getCallbackUrl();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getCallbackUrl() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
188
    }
189
190
    public function verifyCallback($args)
191
    {
192
        if ($this->authProvider->verifyCallback($args)) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method verifyCallback() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
193
            switch ($this->authProvider->getAuthProviderSlug()) {
194
                case 'facebook':
195
                    $socialAuth = SocialAuthQuery::create()
196
                        ->filterByPlatform($this->authProvider->getAuthProviderSlug())
197
                        ->filterBySocialId($this->authProvider->getUserId())
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getUserId() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth, TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
198
                        ->filterByRevoked(false)
199
                        ->findOne();
200
201
                    if (!is_null($socialAuth)) {
202
                        //$socialAuth->setMeta($this->authProvider->getMeta());
0 ignored issues
show
Unused Code Comprehensibility introduced by
77% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
203
                        //$socialAuth->save();
0 ignored issues
show
Unused Code Comprehensibility introduced by
84% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
204
                        $user = $socialAuth->getUser();
205
                    }
206
                break;
207
            }
208
209
            return $this->loginSuccess($user);
0 ignored issues
show
Bug introduced by
The variable $user does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
210
        }
211
212
        return false;
213
    }
214
215
    public function getAuthProviderSlug()
216
    {
217
        return $this->authProvider->getAuthProviderSlug();
218
    }
219
220
    public function isCallback()
221
    {
222
        return $this->authProvider instanceof CallbackInterface;
223
    }
224
225
    public function isCredential()
226
    {
227
        return $this->authProvider instanceof UsernamePasswordInterface;
228
    }
229
230
    public function getSocialUserId()
231
    {
232
        return $this->authProvider->getUserId();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getUserId() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth, TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
233
    }
234
235
    public function getMeta()
236
    {
237
        return $this->authProvider->getMeta();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechWilk\Rota\AuthProviderInterface as the method getMeta() does only exist in the following implementations of said interface: TechWilk\Rota\AuthProvider\Callback\FacebookAuth, TechWilk\Rota\AuthProvid...amePassword\OneBodyAuth.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
238
    }
239
}
240