iranianpep/code-jetter

View on GitHub
components/user/services/UserAuthentication.php

Summary

Maintainability
D
2 days
Test Coverage

removeLoggedInUserFromSession accesses the super-global variable $_SESSION.
Open

    public function removeLoggedInUserFromSession(User $user)
    {
        /**
         * model name is used to manage the case when an admin and a user logged in on the same browser
         * otherwise if one of the user logged out, the other one will be logged out as well.

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

updateLastActivityAt accesses the super-global variable $_SESSION.
Open

    public function updateLastActivityAt(User $user)
    {
        $userModel = $user->getClassNameFromNamespace();

        if (isset($_SESSION['loggedIn'][$userModel])) {

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

getAllLoggedIn accesses the super-global variable $_SESSION.
Open

    public function getAllLoggedIn(array $whiteList = null)
    {
        if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
            return false;
        }

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

getAllLoggedIn accesses the super-global variable $_SESSION.
Open

    public function getAllLoggedIn(array $whiteList = null)
    {
        if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
            return false;
        }

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

updateLastActivityAt accesses the super-global variable $_SESSION.
Open

    public function updateLastActivityAt(User $user)
    {
        $userModel = $user->getClassNameFromNamespace();

        if (isset($_SESSION['loggedIn'][$userModel])) {

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

login accesses the super-global variable $_SESSION.
Open

    public function login(User $user, $password)
    {
        $output = new Output();

        $userModel = $user->getClassNameFromNamespace();

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

removeLoggedInUserFromSession accesses the super-global variable $_SESSION.
Open

    public function removeLoggedInUserFromSession(User $user)
    {
        /**
         * model name is used to manage the case when an admin and a user logged in on the same browser
         * otherwise if one of the user logged out, the other one will be logged out as well.

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

getAllLoggedIn accesses the super-global variable $_SESSION.
Open

    public function getAllLoggedIn(array $whiteList = null)
    {
        if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
            return false;
        }

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

getAllLoggedIn accesses the super-global variable $_SESSION.
Open

    public function getAllLoggedIn(array $whiteList = null)
    {
        if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
            return false;
        }

Superglobals

Since: 0.2

Accessing a super-global variable directly is considered a bad practice. These variables should be encapsulated in objects that are provided by a framework, for instance.

Example

class Foo {
    public function bar() {
        $name = $_POST['foo'];
    }
}

Source

Function getAllLoggedIn has a Cognitive Complexity of 25 (exceeds 5 allowed). Consider refactoring.
Open

    public function getAllLoggedIn(array $whiteList = null)
    {
        if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
            return false;
        }
Severity: Minor
Found in components/user/services/UserAuthentication.php - About 3 hrs to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

The class UserAuthentication has an overall complexity of 64 which is very high. The configured complexity threshold is 50.
Open

class UserAuthentication
{
    /**
     * @param User $user
     * @param      $password

File UserAuthentication.php has 278 lines of code (exceeds 250 allowed). Consider refactoring.
Open

<?php
/**
 * Created by PhpStorm.
 * User: ehsanabbasi
 * Date: 24/02/16
Severity: Minor
Found in components/user/services/UserAuthentication.php - About 2 hrs to fix

    Method resetPassword has 46 lines of code (exceeds 25 allowed). Consider refactoring.
    Open

        public function resetPassword(User $user, $token, $password, $passwordConfirmation)
        {
            $output = new Output();
            /**
             * Start validating passwords - email & resetPasswordToken are already validated.
    Severity: Minor
    Found in components/user/services/UserAuthentication.php - About 1 hr to fix

      Method forgetPassword has 42 lines of code (exceeds 25 allowed). Consider refactoring.
      Open

          public function forgetPassword(User $user)
          {
              $output = new Output();
      
              if ($this->isUserActive($user) !== true) {
      Severity: Minor
      Found in components/user/services/UserAuthentication.php - About 1 hr to fix

        Method getAllLoggedIn has 34 lines of code (exceeds 25 allowed). Consider refactoring.
        Open

            public function getAllLoggedIn(array $whiteList = null)
            {
                if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
                    return false;
                }
        Severity: Minor
        Found in components/user/services/UserAuthentication.php - About 1 hr to fix

          Method login has 31 lines of code (exceeds 25 allowed). Consider refactoring.
          Open

              public function login(User $user, $password)
              {
                  $output = new Output();
          
                  $userModel = $user->getClassNameFromNamespace();
          Severity: Minor
          Found in components/user/services/UserAuthentication.php - About 1 hr to fix

            Function resetPassword has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
            Open

                public function resetPassword(User $user, $token, $password, $passwordConfirmation)
                {
                    $output = new Output();
                    /**
                     * Start validating passwords - email & resetPasswordToken are already validated.
            Severity: Minor
            Found in components/user/services/UserAuthentication.php - About 45 mins to fix

            Cognitive Complexity

            Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

            A method's cognitive complexity is based on a few simple rules:

            • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
            • Code is considered more complex for each "break in the linear flow of the code"
            • Code is considered more complex when "flow breaking structures are nested"

            Further reading

            Avoid too many return statements within this method.
            Open

                    return $loggedIn[$roles[$routeInfo->getAccessRole()]['user']];
            Severity: Major
            Found in components/user/services/UserAuthentication.php - About 30 mins to fix

              Avoid too many return statements within this method.
              Open

                      return $output;
              Severity: Major
              Found in components/user/services/UserAuthentication.php - About 30 mins to fix

                Avoid too many return statements within this method.
                Open

                        return $finalOutput;
                Severity: Major
                Found in components/user/services/UserAuthentication.php - About 30 mins to fix

                  Avoid too many return statements within this method.
                  Open

                          return true;
                  Severity: Major
                  Found in components/user/services/UserAuthentication.php - About 30 mins to fix

                    Function forgetPassword has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
                    Open

                        public function forgetPassword(User $user)
                        {
                            $output = new Output();
                    
                            if ($this->isUserActive($user) !== true) {
                    Severity: Minor
                    Found in components/user/services/UserAuthentication.php - About 25 mins to fix

                    Cognitive Complexity

                    Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

                    A method's cognitive complexity is based on a few simple rules:

                    • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
                    • Code is considered more complex for each "break in the linear flow of the code"
                    • Code is considered more complex when "flow breaking structures are nested"

                    Further reading

                    The method getAllLoggedIn() has an NPath complexity of 2308. The configured NPath complexity threshold is 200.
                    Open

                        public function getAllLoggedIn(array $whiteList = null)
                        {
                            if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
                                return false;
                            }

                    NPathComplexity

                    Since: 0.1

                    The NPath complexity of a method is the number of acyclic execution paths through that method. A threshold of 200 is generally considered the point where measures should be taken to reduce complexity.

                    Example

                    class Foo {
                        function bar() {
                            // lots of complicated code
                        }
                    }

                    Source https://phpmd.org/rules/codesize.html#npathcomplexity

                    The method getAllLoggedIn() has a Cyclomatic Complexity of 16. The configured cyclomatic complexity threshold is 10.
                    Open

                        public function getAllLoggedIn(array $whiteList = null)
                        {
                            if (!isset($_SESSION['loggedIn']) || empty($_SESSION['loggedIn']) || !is_array($_SESSION['loggedIn'])) {
                                return false;
                            }

                    CyclomaticComplexity

                    Since: 0.1

                    Complexity is determined by the number of decision points in a method plus one for the method entry. The decision points are 'if', 'while', 'for', and 'case labels'. Generally, 1-4 is low complexity, 5-7 indicates moderate complexity, 8-10 is high complexity, and 11+ is very high complexity.

                    Example

                    // Cyclomatic Complexity = 11
                    class Foo {
                    1   public function example() {
                    2       if ($a == $b) {
                    3           if ($a1 == $b1) {
                                    fiddle();
                    4           } elseif ($a2 == $b2) {
                                    fiddle();
                                } else {
                                    fiddle();
                                }
                    5       } elseif ($c == $d) {
                    6           while ($c == $d) {
                                    fiddle();
                                }
                    7        } elseif ($e == $f) {
                    8           for ($n = 0; $n < $h; $n++) {
                                    fiddle();
                                }
                            } else {
                                switch ($z) {
                    9               case 1:
                                        fiddle();
                                        break;
                    10              case 2:
                                        fiddle();
                                        break;
                    11              case 3:
                                        fiddle();
                                        break;
                                    default:
                                        fiddle();
                                        break;
                                }
                            }
                        }
                    }

                    Source https://phpmd.org/rules/codesize.html#cyclomaticcomplexity

                    The method isLoggedIn has a boolean flag argument $updateLastActivityAt, which is a certain sign of a Single Responsibility Principle violation.
                    Open

                        public function isLoggedIn(User $user, $updateLastActivityAt = true)

                    BooleanArgumentFlag

                    Since: 1.4.0

                    A boolean flag argument is a reliable indicator for a violation of the Single Responsibility Principle (SRP). You can fix this problem by extracting the logic in the boolean flag into its own class or method.

                    Example

                    class Foo {
                        public function bar($flag = true) {
                        }
                    }

                    Source https://phpmd.org/rules/cleancode.html#booleanargumentflag

                    The method updateLastActivityAt uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them.
                    Open

                            } else {
                                return false;
                            }

                    ElseExpression

                    Since: 1.4.0

                    An if expression with an else branch is basically not necessary. You can rewrite the conditions in a way that the else clause is not necessary and the code becomes simpler to read. To achieve this, use early return statements, though you may need to split the code it several smaller methods. For very simple assignments you could also use the ternary operations.

                    Example

                    class Foo
                    {
                        public function bar($flag)
                        {
                            if ($flag) {
                                // one branch
                            } else {
                                // another branch
                            }
                        }
                    }

                    Source https://phpmd.org/rules/cleancode.html#elseexpression

                    The method resetPassword uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them.
                    Open

                            } else {
                                $tokenExpired = true;
                            }

                    ElseExpression

                    Since: 1.4.0

                    An if expression with an else branch is basically not necessary. You can rewrite the conditions in a way that the else clause is not necessary and the code becomes simpler to read. To achieve this, use early return statements, though you may need to split the code it several smaller methods. For very simple assignments you could also use the ternary operations.

                    Example

                    class Foo
                    {
                        public function bar($flag)
                        {
                            if ($flag) {
                                // one branch
                            } else {
                                // another branch
                            }
                        }
                    }

                    Source https://phpmd.org/rules/cleancode.html#elseexpression

                    The method resetPassword uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them.
                    Open

                            } else {
                                $finalOutput->setSuccess(false);
                                $finalOutput->setMessage($output->getMessage());
                            }

                    ElseExpression

                    Since: 1.4.0

                    An if expression with an else branch is basically not necessary. You can rewrite the conditions in a way that the else clause is not necessary and the code becomes simpler to read. To achieve this, use early return statements, though you may need to split the code it several smaller methods. For very simple assignments you could also use the ternary operations.

                    Example

                    class Foo
                    {
                        public function bar($flag)
                        {
                            if ($flag) {
                                // one branch
                            } else {
                                // another branch
                            }
                        }
                    }

                    Source https://phpmd.org/rules/cleancode.html#elseexpression

                    The method login uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them.
                    Open

                            } else {
                                $output->setSuccess(false);
                                $output->setMessage('Please try again');
                            }

                    ElseExpression

                    Since: 1.4.0

                    An if expression with an else branch is basically not necessary. You can rewrite the conditions in a way that the else clause is not necessary and the code becomes simpler to read. To achieve this, use early return statements, though you may need to split the code it several smaller methods. For very simple assignments you could also use the ternary operations.

                    Example

                    class Foo
                    {
                        public function bar($flag)
                        {
                            if ($flag) {
                                // one branch
                            } else {
                                // another branch
                            }
                        }
                    }

                    Source https://phpmd.org/rules/cleancode.html#elseexpression

                    The method removeLoggedInUserFromSession uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them.
                    Open

                            } else {
                                return false;
                            }

                    ElseExpression

                    Since: 1.4.0

                    An if expression with an else branch is basically not necessary. You can rewrite the conditions in a way that the else clause is not necessary and the code becomes simpler to read. To achieve this, use early return statements, though you may need to split the code it several smaller methods. For very simple assignments you could also use the ternary operations.

                    Example

                    class Foo
                    {
                        public function bar($flag)
                        {
                            if ($flag) {
                                // one branch
                            } else {
                                // another branch
                            }
                        }
                    }

                    Source https://phpmd.org/rules/cleancode.html#elseexpression

                    The method forgetPassword uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them.
                    Open

                            } else {
                                $output->setMessage("An email including a link to reset your password is sent to {$email}");
                                $output->setSuccess(true);
                            }

                    ElseExpression

                    Since: 1.4.0

                    An if expression with an else branch is basically not necessary. You can rewrite the conditions in a way that the else clause is not necessary and the code becomes simpler to read. To achieve this, use early return statements, though you may need to split the code it several smaller methods. For very simple assignments you could also use the ternary operations.

                    Example

                    class Foo
                    {
                        public function bar($flag)
                        {
                            if ($flag) {
                                // one branch
                            } else {
                                // another branch
                            }
                        }
                    }

                    Source https://phpmd.org/rules/cleancode.html#elseexpression

                    The method logout() contains an exit expression.
                    Open

                            exit;

                    ExitExpression

                    Since: 0.2

                    An exit-expression within regular code is untestable and therefore it should be avoided. Consider to move the exit-expression into some kind of startup script where an error/exception code is returned to the calling environment.

                    Example

                    class Foo {
                        public function bar($param)  {
                            if ($param === 42) {
                                exit(23);
                            }
                        }
                    }

                    Source https://phpmd.org/rules/design.html#exitexpression

                    Identical blocks of code found in 2 locations. Consider refactoring.
                    Open

                            if ($user->getToken() === $token) {
                                // check token lifetime
                                $tokenGeneratedAt = $user->getTokenGeneratedAt();
                                $tokenLivedTime = time() - strtotime($tokenGeneratedAt);
                    
                    
                    Severity: Minor
                    Found in components/user/services/UserAuthentication.php and 1 other location - About 50 mins to fix
                    components/user/models/User.php on lines 451..460

                    Duplicated Code

                    Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:

                    Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

                    When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).

                    Tuning

                    This issue has a mass of 97.

                    We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.

                    The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.

                    If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.

                    See codeclimate-duplication's documentation for more information about tuning the mass threshold in your .codeclimate.yml.

                    Refactorings

                    Further Reading

                    A file should declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it should execute logic with side effects, but should not do both. The first symbol is defined on line 28 and the first side effect is on line 22.
                    Open

                    <?php

                    The 'getCurrentLoggedIn()' method which returns a boolean should be named 'is...()' or 'has...()'
                    Open

                        public function getCurrentLoggedIn()
                        {
                            $routeInfo = Registry::getRouterClass()->getLastRoute();
                    
                            if (!$routeInfo instanceof RouteInfo) {

                    BooleanGetMethodName

                    Since: 0.2

                    Looks for methods named 'getX()' with 'boolean' as the return type. The convention is to name these methods 'isX()' or 'hasX()'.

                    Example

                    class Foo {
                        /**
                         * @return boolean
                         */
                        public function getFoo() {} // bad
                        /**
                         * @return bool
                         */
                        public function isFoo(); // ok
                        /**
                         * @return boolean
                         */
                        public function getFoo($bar); // ok, unless checkParameterizedMethods=true
                    }

                    Source https://phpmd.org/rules/naming.html#booleangetmethodname

                    There are no issues that match your filters.

                    Category
                    Status