Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEW Log out all devices for members with no admin access #56

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/Model/LoginSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
58 changes: 54 additions & 4 deletions src/Security/LogOutAuthenticationHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -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
Copy link
Member Author

@emteknetnz emteknetnz Apr 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was copy pasted from https://github.com/silverstripe/silverstripe-mfa/blob/4/src/Service/EnforcementManager.php#L221

It's a little odd, though it works

It's a private function so easy to remove in the future if we add a function like this to core

{
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;
});
}
}
63 changes: 63 additions & 0 deletions tests/php/Security/LogOutAuthenticationHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
}
}
43 changes: 42 additions & 1 deletion tests/php/Security/LogOutAuthenticationHandlerTest.yml
Original file line number Diff line number Diff line change
@@ -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: '[email protected]'
admin_access:
FirstName: 'AdminAccess'
Email: '[email protected]'
Groups: =>SilverStripe\Security\Group.editor
no_admin_access:
FirstName: 'NoAdminAccess'
Email: '[email protected]'
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