Completed
Pull Request — master (#7026)
by Damian
08:24
created

SecurityTest::getValidationResult()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 8
Code Lines 5

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 2
eloc 5
nc 2
nop 0
dl 0
loc 8
rs 9.4285
c 0
b 0
f 0
1
<?php
2
3
namespace SilverStripe\Security\Tests;
4
5
use SilverStripe\Dev\Debug;
6
use SilverStripe\ORM\DataObject;
7
use SilverStripe\ORM\FieldType\DBDatetime;
8
use SilverStripe\ORM\FieldType\DBClassName;
9
use SilverStripe\ORM\DB;
10
use SilverStripe\ORM\ValidationResult;
11
use SilverStripe\Security\LoginAttempt;
12
use SilverStripe\Security\Member;
13
use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
14
use SilverStripe\Security\Security;
15
use SilverStripe\Core\Config\Config;
16
use SilverStripe\Core\Convert;
17
use SilverStripe\Dev\FunctionalTest;
18
use SilverStripe\Dev\TestOnly;
19
use SilverStripe\Control\HTTPResponse;
20
use SilverStripe\Control\Session;
21
use SilverStripe\Control\Director;
22
use SilverStripe\Control\Controller;
23
use SilverStripe\i18n\i18n;
24
25
/**
26
 * Test the security class, including log-in form, change password form, etc
27
 */
28
class SecurityTest extends FunctionalTest
29
{
30
    protected static $fixture_file = 'MemberTest.yml';
31
32
    protected $autoFollowRedirection = false;
33
34
    protected static $extra_controllers = [
35
        SecurityTest\NullController::class,
36
        SecurityTest\SecuredController::class,
37
    ];
38
39
    protected function setUp()
40
    {
41
        // Set to an empty array of authenticators to enable the default
42
        Config::modify()->set(MemberAuthenticator::class, 'authenticators', []);
43
        Config::modify()->set(MemberAuthenticator::class, 'default_authenticator', MemberAuthenticator::class);
44
45
        /**
46
         * @skipUpgrade
47
         */
48
        Member::config()->unique_identifier_field = 'Email';
0 ignored issues
show
Documentation introduced by
The property unique_identifier_field does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
49
50
        parent::setUp();
51
52
        Config::modify()->merge('SilverStripe\\Control\\Director', 'alternate_base_url', '/');
53
    }
54
55
    public function testAccessingAuthenticatedPageRedirectsToLoginForm()
56
    {
57
        $this->autoFollowRedirection = false;
58
59
        $response = $this->get('SecurityTest_SecuredController');
60
        $this->assertEquals(302, $response->getStatusCode());
61
        $this->assertContains(
62
            Config::inst()->get(Security::class, 'login_url'),
63
            $response->getHeader('Location')
64
        );
65
66
        $this->logInWithPermission('ADMIN');
67
        $response = $this->get('SecurityTest_SecuredController');
68
        $this->assertEquals(200, $response->getStatusCode());
69
        $this->assertContains('Success', $response->getBody());
70
71
        $this->autoFollowRedirection = true;
72
    }
73
74
    public function testPermissionFailureSetsCorrectFormMessages()
75
    {
76
        Config::nest();
77
78
        // Controller that doesn't attempt redirections
79
        $controller = new SecurityTest\NullController();
80
        $controller->setResponse(new HTTPResponse());
81
82
        Security::permissionFailure($controller, array('default' => 'Oops, not allowed'));
83
        $this->assertEquals('Oops, not allowed', Session::get('Security.Message.message'));
84
85
        // Test that config values are used correctly
86
        Config::inst()->update(Security::class, 'default_message_set', 'stringvalue');
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface SilverStripe\Config\Coll...nfigCollectionInterface as the method update() does only exist in the following implementations of said interface: SilverStripe\Config\Coll...\MemoryConfigCollection.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
87
        Security::permissionFailure($controller);
88
        $this->assertEquals(
89
            'stringvalue',
90
            Session::get('Security.Message.message'),
91
            'Default permission failure message value was not present'
92
        );
93
94
        Config::modify()->remove(Security::class, 'default_message_set');
95
        Config::modify()->merge(Security::class, 'default_message_set', array('default' => 'arrayvalue'));
96
        Security::permissionFailure($controller);
97
        $this->assertEquals(
98
            'arrayvalue',
99
            Session::get('Security.Message.message'),
100
            'Default permission failure message value was not present'
101
        );
102
103
        // Test that non-default messages work.
104
        // NOTE: we inspect the response body here as the session message has already
105
        // been fetched and output as part of it, so has been removed from the session
106
        $this->logInWithPermission('EDITOR');
107
108
        Config::inst()->update(
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface SilverStripe\Config\Coll...nfigCollectionInterface as the method update() does only exist in the following implementations of said interface: SilverStripe\Config\Coll...\MemoryConfigCollection.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
109
            Security::class,
110
            'default_message_set',
111
            array('default' => 'default', 'alreadyLoggedIn' => 'You are already logged in!')
112
        );
113
        Security::permissionFailure($controller);
114
        $this->assertContains(
115
            'You are already logged in!',
116
            $controller->getResponse()->getBody(),
117
            'Custom permission failure message was ignored'
118
        );
119
120
        Security::permissionFailure(
121
            $controller,
122
            array('default' => 'default', 'alreadyLoggedIn' => 'One-off failure message')
123
        );
124
        $this->assertContains(
125
            'One-off failure message',
126
            $controller->getResponse()->getBody(),
127
            "Message set passed to Security::permissionFailure() didn't override Config values"
128
        );
129
130
        Config::unnest();
131
    }
132
133
    /**
134
     * Follow all redirects recursively
135
     *
136
     * @param  string $url
137
     * @param  int    $limit Max number of requests
138
     * @return HTTPResponse
139
     */
140
    protected function getRecursive($url, $limit = 10)
141
    {
142
        $this->cssParser = null;
143
        $response = $this->mainSession->get($url);
144
        while (--$limit > 0 && $response instanceof HTTPResponse && $response->getHeader('Location')) {
145
            $response = $this->mainSession->followRedirection();
146
        }
147
        return $response;
148
    }
149
150
    public function testAutomaticRedirectionOnLogin()
151
    {
152
        // BackURL with permission error (not authenticated) should not redirect
153
        if ($member = Security::getCurrentUser()) {
154
            Security::setCurrentUser(null);
155
        }
156
        $response = $this->getRecursive('SecurityTest_SecuredController');
157
        $this->assertContains(Convert::raw2xml("That page is secured."), $response->getBody());
158
        $this->assertContains('<input type="submit" name="action_doLogin"', $response->getBody());
159
160
        // Non-logged in user should not be redirected, but instead shown the login form
161
        // No message/context is available as the user has not attempted to view the secured controller
162
        $response = $this->getRecursive('Security/login?BackURL=SecurityTest_SecuredController/');
163
        $this->assertNotContains(Convert::raw2xml("That page is secured."), $response->getBody());
164
        $this->assertNotContains(Convert::raw2xml("You don't have access to this page"), $response->getBody());
165
        $this->assertContains('<input type="submit" name="action_doLogin"', $response->getBody());
166
167
        // BackURL with permission error (wrong permissions) should not redirect
168
        $this->logInAs('grouplessmember');
169
        $response = $this->getRecursive('SecurityTest_SecuredController');
170
        $this->assertContains(Convert::raw2xml("You don't have access to this page"), $response->getBody());
171
        $this->assertContains(
172
            '<input type="submit" name="action_logout" value="Log in as someone else"',
173
            $response->getBody()
174
        );
175
176
        // Directly accessing this page should attempt to follow the BackURL, but stop when it encounters the error
177
        $response = $this->getRecursive('Security/login?BackURL=SecurityTest_SecuredController/');
178
        $this->assertContains(Convert::raw2xml("You don't have access to this page"), $response->getBody());
179
        $this->assertContains(
180
            '<input type="submit" name="action_logout" value="Log in as someone else"',
181
            $response->getBody()
182
        );
183
184
        // Check correctly logged in admin doesn't generate the same errors
185
        $this->logInAs('admin');
186
        $response = $this->getRecursive('SecurityTest_SecuredController');
187
        $this->assertContains(Convert::raw2xml("Success"), $response->getBody());
188
189
        // Directly accessing this page should attempt to follow the BackURL and succeed
190
        $response = $this->getRecursive('Security/login?BackURL=SecurityTest_SecuredController/');
191
        $this->assertContains(Convert::raw2xml("Success"), $response->getBody());
192
    }
193
194
    public function testLogInAsSomeoneElse()
195
    {
196
        $member = DataObject::get_one(Member::class);
197
198
        /* Log in with any user that we can find */
199
        Security::setCurrentUser($member);
200
201
        /* View the Security/login page */
202
        $response = $this->get(Config::inst()->get(Security::class, 'login_url'));
203
204
        $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.action');
205
206
        /* We have only 1 input, one to allow the user to log in as someone else */
207
        $this->assertEquals(count($items), 1, 'There is 1 input, allowing the user to log in as someone else.');
208
209
        $this->autoFollowRedirection = true;
210
211
        /* Submit the form, using only the logout action and a hidden field for the authenticator */
212
        $response = $this->submitForm(
213
            'MemberLoginForm_LoginForm',
214
            null,
215
            array(
216
                'action_logout' => 1,
217
            )
218
        );
219
220
        /* We get a good response */
221
        $this->assertEquals($response->getStatusCode(), 200, 'We have a 200 OK response');
222
        $this->assertNotNull($response->getBody(), 'There is body content on the page');
223
224
        /* Log the user out */
225
        Security::setCurrentUser(null);
226
    }
227
228
    public function testMemberIDInSessionDoesntExistInDatabaseHasToLogin()
229
    {
230
        /* Log in with a Member ID that doesn't exist in the DB */
231
        $this->session()->inst_set('loggedInAs', 500);
232
233
        $this->autoFollowRedirection = true;
234
235
        /* Attempt to get into the admin section */
236
        $response = $this->get(Config::inst()->get(Security::class, 'login_url'));
237
238
        $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.text');
239
240
        /* We have 2 text inputs - one for email, and another for the password */
241
        $this->assertEquals(count($items), 2, 'There are 2 inputs - one for email, another for password');
242
243
        $this->autoFollowRedirection = false;
244
245
        /* Log the user out */
246
        $this->session()->inst_set('loggedInAs', null);
247
    }
248
249
    public function testLoginUsernamePersists()
250
    {
251
        // Test that username does not persist
252
        $this->session()->inst_set('SessionForms.MemberLoginForm.Email', '[email protected]');
253
        Security::config()->remember_username = false;
0 ignored issues
show
Documentation introduced by
The property remember_username does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
254
        $this->get(Config::inst()->get(Security::class, 'login_url'));
255
        $items = $this
256
            ->cssParser()
257
            ->getBySelector('#MemberLoginForm_LoginForm #MemberLoginForm_LoginForm_Email');
258
        $this->assertEquals(1, count($items));
259
        $this->assertEmpty((string)$items[0]->attributes()->value);
260
        $this->assertEquals('off', (string)$items[0]->attributes()->autocomplete);
261
        $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm');
262
        $this->assertEquals(1, count($form));
263
        $this->assertEquals('off', (string)$form[0]->attributes()->autocomplete);
264
265
        // Test that username does persist when necessary
266
        $this->session()->inst_set('SessionForms.MemberLoginForm.Email', '[email protected]');
267
        Security::config()->remember_username = true;
0 ignored issues
show
Documentation introduced by
The property remember_username does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
268
        $this->get(Config::inst()->get(Security::class, 'login_url'));
269
        $items = $this
270
            ->cssParser()
271
            ->getBySelector('#MemberLoginForm_LoginForm #MemberLoginForm_LoginForm_Email');
272
        $this->assertEquals(1, count($items));
273
        $this->assertEquals('[email protected]', (string)$items[0]->attributes()->value);
274
        $this->assertNotEquals('off', (string)$items[0]->attributes()->autocomplete);
275
        $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm');
276
        $this->assertEquals(1, count($form));
277
        $this->assertNotEquals('off', (string)$form[0]->attributes()->autocomplete);
278
    }
279
280
    public function testExternalBackUrlRedirectionDisallowed()
281
    {
282
        // Test internal relative redirect
283
        $response = $this->doTestLoginForm('[email protected]', '1nitialPassword', 'testpage');
284
        $this->assertEquals(302, $response->getStatusCode());
285
        $this->assertRegExp(
286
            '/testpage/',
287
            $response->getHeader('Location'),
288
            "Internal relative BackURLs work when passed through to login form"
289
        );
290
        // Log the user out
291
        $this->session()->inst_set('loggedInAs', null);
292
293
        // Test internal absolute redirect
294
        $response = $this->doTestLoginForm(
295
            '[email protected]',
296
            '1nitialPassword',
297
            Director::absoluteBaseURL() . 'testpage'
298
        );
299
        // for some reason the redirect happens to a relative URL
300
        $this->assertRegExp(
301
            '/^' . preg_quote(Director::absoluteBaseURL(), '/') . 'testpage/',
302
            $response->getHeader('Location'),
303
            "Internal absolute BackURLs work when passed through to login form"
304
        );
305
        // Log the user out
306
        $this->session()->inst_set('loggedInAs', null);
307
308
        // Test external redirect
309
        $response = $this->doTestLoginForm('[email protected]', '1nitialPassword', 'http://myspoofedhost.com');
310
        $this->assertNotRegExp(
311
            '/^' . preg_quote('http://myspoofedhost.com', '/') . '/',
312
            (string)$response->getHeader('Location'),
313
            "Redirection to external links in login form BackURL gets prevented as a measure against spoofing attacks"
314
        );
315
316
        // Test external redirection on ChangePasswordForm
317
        $this->get('Security/changepassword?BackURL=http://myspoofedhost.com');
318
        $changedResponse = $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword');
319
        $this->assertNotRegExp(
320
            '/^' . preg_quote('http://myspoofedhost.com', '/') . '/',
321
            (string)$changedResponse->getHeader('Location'),
322
            "Redirection to external links in change password form BackURL gets prevented to stop spoofing attacks"
323
        );
324
325
        // Log the user out
326
        $this->session()->inst_set('loggedInAs', null);
327
    }
328
329
    /**
330
     * Test that the login form redirects to the change password form after logging in with an expired password
331
     */
332
    public function testExpiredPassword()
333
    {
334
        /* BAD PASSWORDS ARE LOCKED OUT */
335
        $badResponse = $this->doTestLoginForm('[email protected]', 'badpassword');
336
        $this->assertEquals(302, $badResponse->getStatusCode());
337
        $this->assertRegExp('/Security\/login/', $badResponse->getHeader('Location'));
338
        $this->assertNull($this->session()->inst_get('loggedInAs'));
339
340
        /* UNEXPIRED PASSWORD GO THROUGH WITHOUT A HITCH */
341
        $goodResponse = $this->doTestLoginForm('[email protected]', '1nitialPassword');
342
        $this->assertEquals(302, $goodResponse->getStatusCode());
343
        $this->assertEquals(
344
            Controller::join_links(Director::absoluteBaseURL(), 'test/link'),
345
            $goodResponse->getHeader('Location')
346
        );
347
        $this->assertEquals($this->idFromFixture(Member::class, 'test'), $this->session()->inst_get('loggedInAs'));
348
349
        $this->logOut();
350
351
        /* EXPIRED PASSWORDS ARE SENT TO THE CHANGE PASSWORD FORM */
352
        $expiredResponse = $this->doTestLoginForm('[email protected]', '1nitialPassword');
353
        $this->assertEquals(302, $expiredResponse->getStatusCode());
354
        $this->assertEquals(
355
            Director::absoluteURL('Security/changepassword').'?BackURL=test%2Flink',
356
            Director::absoluteURL($expiredResponse->getHeader('Location'))
357
        );
358
        $this->assertEquals(
359
            $this->idFromFixture(Member::class, 'expiredpassword'),
360
            $this->session()->inst_get('loggedInAs')
361
        );
362
363
        // Make sure it redirects correctly after the password has been changed
364
        $this->mainSession->followRedirection();
365
        $changedResponse = $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword');
366
        $this->assertEquals(302, $changedResponse->getStatusCode());
367
        $this->assertEquals(
368
            Controller::join_links(Director::absoluteBaseURL(), 'test/link'),
369
            $changedResponse->getHeader('Location')
370
        );
371
    }
372
373
    public function testChangePasswordForLoggedInUsers()
374
    {
375
        $goodResponse = $this->doTestLoginForm('[email protected]', '1nitialPassword');
376
377
        // Change the password
378
        $this->get('Security/changepassword?BackURL=test/back');
379
        $changedResponse = $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword');
380
        $this->assertEquals(302, $changedResponse->getStatusCode());
381
        $this->assertEquals(
382
            Controller::join_links(Director::absoluteBaseURL(), 'test/back'),
383
            $changedResponse->getHeader('Location')
384
        );
385
        $this->assertEquals($this->idFromFixture(Member::class, 'test'), $this->session()->inst_get('loggedInAs'));
386
387
        // Check if we can login with the new password
388
        $this->logOut();
389
        $goodResponse = $this->doTestLoginForm('[email protected]', 'changedPassword');
390
        $this->assertEquals(302, $goodResponse->getStatusCode());
391
        $this->assertEquals(
392
            Controller::join_links(Director::absoluteBaseURL(), 'test/link'),
393
            $goodResponse->getHeader('Location')
394
        );
395
        $this->assertEquals($this->idFromFixture(Member::class, 'test'), $this->session()->inst_get('loggedInAs'));
396
    }
397
398
    public function testChangePasswordFromLostPassword()
399
    {
400
        $admin = $this->objFromFixture(Member::class, 'test');
401
        $admin->FailedLoginCount = 99;
0 ignored issues
show
Documentation introduced by
The property FailedLoginCount does not exist on object<SilverStripe\ORM\DataObject>. Since you implemented __set, maybe consider adding a @property annotation.

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...
402
        $admin->LockedOutUntil = DBDatetime::now()->getValue();
0 ignored issues
show
Documentation introduced by
The property LockedOutUntil does not exist on object<SilverStripe\ORM\DataObject>. Since you implemented __set, maybe consider adding a @property annotation.

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...
403
        $admin->write();
404
405
        $this->assertNull($admin->AutoLoginHash, 'Hash is empty before lost password');
406
407
        // Request new password by email
408
        $response = $this->get('Security/lostpassword');
409
        $response = $this->post('Security/lostpassword/LostPasswordForm', array('Email' => '[email protected]'));
410
411
        $this->assertEmailSent('[email protected]');
412
413
        // Load password link from email
414
        $admin = DataObject::get_by_id(Member::class, $admin->ID);
415
        $this->assertNotNull($admin->AutoLoginHash, 'Hash has been written after lost password');
416
417
        // We don't have access to the token - generate a new token and hash pair.
418
        $token = $admin->generateAutologinTokenAndStoreHash();
419
420
        // Check.
421
        $response = $this->get('Security/changepassword/?m='.$admin->ID.'&t=' . $token);
422
        $this->assertEquals(302, $response->getStatusCode());
423
        $this->assertEquals(
424
            Director::absoluteURL('Security/changepassword'),
425
            Director::absoluteURL($response->getHeader('Location'))
426
        );
427
428
        // Follow redirection to form without hash in GET parameter
429
        $response = $this->get('Security/changepassword');
430
        $changedResponse = $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword');
431
        $this->assertEquals($this->idFromFixture(Member::class, 'test'), $this->session()->inst_get('loggedInAs'));
432
433
        // Check if we can login with the new password
434
        $this->logOut();
435
        $goodResponse = $this->doTestLoginForm('[email protected]', 'changedPassword');
436
        $this->assertEquals(302, $goodResponse->getStatusCode());
437
        $this->assertEquals($this->idFromFixture(Member::class, 'test'), $this->session()->inst_get('loggedInAs'));
438
439
        $admin = DataObject::get_by_id(Member::class, $admin->ID, false);
440
        $this->assertNull($admin->LockedOutUntil);
441
        $this->assertEquals(0, $admin->FailedLoginCount);
442
    }
443
444
    public function testRepeatedLoginAttemptsLockingPeopleOut()
445
    {
446
        $local = i18n::get_locale();
447
        i18n::set_locale('en_US');
448
449
        Member::config()->lock_out_after_incorrect_logins = 5;
0 ignored issues
show
Documentation introduced by
The property lock_out_after_incorrect_logins does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
450
        Member::config()->lock_out_delay_mins = 15;
0 ignored issues
show
Documentation introduced by
The property lock_out_delay_mins does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
451
452
        // Login with a wrong password for more than the defined threshold
453
        for ($i = 1; $i <= (Member::config()->lock_out_after_incorrect_logins+1); $i++) {
454
            $this->doTestLoginForm('[email protected]', 'incorrectpassword');
455
            $member = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test'));
456
457
            if ($i < Member::config()->get('lock_out_after_incorrect_logins')) {
458
                $this->assertNull(
459
                    $member->LockedOutUntil,
460
                    'User does not have a lockout time set if under threshold for failed attempts'
461
                );
462
                $this->assertHasMessage(
463
                    _t(
464
                        'SilverStripe\\Security\\Member.ERRORWRONGCRED',
465
                        'The provided details don\'t seem to be correct. Please try again.'
466
                    )
467
                );
468
            } else {
469
                // Fuzzy matching for time to avoid side effects from slow running tests
470
                $this->assertGreaterThan(
471
                    time() + 14*60,
472
                    strtotime($member->LockedOutUntil),
473
                    'User has a lockout time set after too many failed attempts'
474
                );
475
            }
476
        }
477
        $msg = _t(
478
            'SilverStripe\\Security\\Member.ERRORLOCKEDOUT2',
479
            'Your account has been temporarily disabled because of too many failed attempts at ' .
480
            'logging in. Please try again in {count} minutes.',
481
            null,
482
            array('count' => Member::config()->lock_out_delay_mins)
483
        );
484
        $this->assertHasMessage($msg);
485
486
487
        $this->doTestLoginForm('[email protected]', '1nitialPassword');
488
        $this->assertNull(
489
            $this->session()->inst_get('loggedInAs'),
490
            'The user can\'t log in after being locked out, even with the right password'
491
        );
492
493
        // (We fake this by re-setting LockedOutUntil)
494
        $member = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test'));
495
        $member->LockedOutUntil = date('Y-m-d H:i:s', time() - 30);
0 ignored issues
show
Documentation introduced by
The property LockedOutUntil does not exist on object<SilverStripe\ORM\DataObject>. Since you implemented __set, maybe consider adding a @property annotation.

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...
496
        $member->write();
497
        $this->doTestLoginForm('[email protected]', '1nitialPassword');
498
        $this->assertEquals(
499
            $this->session()->inst_get('loggedInAs'),
500
            $member->ID,
501
            'After lockout expires, the user can login again'
502
        );
503
504
        // Log the user out
505
        $this->logOut();
506
507
        // Login again with wrong password, but less attempts than threshold
508
        for ($i = 1; $i < Member::config()->lock_out_after_incorrect_logins; $i++) {
509
            $this->doTestLoginForm('[email protected]', 'incorrectpassword');
510
        }
511
        $this->assertNull($this->session()->inst_get('loggedInAs'));
512
        $this->assertHasMessage(
513
            _t('SilverStripe\\Security\\Member.ERRORWRONGCRED', 'The provided details don\'t seem to be correct. Please try again.'),
514
            'The user can retry with a wrong password after the lockout expires'
515
        );
516
517
        $this->doTestLoginForm('[email protected]', '1nitialPassword');
518
        $this->assertEquals(
519
            $this->session()->inst_get('loggedInAs'),
520
            $member->ID,
521
            'The user can login successfully after lockout expires, if staying below the threshold'
522
        );
523
524
        i18n::set_locale($local);
525
    }
526
527
    public function testAlternatingRepeatedLoginAttempts()
528
    {
529
        Member::config()->lock_out_after_incorrect_logins = 3;
0 ignored issues
show
Documentation introduced by
The property lock_out_after_incorrect_logins does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
530
531
        // ATTEMPTING LOG-IN TWICE WITH ONE ACCOUNT AND TWICE WITH ANOTHER SHOULDN'T LOCK ANYBODY OUT
532
533
        $this->doTestLoginForm('[email protected]', 'incorrectpassword');
534
        $this->doTestLoginForm('[email protected]', 'incorrectpassword');
535
536
        $this->doTestLoginForm('[email protected]', 'incorrectpassword');
537
        $this->doTestLoginForm('[email protected]', 'incorrectpassword');
538
539
        $member1 = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test'));
540
        $member2 = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'noexpiry'));
541
542
        $this->assertNull($member1->LockedOutUntil);
543
        $this->assertNull($member2->LockedOutUntil);
544
545
        // BUT, DOING AN ADDITIONAL LOG-IN WITH EITHER OF THEM WILL LOCK OUT, SINCE THAT IS THE 3RD FAILURE IN
546
        // THIS SESSION
547
548
        $this->doTestLoginForm('[email protected]', 'incorrectpassword');
549
        $member1 = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test'));
550
        $this->assertNotNull($member1->LockedOutUntil);
551
552
        $this->doTestLoginForm('[email protected]', 'incorrectpassword');
553
        $member2 = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'noexpiry'));
