YubikeyAuthenticator::authenticate() - Code Metrics - Inspection of "Strip out the form update to a separate method. Le..." - Firesphere/silverstripe-yubiauth - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Branch — develop (1dbf6d)

by Simon

created 2016-11-12 01:38 UTC

YubikeyAuthenticator::authenticate() D

↳ Parent: YubikeyAuthenticator

Complexity

Conditions	17
Paths	14

Size

Total Lines	82
Code Lines	56

Duplication

Lines	17
Ratio	20.73 %

Importance

Changes	2
Bugs	0	Features	0

Metric	Value
c	2
b	0
f	0
dl	17
loc	82
rs	4.9359
cc	17
eloc	56
nc	14
nop	2

How to fix Long Method Complexity

<?php
namespace Firesphere\YubiAuth;

use Config;
use Controller;
use DateTime;
use Form;
use Member;
use MemberAuthenticator;
use ValidationResult;

/**
 * Class YubikeyAuthenticator
 *
 * Enable Yubikey Authentication.
 */
class YubikeyAuthenticator extends MemberAuthenticator
{

    /**
     * @inheritdoc
     *
     * @param array $data
     * @param Form|null $form
     *
     * @return bool|Member
     */
    public static function authenticate($data, Form $form = null)
    {
        Config::inst()->update('Security', 'login_recording', false); // Disable login_recording for this auth.
        // First, let's see if we know the member
        $member = parent::authenticate($data, $form);
        Config::inst()->update('Security', 'login_recording', true);
        $validationError = ValidationResult::create(false,
            _t('YubikeyAuthenticator.ERRORYUBIKEY', 'Yubikey authentication error'));
        if ($member && $member instanceof Member) {
            // If we know the member, and it's YubiAuth enabled, continue.
            if ($member &&
                ($member->YubiAuthEnabled || $data['Yubikey'] !== '')
            ) {
                $data['Yubikey'] = strtolower($data['Yubikey']);
                $yubiCode = QwertyConvertor::convertString($data['Yubikey']);
                $yubiFingerprint = substr($yubiCode, 0, -32);
                // If the member has a yubikey ID set, compare it to the fingerprint.
                if ($member->Yubikey && strpos($yubiFingerprint, $member->Yubikey) !== 0) {
                    self::updateForm($validationError, $form);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}

                    return null; // Yubikey id doesn't match the member.
                }
                $clientID = YUBIAUTH_CLIENTID;
                $apiKey = YUBIAUTH_APIKEY;
                $service = new \Yubikey\Validate($apiKey, $clientID);
                if ($url = self::config()->get('AuthURL')) {
                    $service->setHost($url);
                }
                $result = $service->check($yubiCode);

                if ($result->success() === true) {
                    self::updateMember($member, $yubiFingerprint);
                    if ($member) {
                        $member->registerSuccessfulLogin();
                        $member->MaxNoYubiLogins = 0;
                        $member->write();
                    }

                    return $member;
                } else {
                    self::updateForm($validationError, $form);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}

                    return null;
                }
            } elseif (!$member->YubiAuthEnabled) { // We do not have to check the YubiAuth for now.
                $member->NoYubikeyCount += 1;
                $member->write();
                $maxNoYubi = Config::inst()->get('YubikeyAuthenticator', 'MaxNoYubiLogin');
                if ($maxNoYubi > 0 && $maxNoYubi <= $member->NoYubikeyCount) {

                    $validationError = ValidationResult::create(false,
                        _t('YubikeyAuthenticator.ERRORMAXYUBIKEY', 'Maximum login without yubikey exceeded'));
                    self::updateForm($validationError, $form);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}
                    $member->registerFailedLogin();

                    return null;
                }
                $date1 = new DateTime($member->Created);
                $date2 = new DateTime(date('Y-m-d'));

                $diff = $date2->diff($date1)->format("%a");
                $maxNoYubiDays = Config::inst()->get('YubikeyAuthenticator', 'MaxNoYubiLoginDays');

                if ($maxNoYubiDays > 0 && $diff >= $maxNoYubiDays) {

                    $validationError = ValidationResult::create(false,
                        _t('YubikeyAuthenticator.ERRORMAXYUBIKEYDAYS', 'Maximum days without yubikey exceeded'));
                    self::updateForm($validationError, $form);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}
                    $member->registerFailedLogin();

