ENH PasswordExpirationMiddleware implementation (#9207)

This commit is contained in:
Serge Latyntsev 2019-09-12 14:34:06 +12:00 committed by Aaron Carlino
parent 6d6c4c652c
commit 233e0e7aa0
6 changed files with 746 additions and 4 deletions

View File

@ -27,6 +27,7 @@ SilverStripe\Core\Injector\Injector:
Middlewares: Middlewares:
AuthenticationMiddleware: '%$SilverStripe\Security\AuthenticationMiddleware' AuthenticationMiddleware: '%$SilverStripe\Security\AuthenticationMiddleware'
BasicAuthMiddleware: '%$SilverStripe\Security\BasicAuthMiddleware' BasicAuthMiddleware: '%$SilverStripe\Security\BasicAuthMiddleware'
PasswordExpirationMiddleware: '%$SilverStripe\Security\PasswordExpirationMiddleware'
SilverStripe\Security\AuthenticationMiddleware: SilverStripe\Security\AuthenticationMiddleware:
properties: properties:
AuthenticationHandler: '%$SilverStripe\Security\AuthenticationHandler' AuthenticationHandler: '%$SilverStripe\Security\AuthenticationHandler'
@ -42,3 +43,11 @@ SilverStripe\Core\Injector\Injector:
Authenticators: Authenticators:
cms: '%$SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator' cms: '%$SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator'
SilverStripe\Security\IdentityStore: '%$SilverStripe\Security\AuthenticationHandler' SilverStripe\Security\IdentityStore: '%$SilverStripe\Security\AuthenticationHandler'
SilverStripe\Security\PasswordExpirationMiddleware:
default_redirect: Security/changepassword
whitelisted_url_startswith:
- Security/basicauthlogin/
- Security/changepassword/
- Security/login/
- Security/logout/

View File

@ -50,6 +50,33 @@ no longer apply when this module is installed. If you have customised the login
by adding form fields, or through custom handlers such as SAML or LDAP, by adding form fields, or through custom handlers such as SAML or LDAP,
you'll need to review those before starting to use the module. you'll need to review those before starting to use the module.
## New PasswordExpirationMiddleware now proactively invalidates members with expired passwords
A new PasswordExpirationMiddleware has been implemented.
It checks passwords of authenticated users for expiration and either enforces a redirection
to a change password form, or resets the user for a request being processed (sets current user to null).
This is considered to be a security enhancement, but potentially might interfere with some custom logic
around password expiration if you have it implemented.
Ideally you should test your setup when upgrading if you use the password expiration functionality.
If you'd like to deactivate the middleware, you can unregister it in your application config like this:
```yml
---
Name: disable-passwordExpirationMiddleware
After:
- '#coresecurity'
---
SilverStripe\Core\Injector\Injector:
SilverStripe\Control\Director:
properties:
Middlewares:
PasswordExpirationMiddleware: null
```
## Deprecation ## Deprecation
* `PasswordValidator` methods `minLength`, `characterStrength`, and `checkHistoricalPasswords` are now deprecated from * `PasswordValidator` methods `minLength`, `characterStrength`, and `checkHistoricalPasswords` are now deprecated from

View File

@ -2,9 +2,12 @@
namespace SilverStripe\Security\MemberAuthenticator; namespace SilverStripe\Security\MemberAuthenticator;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Director; use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse;
use SilverStripe\Core\Convert; use SilverStripe\Core\Convert;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Security\PasswordExpirationMiddleware;
use SilverStripe\Security\CMSSecurity; use SilverStripe\Security\CMSSecurity;
use SilverStripe\Security\Security; use SilverStripe\Security\Security;
@ -65,7 +68,15 @@ class CMSLoginHandler extends LoginHandler
); );
// Get redirect url // Get redirect url
$changePasswordURL = $this->addBackURLParam(Security::singleton()->Link('changepassword')); $changedPasswordLink = Security::singleton()->Link('changepassword');
$changePasswordURL = $this->addBackURLParam($changedPasswordLink);
if (Injector::inst()->has(PasswordExpirationMiddleware::class)) {
$session = $this->getRequest()->getSession();
$passwordExpirationMiddleware = Injector::inst()->get(PasswordExpirationMiddleware::class);
$passwordExpirationMiddleware->allowCurrentRequest($session);
}
$changePasswordURLATT = Convert::raw2att($changePasswordURL); $changePasswordURLATT = Convert::raw2att($changePasswordURL);
$changePasswordURLJS = Convert::raw2js($changePasswordURL); $changePasswordURLJS = Convert::raw2js($changePasswordURL);
$message = _t( $message = _t(

View File

@ -9,6 +9,7 @@ use SilverStripe\Control\RequestHandler;
use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Injector\Injector;
use SilverStripe\ORM\ValidationResult; use SilverStripe\ORM\ValidationResult;
use SilverStripe\Security\Authenticator; use SilverStripe\Security\Authenticator;
use SilverStripe\Security\PasswordExpirationMiddleware;
use SilverStripe\Security\IdentityStore; use SilverStripe\Security\IdentityStore;
use SilverStripe\Security\Member; use SilverStripe\Security\Member;
use SilverStripe\Security\Security; use SilverStripe\Security\Security;
@ -263,7 +264,8 @@ class LoginHandler extends RequestHandler
'good' 'good'
); );
$changedPasswordLink = Security::singleton()->Link('changepassword'); $changedPasswordLink = Security::singleton()->Link('changepassword');
$changePasswordUrl = $this->addBackURLParam($changedPasswordLink);
return $this->redirect($this->addBackURLParam($changedPasswordLink)); return $this->redirect($changePasswordUrl);
} }
} }

