Authenticator::authenticateMember() - Code Metrics - Inspection of "FIX: Refactor delegateToHandler / delegateToHandle..." - sminnee/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — authenticator-refactor ( e3c944...b14200 )

by Sam

created 2017-04-23 09:24 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()
    {
        // Bitwise-OR of all the supported services, to make a bitmask
        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			// Bitwise-OR of all the supported services, to make a bitmask
25			return BaseAuthenticator::LOGIN \| BaseAuthenticator::LOGOUT \| BaseAuthenticator::CHANGE_PASSWORD
26			\| BaseAuthenticator::RESET_PASSWORD \| BaseAuthenticator::CMS_LOGIN;
27			}
28
29			/**
30			* @inherit
31			*/
32			public function authenticate($data, &$message)
33			{
34			$success = null;
35
36			// Find authenticated member
37			$member = $this->authenticateMember($data, $message, $success);
38
39			// Optionally record every login attempt as a {@link LoginAttempt} object
40			$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 `37` 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...
41
42			if ($member) {
43			Session::clear('BackURL');
44			}
45
46			return $success ? $member : null;
47			}
48
49			/**
50			* Attempt to find and authenticate member if possible from the given data
51			*
52			* @param array $data
53			* @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...
54			* @param bool &$success Success flag
55			* @return Member Found member, regardless of successful login
56			*/
57			protected function authenticateMember($data, &$message, &$success)
58			{
59			// Default success to false
60			$success = false;
61
62			// Attempt to identify by temporary ID
63			$member = null;
64			$email = null;
65			if (!empty($data['tempid'])) {
66			// Find user by tempid, in case they are re-validating an existing session
67			$member = Member::member_from_tempid($data['tempid']);
68			if ($member) {
69			$email = $member->Email;
70			}
71			}
72
73			// Otherwise, get email from posted value instead
74			/** @skipUpgrade */
75			if (!$member && !empty($data['Email'])) {
76			$email = $data['Email'];
77			}
78
79			// Check default login (see Security::setDefaultAdmin())
80			$asDefaultAdmin = $email === Security::default_admin_username();
81			if ($asDefaultAdmin) {
82			// If logging is as default admin, ensure record is setup correctly
83			$member = Member::default_admin();
84			$success = !$member->isLockedOut() && Security::check_default_admin($email, $data['Password']);
85			//protect against failed login
86			if ($success) {
87			return $member;
88			}
89			}
90
91			// Attempt to identify user by email
92			if (!$member && $email) {
93			// Find user by email
94			$member = Member::get()
95			->filter(Member::config()->unique_identifier_field, $email)
96			->first();
97			}
98
99			// Validate against member if possible
100			if ($member && !$asDefaultAdmin) {
101			$result = $member->checkPassword($data['Password']);
102			$success = $result->isValid();
103			} else {
104			$result = ValidationResult::create()->addError(_t(
105			'Member.ERRORWRONGCRED',
106			'The provided details don\'t seem to be correct. Please try again.'
107			));
108			}
109
110			// Emit failure to member and form (if available)
111			if (!$success) {
112			if ($member) {
113			$member->registerFailedLogin();
114			}
115			$message = implode("; ", array_map(
116			function ($message) {
117			return $message['message'];
118			},
119			$result->getMessages()
120			));
121			} else {
122			if ($member) {
123			$member->registerSuccessfulLogin();
124			}
125			}
126
127			return $member;
128			}
129
130			/**
131			* Log login attempt
132			* TODO We could handle this with an extension
133			*
134			* @param array $data
135			* @param Member $member
136			*/
137			protected function recordLoginAttempt($data, $member)
138			{
139			if (!Security::config()->login_recording) {
140			return;
141			}
142
143			// Check email is valid
144			/** @skipUpgrade */
145			$email = isset($data['Email']) ? $data['Email'] : null;
146			if (is_array($email)) {
147			throw new InvalidArgumentException("Bad email passed to MemberAuthenticator::authenticate(): $email");
148			}
149
150			$attempt = new LoginAttempt();
151			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...
152			// successful login (member is existing with matching password)
153			$attempt->MemberID = $member->ID;
154			$attempt->Status = 'Success';
155
156			// Audit logging hook
157			$member->extend('authenticated');
158			} else {
159			// Failed login - we're trying to see if a user exists with this email (disregarding wrong passwords)
160			$attempt->Status = 'Failure';
161			if ($member) {
162			// Audit logging hook
163			$attempt->MemberID = $member->ID;
164			$member->extend('authenticationFailed');
165			} else {
166			// Audit logging hook
167			Member::singleton()->extend('authenticationFailedUnknownUser', $data);
168			}
169			}
170
171			$attempt->Email = $email;
172			$attempt->IP = Controller::curr()->getRequest()->getIP();
173			$attempt->write();
174			}
175
176			/**
177			* @inherit
178			*/
179			public function getLostPasswordHandler($link)
180			{
181			return LostPasswordHandler::create($link, $this);
182			}
183
184			/**
185			* @inherit
186			*/
187			public function getChangePasswordHandler($link)
188			{
189			return ChangePasswordHandler::create($link, $this);
190			}
191
192			/**
193			* @inherit
194			*/
195			public function getLoginHandler($link)
196			{
197			return LoginHandler::create($link, $this);
198			}
199
200			public function getCMSLoginHandler($link)
201			{
202			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...
203			}
204			}
205

sminnee / silverstripe-framework

Push — authenticator-refactor ( e3c944...b14200 )

Authenticator::authenticateMember() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like