554
        $this->assertNotNull($member2->LockedOutUntil);
555
    }
556
557
    public function testUnsuccessfulLoginAttempts()
558
    {
559
        Security::config()->login_recording = true;
0 ignored issues
show
Documentation introduced by
The property login_recording does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
560
561
        /* UNSUCCESSFUL ATTEMPTS WITH WRONG PASSWORD FOR EXISTING USER ARE LOGGED */
562
        $this->doTestLoginForm('[email protected]', 'wrongpassword');
563
        $attempt = DataObject::get_one(
564
            LoginAttempt::class,
565
            array(
566
                '"LoginAttempt"."Email"' => '[email protected]'
567
            )
568
        );
569
        $this->assertTrue(is_object($attempt));
570
        $member = DataObject::get_one(
571
            Member::class,
572
            array(
573
                '"Member"."Email"' => '[email protected]'
574
            )
575
        );
576
        $this->assertEquals($attempt->Status, 'Failure');
577
        $this->assertEquals($attempt->Email, '[email protected]');
578
        $this->assertEquals($attempt->Member()->toMap(), $member->toMap());
579
580
        /* UNSUCCESSFUL ATTEMPTS WITH NONEXISTING USER ARE LOGGED */
581
        $this->doTestLoginForm('[email protected]', 'wrongpassword');
582
        $attempt = DataObject::get_one(
583
            LoginAttempt::class,
584
            array(
585
            '"LoginAttempt"."Email"' => '[email protected]'
586
            )
587
        );
588
        $this->assertTrue(is_object($attempt));
589
        $this->assertEquals($attempt->Status, 'Failure');
590
        $this->assertEquals($attempt->Email, '[email protected]');
591
        $this->assertNotEmpty($this->getValidationResult()->getMessages(), 'An invalid email returns a message.');
592
    }
