LDAPLostPasswordHandler::forgotPassword() - Code Metrics - Inspection of "SS4 security changes and unit test fixes" - silverstripe/silverstripe-activedirectory - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#116)

by Franco

created 2017-09-03 01:40 UTC

LDAPLostPasswordHandler::forgotPassword() C

↳ Parent: LDAPLostPasswordHandler

Complexity

Conditions	12
Paths	23

Size

Total Lines	85
Code Lines	52

Duplication

Lines	4
Ratio	4.71 %

Importance

Changes

Metric	Value
dl	4
loc	85
rs	5.034
c	0
b	0
f	0
cc	12
eloc	52
nc	23
nop	2

How to fix Long Method Complexity

<?php

namespace SilverStripe\ActiveDirectory\Authenticators;

use SilverStripe\ActiveDirectory\Services\LDAPService;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Email\Email;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Convert;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Forms\EmailField;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Form;
use SilverStripe\Forms\FormAction;
use SilverStripe\Forms\TextField;
use SilverStripe\ORM\FieldType\DBField;
use SilverStripe\Security\Member;
use SilverStripe\Security\MemberAuthenticator\LostPasswordForm;
use SilverStripe\Security\MemberAuthenticator\LostPasswordHandler;
use SilverStripe\Security\Security;

class LDAPLostPasswordHandler extends LostPasswordHandler
{
    /**
     * Since the logout and dologin actions may be conditionally removed, it's necessary to ensure these
     * remain valid actions regardless of the member login state.
     *
     * @var array
     * @config
     */
    private static $allowed_actions = [

        'lostpassword',
        'LostPasswordForm',
        'passwordsent',
    ];


    /**
     * @param string $link The URL to recreate this request handler
     * @param LDAPAuthenticator $authenticator
     */
    public function __construct($link, LDAPAuthenticator $authenticator)
    {
        $this->link = $link;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $this->authenticatorClass = get_class($authenticator);
        parent::__construct($link);
    }

    /**
     * Forgot password form handler method.
     *
     * Called when the user clicks on "I've lost my password".
     *
     * Extensions can use the 'forgotPassword' method to veto executing
     * the logic, by returning FALSE. In this case, the user will be redirected back
     * to the form without further action. It is recommended to set a message
     * in the form detailing why the action was denied.
     *
     * @param array $data Submitted data
     * @param LostPasswordForm $form
     * @return HTTPResponse
     */
    public function forgotPassword($data, $form)
    {
        /** @var Controller $controller */
        $controller = $form->getController();

        // No need to protect against injections, LDAPService will ensure that this is safe
        $login = trim($data['Login']);

        $service = Injector::inst()->get(LDAPService::class);
        if (Email::is_valid_address($login)) {
            if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') != 'yes') {
                $form->sessionMessage(
                    _t(
                        'LDAPLoginForm.USERNAMEINSTEADOFEMAIL',
                        'Please enter your username instead of your email to get a password reset link.'
                    ),
                    'bad'
                );
                return $controller->redirect($controller->Link('lostpassword'));
            }
            $userData = $service->getUserByEmail($login);
        } else {
            $userData = $service->getUserByUsername($login);
        }

        // Avoid information disclosure by displaying the same status,
        // regardless whether the email address actually exists
        if (!isset($userData['objectguid'])) {
            return $controller->redirect($controller->Link('passwordsent/')
                . urlencode($data['Login']));
        }

        $member = Member::get()->filter('GUID', $userData['objectguid'])->limit(1)->first();
        // User haven't been imported yet so do that now
        if (!($member && $member->exists())) {

            $member = new Member();
            $member->GUID = $userData['objectguid'];
        }

        // Update the users from LDAP so we are sure that the email is correct.
        // This will also write the Member record.
        $service->updateMemberFromLDAP($member);

        // Allow vetoing forgot password requests
        $results = $this->extend('forgotPassword', $member);
        if ($results && is_array($results) && in_array(false, $results, true)) {

            return $controller->redirect('lostpassword');
        }

