Authenticator::authenticateMember() - Code Metrics - Inspection of "CMS Login Handling" - sminnee/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — authenticator-refactor ( 4aec3f...04850c )

by Simon

created 2017-04-30 05:05 UTC

Authenticator::authenticateMember() C

↳ Parent: Authenticator

Complexity

Conditions	12
Paths	98

Size

Total Lines	57
Code Lines	31

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	12
eloc	31
nc	98
nop	4
dl	0
loc	57
rs	6.62
c	0
b	0
f	0

How to fix Long Method Complexity

<?php

namespace SilverStripe\Security\MemberAuthenticator;

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

/**
 * 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;
    }

    /**
     * @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 Form submitted data
     * @param $message
     * @param bool &$success Success flag
     * @param null|Member $member If the parent method already identified the member, it can be passed in
     * @return Member Found member, regardless of successful login
     */
    protected function authenticateMember($data, &$message, &$success, $member = null)
    {
        // Default success to false
        $success = false;
        $email = !empty($data['Email']) ? $data['Email'] : null ;
        
        // 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->canLogin()->isValid() && 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
            /** @var Member $member */
            $member = Member::get()
                ->filter([Member::config()->get('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(
                'SilverStripe\\Security\\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) { // How can success be true and member false?
                $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, $success)
    {
        if (!Security::config()->get('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 = LoginAttempt::create();
        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);
    }
}


1			<?php
2
3			namespace SilverStripe\Security\MemberAuthenticator;
4
5			use SilverStripe\Control\Controller;
6			use SilverStripe\Control\Session;
7			use SilverStripe\ORM\ValidationResult;
8			use InvalidArgumentException;
9			use SilverStripe\Security\Authenticator as BaseAuthenticator;
10			use SilverStripe\Security\Security;
11			use SilverStripe\Security\Member;
12			use SilverStripe\Security\LoginAttempt;
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;
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...
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 Form submitted data
53			* @param $message
54			* @param bool &$success Success flag
55			* @param null\|Member $member If the parent method already identified the member, it can be passed in
56			* @return Member Found member, regardless of successful login
57			*/
58			protected function authenticateMember($data, &$message, &$success, $member = null)
59			{
60			// Default success to false
61			$success = false;
62			$email = !empty($data['Email']) ? $data['Email'] : null ;
63
64			// Check default login (see Security::setDefaultAdmin())
65			$asDefaultAdmin = $email === Security::default_admin_username();
66			if ($asDefaultAdmin) {
67			// If logging is as default admin, ensure record is setup correctly
68			$member = Member::default_admin();
69			$success = $member->canLogin()->isValid() && Security::check_default_admin($email, $data['Password']);
70			//protect against failed login
71			if ($success) {
72			return $member;
73			}
74			}
75
76			// Attempt to identify user by email
77			if (!$member && $email) {
78			// Find user by email
79			/** @var Member $member */
80			$member = Member::get()
81			->filter([Member::config()->get('unique_identifier_field') => $email])
82			->first();
83			}
84
85			// Validate against member if possible
86			if ($member && !$asDefaultAdmin) {
87			$result = $member->checkPassword($data['Password']);
88			$success = $result->isValid();
89			} else {
90			$result = ValidationResult::create()->addError(_t(
91			'SilverStripe\\Security\\Member.ERRORWRONGCRED',
92			'The provided details don\'t seem to be correct. Please try again.'
93			));
94			}
95
96			// Emit failure to member and form (if available)
97			if (!$success) {
98			if ($member) {
99			$member->registerFailedLogin();
100			}
101			$message = implode("; ", array_map(
102			function ($message) {
103			return $message['message'];
104			},
105			$result->getMessages()
106			));
107			} else {
108			if ($member) { // How can success be true and member false?
109			$member->registerSuccessfulLogin();
110			}
111			}
112
113			return $member;
114			}
115
116			/**
117			* Log login attempt
118			* TODO We could handle this with an extension
119			*
120			* @param array $data
121			* @param Member $member
122			*/
123			protected function recordLoginAttempt($data, $member, $success)
124			{
125			if (!Security::config()->get('login_recording')) {
126			return;
127			}
128
129			// Check email is valid
130			/** @skipUpgrade */
131			$email = isset($data['Email']) ? $data['Email'] : null;
132			if (is_array($email)) {
133			throw new InvalidArgumentException("Bad email passed to MemberAuthenticator::authenticate(): $email");
134			}
135
136			$attempt = LoginAttempt::create();
137			if ($success) {
138			// successful login (member is existing with matching password)
139			$attempt->MemberID = $member->ID;
140			$attempt->Status = 'Success';
141
142			// Audit logging hook
143			$member->extend('authenticated');
144			} else {
145			// Failed login - we're trying to see if a user exists with this email (disregarding wrong passwords)
146			$attempt->Status = 'Failure';
147			if ($member) {
148			// Audit logging hook
149			$attempt->MemberID = $member->ID;
150			$member->extend('authenticationFailed');
151			} else {
152			// Audit logging hook
153			Member::singleton()->extend('authenticationFailedUnknownUser', $data);
154			}
155			}
156
157			$attempt->Email = $email;
158			$attempt->IP = Controller::curr()->getRequest()->getIP();
159			$attempt->write();
160			}
161
162			/**
163			* @inherit
164			*/
165			public function getLostPasswordHandler($link)
166			{
167			return LostPasswordHandler::create($link, $this);
168			}
169
170			/**
171			* @inherit
172			*/
173			public function getChangePasswordHandler($link)
174			{
175			return ChangePasswordHandler::create($link, $this);
176			}
177
178			/**
179			* @inherit
180			*/
181			public function getLoginHandler($link)
182			{
183			return LoginHandler::create($link, $this);
184			}
185			}
186

sminnee / silverstripe-framework

Push — authenticator-refactor ( 4aec3f...04850c )

Authenticator::authenticateMember() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like