593
594
    public function testSuccessfulLoginAttempts()
595
    {
596
        Security::config()->login_recording = true;
0 ignored issues
show
Documentation introduced by
The property login_recording does not exist on object<SilverStripe\Core\Config\Config_ForClass>. Since you implemented __set, maybe consider adding a @property annotation.

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...
597
598
        /* SUCCESSFUL ATTEMPTS ARE LOGGED */
599
        $this->doTestLoginForm('[email protected]', '1nitialPassword');
600
        $attempt = DataObject::get_one(
601
            LoginAttempt::class,
602
            array(
603
            '"LoginAttempt"."Email"' => '[email protected]'
604
            )
605
        );
606
        $member = DataObject::get_one(
607
            Member::class,
608
            array(
609
            '"Member"."Email"' => '[email protected]'
610
            )
611
        );
612
        $this->assertTrue(is_object($attempt));
613
        $this->assertEquals($attempt->Status, 'Success');
614
        $this->assertEquals($attempt->Email, '[email protected]');
615
        $this->assertEquals($attempt->Member()->toMap(), $member->toMap());
616
    }
617
618
    public function testDatabaseIsReadyWithInsufficientMemberColumns()
619
    {
620
        Security::clear_database_is_ready();
621
        DBClassName::clear_classname_cache();
622
623
        // Assumption: The database has been built correctly by the test runner,
624
        // and has all columns present in the ORM
625
        /**
626
         * @skipUpgrade
627
         */
628
        DB::get_schema()->renameField('Member', 'Email', 'Email_renamed');
629
630
        // Email column is now missing, which means we're not ready to do permission checks
631
        $this->assertFalse(Security::database_is_ready());
632
633
        // Rebuild the database (which re-adds the Email column), and try again
634
        static::resetDBSchema(true);
635
        $this->assertTrue(Security::database_is_ready());
636
    }