View File

@ -0,0 +1,247 @@
<?php declare(strict_types=1);
namespace SilverStripe\Security;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Control\Middleware\HTTPMiddleware;
use SilverStripe\Control\Session;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\ORM\Connect\DatabaseException;
/**
* Check if authenticated user has password expired.
* Depending on the configuration there are following outcomes:
* - if the current requested URL whitelisted, then allow to process further
* - else if the change password form URL is set, then redirect to it
* - else set current user to null (deauthenticate for the current request) and process further
*/
class PasswordExpirationMiddleware implements HTTPMiddleware
{
use Configurable;
/**
* Session key for persisting URL of the password change form
*/
const SESSION_KEY_REDIRECT = __CLASS__.'.change password redirect';
/**
* Session key for persisting a flag allowing to process the current request
* without performing password expiration check
*/
const SESSION_KEY_ALLOW_CURRENT_REQUEST = __CLASS__.'.allow current request';
/**
* List of URL patterns allowed for users to visit where
* URL starts with the pattern
*
* @var string[]
*
* @config
*/
private static $whitelisted_url_startswith = [];
/**
* Where users with expired passwords get redirected by default
* when login form didn't register a custom one with
* {@see SilverStripe\Security\AuthenticationMiddleware::setRedirect}
*
* @var string
*
* @config
*/
private static $default_redirect = null;
/**
* The list of mimetypes allowing a redirect to a change password form.
* By default this is (x)HTML
*
* @var string[]
*
* @config
*/
private static $mimetypes_allowing_redirect = [
'*/*',
'text/*',
'text/html',
'application/xhtml+xml',
'text/xml',
'application/xml'
];
public function process(HTTPRequest $request, callable $delegate)
{
try {
if ($response = $this->checkForExpiredPassword($request)) {
return $response;
}
} catch (DatabaseException $e) {
// Database isn't ready, carry on.
}
return $delegate($request);
}
/**
* Check if the just authenticated member has the password expired.
* Returns a response if the current request should not be
* processed as usual.
*
* @param HTTPRequest $request
*
* @return HTTPResponse|null
*/
protected function checkForExpiredPassword(HTTPRequest $request): ?HTTPResponse
{
$session = $request->getSession();
if ($session && $session->get(static::SESSION_KEY_ALLOW_CURRENT_REQUEST)) {
// allow current request and skip the expiration check, but for only the current
// request, so we're deleting the flag from the session so it's not affecting other
// requests.
// This flag would usually be set from within $handler->authenticateRequest()
$session->clear(static::SESSION_KEY_ALLOW_CURRENT_REQUEST);
return null;
}
$user = Security::getCurrentUser();
if ($user && $user->isPasswordExpired()) {
if ($response = $this->handleExpiredPassword($request)) {
return $response;
}
}
return null;
}
/**
* Check if we have a redirect to a password change form registered
* and redirect there if possible.
* Otherwise, deauthenticate the user by resetting it for this request,
* since we should treat ones with expired passwords as unauthorised.
*
* @param HTTPRequest $request
*
* @return HTTPResponse|null
*/
protected function handleExpiredPassword(HTTPRequest $request): ?HTTPResponse
{
$session = $request->getSession();
$sessionRedirectUrl = $session->get(static::SESSION_KEY_REDIRECT);
$defaultRedirectUrl = static::config()->get('default_redirect');
if ($sessionRedirectUrl || $defaultRedirectUrl) {
$redirectUrl = $this->absoluteUrl($sessionRedirectUrl ?? $defaultRedirectUrl);
} else {
$redirectUrl = null;
}
if (!$session || !$redirectUrl) {
Security::setCurrentUser(null);
return null;
}
$currentUrl = $this->absoluteUrl($request->getURL(true));
if ($currentUrl === $redirectUrl) {
return null;
}
$allowedStartswith = static::config()->get('whitelisted_url_startswith');
if (is_array($allowedStartswith)) {
foreach ($allowedStartswith as $pattern) {
$startswith = $this->absoluteUrl($pattern);
if (strncmp($currentUrl, $startswith, strlen($startswith)) === 0) {
return null;
}
}
}
return $this->redirectOrForbid($request, $redirectUrl);
}
/**
* Builds an absolute URL for the given path, adds base url
* if the path configured as absolute
*
* @param string $url
*
* @return string
*/
protected static function absoluteUrl($url): string
{
if (substr($url, 0, 1) === '/' && substr($url, 1, 1) !== '/') {
// add BASE_URL explicitly if not absolute
$url = Controller::join_links(Director::absoluteBaseURL(), $url);
} else {
$url = Director::absoluteURL($url) ?: Controller::join_links(Director::absoluteBaseURL(), $url);
}
if (substr($url, -1) === '/') {
$url = substr($url, 0, -1);
}
return $url;
}
/**
* Returns a redirect to the URL if text/html is acceptable, otherwise
* deauthenticates the current request by Security::setCurrentUser(null)
*
* @param HTTPRequest $request
* @param string $redirectUrl
*
* @return HTTPResponse|null
*/
private function redirectOrForbid(HTTPRequest $request, $redirectUrl): ?HTTPResponse
{
$acceptableTypes = $request->getAcceptMimetypes();
$allowedTypes = static::config()->get('mimetypes_allowing_redirect') ?? [];
if (count(array_intersect($allowedTypes, $acceptableTypes)) > 0) {
$redirectAllowed = true;
} else {
// if browser didn't send the Accept header
// with mimetypes, let's redirect anyway
$redirectAllowed = count($acceptableTypes) === 0;
}
if ($redirectAllowed) {
$response = new HTTPResponse();
$response->redirect($redirectUrl);
return $response;
}
Security::setCurrentUser(null);
return null;
}
/**
* Preserve the password change URL in the session
* That URL is to be redirected to to force users change expired passwords
*
* @param Session $session Session where we persist the redirect URL
* @param string $url change password form address
*/
public static function setRedirect(Session $session, $url)
{
$session->set(static::SESSION_KEY_REDIRECT, $url);
}
/**
* Allow the current request to be finished without password expiration check
*
* @param Session $session Session where we persist the redirect URL
* @param string $url change password form address
*/
public static function allowCurrentRequest(Session $session)
{
$session->set(static::SESSION_KEY_ALLOW_CURRENT_REQUEST, true);
}
}

