Authenticator::authenticateMember() - Code Metrics - Inspection of "WIP - refactoring of authenticators" - sminnee/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — authenticator-refactor ( d33fab...54d3b4 )

by Sam

created 2017-04-22 04:56 UTC

Authenticator::authenticateMember() D

↳ Parent: Authenticator

Complexity

Conditions	15
Paths	294

Size

Total Lines	72
Code Lines	38

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	72
c	0
b	0
f	0
cc	15
eloc	38
nc	294
nop	3
rs	4.1144

How to fix Long Method Complexity

<?php

namespace SilverStripe\Security\MemberAuthenticator;

use SilverStripe\Control\Controller;
use SilverStripe\Control\Session;
use SilverStripe\Forms\Form;
use SilverStripe\ORM\ValidationResult;
use InvalidArgumentException;
use SilverStripe\Security\Authenticator as BaseAuthenticator;
use SilverStripe\Security\Security;
use SilverStripe\Security\Member;

/**
 * Authenticator for the default "member" method
 *
 * @author Markus Lanthaler <[email protected]>
 */
class Authenticator implements BaseAuthenticator
{

    public function supportedServices()
    {
        return BaseAuthenticator::LOGIN || BaseAuthenticator::LOGOUT| BaseAuthenticator::CHANGE_PASSWORD
            | BaseAuthenticator::RESET_PASSWORD | BaseAuthenticator::CMS_LOGIN;
    }

    /**
     * @inherit
     */
    public function authenticate($data, &$message)
    {
        $success = null;

        // Find authenticated member
        $member = $this->authenticateMember($data, $message, $success);

        // Optionally record every login attempt as a {@link LoginAttempt} object
        $this->recordLoginAttempt($data, $member, $success);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

        if ($member) {
            Session::clear('BackURL');
        }

        return $success ? $member : null;
    }

    /**
     * Attempt to find and authenticate member if possible from the given data
     *
     * @param array $data
     * @param Form $form
/**
 * @param array $germany
 * @param array $island
 * @param array $italy
 */
function finale($germany, $island) {
    return "2:1";
}
     * @param bool &$success Success flag
     * @return Member Found member, regardless of successful login
     */
    protected function authenticateMember($data, &$message, &$success)
    {
        // Default success to false
        $success = false;

        // Attempt to identify by temporary ID
        $member = null;
        $email = null;
        if (!empty($data['tempid'])) {
            // Find user by tempid, in case they are re-validating an existing session
            $member = Member::member_from_tempid($data['tempid']);
            if ($member) {
                $email = $member->Email;
            }
        }

        // Otherwise, get email from posted value instead
        /** @skipUpgrade */
        if (!$member && !empty($data['Email'])) {
            $email = $data['Email'];
        }

        // Check default login (see Security::setDefaultAdmin())
        $asDefaultAdmin = $email === Security::default_admin_username();
        if ($asDefaultAdmin) {
            // If logging is as default admin, ensure record is setup correctly
            $member = Member::default_admin();
            $success = !$member->isLockedOut() && Security::check_default_admin($email, $data['Password']);
            //protect against failed login
            if ($success) {
                return $member;
            }
        }

        // Attempt to identify user by email
        if (!$member && $email) {
            // Find user by email
            $member = Member::get()
                ->filter(Member::config()->unique_identifier_field, $email)
                ->first();
        }

        // Validate against member if possible
        if ($member && !$asDefaultAdmin) {
            $result = $member->checkPassword($data['Password']);
            $success = $result->isValid();
        } else {
            $result = ValidationResult::create()->addError(_t(
                'Member.ERRORWRONGCRED',
                'The provided details don\'t seem to be correct. Please try again.'
            ));
        }

        // Emit failure to member and form (if available)
        if (!$success) {
            if ($member) {
                $member->registerFailedLogin();
            }
            $message = implode("; ", array_map(
                function ($message) {
                    return $message['message'];
                },
                $result->getMessages()
            ));
        } else {
            if ($member) {
                $member->registerSuccessfulLogin();
            }
        }

        return $member;
    }