637
638
    public function testSecurityControllerSendsRobotsTagHeader()
639
    {
640
        $response = $this->get(Config::inst()->get(Security::class, 'login_url'));
641
        $robotsHeader = $response->getHeader('X-Robots-Tag');
642
        $this->assertNotNull($robotsHeader);
643
        $this->assertContains('noindex', $robotsHeader);
644
    }
645
646
    public function testDoNotSendEmptyRobotsHeaderIfNotDefined()
647
    {
648
        Config::inst()->remove(Security::class, 'robots_tag');
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface SilverStripe\Config\Coll...nfigCollectionInterface as the method remove() does only exist in the following implementations of said interface: SilverStripe\Config\Coll...\MemoryConfigCollection.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
649
        $response = $this->get(Config::inst()->get(Security::class, 'login_url'));
650
        $robotsHeader = $response->getHeader('X-Robots-Tag');
651
        $this->assertNull($robotsHeader);
652
    }
653
654
    /**
655
     * Execute a log-in form using Director::test().
656
     * Helper method for the tests above
657
     */
658
    public function doTestLoginForm($email, $password, $backURL = 'test/link')
659
    {
660
        $this->get(Config::inst()->get(Security::class, 'logout_url'));
661
        $this->session()->inst_set('BackURL', $backURL);
662
        $this->get(Config::inst()->get(Security::class, 'login_url'));
663
664
        return $this->submitForm(
665
            "MemberLoginForm_LoginForm",
666
            null,
667
            array(
668
                'Email' => $email,
669
                'Password' => $password,
670
                'AuthenticationMethod' => MemberAuthenticator::class,
671
                'action_doLogin' => 1,
672
            )
673
        );
674
    }
