Completed
Push — develop ( dbf1eb...cd67db )
by
unknown
24:15 queued 13:09
created

IndexController   B

Complexity

Total Complexity 47

Size/Duplication

Total Lines 403
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 15

Importance

Changes 2
Bugs 0 Features 0
Metric Value
wmc 47
c 2
b 0
f 0
lcom 1
cbo 15
dl 0
loc 403
rs 7.4481

6 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 7 1
D indexAction() 0 97 15
C loginAction() 0 58 9
D loginExternAction() 0 103 13
B groupAction() 0 63 7
A logoutAction() 0 13 2

How to fix   Complexity   

Complex Class

Complex classes like IndexController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use IndexController, and based on these observations, apply Extract Interface, too.

1
<?php
2
/**
3
 * YAWIK
4
 *
5
 * @filesource
6
 * @copyright (c) 2013-2015 Cross Solution (http://cross-solution.de)
7
 * @license   MIT
8
 */
9
10
/** Auth controller */
11
namespace Auth\Controller;
12
13
use Auth\AuthenticationService;
14
use Auth\Service\Exception;
15
use Auth\Options\ModuleOptions;
16
use Zend\Mvc\Controller\AbstractActionController;
17
use Zend\Log\LoggerInterface;
18
use Zend\View\Model\ViewModel;
19
use Zend\View\Model\JsonModel;
20
use Zend\Stdlib\Parameters;
21
22
/**
23
 * Main Action Controller for Authentication module.
24
 */
