Completed
Push — develop ( c603db...77b219 )
by
unknown
16:24 queued 08:23
created

IndexController   C

Complexity

Total Complexity 48

Size/Duplication

Total Lines 426
Duplicated Lines 2.11 %

Coupling/Cohesion

Components 1
Dependencies 18

Importance

Changes 3
Bugs 0 Features 1
Metric Value
wmc 48
c 3
b 0
f 1
lcom 1
cbo 18
dl 9
loc 426
rs 5.3384

6 Methods

Rating   Name   Duplication   Size   Complexity  
D loginExternAction() 0 102 13
A __construct() 0 7 1
D indexAction() 9 109 15
C loginAction() 0 58 9
C groupAction() 0 69 8
A logoutAction() 0 13 2

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

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 - 2016 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 Auth\Form\Login;
17
use Auth\Form\Register;
18
use Zend\Mvc\Controller\AbstractActionController;
19
use Zend\Log\LoggerInterface;
20
use Zend\View\Model\ViewModel;
21
use Zend\View\Model\JsonModel;
22
use Zend\Stdlib\Parameters;
23
24
/**
25
 *
26
 * @method \Core\Controller\Plugin\Notification notification
27
 * @method \Core\Controller\Plugin\Mailer mailer
28
 *
29
 * Main Action Controller for Authentication module.
30
 */
31
class IndexController extends AbstractActionController
32
{
33
34
    const LOGIN='login';
35
    const REGISTER='register';
36
37
    /**
38
     * @var AuthenticationService
39
     */
40
    protected $auth;
41
42
    /**
43
     * @var array
44
     */
45
    protected $forms;
46
47
    /**
48
     * @var LoggerInterface
49
     */
50
    protected $logger;
51
52
    /**
53
     * @var ModuleOptions
54
     */
55
    protected $options;
56
57
    /**
58
     * @param $auth  AuthenticationService
59
     * @param $logger LoggerInterface
60
     * @param $forms
61
     * @param $options ModuleOptions
62
     */
63
    public function __construct(AuthenticationService $auth, LoggerInterface $logger, array $forms, $options)
64
    {
65
        $this->auth = $auth;
66
        $this->forms = $forms;
67
        $this->logger = $logger;
68
        $this->options = $options;
69
    }
70
71
    /**
72
     * Login with username and password
73
     *
74
     * @return \Zend\Http\Response|ViewModel
75
     */
76
    public function indexAction()
77
    {
78
        if ($this->auth->hasIdentity()) {
79
            return $this->redirect()->toRoute('lang');
80
        }
81
82
        $viewModel        = new ViewModel();
83
        $services         = $this->getServiceLocator();
84
85
        /* @var $loginForm Login */
86
        $loginForm        = $this->forms[self::LOGIN];
87
        /* @var $registerForm Register */
88
        $registerForm = $this->forms[self::REGISTER];
89
90
        /* @var $request \Zend\Http\Request */
91
        $request   = $this->getRequest();
92
93
        if ($request->isPost()) {
94
            $data                          = $this->params()->fromPost();
95
            $adapter                       = $services->get('Auth/Adapter/UserLogin');
96
            // inject suffixes via shared Events
97
            $loginSuffix                   = '';
98
            // @TODO: replace this by the Plugin LoginFilter
99
            $e                             = $this->getEvent();
100
            $loginSuffixResponseCollection = $this->getEventManager()->trigger('login.getSuffix', $e);
101
            if (!$loginSuffixResponseCollection->isEmpty()) {
102
                $loginSuffix = $loginSuffixResponseCollection->last();
103
            }
104
105
            $loginForm->setData($data);
106
            if (array_key_exists('credentials', $data) &&
107
                array_key_exists('login', $data['credentials']) &&
108
                array_key_exists('credential', $data['credentials'])) {
109
                $adapter->setIdentity($data['credentials']['login'] . $loginSuffix)
110
                    ->setCredential($data['credentials']['credential']);
111
            }
112
            
113
            $auth   = $this->auth;
114
            $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...
115
            
116
            
117
            if ($result->isValid()) {
118
                $user = $auth->getUser();
119
                $settings = $user->getSettings('Core');
120
                $language = $settings->localization->language;
121 View Code Duplication
                if (!$language) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
122
                    $headers = $request->getHeaders();
123
                    if ($headers->has('Accept-Language')) {
124
                        $locales = $headers->get('Accept-Language')->getPrioritized();
125
                        $language  = $locales[0]->type;
126
                    } else {
127
                        $language = 'en';
128
                    }
129
                }
130
                $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...
131
                
132
                $ref = $this->params()->fromQuery('ref', false);
133
134
                if ($ref) {
135
                    $ref = urldecode($ref);
136
                    $url = preg_replace('~/[a-z]{2}(/|$)~', '/' . $language . '$1', $ref);
137
                    $url = $request->getBasePath() . $url;
138
                } else {
139
                    $urlHelper = $services->get('ViewHelperManager')->get('url');
140
                    $url = $urlHelper('lang', array('lang' => $language));
141
                }
142
                $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...
143
                return $this->redirect()->toUrl($url);
144
            } else {
145
                $loginName = $data['credentials']['login'];
146
                if (!empty($loginSuffix)) {
147
                    $loginName = $loginName . ' (' . $loginName . $loginSuffix . ')';
148
                }
149
                $this->logger->info('Failed to authenticate User ' . $loginName);
150
                $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...
151
            }
152
        }
153
        
154
        $ref = $this->params()->fromQuery('ref', false);
155
        
156
        if ($ref) {
157
            $req = $this->params()->fromQuery('req', false);
158
            if ($req) {
159
                $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...
160
                $viewModel->setVariable('required', true);
161
            }
162
            $viewModel->setVariable('ref', $ref);
163
        }
164
165
        $allowRegister = $this->options->getEnableRegistration();
166
        $allowResetPassword = $this->options->getEnableResetPassword();
167
        if (isset($allowRegister)) {
168
            $viewModel->setVariables(
169
                [
170
                    'allowRegister' => $allowRegister,
171
                    'allowResetPassword' => $allowResetPassword
172
                ]
173
            );
174
        }
175
176
        $viewModel->setVariable('loginForm', $loginForm);
177
        $viewModel->setVariable('registerForm', $registerForm);
178
179
        /* @deprecated use loginForm instead of form in your view scripts */
180
        $viewModel->setVariable('form', $loginForm);
181
182
183
        return $viewModel;
184
    }
