From 1535bc2cc20a2dbf5f64e2d11941a17e0c98643d Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Mon, 12 Apr 2021 17:39:56 +1200 Subject: [PATCH] NEW Log out all devices for members with no admin access --- README.md | 14 ++--- src/Model/LoginSession.php | 2 +- src/Security/LogOutAuthenticationHandler.php | 58 +++++++++++++++-- .../LogOutAuthenticationHandlerTest.php | 63 +++++++++++++++++++ .../LogOutAuthenticationHandlerTest.yml | 43 ++++++++++++- 5 files changed, 166 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 4ba0586..c6482d1 100644 --- a/README.md +++ b/README.md @@ -53,18 +53,16 @@ It is also compatible with the [Silverstripe MFA module suite](https://github.co ## Configuration -### Logout across devices +### Logout all devices for members without admin access -This module respects the `SilverStripe\Security\RememberLoginHash.logout_across_devices` config setting, which defaults to `true`. This means that the default behaviour is to revoke _all_ a user’s sessions when they log out. +Some sites allow the creation of members with no admin access, for example sites that allow website users to create an account with a corresponding entry in the member table. These members have no way to access the session manager and instantly log out an unwanted device. -To change this so that logging out will only revoke the session for that one device, use the following config setting: +For these members who cannot access admin, when they log out from one device, any existing logged in devices will automatically be logged out. If they had ticked the 'Remember me' checkbox when logging in, they will not auto-log back in. -```yml -SilverStripe\Security\RememberLoginHash: - logout_across_devices: false -``` +This functionality is on by default and can be turned off by setting +`SilverStripe\SessionManager\Security\LogOutAuthenticationHandler.no_admin_access_revoke_all_on_logout` to false -**Important:** do not set this value to false if users do not have access to the CMS (or a custom UI where they can revoke sessions). Doing so would make it impossible to a user to revoke a session if they suspect their device has been compromised. +Note: The existing config setting `SilverStripe\Security\RememberLoginHash.logout_across_devices` set to true only prevents devices from auto-logging in ("Remember me" checkbox) in the future, it does not instantly log out devices. ### Session timeout diff --git a/src/Model/LoginSession.php b/src/Model/LoginSession.php index cdf2757..9499e1c 100644 --- a/src/Model/LoginSession.php +++ b/src/Model/LoginSession.php @@ -174,7 +174,7 @@ public function handlePermission(string $funcName, $member): bool } // Members can manage their own sessions - if ($this->ID == $member->ID) { + if ($this->MemberID === $member->ID) { return true; } diff --git a/src/Security/LogOutAuthenticationHandler.php b/src/Security/LogOutAuthenticationHandler.php index d065819..dd87be9 100644 --- a/src/Security/LogOutAuthenticationHandler.php +++ b/src/Security/LogOutAuthenticationHandler.php @@ -3,12 +3,14 @@ namespace SilverStripe\SessionManager\Security; use InvalidArgumentException; +use SilverStripe\Admin\LeftAndMain; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Security\AuthenticationHandler; use SilverStripe\Security\Member; -use SilverStripe\Security\RememberLoginHash; use SilverStripe\Security\Security; use SilverStripe\SessionManager\Model\LoginSession; @@ -18,6 +20,16 @@ */ class LogOutAuthenticationHandler implements AuthenticationHandler { + use Configurable; + + /** + * Members with no admin access i.e. website user accounts - revoke all sessions on logout + * + * @config + * @var bool + */ + private static $no_admin_access_revoke_all_on_logout = true; + /** * @param HTTPRequest $request * @return Member|null @@ -58,9 +70,14 @@ public function logOut(HTTPRequest $request = null) $loginHandler = Injector::inst()->get(LogInAuthenticationHandler::class); $member = Security::getCurrentUser(); - if (RememberLoginHash::config()->get('logout_across_devices')) { - foreach ($member->LoginSessions() as $session) { - $session->delete(); + // Members without admin access are unable to access session manager and individually revoke login sessions + // These types of members often exist on sites where website users can create accounts via the frontend + // and a corresponding record is created on the Member table + // Since there's no other way for these users to logout any malicious devices, auto log out of all devices + // when one device is logged out + if (static::config()->get('no_admin_access_revoke_all_on_logout') && !$this->hasAdminAccess($member)) { + foreach ($member->LoginSessions() as $loginSession) { + $loginSession->delete(); } } else { $loginSessionID = $request->getSession()->get($loginHandler->getSessionVariable()); @@ -72,4 +89,37 @@ public function logOut(HTTPRequest $request = null) $request->getSession()->clear($loginHandler->getSessionVariable()); } + + /** + * Decides whether the provided user has access to any LeftAndMain controller, which indicates some level + * of access to the CMS. + * + * @see LeftAndMain::init() + * @param Member $member + * @return bool + */ + private function hasAdminAccess(Member $member): bool + { + return Member::actAs($member, function () use ($member) { + $leftAndMain = LeftAndMain::singleton(); + if ($leftAndMain->canView($member)) { + return true; + } + + // Look through all LeftAndMain subclasses to find if one permits the member to view + $menu = $leftAndMain->MainMenu(false); + foreach ($menu as $candidate) { + if ( + $candidate->Link + && $candidate->Link !== $leftAndMain->Link() + && $candidate->MenuItem->controller + && singleton($candidate->MenuItem->controller)->canView($member) + ) { + return true; + } + } + + return false; + }); + } } diff --git a/tests/php/Security/LogOutAuthenticationHandlerTest.php b/tests/php/Security/LogOutAuthenticationHandlerTest.php index 2d01c9e..32c2826 100644 --- a/tests/php/Security/LogOutAuthenticationHandlerTest.php +++ b/tests/php/Security/LogOutAuthenticationHandlerTest.php @@ -4,6 +4,7 @@ use SilverStripe\Control\Session; use SilverStripe\Control\Tests\HttpRequestMockBuilder; +use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; use SilverStripe\Security\Member; use SilverStripe\Security\Security; @@ -35,4 +36,66 @@ public function testLogout() "Login session is deleted on logout" ); } + + public function testMemberAdminAccessLogout() + { + $deviceALoginSessionID = $this->objFromFixture(LoginSession::class, 'admin_access_device_a')->ID; + $session = new Session(['activeLoginSession' => $deviceALoginSessionID]); + $request = $this->buildRequestMock('/', [], [], null, $session); + $request->method('getIP')->willReturn('192.168.0.1'); + + $member = $this->objFromFixture(Member::class, 'admin_access'); + Security::setCurrentUser($member); + + $logOutAuthenticationHandler = new LogOutAuthenticationHandler(); + $logOutAuthenticationHandler->logOut($request); + + $loginSession = $member->LoginSessions()->find('UserAgent', 'Admin Access Device A'); + $this->assertNull($loginSession, 'Login session A is deleted on logout'); + + $loginSession = $member->LoginSessions()->find('UserAgent', 'Admin Access Device B'); + $this->assertNotNull($loginSession, 'Login session B is not deleted on logout'); + } + + public function testMemberNoAdminAccessLogout() + { + $deviceALoginSessionID = $this->objFromFixture(LoginSession::class, 'no_admin_access_device_a')->ID; + $session = new Session(['activeLoginSession' => $deviceALoginSessionID]); + $request = $this->buildRequestMock('/', [], [], null, $session); + $request->method('getIP')->willReturn('192.168.0.1'); + + $member = $this->objFromFixture(Member::class, 'no_admin_access'); + Security::setCurrentUser($member); + + $logOutAuthenticationHandler = new LogOutAuthenticationHandler(); + $logOutAuthenticationHandler->logOut($request); + + $loginSession = $member->LoginSessions()->find('UserAgent', 'No Admin Access Device A'); + $this->assertNull($loginSession, 'Login session A is deleted on logout'); + + $loginSession = $member->LoginSessions()->find('UserAgent', 'No Admin Access Device B'); + $this->assertNull($loginSession, 'Login session B is deleted on logout'); + } + + public function testMemberNoAdminAccessLogoutRevokeAllFalse() + { + Config::modify()->set(LogOutAuthenticationHandler::class, 'no_admin_access_revoke_all_on_logout', false); + + $deviceALoginSessionID = $this->objFromFixture(LoginSession::class, 'no_admin_access_device_a')->ID; + $session = new Session(['activeLoginSession' => $deviceALoginSessionID]); + $request = $this->buildRequestMock('/', [], [], null, $session); + $request->method('getIP')->willReturn('192.168.0.1'); + + $member = $this->objFromFixture(Member::class, 'no_admin_access'); + Security::setCurrentUser($member); + + $logOutAuthenticationHandler = new LogOutAuthenticationHandler(); + $logOutAuthenticationHandler->logOut($request); + + $loginSession = $member->LoginSessions()->find('UserAgent', 'No Admin Access Device A'); + $this->assertNull($loginSession, 'Login session A is deleted on logout'); + + $loginSession = $member->LoginSessions()->find('UserAgent', 'No Admin Access Device B'); + $this->assertNotNull($loginSession, 'Login session B is not deleted on logout'); + } } diff --git a/tests/php/Security/LogOutAuthenticationHandlerTest.yml b/tests/php/Security/LogOutAuthenticationHandlerTest.yml index fac1f9e..c64ffd5 100644 --- a/tests/php/Security/LogOutAuthenticationHandlerTest.yml +++ b/tests/php/Security/LogOutAuthenticationHandlerTest.yml @@ -1,11 +1,52 @@ +SilverStripe\Security\Group: + editor: + Title: Editor + website: + Title: WebsiteUser + +SilverStripe\Security\Permission: + editor: + Code: CMS_ACCESS_AssetAdmin + Group: =>SilverStripe\Security\Group.editor + website: + Code: CUSTOM_PERMISSION_LoginToTheWebsiteFrontend + Group: =>SilverStripe\Security\Group.website + SilverStripe\Security\Member: member1: FirstName: 'Andre' Email: 'andre@example.org' + admin_access: + FirstName: 'AdminAccess' + Email: 'admin_access@example.com' + Groups: =>SilverStripe\Security\Group.editor + no_admin_access: + FirstName: 'NoAdminAccess' + Email: 'no_admin_access@example.com' + Groups: =>SilverStripe\Security\Group.website SilverStripe\SessionManager\Model\LoginSession: x1: LastAccessed: '2003-02-15 10:00:00' IPAddress: '192.168.0.1' Member: =>SilverStripe\Security\Member.member1 - + admin_access_device_a: + UserAgent: 'Admin Access Device A' + LastAccessed: '2003-02-15 10:00:00' + IPAddress: '192.168.0.1' + Member: =>SilverStripe\Security\Member.admin_access + admin_access_device_b: + UserAgent: 'Admin Access Device B' + LastAccessed: '2003-02-15 10:00:00' + IPAddress: '192.168.0.1' + Member: =>SilverStripe\Security\Member.admin_access + no_admin_access_device_a: + UserAgent: 'No Admin Access Device A' + LastAccessed: '2003-02-15 10:00:00' + IPAddress: '192.168.0.1' + Member: =>SilverStripe\Security\Member.no_admin_access + no_admin_access_device_b: + UserAgent: 'No Admin Access Device B' + LastAccessed: '2003-02-15 10:00:00' + IPAddress: '192.168.0.1' + Member: =>SilverStripe\Security\Member.no_admin_access