                    return null;

                }

                return $member;
            }
        }
        if ($member) {
            $member->registerFailedLogin();
        }
        self::updateForm($validationError, $form);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}

        return null;
    }

    /**
     * @param Controller $controller
     *
     * @return Form
     */
    public static function get_login_form(Controller $controller)
    {
        return YubikeyLoginForm::create($controller, 'LoginForm');
    }

    public static function get_name()
    {
        return _t('YubikeyAuthenticator.TITLE', 'Yubikey login');
    }

    /**
     * Update the member to forcefully enable YubiAuth
     * Also, register the Yubikey to the member.
     * Documentation:
     * https://developers.yubico.com/yubikey-val/Getting_Started_Writing_Clients.html
     *
     * @param Member $member
     * @param string $yubiString
     */
    private static function updateMember($member, $yubiString)
    {
        if (!$member->YubiAuthEnabled) {
            $member->YubiAuthEnabled = true;
        }
        if (!$member->Yubikey) {
            $member->Yubikey = $yubiString;
        }
        $member->write();
    }

    /**
     * @param ValidationResult $validation
     * @param Form $form
     */
    private static function updateForm($validation, $form)
    {
        if ($form) {
            $form->sessionMessage($validation->message(), 'bad');
        }

    }

}

1		<?php
2		namespace Firesphere\YubiAuth;
3
4		use Config;
5		use Controller;
6		use DateTime;
7		use Form;
8		use Member;
9		use MemberAuthenticator;
10		use ValidationResult;
11
12		/**
13		* Class YubikeyAuthenticator
14		*
15		* Enable Yubikey Authentication.
16		*/
17		class YubikeyAuthenticator extends MemberAuthenticator
18		{
19
20		/**
21		* @inheritdoc
22		*
23		* @param array $data
24		* @param Form\|null $form
25		*
26		* @return bool\|Member
27		*/
28		public static function authenticate($data, Form $form = null)
29		{
30		Config::inst()->update('Security', 'login_recording', false); // Disable login_recording for this auth.
31		// First, let's see if we know the member
32		$member = parent::authenticate($data, $form);
33		Config::inst()->update('Security', 'login_recording', true);
34		$validationError = ValidationResult::create(false,
35		_t('YubikeyAuthenticator.ERRORYUBIKEY', 'Yubikey authentication error'));
36		if ($member && $member instanceof Member) {
37		// If we know the member, and it's YubiAuth enabled, continue.
38		if ($member &&
39		($member->YubiAuthEnabled \|\| $data['Yubikey'] !== '')
40		) {
41		$data['Yubikey'] = strtolower($data['Yubikey']);
42		$yubiCode = QwertyConvertor::convertString($data['Yubikey']);
43		$yubiFingerprint = substr($yubiCode, 0, -32);
44		// If the member has a yubikey ID set, compare it to the fingerprint.
45		if ($member->Yubikey && strpos($yubiFingerprint, $member->Yubikey) !== 0) {
46		self::updateForm($validationError, $form);
		0 ignored issues – show Bug introduced 2016-11-12 01:40 UTC by Report Bug Copy Issue Report It seems like `$form` defined by parameter `$form` on line `28` can be `null`; however, `Firesphere\YubiAuth\Yubi...enticator::updateForm()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history...
47
48		return null; // Yubikey id doesn't match the member.
49		}
50		$clientID = YUBIAUTH_CLIENTID;
51		$apiKey = YUBIAUTH_APIKEY;
52		$service = new \Yubikey\Validate($apiKey, $clientID);
53		if ($url = self::config()->get('AuthURL')) {
54		$service->setHost($url);
55		}
56		$result = $service->check($yubiCode);
57
58		if ($result->success() === true) {
59		self::updateMember($member, $yubiFingerprint);
60		if ($member) {
61		$member->registerSuccessfulLogin();
62		$member->MaxNoYubiLogins = 0;
63		$member->write();
64		}
65
66		return $member;
67		} else {
68		self::updateForm($validationError, $form);
		0 ignored issues – show Bug introduced 2016-11-12 01:40 UTC by Report Bug Copy Issue Report It seems like `$form` defined by parameter `$form` on line `28` can be `null`; however, `Firesphere\YubiAuth\Yubi...enticator::updateForm()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history...
69
70		return null;
71		}
72		} elseif (!$member->YubiAuthEnabled) { // We do not have to check the YubiAuth for now.
73		$member->NoYubikeyCount += 1;
74		$member->write();
75		$maxNoYubi = Config::inst()->get('YubikeyAuthenticator', 'MaxNoYubiLogin');
76	View Code Duplication	if ($maxNoYubi > 0 && $maxNoYubi <= $member->NoYubikeyCount) {
		0 ignored issues – show Duplication introduced 2016-11-12 01:14 UTC by Report Bug Copy Issue Report 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...
77		$validationError = ValidationResult::create(false,
78		_t('YubikeyAuthenticator.ERRORMAXYUBIKEY', 'Maximum login without yubikey exceeded'));
79		self::updateForm($validationError, $form);
		0 ignored issues – show Bug introduced 2016-11-12 01:40 UTC by Report Bug Copy Issue Report It seems like `$form` defined by parameter `$form` on line `28` can be `null`; however, `Firesphere\YubiAuth\Yubi...enticator::updateForm()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history...
80		$member->registerFailedLogin();
81
82		return null;
83		}
84		$date1 = new DateTime($member->Created);
85		$date2 = new DateTime(date('Y-m-d'));
86
87		$diff = $date2->diff($date1)->format("%a");
88		$maxNoYubiDays = Config::inst()->get('YubikeyAuthenticator', 'MaxNoYubiLoginDays');
89
90	View Code Duplication	if ($maxNoYubiDays > 0 && $diff >= $maxNoYubiDays) {
		0 ignored issues – show Duplication introduced 2016-11-12 01:14 UTC by Report Bug Copy Issue Report 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...
91		$validationError = ValidationResult::create(false,
92		_t('YubikeyAuthenticator.ERRORMAXYUBIKEYDAYS', 'Maximum days without yubikey exceeded'));
93		self::updateForm($validationError, $form);
		0 ignored issues – show Bug introduced 2016-11-12 01:40 UTC by Report Bug Copy Issue Report It seems like `$form` defined by parameter `$form` on line `28` can be `null`; however, `Firesphere\YubiAuth\Yubi...enticator::updateForm()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history...
94		$member->registerFailedLogin();
95
96		return null;
97
98		}
99
100		return $member;
101		}
102		}
103		if ($member) {
104		$member->registerFailedLogin();
105		}
106		self::updateForm($validationError, $form);
		0 ignored issues – show Bug introduced 2016-11-12 01:40 UTC by Report Bug Copy Issue Report It seems like `$form` defined by parameter `$form` on line `28` can be `null`; however, `Firesphere\YubiAuth\Yubi...enticator::updateForm()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history...
107
108		return null;
109		}
110
111		/**
112		* @param Controller $controller
113		*
114		* @return Form
115		*/
116		public static function get_login_form(Controller $controller)
117		{
118		return YubikeyLoginForm::create($controller, 'LoginForm');
119		}
120
121		public static function get_name()
122		{
123		return _t('YubikeyAuthenticator.TITLE', 'Yubikey login');
124		}
125
126		/**
127		* Update the member to forcefully enable YubiAuth
128		* Also, register the Yubikey to the member.
129		* Documentation:
130		* https://developers.yubico.com/yubikey-val/Getting_Started_Writing_Clients.html
131		*
132		* @param Member $member
133		* @param string $yubiString
134		*/
135		private static function updateMember($member, $yubiString)
136		{
137		if (!$member->YubiAuthEnabled) {
138		$member->YubiAuthEnabled = true;
139		}
140		if (!$member->Yubikey) {
141		$member->Yubikey = $yubiString;
142		}
143		$member->write();
144		}
145
146		/**
147		* @param ValidationResult $validation
148		* @param Form $form
149		*/
150		private static function updateForm($validation, $form)
151		{
152		if ($form) {
153		$form->sessionMessage($validation->message(), 'bad');
154		}
155
156		}
157
158		}

Firesphere / silverstripe-yubiauth

Branch — develop (1dbf6d)

YubikeyAuthenticator::authenticate() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like