    /**
     * Log login attempt
     * TODO We could handle this with an extension
     *
     * @param array $data
     * @param Member $member
     */
    protected function recordLoginAttempt($data, $member)
    {
        if (!Security::config()->login_recording) {
            return;
        }

        // Check email is valid
        /** @skipUpgrade */
        $email = isset($data['Email']) ? $data['Email'] : null;
        if (is_array($email)) {
            throw new InvalidArgumentException("Bad email passed to MemberAuthenticator::authenticate(): $email");
        }

        $attempt = new LoginAttempt();
        if ($success) {

            // successful login (member is existing with matching password)
            $attempt->MemberID = $member->ID;
            $attempt->Status = 'Success';

            // Audit logging hook
            $member->extend('authenticated');
        } else {
            // Failed login - we're trying to see if a user exists with this email (disregarding wrong passwords)
            $attempt->Status = 'Failure';
            if ($member) {
                // Audit logging hook
                $attempt->MemberID = $member->ID;
                $member->extend('authenticationFailed');
            } else {
                // Audit logging hook
                Member::singleton()->extend('authenticationFailedUnknownUser', $data);
            }
        }

        $attempt->Email = $email;
        $attempt->IP = Controller::curr()->getRequest()->getIP();
        $attempt->write();
    }

    /**
     * @inherit
     */
    public function getLostPasswordHandler($link)
    {
        return LostPasswordHandler::create($link, $this);
    }

    /**
     * @inherit
     */
    public function getChangePasswordHandler($link)
    {
        return ChangePasswordHandler::create($link, $this);
    }

    /**
     * @inherit
     */
    public function getLoginHandler($link)
    {
        return LoginHandler::create($link, $this);
    }

    public function getCMSLoginHandler($link)
    {
        return CMSMemberLoginHandler::create($controller, self::class, "LoginForm");

    }
}