185
    
186
    /**
187
     * Login with HybridAuth
188
     *
189
     * Passed in Params:
190
     * - provider: HybridAuth provider identifier.
191
     *
192
     * Redirects To: Route 'home'
193
     */
194
    public function loginAction()
195
    {
196
        $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...
197
        $provider = $this->params('provider', '--keiner--');
198
        $hauth = $this->getServiceLocator()->get('HybridAuthAdapter');
199
        $hauth->setProvider($provider);
200
        $auth = $this->auth;
201
        $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...
202
        $resultMessage = $result->getMessages();
203
204
        if (array_key_exists('firstLogin', $resultMessage) && $resultMessage['firstLogin'] === true) {
205
            try {
206
                $user          = $auth->getUser();
207
                $password      = substr(md5(uniqid()), 0, 6);
208
                $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...
209
                $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...
210
                $this->logger->debug('first login via ' . $provider . ' as: ' . $externalLogin);
211
212
                $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...
213
                $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...
214
                $user->setRole($this->options->getRole());
215
216
                $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...
217
                $mail->setTemplate('mail/first-socialmedia-login');
218
                $mail->setSubject($this->options->getMailSubjectRegistration());
219
                $mail->setVariables(
220
                    array(
221
                                'displayName'=> $user->getInfo()->getDisplayName(),
222
                                'provider' => $provider,
223
                                'login' => $login,
224
                                'password' => $password,
225
                    )
226
                );
227
                $mail->addTo($user->getInfo()->getEmail());
228
229
                $loggerId = $login . ' (' . $provider . ': ' . $externalLogin . ')';
230
                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...
231
                    $this->logger->info('Mail first-login for ' . $loggerId . ' sent to ' . $user->getInfo()->getEmail());
232
                } else {
233
                    $this->logger->warn('No Mail was sent for ' . $loggerId);
234
                }
235
            } catch (\Exception $e) {
236
                $this->logger->crit($e);
237
                $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...
238
                    /*@translate*/ 'An unexpected error has occurred, please contact your system administrator'
239
                );
240
            }