25
class IndexController extends AbstractActionController
26
{
27
    /**
28
     * @var AuthenticationService
29
     */
30
    protected $auth;
31
32
    /**
33
     * @var
34
     */
35
    protected $loginForm;
36
37
    /**
38
     * @var LoggerInterface
39
     */
40
    protected $logger;
41
42
    /**
43
     * @var ModuleOptions
44
     */
45
    protected $options;
46
47
    /**
48
     * @param $auth  AuthenticationService
49
     * @param $logger LoggerInterface
50
     * @param $loginForm
51
     * @param $options
52
     */
53
    public function __construct(AuthenticationService $auth, LoggerInterface $logger, $loginForm, $options)
54
    {
55
        $this->auth=$auth;
56
        $this->loginForm=$loginForm;
57
        $this->logger=$logger;
58
        $this->options = $options;
59
    }
60
61
    /**
62
     * Login with username and password
63
     */
64
    public function indexAction()
65
    {
66
        if ($this->auth->hasIdentity()) {
67
            return $this->redirect()->toRoute('lang');
68
        }
69
70
        $viewModel = new ViewModel();
71
        $services  = $this->getServiceLocator();
72
        $form      = $this->loginForm;
73
        
74
        if ($this->request->isPost()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method isPost() does only exist in the following implementations of said interface: ZendTest\Mvc\Controller\TestAsset\Request, Zend\Http\PhpEnvironment\Request, Zend\Http\Request.

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...
75
            $data                          = $this->params()->fromPost();
76
            $adapter                       = $services->get('Auth/Adapter/UserLogin');
77
            // inject suffixes via shared Events
78
            $loginSuffix                   = '';
79
            // @TODO: replace this by the Plugin LoginFilter
80
            $e                             = $this->getEvent();
81
            $loginSuffixResponseCollection = $this->getEventManager()->trigger('login.getSuffix', $e);
82
            if (!$loginSuffixResponseCollection->isEmpty()) {
83
                $loginSuffix = $loginSuffixResponseCollection->last();
84
            }
85
86
            $form->setData($data);
87
            if (array_key_exists('credentials', $data) &&
88
                array_key_exists('login', $data['credentials']) &&
89
                array_key_exists('credential', $data['credentials'])) {
90
                $adapter->setIdentity($data['credentials']['login'] . $loginSuffix)
91
                    ->setCredential($data['credentials']['credential']);
92
            }
93
            
94
            $auth       = $this->auth;
95
            $result     = $auth->authenticate($adapter);
0 ignored issues
show
Documentation introduced by
$adapter is of type object|array, but the function expects a null|object<Zend\Authent...apter\AdapterInterface>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
96
            
97
            
98
            if ($result->isValid()) {
99
                $user = $auth->getUser();
100
                $settings = $user->getSettings('Core');
101
                $language = $settings->localization->language;
102
                if (!$language) {
103
                    $headers = $this->getRequest()->getHeaders();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getHeaders() does only exist in the following implementations of said interface: ZendTest\Mvc\Controller\TestAsset\Request, Zend\Http\PhpEnvironment\Request, Zend\Http\Request.

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...
104
                    if ($headers->has('Accept-Language')) {
105
                        $locales = $headers->get('Accept-Language')->getPrioritized();
106
                        $language  = $locales[0]->type;
107
                    } else {
108
                        $language = 'en';
109
                    }
110
                }
111
                $this->logger->info('User ' . $user->login . ' logged in');
0 ignored issues
show
Documentation introduced by
The property $login is declared protected in Auth\Entity\User. Since you implemented __get(), maybe consider adding a @property or @property-read annotation. This makes it easier for IDEs to provide auto-completion.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
112
                
113
                $ref = $this->params()->fromQuery('ref', false);
114
115
                if ($ref) {
116
                    $ref = urldecode($ref);
117
                    $url = preg_replace('~/[a-z]{2}(/|$)~', '/' . $language . '$1', $ref);
118
                    $url = $this->getRequest()->getBasePath() . $url;
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getBasePath() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request.

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...
119
                } else {
120
                    $urlHelper = $services->get('ViewHelperManager')->get('url');
121
                    $url = $urlHelper('lang', array('lang' => $language));
122
                }
123
                $this->notification()->success(/*@translate*/ 'You are now logged in.');
0 ignored issues
show
Documentation Bug introduced by
The method notification does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
124
                return $this->redirect()->toUrl($url);
125
                
126
            } else {
127
                $loginName = $data['credentials']['login'];
128
                if (!empty($loginSuffix)) {
129
                    $loginName = $loginName . ' (' . $loginName . $loginSuffix . ')';
130
                }
131
                $this->logger->info('Failed to authenticate User ' . $loginName );
132
                $this->notification()->danger(/*@translate*/ 'Authentication failed.');
0 ignored issues
show
Documentation Bug introduced by
The method notification does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
133
            }
134
        }
135
        
136
        $ref = $this->params()->fromQuery('ref', false);
137
        
138
        if ($ref) {
139
            $req = $this->params()->fromQuery('req', false);
140
            if ($req) {
141
                $this->getResponse()->setStatusCode(403);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\ResponseInterface as the method setStatusCode() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Response, Zend\Http\Response, Zend\Http\Response\Stream.

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...
142
                $viewModel->setVariable('required', true);
143
            }
144
            $viewModel->setVariable('ref', $ref);
145
        }
146
147
        $allowRegister = $this->options->getEnableRegistration();
148
        $allowResetPassword = $this->options->getEnableResetPassword();
149
        if (isset($allowRegister)) {
150
            $viewModel->setVariables(
151
                [
152
                    'allowRegister' => $allowRegister,
153
                    'allowResetPassword' => $allowResetPassword
154
                ]
155
            );
156
        }
157
        
158
        $viewModel->setVariable('form', $form);
159
        return $viewModel;
160
    }
161
    
162
    /**
163
     * Login with HybridAuth
164
     *
165
     * Passed in Params:
166
     * - provider: HybridAuth provider identifier.
167
     *
168
     * Redirects To: Route 'home'
169
     */
170
    public function loginAction()
171
    {
172
        $ref = urldecode($this->getRequest()->getBasePath().$this->params()->fromQuery('ref'));
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getBasePath() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request.

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...
173
        $provider = $this->params('provider', '--keiner--');
174
        $hauth = $this->getServiceLocator()->get('HybridAuthAdapter');
175
        $hauth->setProvider($provider);
176
        $auth = $this->auth;
177
        $result = $auth->authenticate($hauth);
0 ignored issues
show
Documentation introduced by
$hauth is of type object|array, but the function expects a null|object<Zend\Authent...apter\AdapterInterface>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
178
        $resultMessage = $result->getMessages();
179
180
        if (array_key_exists('firstLogin', $resultMessage) && $resultMessage['firstLogin'] === true) {
181
            try {
182
                $user          = $auth->getUser();
183
                $password      = substr(md5(uniqid()), 0, 6);
184
                $login         = uniqid() . ($this->options->auth_suffix != "" ? '@' . $this->options->auth_suffix : '');
0 ignored issues
show
Documentation introduced by
The property auth_suffix does not exist on object<Auth\Options\ModuleOptions>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
185
                $externalLogin = isset($user->login)?$user->login:'-- not communicated --';
0 ignored issues
show
Documentation introduced by
The property $login is declared protected in Auth\Entity\User. Since you implemented __get(), maybe consider adding a @property or @property-read annotation. This makes it easier for IDEs to provide auto-completion.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
186
                $this->logger->debug('first login via ' . $provider . ' as: ' . $externalLogin);
187
188
                $user->login=$login;
0 ignored issues
show
Documentation introduced by
The property $login is declared protected in Auth\Entity\User. Since you implemented __set(), maybe consider adding a @property or @property-write annotation. This makes it easier for IDEs to provide auto-completion.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
189
                $user->setPassword($password);
0 ignored issues
show
Unused Code introduced by
The call to the method Auth\Entity\AnonymousUser::setPassword() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
190
                $user->role = $this->options->getRole();
0 ignored issues
show
Documentation introduced by
The property $role is declared protected in Auth\Entity\User. Since you implemented __set(), maybe consider adding a @property or @property-write annotation. This makes it easier for IDEs to provide auto-completion.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
191
192
                $mail = $this->mailer('htmltemplate');
0 ignored issues
show
Documentation Bug introduced by
The method mailer does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
193
                $mail->setTemplate('mail/first-socialmedia-login');
194
                $mail->setSubject($this->options->getMailSubjectRegistration());
195
                $mail->setVariables(
196
                    array(
197
                                'displayName'=> $user->getInfo()->getDisplayName(),
198
                                'provider' => $provider,
199
                                'login' => $login,
200
                                'password' => $password,
201
                    )
202
                );
203
                $mail->addTo($user->getInfo()->getEmail());
204
205
                $loggerId = $login . ' (' . $provider . ': ' . $externalLogin . ')';
206
                if (isset($mail) && $this->mailer($mail)) {
0 ignored issues
show
Documentation Bug introduced by
The method mailer does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
207
                    $this->logger->info('Mail first-login for ' . $loggerId . ' sent to ' . $user->getInfo()->getEmail());
208
                } else {
209
                    $this->logger->warn('No Mail was sent for ' . $loggerId);
210
                }
211
            } catch (\Exception $e) {
212
                $this->logger->crit($e);
213
                $this->notification()->danger(
0 ignored issues
show
Documentation Bug introduced by
The method notification does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
214
                    /*@translate*/ 'An unexpected error has occurred, please contact your system administrator'
215
                );
216
            }
217
        }
218
        
219
        $user = $auth->getUser();
220
        $this->logger->info('User ' . $auth->getUser()->getInfo()->getDisplayName() . ' logged in via ' . $provider);
221
        $settings = $user->getSettings('Core');
222
        if (null !== $settings->localization->language) {
223
            $basePath = $this->getRequest()->getBasePath();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getBasePath() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request.

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...
224
            $ref = preg_replace('~^'.$basePath . '/[a-z]{2}(/)?~', $basePath . '/' . $settings->localization->language . '$1', $ref);
225
        }
226
        return $this->redirect()->toUrl($ref);
227
    }
228
    
229
    /**
230
     * Login via an external Application. This will get obsolet as soon we'll have a full featured Rest API.
231
     *
232
     * Passed in params:
233
     * - appKey: Application identifier key
234
     * - user: Name of the user to log in
235
     * - pass: Password of the user to log in
236
     *
237
     * Returns an json response with the session-id.
238
     * Non existant users will be created!
239
     *
240
     */
241
    public function loginExternAction()
242
    {
243
        $services   = $this->getServiceLocator();
244
        $adapter    = $services->get('ExternalApplicationAdapter');
245
        $appKey     = $this->params()->fromPost('appKey');
246
247
        $adapter->setIdentity($this->params()->fromPost('user'))
248
                ->setCredential($this->params()->fromPost('pass'))
249
                ->setApplicationKey($appKey);
250
        
251
        $auth       = $this->auth;
252
        $result     = $auth->authenticate($adapter);
0 ignored issues
show
Documentation introduced by
$adapter is of type object|array, but the function expects a null|object<Zend\Authent...apter\AdapterInterface>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
253
        
254
        if ($result->isValid()) {
255
            $this->logger->info(
256
                'User ' . $this->params()->fromPost('user') .
257
                ' logged via ' . $appKey
258
            );
259
            
260
            // the external login may include some parameters for an update
261
            $updateParams = $this->params()->fromPost();
262
            unset($updateParams['user'], $updateParams['pass'], $updateParams['appKey']);
263
            $resultMessage = $result->getMessages();
264
            $password = null;
265
            if (array_key_exists('firstLogin', $resultMessage) && $resultMessage['firstLogin'] === true) {
266
                $password = substr(md5(uniqid()), 0, 6);
267
                $updateParams['password'] = $password;
268
            }
269
            if (!empty($updateParams)) {
270
                $user = $auth->getUser();
271
                try {
272
                    foreach ($updateParams as $updateKey => $updateValue) {
273
                        if ('email' == $updateKey) {
274
                            $user->info->email = $updateValue;
0 ignored issues
show
Documentation introduced by
The property $info is declared protected in Auth\Entity\User. Since you implemented __get(), maybe consider adding a @property or @property-read annotation. This makes it easier for IDEs to provide auto-completion.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
Bug introduced by
Accessing email on the interface Auth\Entity\InfoInterface suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
275
                        }
276
                        $user->$updateKey = $updateValue;
277
                    }
278
                } catch (Exception $e) {
0 ignored issues
show
Unused Code introduced by
catch (\Auth\Service\Exception $e) { } 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...
Coding Style Comprehensibility introduced by
Consider adding a comment why this CATCH block is empty.
Loading history...
Bug introduced by
The class Auth\Service\Exception does not exist. Did you forget a USE statement, or did you not list all dependencies?

Scrutinizer analyzes your composer.json/composer.lock file if available to determine the classes, and functions that are defined by your dependencies.

It seems like the listed class was neither found in your dependencies, nor was it found in the analyzed files in your repository. If you are using some other form of dependency management, you might want to disable this analysis.

Loading history...
279
                }
280
                $services->get('repositories')->store($user);
281
            }
282
            
283
            $resultMessage = $result->getMessages();
284
            // TODO: send a mail also when required (maybe first mail failed or email has changed)
285
            if (array_key_exists('firstLogin', $resultMessage) && $resultMessage['firstLogin'] === true) {
286
                // first external Login
287
                $userName = $this->params()->fromPost('user');
288
                $this->logger->debug('first login for User: ' .  $userName);
289
                //
290
                if (preg_match("/^(.*)@\w+$/", $userName, $realUserName)) {
291
                    $userName = $realUserName[1];
292
                }
293
294
                $mail = $this->mailer('htmltemplate'); /* @var $mail \Core\Mail\HTMLTemplateMessage */
0 ignored issues
show
Documentation Bug introduced by
The method mailer does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
295
                $apps = $this->config('external_applications');
0 ignored issues
show
Documentation Bug introduced by
The method config does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
296
                $apps = array_flip($apps);
297
                $application = isset($apps[$appKey]) ? $apps[$appKey] : null;
298
299
                $mail->setVariables(
300
                    array(
301
                    'application' => $application,
302
                    'login'=>$userName,
303
                    'password' => $password,
304
                    )
305
                );
306
                $mail->setSubject($this->options->getMailSubjectRegistration());
307
                $mail->setTemplate('mail/first-external-login');
308
                $mail->addTo($user->getInfo()->getEmail());
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...
309
310
                try {
311
                    $this->mailer($mail);
0 ignored issues
show
Documentation Bug introduced by
The method mailer does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
312
                    $this->logger->info('Mail first-login sent to ' . $userName);
313
                } catch (\Zend\Mail\Transport\Exception\ExceptionInterface $e) {
314
                    $this->logger->warn('No Mail was sent');
315
                    $this->logger->debug($e);
0 ignored issues
show
Documentation introduced by
$e is of type object<Zend\Mail\Transpo...ion\ExceptionInterface>, but the function expects a string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
316
                }
317
            }
318
319
            return new JsonModel(
320
                array(
321
                'status' => 'success',
322
                'token' => session_id()
323
                )
324
            );
325
        } else {
326
            $this->logger->info(
327
                'Failed to authenticate User ' . $this->params()->fromPost('user') .
328
                ' via ' . $this->params()->fromPost('appKey')
329
            );
330
            
331
            $this->getResponse()->setStatusCode(403);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\ResponseInterface as the method setStatusCode() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Response, Zend\Http\Response, Zend\Http\Response\Stream.

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...
332
            return new JsonModel(
333
                array(
334
                'status' => 'failure',
335
                'user' => $this->params()->fromPost('user'),
336
                'appKey' => $this->params()->fromPost('appKey'),
337
                'code'   => $result->getCode(),
338
                'messages' => $result->getMessages(),
339
                )
340
            );
341
        }
342
        
343
    }
344
    
345
    public function groupAction()
346
    {
347
        //$adapter = $this->getServiceLocator()->get('ExternalApplicationAdapter');
0 ignored issues
show
Unused Code Comprehensibility introduced by
67% 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...
348
        if (false) {
0 ignored issues
show
Bug introduced by
Avoid IF statements that are always true or false
Loading history...
349
             $this->request->setMethod('get');
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method setMethod() does only exist in the following implementations of said interface: ZendTest\Mvc\Controller\TestAsset\Request, Zend\Http\PhpEnvironment\Request, Zend\Http\Request.

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...
350
            $params = new Parameters(
351
                array(
352
                'format' => 'json',
353
                    'group' => array (
354
                        0 => 'testuser4711', 1 => 'flatscreen', 2 => 'flatscreen1', 3 => 'flatscreen2', 4 => 'flatscreen3',  5 => 'flatscreen4',
355
                        6 => 'flatscreen5', 7 => 'flatscreen6', 8 => 'flatscreen7',  9 => 'flatscreen8', 10 => 'flatscreen9'
356
                    ),
357
                    'name' => '(die) Rauscher – Unternehmensberatung & Consulting',
358
                )
359
            );
360
            $this->getRequest()->setQuery($params);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method setQuery() does only exist in the following implementations of said interface: ZendTest\Mvc\Controller\TestAsset\Request, Zend\Http\PhpEnvironment\Request, Zend\Http\Request.

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...
361
             
362
        }
363
        $auth = $this->auth;
364
        $userGrpAdmin = $auth->getUser();
365
        $this->logger->info('User ' . $auth->getUser()->getInfo()->getDisplayName());
366
        $grp = $this->params()->fromQuery('group');
0 ignored issues
show
Unused Code introduced by
$grp is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
367
        
368
        // if the request is made by an external host, add his identification-key to the name
369
        $loginSuffix = '';
370
        // @TODO: replace this by the Plugin LoginFilter
371
        $e = $this->getEvent();
372
        $loginSuffixResponseCollection = $this->getEventManager()->trigger('login.getSuffix', $e);
373
        if (!$loginSuffixResponseCollection->isEmpty()) {
374
            $loginSuffix = $loginSuffixResponseCollection->last();
375
        }
376
        // make out of the names a list of Ids
377
        $params = $this->getRequest()->getQuery();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: ZendTest\Mvc\Controller\TestAsset\Request, Zend\Http\PhpEnvironment\Request, Zend\Http\Request.

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...
378
        $groupUserId = array();
379
        $notFoundUsers = array();
380
        //$users = $this->getRepository();
0 ignored issues
show
Unused Code Comprehensibility introduced by
60% 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...
381
        $users = $this->getServiceLocator()->get('repositories')->get('Auth/User');
382
        if (!empty($params->group)) {
383
            foreach ($params->group as $grp_member) {
0 ignored issues
show
Coding Style introduced by
$grp_member does not seem to conform to the naming convention (^[a-z][a-zA-Z0-9]*$).

This check examines a number of code elements and verifies that they conform to the given naming conventions.

You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods.

Loading history...
384
                $user = $users->findByLogin($grp_member . $loginSuffix);
0 ignored issues
show
Coding Style introduced by
$grp_member does not seem to conform to the naming convention (^[a-z][a-zA-Z0-9]*$).

This check examines a number of code elements and verifies that they conform to the given naming conventions.

You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods.

Loading history...
385
                if (!empty($user)) {
386
                    $groupUserId[] = $user->id;
387
                } else {
388
                    $notFoundUsers[] = $grp_member . $loginSuffix;
0 ignored issues
show
Coding Style introduced by
$grp_member does not seem to conform to the naming convention (^[a-z][a-zA-Z0-9]*$).

This check examines a number of code elements and verifies that they conform to the given naming conventions.

You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods.

Loading history...
389
                }
390
            }
391
        }
392
        $name = $params->name;
393
        if (!empty($params->name)) {
394
            $group = $this->auth()->getUser()->getGroup($params->name, /*create*/ true);
0 ignored issues
show
Documentation Bug introduced by
The method auth does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
395
            $group->setUsers($groupUserId);
396
        }
397
        $this->logger->info(
398
            'Update Group Name: ' . $name . PHP_EOL . str_repeat(' ', 36) . 'Group Owner: ' . $userGrpAdmin->getLogin() . PHP_EOL .
399
            str_repeat(' ', 36) . 'Group Members Param: ' . implode(',', $params->group) . PHP_EOL .
400
            str_repeat(' ', 36) . 'Group Members: ' . count($groupUserId) . PHP_EOL . str_repeat(' ', 36) . 'Group Members not found: ' . implode(',', $notFoundUsers)
401
        );
402
        
403
        return new JsonModel(
404
            array(
405
            )
406
        );
407
    }
408
    
409
    /**
410
     * Logout
411
     *
412
     * Redirects To: Route 'home'
413
     */
414
    public function logoutAction()
0 ignored issues
show
Coding Style introduced by
logoutAction 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...
415
    {
416
        $auth = $this->auth;
417
        $this->logger->info('User ' . ($auth->getUser()->login==''?$auth->getUser()->info->displayName:$auth->getUser()->login) . ' logged out');
0 ignored issues
show
Documentation introduced by
The property $login is declared protected in Auth\Entity\User. Since you implemented __get(), maybe consider adding a @property or @property-read annotation. This makes it easier for IDEs to provide auto-completion.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
Documentation introduced by
The property $info is declared protected in Auth\Entity\User. Since you implemented __get(), maybe consider adding a @property or @property-read annotation. This makes it easier for IDEs to provide auto-completion.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
Bug introduced by
Accessing displayName on the interface Auth\Entity\InfoInterface suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
418
        $auth->clearIdentity();
419
        unset($_SESSION['HA::STORE']);
420
421
        $this->notification()->success(/*@translate*/ 'You are now logged out');
0 ignored issues
show
Documentation Bug introduced by
The method notification does not exist on object<Auth\Controller\IndexController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
422
        return $this->redirect()->toRoute(
423
            'lang',
424
            array('lang' => $this->params('lang'))
425
        );
426
    }
427
}
428