        if ($member) {
            /** @see MemberLoginForm::forgotPassword */
            $token = $member->generateAutologinTokenAndStoreHash();
            $e = Email::create()
                ->setSubject(_t('Member.SUBJECTPASSWORDRESET', 'Your password reset link', 'Email subject'))
                ->setHTMLTemplate('ForgotPasswordEmail')
                ->setData($member)
                ->setData(['PasswordResetLink' => Security::getPasswordResetLink($member, $token)]);
            $e->setTo($member->Email);
            $e->send();
            return $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login']));
        } elseif ($data['Login']) {
            // Avoid information disclosure by displaying the same status,
            // regardless whether the email address actually exists
            return $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login']));
        } else {
            if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') {
                $form->sessionMessage(
                    _t(
                        'LDAPLoginForm.ENTERUSERNAMEOREMAIL',
                        'Please enter your username or your email address to get a password reset link.'
                    ),
                    'bad'
                );
            } else {
                $form->sessionMessage(
                    _t(
                        'LDAPLoginForm.ENTERUSERNAME',
                        'Please enter your username to get a password reset link.'
                    ),
                    'bad'
                );
            }
            return $controller->redirect($controller->Link('lostpassword'));
        }
    }

    /**
     * Factory method for the lost password form
     *
     * @return Form Returns the lost password form
     */
    public function lostPasswordForm()
    {
        if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') {

            $loginField = TextField::create(
                'Login',
                _t('LDAPLoginForm.USERNAMEOREMAIL', 'Username or email'),
                null,
                null,
                $this
            );
        } else {
            $loginField = TextField::create('Login', _t('LDAPLoginForm.USERNAME', 'Username'), null, null, $this);
        }

        $action = FormAction::create('forgotPassword', _t('Security.BUTTONSEND', 'Send me the password reset link'));
        return LostPasswordForm::create(
            $this,
            $this->authenticatorClass,
            'LostPasswordForm',
            FieldList::create([$loginField]),
            FieldList::create([$action]),
            false
        );
    }

    public function lostpassword()
    {
        if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') {
            $message = _t(
                'LDAPLostPasswordHandler.NOTERESETPASSWORDUSERNAMEOREMAIL',
                'Enter your username or your email address and we will send you a link with which '
                . 'you can reset your password'
            );
        } else {
            $message = _t(
                'LDAPLostPasswordHandler.NOTERESETPASSWORDUSERNAME',
                'Enter your username and we will send you a link with which you can reset your password'
            );
        }

        return [
            'Content' => DBField::create_field('HTMLFragment', "<p>$message</p>"),
            'Form' => $this->lostPasswordForm(),
        ];
    }

    public function passwordsent()
    {
        $username = Convert::raw2xml(rawurldecode($this->getRequest()->param('ID')));

        return [
            'Title' => _t(
                'LDAPSecurity.PASSWORDSENTHEADER',
                "Password reset link sent to '{username}'",
                ['username' => $username]
            ),
            'Content' =>
                _t(
                    'LDAPSecurity.PASSWORDSENTTEXT',
                    "Thank you! A reset link has been sent to '{username}', provided an account exists.",
                    ['username' => $username]
                ),
            'Username' => $username
        ];
    }
}