241
        }
242
        
243
        $user = $auth->getUser();
244
        $this->logger->info('User ' . $auth->getUser()->getInfo()->getDisplayName() . ' logged in via ' . $provider);
245
        $settings = $user->getSettings('Core');
246
        if (null !== $settings->localization->language) {
247
            $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...
248
            $ref = preg_replace('~^'.$basePath . '/[a-z]{2}(/)?~', $basePath . '/' . $settings->localization->language . '$1', $ref);
249
        }
250
        return $this->redirect()->toUrl($ref);
251
    }
252
    
253
    /**
254
     * Login via an external Application. This will get obsolet as soon we'll have a full featured Rest API.
255
     *
256
     * Passed in params:
257
     * - appKey: Application identifier key
258
     * - user: Name of the user to log in
259
     * - pass: Password of the user to log in
260
     *
261
     * Returns an json response with the session-id.
262
     * Non existent users will be created!
263
     *
264
     */
265
    public function loginExternAction()
266
    {
267
        $services   = $this->getServiceLocator();
268
        $adapter    = $services->get('ExternalApplicationAdapter');
269
        $appKey     = $this->params()->fromPost('appKey');
270
271
        $adapter->setIdentity($this->params()->fromPost('user'))
272
                ->setCredential($this->params()->fromPost('pass'))
273
                ->setApplicationKey($appKey);
274
        
275
        $auth       = $this->auth;
276
        $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...
277
        
278
        if ($result->isValid()) {
279
            $this->logger->info(
280
                'User ' . $this->params()->fromPost('user') .
281
                ' logged via ' . $appKey
282
            );
283
            
284
            // the external login may include some parameters for an update
285
            $updateParams = $this->params()->fromPost();
286
            unset($updateParams['user'], $updateParams['pass'], $updateParams['appKey']);
287
            $resultMessage = $result->getMessages();
288
            $password = null;
289
            if (array_key_exists('firstLogin', $resultMessage) && $resultMessage['firstLogin'] === true) {
290
                $password = substr(md5(uniqid()), 0, 6);
291
                $updateParams['password'] = $password;
292
            }
293
            if (!empty($updateParams)) {
294
                $user = $auth->getUser();
295
                try {
296
                    foreach ($updateParams as $updateKey => $updateValue) {
297
                        if ('email' == $updateKey) {
298
                            $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...
299
                        }
300
                        $user->$updateKey = $updateValue;
301
                    }
302
                } 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...
303
                }
304
                $services->get('repositories')->store($user);
305
            }
306
            
307
            $resultMessage = $result->getMessages();
308
            // TODO: send a mail also when required (maybe first mail failed or email has changed)
309
            if (array_key_exists('firstLogin', $resultMessage) && $resultMessage['firstLogin'] === true) {
310
                // first external Login
311
                $userName = $this->params()->fromPost('user');
312
                $this->logger->debug('first login for User: ' .  $userName);
313
                //
314
                if (preg_match("/^(.*)@\w+$/", $userName, $realUserName)) {
315
                    $userName = $realUserName[1];
316
                }
317
318
                $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...
319
                $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...
320
                $apps = array_flip($apps);
321
                $application = isset($apps[$appKey]) ? $apps[$appKey] : null;
322
323
                $mail->setVariables(
324
                    array(
325
                    'application' => $application,
326
                    'login'=>$userName,
327
                    'password' => $password,
328
                    )
329
                );
330
                $mail->setSubject($this->options->getMailSubjectRegistration());
331
                $mail->setTemplate('mail/first-external-login');
332
                $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...
333
334
                try {
335
                    $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...
336
                    $this->logger->info('Mail first-login sent to ' . $userName);
337
                } catch (\Zend\Mail\Transport\Exception\ExceptionInterface $e) {
338
                    $this->logger->warn('No Mail was sent');
339
                    $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...
340
                }
341
            }
342
343
            return new JsonModel(
344
                array(
345
                'status' => 'success',
346
                'token' => session_id()
347
                )
348
            );
349
        } else {
350
            $this->logger->info(
351
                'Failed to authenticate User ' . $this->params()->fromPost('user') .
352
                ' via ' . $this->params()->fromPost('appKey')
353
            );
354
            
355
            $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...
356
            return new JsonModel(
357
                array(
358
                'status' => 'failure',
359
                'user' => $this->params()->fromPost('user'),
360
                'appKey' => $this->params()->fromPost('appKey'),
361
                'code'   => $result->getCode(),
362
                'messages' => $result->getMessages(),
363
                )
364
            );
365
        }