View File

@ -0,0 +1,446 @@
<?php
namespace SilverStripe\Security\Tests;
use SilverStripe\Control\Director;
use SilverStripe\Control\Tests\HttpRequestMockBuilder;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\ValidationException;
use SilverStripe\Security\PasswordExpirationMiddleware;
use SilverStripe\Security\Member;
use SilverStripe\Security\Security;
class PasswordExpirationMiddlewareTest extends SapphireTest
{
use HttpRequestMockBuilder;
protected function setUp()
{
parent::setUp();
Director::config()->set('alternate_base_url', 'http://localhost/custom-base/');
PasswordExpirationMiddleware::config()->set('default_redirect', null);
PasswordExpirationMiddleware::config()->set('whitelisted_url_startswith', []);
}
/**
* Returns Member mock object
*
* @param bool $isPasswordExpired result of the function {@see SilverStripe\Security\Member::isPasswordExpired}
*
* @return Member
*/
private function getMemberMock($isPasswordExpired) : Member
{
$mock = $this->createMock(Member::class);
$mock->method('isPasswordExpired')->will($this->returnValue($isPasswordExpired));
return $mock;
}
public function test200()
{
$member = $this->getMemberMock(false);
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$executed = false;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed) {
$executed = true;
return "delegated";
});
$this->assertEquals($member, Security::getCurrentUser());
$this->assertTrue($executed);
$this->assertEquals("delegated", $response);
}
/**
* Check a member with an expired password is allowed to process the request in
* deauthorised mode (Security::getCurrentUser() === null) if there are no
* change password redirects registered
*
* @depends test200
*/
public function testDeauthorised()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertTrue($executed);
$this->assertNull($activeMember);
}
/**
* Check a member with an expired password is redirected to a change password form
* instead of processing its original request
*
* @depends test200
*/
public function testRedirected()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, '/redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertFalse($executed);
$this->assertNull($activeMember);
$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(Director::absoluteURL('redirect-address-custom'), $response->getHeader('Location'));
}
/**
* Check we handle network locations correctly (the relative urls starting with //)
*
* @depends testRedirected
*/
public function testNetworkLocationRedirect()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, '//localhost/custom-base/redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertFalse($executed);
$this->assertNull($activeMember);
$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(Director::absoluteURL('redirect-address-custom'), $response->getHeader('Location'));
}
/**
* Check we can allow the current request handling even with an expired password
*
* @depends test200
* @depends testDeauthorised
*/
public function testAllowRequest()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$session = $request->getSession();
PasswordExpirationMiddleware::allowCurrentRequest($session);
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertTrue($executed);
$this->assertEquals($member, $activeMember);
}
/**
* Check a member with an expired password is redirected to a default change password form
* if a custom not set
*
* @depends testRedirected
*/
public function testDefaultRedirect()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('default_redirect', 'redirect-address-default');
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertFalse($executed);
$this->assertNull($activeMember);
$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(Director::absoluteURL('redirect-address-default'), $response->getHeader('Location'));
}
/**
* Check a member with an expired password is redirected to a default change password form
* if a custom not set
*
* @depends testDefaultRedirect
*/
public function testCustomRedirect()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('default_redirect', '/redirect-address-default');
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertFalse($executed);
$this->assertNull($activeMember);
$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(Director::absoluteURL('redirect-address-default'), $response->getHeader('Location'));
}
/**
* Check a custom redirect URL overrides the default one
*
* @depends testCustomRedirect
*/
public function testCustomOverDefaultRedirect()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('default_redirect', '/redirect-address-default');
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, '/redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertFalse($executed);
$this->assertNull($activeMember);
$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(Director::absoluteURL('redirect-address-custom'), $response->getHeader('Location'));
}
/**
* Test we can allow URLs to be visited without redirections through config
*
* @depends testRedirected
*/
public function testAllowedUrlStartswithNegative()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('whitelisted_url_startswith', [
'/allowed-address-configured/'
]);
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/not-allowed');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, '/redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertFalse($executed);
$this->assertNull($activeMember);
$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(Director::absoluteURL('redirect-address-custom'), $response->getHeader('Location'));
}
/**
* Test we can allow URLs to be visited without redirections through config
*
* @depends testRedirected
*/
public function testAllowedUrlStartswithPositivePattern()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('whitelisted_url_startswith', [
'/allowed-address-configured/'
]);
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/allowed-address-configured/subsection1/subsection2/');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, '/redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertTrue($executed);
$this->assertEquals($member, $activeMember);
}
public function testAllowedUrlStartswithPositiveTrailingSlash()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('whitelisted_url_startswith', [
'/allowed-address-configured/'
]);
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/allowed-address-configured?foo=bar');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, '/redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertTrue($executed);
$this->assertEquals($member, $activeMember);
}
public function testAllowedUrlStartswithPositiveRelativeUrl()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('whitelisted_url_startswith', [
'allowed-address-configured/'
]);
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/allowed-address-configured?foo=bar');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, 'redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertTrue($executed);
$this->assertEquals($member, $activeMember);
}
/**
* Test we can allow URLs to be visited without redirections through config
*
* @depends testRedirected
*/
public function testAllowedUrlStartswithPositiveExactUrl()
{
$member = $this->getMemberMock(true);
$this->assertTrue($member->isPasswordExpired());
PasswordExpirationMiddleware::config()->set('whitelisted_url_startswith', [
'/allowed-address-configured/'
]);
$a = new PasswordExpirationMiddleware();
$request = $this->buildRequestMock('/allowed-address-configured/');
$request->method('getAcceptMimetypes')->will($this->returnValue(['*/*']));
$session = $request->getSession();
$a->setRedirect($session, '/redirect-address-custom');
$executed = false;
$activeMember = null;
Security::setCurrentUser($member);
$response = $a->process($request, static function () use (&$executed, &$activeMember) {
$executed = true;
$activeMember = Security::getCurrentUser();
});
$this->assertTrue($executed);
$this->assertEquals($member, $activeMember);
}
}