Completed
Pull Request — master (#116)
by Franco
01:50
created

LDAPAuthenticator   B

Complexity

Total Complexity 24

Size/Duplication

Total Lines 189
Duplicated Lines 2.12 %

Coupling/Cohesion

Components 0
Dependencies 16

Importance

Changes 0
Metric Value
wmc 24
lcom 0
cbo 16
dl 4
loc 189
rs 8.4614
c 0
b 0
f 0

7 Methods

Rating   Name   Duplication   Size   Complexity  
A get_name() 0 4 1
A get_login_form() 0 4 1
C authenticate() 4 81 14
A fallbackAuthenticate() 0 19 4
A getLoginHandler() 0 4 1
A supportedServices() 0 9 2
A getLostPasswordHandler() 0 4 1

How to fix   Duplicated Code   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

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\HTTPRequest;
9
use SilverStripe\Control\Session;
10
use SilverStripe\Core\Config\Config;
11
use SilverStripe\Core\Injector\Injector;
12
use SilverStripe\ORM\ValidationResult;
13
use SilverStripe\Security\Authenticator;
14
use SilverStripe\Security\Member;
15
use SilverStripe\Security\MemberAuthenticator\LogoutHandler;
16
use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
17
use Zend\Authentication\Result;
18
19
/**
20
 * Class LDAPAuthenticator
21
 *
22
 * Authenticate a user against LDAP, without the single sign-on component.
23
 *
24
 * See SAMLAuthenticator for further information.
25
 *
26
 * @package activedirectory
27
 */
28
class LDAPAuthenticator extends MemberAuthenticator
29
{
30
    /**
31
     * @var string
32
     */
33
    private $name = 'LDAP';
0 ignored issues
show
Unused Code introduced by
The property $name 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...
34
35
    /**
36
     * Set to 'yes' to indicate if this module should look up usernames in LDAP by matching the email addresses.
37
     *
38
     * CAVEAT #1: only set to 'yes' for systems that enforce email uniqueness.
39
     * Otherwise only the first LDAP user with matching email will be accessible.
40
     *
41
     * CAVEAT #2: this is untested for systems that use LDAP with principal style usernames (i.e. [email protected]).
42
     * The system will misunderstand emails for usernames with uncertain outcome.
43
     *
44
     * @var string 'no' or 'yes'
45
     */
46
    private static $allow_email_login = 'no';
0 ignored issues
show
Unused Code introduced by
The property $allow_email_login 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...
47
48
    /**
49
     * Set to 'yes' to fallback login attempts to {@link $fallback_authenticator}.
50
     * This will occur if LDAP fails to authenticate the user.
51
     *
52
     * @var string 'no' or 'yes'
53
     */
54
    private static $fallback_authenticator = 'no';
0 ignored issues
show
Unused Code introduced by
The property $fallback_authenticator 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...
55
56
    /**
57
     * The class of {@link Authenticator} to use as the fallback authenticator.
58
     *
59
     * @var string
60
     */
61
    private static $fallback_authenticator_class = MemberAuthenticator::class;
0 ignored issues
show
Unused Code introduced by
The property $fallback_authenticator_class 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...
62
63
    /**
64
     * @return string
65
     */
66
    public static function get_name()
67
    {
68
        return Config::inst()->get(self::class, 'name');
69
    }
70
71
    /**
72
     * @param Controller $controller
73
     * @return LDAPLoginForm
74
     */
75
    public static function get_login_form(Controller $controller)
76
    {
77
        return LDAPLoginForm::create($controller, LDAPAuthenticator::class, 'LoginForm');
78
    }
79
80
    /**
81
     * Performs the login, but will also create and sync the Member record on-the-fly, if not found.
82
     *
83
     * @param array $data
84
     * @param HTTPRequest $request
85
     * @param ValidationResult|null $result
86
     * @return null|Member
87
     */
88
    public function authenticate(array $data, HTTPRequest $request, ValidationResult &$result = null)
89
    {
90
        $result = $result ?: ValidationResult::create();
91
        /** @var LDAPService $service */
92
        $service = Injector::inst()->get(LDAPService::class);
93
        $login = trim($data['Login']);
94
        if (Email::is_valid_address($login)) {
95
            if (Config::inst()->get(self::class, 'allow_email_login') != 'yes') {
96
                $result->addError(
97
                    _t(
98
                        'LDAPAuthenticator.PLEASEUSEUSERNAME',
99
                        'Please enter your username instead of your email to log in.'
100
                    )
101
                );
102
                return null;
103
            }
104
            $username = $service->getUsernameByEmail($login);
105
106
            // No user found with this email.
107
            if (!$username) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $username of type null|string is loosely compared to false; this is ambiguous if the string can be empty. You might want to explicitly use === null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
108
                if (Config::inst()->get(self::class, 'fallback_authenticator') === 'yes') {
109
                    if ($fallbackMember = $this->fallbackAuthenticate($data, $request)) {
110
                        {
111
                            return $fallbackMember;
112
                        }
113
                    }
114
                }
115
116
                $result->addError(_t('LDAPAuthenticator.INVALIDCREDENTIALS', 'Invalid credentials'));
117
                return null;
118
            }
119
        } else {
120
            $username = $login;
121
        }
122
123
        $serviceAuthenticationResult = $service->authenticate($username, $data['Password']);
124
        $success = $serviceAuthenticationResult['success'] === true;
125
126
        if (!$success) {
127
            /*
128
             * Try the fallback method  unless the reason was invalid credentials. This is to avoid
129
             * having an unhandled exception error thrown by PasswordEncryptor::create_for_algorithm()
130
             */
131
            if (Config::inst()->get(self::class, 'fallback_authenticator') === 'yes'
132
                && !in_array($serviceAuthenticationResult['code'], [Result::FAILURE_CREDENTIAL_INVALID])
133
            ) {
134
                if ($fallbackMember = $this->fallbackAuthenticate($data, $request)) {
135
                    return $fallbackMember;
136
                }
137
            }
138
139
            $result->addError($serviceAuthenticationResult['message']);
140
141
            return null;
142
        }
143
        $data = $service->getUserByUsername($serviceAuthenticationResult['identity']);
144
        if (!$data) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $data 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...
145
            $result->addError(
146
                _t(
147
                    'LDAPAuthenticator.PROBLEMFINDINGDATA',
148
                    'There was a problem retrieving your user data'
149
                )
150
            );
151
            return null;
152
        }
153
154
        // LDAPMemberExtension::memberLoggedIn() will update any other AD attributes mapped to Member fields
155
        $member = Member::get()->filter('GUID', $data['objectguid'])->limit(1)->first();
156 View Code Duplication
        if (!($member && $member->exists())) {
0 ignored issues
show
Duplication introduced by
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...
157
            $member = new Member();
158
            $member->GUID = $data['objectguid'];
159
        }
160
161
        // Update the users from LDAP so we are sure that the email is correct.
162
        // This will also write the Member record.
163
        $service->updateMemberFromLDAP($member);
0 ignored issues
show
Compatibility introduced by
$member of type object<SilverStripe\ORM\DataObject> is not a sub-type of object<SilverStripe\Security\Member>. It seems like you assume a child class of the class SilverStripe\ORM\DataObject to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
164
165
        $request->getSession()->clear('BackURL');
166
167
        return $member;
168
    }