675
676
    /**
677
     * Helper method to execute a change password form
678
     */
679
    public function doTestChangepasswordForm($oldPassword, $newPassword)
680
    {
681
        return $this->submitForm(
682
            "ChangePasswordForm_ChangePasswordForm",
683
            null,
684
            array(
685
                'OldPassword' => $oldPassword,
686
                'NewPassword1' => $newPassword,
687
                'NewPassword2' => $newPassword,
688
                'action_doChangePassword' => 1,
689
            )
690
        );
691
    }
692
693
    /**
694
     * Assert this message is in the current login form errors
695
     *
696
     * @param string $expected
697
     * @param string $errorMessage
698
     */
699
    protected function assertHasMessage($expected, $errorMessage = null)
700
    {
701
        $messages = [];
702
        $result = $this->getValidationResult();
703
        if ($result) {
704
            foreach ($result->getMessages() as $message) {
705
                $messages[] = $message['message'];
706
            }
707
        }
708
709
        $this->assertContains($expected, $messages, $errorMessage);
710
    }
711
712
    /**
713
     * Get validation result from last login form submission
714
     *
715
     * @return ValidationResult
716
     */
717
    protected function getValidationResult()
718
    {
719
        $result = $this->session()->inst_get('FormInfo.MemberLoginForm_LoginForm.result');
720
        if ($result) {
721
            return unserialize($result);
722
        }
723
        return null;
724
    }
725
}
726