1			<?php
2
3			namespace SilverStripe\Security\MemberAuthenticator;
4
5			use SilverStripe\Control\Controller;
6			use SilverStripe\Control\Session;
7			use SilverStripe\Forms\Form;
8			use SilverStripe\ORM\ValidationResult;
9			use InvalidArgumentException;
10			use SilverStripe\Security\Authenticator as BaseAuthenticator;
11			use SilverStripe\Security\Security;
12			use SilverStripe\Security\Member;
13
14			/**
15			* Authenticator for the default "member" method
16			*
17			* @author Markus Lanthaler <[email protected]>
18			*/
19			class Authenticator implements BaseAuthenticator
20			{
21
22			public function supportedServices()
23			{
24			return BaseAuthenticator::LOGIN \|\| BaseAuthenticator::LOGOUT\| BaseAuthenticator::CHANGE_PASSWORD
25			\| BaseAuthenticator::RESET_PASSWORD \| BaseAuthenticator::CMS_LOGIN;
26			}
27
28			/**
29			* @inherit
30			*/
31			public function authenticate($data, &$message)
32			{
33			$success = null;
34
35			// Find authenticated member
36			$member = $this->authenticateMember($data, $message, $success);
37
38			// Optionally record every login attempt as a {@link LoginAttempt} object
39			$this->recordLoginAttempt($data, $member, $success);
			0 ignored issues – show Bug introduced 2017-04-22 04:42 UTC by Report Bug Copy Issue Report It seems like `$member` defined by `$this->authenticateMembe...ta, $message, $success)` on line `36` can be `null`; however, `SilverStripe\Security\Me...r::recordLoginAttempt()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history... Unused Code introduced 2017-04-22 04:35 UTC by Report Bug Copy Issue Report The call to `Authenticator::recordLoginAttempt()` has too many arguments starting with `$success`. This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue. If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. In this case you can add the `@ignore` PhpDoc annotation to the duplicate definition and it will be ignored. Loading history...
40
41			if ($member) {
42			Session::clear('BackURL');
43			}
44
45			return $success ? $member : null;
46			}
47
48			/**
49			* Attempt to find and authenticate member if possible from the given data
50			*
51			* @param array $data
52			* @param Form $form
			0 ignored issues – show Bug introduced 2017-04-22 04:35 UTC by Report Bug Copy Issue Report There is no parameter named `$form`. Was it maybe removed? This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. Consider the following example. The parameter `$italy` is not defined by the method `finale(...)`. /** * @param array $germany * @param array $island * @param array $italy */ function finale($germany, $island) { return "2:1"; } The most likely cause is that the parameter was removed, but the annotation was not. Loading history...
53			* @param bool &$success Success flag
54			* @return Member Found member, regardless of successful login
55			*/
56			protected function authenticateMember($data, &$message, &$success)
57			{
58			// Default success to false
59			$success = false;
60
61			// Attempt to identify by temporary ID
62			$member = null;
63			$email = null;
64			if (!empty($data['tempid'])) {
65			// Find user by tempid, in case they are re-validating an existing session
66			$member = Member::member_from_tempid($data['tempid']);
67			if ($member) {
68			$email = $member->Email;
69			}
70			}
71
72			// Otherwise, get email from posted value instead
73			/** @skipUpgrade */
74			if (!$member && !empty($data['Email'])) {
75			$email = $data['Email'];
76			}
77
78			// Check default login (see Security::setDefaultAdmin())
79			$asDefaultAdmin = $email === Security::default_admin_username();
80			if ($asDefaultAdmin) {
81			// If logging is as default admin, ensure record is setup correctly
82			$member = Member::default_admin();
83			$success = !$member->isLockedOut() && Security::check_default_admin($email, $data['Password']);
84			//protect against failed login
85			if ($success) {
86			return $member;
87			}
88			}
89
90			// Attempt to identify user by email
91			if (!$member && $email) {
92			// Find user by email
93			$member = Member::get()
94			->filter(Member::config()->unique_identifier_field, $email)
95			->first();
96			}
97
98			// Validate against member if possible
99			if ($member && !$asDefaultAdmin) {
100			$result = $member->checkPassword($data['Password']);
101			$success = $result->isValid();
102			} else {
103			$result = ValidationResult::create()->addError(_t(
104			'Member.ERRORWRONGCRED',
105			'The provided details don\'t seem to be correct. Please try again.'
106			));
107			}
108
109			// Emit failure to member and form (if available)
110			if (!$success) {
111			if ($member) {
112			$member->registerFailedLogin();
113			}
114			$message = implode("; ", array_map(
115			function ($message) {
116			return $message['message'];
117			},
118			$result->getMessages()
119			));
120			} else {
121			if ($member) {
122			$member->registerSuccessfulLogin();
123			}
124			}
125
126			return $member;
127			}
128
129			/**
130			* Log login attempt
131			* TODO We could handle this with an extension
132			*
133			* @param array $data
134			* @param Member $member
135			*/
136			protected function recordLoginAttempt($data, $member)
137			{
138			if (!Security::config()->login_recording) {
139			return;
140			}
141
142			// Check email is valid
143			/** @skipUpgrade */
144			$email = isset($data['Email']) ? $data['Email'] : null;
145			if (is_array($email)) {
146			throw new InvalidArgumentException("Bad email passed to MemberAuthenticator::authenticate(): $email");
147			}
148
149			$attempt = new LoginAttempt();
150			if ($success) {
			0 ignored issues – show Bug introduced 2017-04-22 04:35 UTC by Report Bug Copy Issue Report The variable `$success` does not exist. Did you forget to declare it? This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug. Loading history...
151			// successful login (member is existing with matching password)
152			$attempt->MemberID = $member->ID;
153			$attempt->Status = 'Success';
154
155			// Audit logging hook
156			$member->extend('authenticated');
157			} else {
158			// Failed login - we're trying to see if a user exists with this email (disregarding wrong passwords)
159			$attempt->Status = 'Failure';
160			if ($member) {
161			// Audit logging hook
162			$attempt->MemberID = $member->ID;
163			$member->extend('authenticationFailed');
164			} else {
165			// Audit logging hook
166			Member::singleton()->extend('authenticationFailedUnknownUser', $data);
167			}
168			}
169
170			$attempt->Email = $email;
171			$attempt->IP = Controller::curr()->getRequest()->getIP();
172			$attempt->write();
173			}
174
175			/**
176			* @inherit
177			*/
178			public function getLostPasswordHandler($link)
179			{
180			return LostPasswordHandler::create($link, $this);
181			}
182
183			/**
184			* @inherit
185			*/
186			public function getChangePasswordHandler($link)
187			{
188			return ChangePasswordHandler::create($link, $this);
189			}
190
191			/**
192			* @inherit
193			*/
194			public function getLoginHandler($link)
195			{
196			return LoginHandler::create($link, $this);
197			}
198
199			public function getCMSLoginHandler($link)
200			{
201			return CMSMemberLoginHandler::create($controller, self::class, "LoginForm");
			0 ignored issues – show Bug introduced 2017-04-22 04:35 UTC by Report Bug Copy Issue Report The variable `$controller` does not exist. Did you forget to declare it? This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug. Loading history...
202			}
203			}
204

sminnee / silverstripe-framework

Push — authenticator-refactor ( d33fab...54d3b4 )

Authenticator::authenticateMember() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like