366
    }
367
    
368
    public function groupAction()
369
    {
370
        //$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...
371
        if (false) {
0 ignored issues
show
Bug introduced by
Avoid IF statements that are always true or false
Loading history...
372
            $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: 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...
373
            $params = new Parameters(
374
                array(
375
                'format' => 'json',
376
                    'group' => array(
377
                        0 => 'testuser4711', 1 => 'flatscreen', 2 => 'flatscreen1', 3 => 'flatscreen2', 4 => 'flatscreen3',  5 => 'flatscreen4',
378
                        6 => 'flatscreen5', 7 => 'flatscreen6', 8 => 'flatscreen7',  9 => 'flatscreen8', 10 => 'flatscreen9'
379
                    ),
380
                    'name' => '(die) Rauscher – Unternehmensberatung & Consulting',
381
                )
382
            );
383
            $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: 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...
384
        }
385
        $auth = $this->auth;
386
        $userGrpAdmin = $auth->getUser();
387
        $this->logger->info('User ' . $auth->getUser()->getInfo()->getDisplayName());
388
        $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...
389
        
390
        // if the request is made by an external host, add his identification-key to the name
391
        $loginSuffix = '';
392
        // @TODO: replace this by the Plugin LoginFilter
393
        $e = $this->getEvent();
394
        $loginSuffixResponseCollection = $this->getEventManager()->trigger('login.getSuffix', $e);
395
        if (!$loginSuffixResponseCollection->isEmpty()) {
396
            $loginSuffix = $loginSuffixResponseCollection->last();
397
        }
398
        // make out of the names a list of Ids
399
        $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: 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...
400
        $groupUserId = array();
401
        $notFoundUsers = array();
402
        //$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...
403
        $users = $this->getServiceLocator()->get('repositories')->get('Auth/User');
404
        if (!empty($params->group)) {
405
            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...
406
                try
407
                {
408
                    $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...
409
                    if (!empty($user)) {
410
                        $groupUserId[] = $user->id;
411
                    } else {
412
                        $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...
413
                    }
414
                }
415
                catch (\Auth\Exception\UserDeactivatedException $e)
416
                {
417
                    $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...
418
                }
419
            }
420
        }
421
        $name = $params->name;
422
        if (!empty($params->name)) {
423
            $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...
424
            $group->setUsers($groupUserId);
425
        }
426
        $this->logger->info(
427
            'Update Group Name: ' . $name . PHP_EOL . str_repeat(' ', 36) . 'Group Owner: ' . $userGrpAdmin->getLogin() . PHP_EOL .
428
            str_repeat(' ', 36) . 'Group Members Param: ' . implode(',', $params->group) . PHP_EOL .
429
            str_repeat(' ', 36) . 'Group Members: ' . count($groupUserId) . PHP_EOL . str_repeat(' ', 36) . 'Group Members not found: ' . implode(',', $notFoundUsers)
430
        );
431
        
432
        return new JsonModel(
433
            array(
434
            )
435
        );
436
    }
437
    
438
    /**
439
     * Logout
440
     *
441
     * Redirects To: Route 'home'
442
     */
443
    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...
444
    {
445
        $auth = $this->auth;
446
        $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...
447
        $auth->clearIdentity();
448
        unset($_SESSION['HA::STORE']);
449
450
        $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...
451
        return $this->redirect()->toRoute(
452
            'lang',
453
            array('lang' => $this->params('lang'))
454
        );
455
    }
456
}
457