1		<?php
2
3		namespace SilverStripe\ActiveDirectory\Authenticators;
4
5		use SilverStripe\ActiveDirectory\Services\LDAPService;
6		use SilverStripe\Control\Controller;
7		use SilverStripe\Control\Email\Email;
8		use SilverStripe\Control\HTTPResponse;
9		use SilverStripe\Core\Config\Config;
10		use SilverStripe\Core\Convert;
11		use SilverStripe\Core\Injector\Injector;
12		use SilverStripe\Forms\EmailField;
13		use SilverStripe\Forms\FieldList;
14		use SilverStripe\Forms\Form;
15		use SilverStripe\Forms\FormAction;
16		use SilverStripe\Forms\TextField;
17		use SilverStripe\ORM\FieldType\DBField;
18		use SilverStripe\Security\Member;
19		use SilverStripe\Security\MemberAuthenticator\LostPasswordForm;
20		use SilverStripe\Security\MemberAuthenticator\LostPasswordHandler;
21		use SilverStripe\Security\Security;
22
23		class LDAPLostPasswordHandler extends LostPasswordHandler
24		{
25		/**
26		* Since the logout and dologin actions may be conditionally removed, it's necessary to ensure these
27		* remain valid actions regardless of the member login state.
28		*
29		* @var array
30		* @config
31		*/
32		private static $allowed_actions = [
		0 ignored issues – show Comprehensibility introduced 2017-01-16 21:03 UTC by Report Bug Copy Issue Report Consider using a different property name as you override a private property of the parent class. Loading history... Unused Code introduced 2017-01-16 21:03 UTC by Report Bug Copy Issue Report The property `$allowed_actions` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
33		'lostpassword',
34		'LostPasswordForm',
35		'passwordsent',
36		];
37
38
39		/**
40		* @param string $link The URL to recreate this request handler
41		* @param LDAPAuthenticator $authenticator
42		*/
43		public function __construct($link, LDAPAuthenticator $authenticator)
44		{
45		$this->link = $link;
		0 ignored issues – show Documentation introduced 2017-09-03 01:42 UTC by Report Bug Copy Issue Report The property `$link` is declared private in `SilverStripe\Security\Me...tor\LostPasswordHandler`. Since you implemented `__set()`, maybe consider adding a @property or @property-write annotation. This makes it easier for IDEs to provide auto-completion. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
46		$this->authenticatorClass = get_class($authenticator);
47		parent::__construct($link);
48		}
49
50		/**
51		* Forgot password form handler method.
52		*
53		* Called when the user clicks on "I've lost my password".
54		*
55		* Extensions can use the 'forgotPassword' method to veto executing
56		* the logic, by returning FALSE. In this case, the user will be redirected back
57		* to the form without further action. It is recommended to set a message
58		* in the form detailing why the action was denied.
59		*
60		* @param array $data Submitted data
61		* @param LostPasswordForm $form
62		* @return HTTPResponse
63		*/
64		public function forgotPassword($data, $form)
65		{
66		/** @var Controller $controller */
67		$controller = $form->getController();
68
69		// No need to protect against injections, LDAPService will ensure that this is safe
70		$login = trim($data['Login']);
71
72		$service = Injector::inst()->get(LDAPService::class);
73		if (Email::is_valid_address($login)) {
74		if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') != 'yes') {
75		$form->sessionMessage(
76		_t(
77		'LDAPLoginForm.USERNAMEINSTEADOFEMAIL',
78		'Please enter your username instead of your email to get a password reset link.'
79		),
80		'bad'
81		);
82		return $controller->redirect($controller->Link('lostpassword'));
83		}
84		$userData = $service->getUserByEmail($login);
85		} else {
86		$userData = $service->getUserByUsername($login);
87		}
88
89		// Avoid information disclosure by displaying the same status,
90		// regardless whether the email address actually exists
91		if (!isset($userData['objectguid'])) {
92		return $controller->redirect($controller->Link('passwordsent/')
93		. urlencode($data['Login']));
94		}
95
96		$member = Member::get()->filter('GUID', $userData['objectguid'])->limit(1)->first();
97		// User haven't been imported yet so do that now
98	View Code Duplication	if (!($member && $member->exists())) {
		0 ignored issues – show Duplication introduced 2017-04-11 01:05 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...
99		$member = new Member();
100		$member->GUID = $userData['objectguid'];
101		}
102
103		// Update the users from LDAP so we are sure that the email is correct.
104		// This will also write the Member record.
105		$service->updateMemberFromLDAP($member);
106
107		// Allow vetoing forgot password requests
108		$results = $this->extend('forgotPassword', $member);
109		if ($results && is_array($results) && in_array(false, $results, true)) {
		0 ignored issues – show Bug Best Practice introduced 2017-01-16 21:03 UTC by Report Bug Copy Issue Report The expression `$results` of type `array` is implicitly converted to a boolean; are you sure this is intended? If so, consider using `! empty($expr)` instead to make it clear that you intend to check for an array without elements. This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent. Consider making the comparison explicit by using `empty(..)` or `! empty(...)` instead. Loading history...
110		return $controller->redirect('lostpassword');
111		}
112
113		if ($member) {
114		/** @see MemberLoginForm::forgotPassword */
115		$token = $member->generateAutologinTokenAndStoreHash();
116		$e = Email::create()
117		->setSubject(_t('Member.SUBJECTPASSWORDRESET', 'Your password reset link', 'Email subject'))
118		->setHTMLTemplate('ForgotPasswordEmail')
119		->setData($member)
120		->setData(['PasswordResetLink' => Security::getPasswordResetLink($member, $token)]);
121		$e->setTo($member->Email);
122		$e->send();
123		return $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login']));
124		} elseif ($data['Login']) {
125		// Avoid information disclosure by displaying the same status,
126		// regardless whether the email address actually exists
127		return $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login']));
128		} else {
129		if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') {
130		$form->sessionMessage(
131		_t(
132		'LDAPLoginForm.ENTERUSERNAMEOREMAIL',
133		'Please enter your username or your email address to get a password reset link.'
134		),
135		'bad'
136		);
137		} else {
138		$form->sessionMessage(
139		_t(
140		'LDAPLoginForm.ENTERUSERNAME',
141		'Please enter your username to get a password reset link.'
142		),
143		'bad'
144		);
145		}
146		return $controller->redirect($controller->Link('lostpassword'));
147		}
148		}
149
150		/**
151		* Factory method for the lost password form
152		*
153		* @return Form Returns the lost password form
154		*/
155		public function lostPasswordForm()
156		{
157	View Code Duplication	if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') {
		0 ignored issues – show Duplication introduced 2017-09-03 01:42 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...
158		$loginField = TextField::create(
159		'Login',
160		_t('LDAPLoginForm.USERNAMEOREMAIL', 'Username or email'),
161		null,
162		null,
163		$this
164		);
165		} else {
166		$loginField = TextField::create('Login', _t('LDAPLoginForm.USERNAME', 'Username'), null, null, $this);
167		}
168
169		$action = FormAction::create('forgotPassword', _t('Security.BUTTONSEND', 'Send me the password reset link'));
170		return LostPasswordForm::create(
171		$this,
172		$this->authenticatorClass,
173		'LostPasswordForm',
174		FieldList::create([$loginField]),
175		FieldList::create([$action]),
176		false
177		);
178		}
179
180		public function lostpassword()
181		{
182		if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') {
183		$message = _t(
184		'LDAPLostPasswordHandler.NOTERESETPASSWORDUSERNAMEOREMAIL',
185		'Enter your username or your email address and we will send you a link with which '
186		. 'you can reset your password'
187		);
188		} else {
189		$message = _t(
190		'LDAPLostPasswordHandler.NOTERESETPASSWORDUSERNAME',
191		'Enter your username and we will send you a link with which you can reset your password'
192		);
193		}
194
195		return [
196		'Content' => DBField::create_field('HTMLFragment', "<p>$message</p>"),
197		'Form' => $this->lostPasswordForm(),
198		];
199		}
200
201		public function passwordsent()
202		{
203		$username = Convert::raw2xml(rawurldecode($this->getRequest()->param('ID')));
204
205		return [
206		'Title' => _t(
207		'LDAPSecurity.PASSWORDSENTHEADER',
208		"Password reset link sent to '{username}'",
209		['username' => $username]
210		),
211		'Content' =>
212		_t(
213		'LDAPSecurity.PASSWORDSENTTEXT',
214		"Thank you! A reset link has been sent to '{username}', provided an account exists.",
215		['username' => $username]
216		),
217		'Username' => $username
218		];
219		}
220		}
221

silverstripe / silverstripe-activedirectory

Pull Request — master (#116)

LDAPLostPasswordHandler::forgotPassword() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like