169
170
    /**
171
     * Try to authenticate using the fallback authenticator.
172
     *
173
     * @param array $data
174
     * @param HTTPRequest $request
175
     * @return null|Member
176
     */
177
    protected function fallbackAuthenticate($data, HTTPRequest $request)
178
    {
179
        // Set Email from Login
180
        if (array_key_exists('Login', $data) && !array_key_exists('Email', $data)) {
181
            $data['Email'] = $data['Login'];
182
        }
183
        $authenticatorClass = Config::inst()->get(self::class, 'fallback_authenticator_class');
184
        if ($authenticator = Injector::inst()->get($authenticatorClass)) {
185
            $result = call_user_func(
186
                [
187
                    $authenticator,
188
                    'authenticate'
189
                ],
190
                $data,
191
                $request
192
            );
193
            return $result;
194
        }
195
    }
196
197
    public function getLoginHandler($link)
198
    {
199
        return LDAPLoginHandler::create($link, $this);
200
    }
201
202
    public function supportedServices()
203
    {
204
        $result = Authenticator::LOGIN | Authenticator::LOGOUT | Authenticator::RESET_PASSWORD;
205
206
        if ((bool)LDAPService::config()->get('allow_password_change')) {
207
            $result |= Authenticator::CHANGE_PASSWORD;
208
        }
209
        return $result;
210
    }
211
212
    public function getLostPasswordHandler($link)
213
    {
214
        return LDAPLostPasswordHandler::create($link, $this);
215
    }
216
}
217