diff --git a/src/Security/RememberLoginHash.php b/src/Security/RememberLoginHash.php index cdde1bf7887..332ac1d2f0f 100644 --- a/src/Security/RememberLoginHash.php +++ b/src/Security/RememberLoginHash.php @@ -172,19 +172,22 @@ public function renew() * only the token for the provided device ID will be removed * * @param Member $member - * @param string $alcDevice + * @param string|null $alcDevice Null when logging out of non-persi-tien session */ public static function clear(Member $member, $alcDevice = null) { if (!$member->exists()) { + // If we don't have a valid user, we can't clear any "Remember me" tokens return; } - $filter = ['MemberID'=>$member->ID]; - if (!static::config()->logout_across_devices && $alcDevice) { - $filter['DeviceID'] = $alcDevice; + + if (static::config()->logout_across_devices) { + self::get()->filter(['MemberID' => $member->ID])->removeAll(); + } elseif ($alcDevice) { + self::get()->filter([ + 'MemberID' => $member->ID, + 'DeviceID' => $alcDevice + ])->removeAll(); } - RememberLoginHash::get() - ->filter($filter) - ->removeAll(); } } diff --git a/tests/php/Security/RememberLoginHashTest.php b/tests/php/Security/RememberLoginHashTest.php new file mode 100644 index 00000000000..e4987c12508 --- /dev/null +++ b/tests/php/Security/RememberLoginHashTest.php @@ -0,0 +1,75 @@ +objFromFixture(Member::class, 'main'); + + /** @var Member $secondary */ + $secondary = $this->objFromFixture(Member::class, 'secondary'); + + $this->loginHash = [ + 'current' => RememberLoginHash::generate($main), + 'other' => RememberLoginHash::generate($main), + 'secondary' => RememberLoginHash::generate($secondary), + ]; + } + + public function clearScenarios() + { + return [ + 'logout across devices' => [true, 'current', ['secondary'], ['current', 'other']], + 'logout across devices on non-persistent session' => [true, false, ['secondary'], ['current', 'other']], + 'logout single device' => [false, 'current', ['secondary', 'other'], ['current']], + 'logout single device on non-persistent session' => [false, false, ['secondary', 'current', 'other'], []], + ]; + } + + /** + * @param bool $logoutAcrossDevices + * @param string $deviceId + * @param array $expected + * @param array $unexpected + * @dataProvider clearScenarios + */ + public function testClear(bool $logoutAcrossDevices, string $deviceId, array $expected, array $unexpected) + { + RememberLoginHash::config()->set('logout_across_devices', $logoutAcrossDevices); + + RememberLoginHash::clear( + $this->objFromFixture(Member::class, 'main'), + $deviceId ? $this->loginHash[$deviceId]->DeviceID : null + ); + + foreach ($expected as $key) { + $ID = $this->loginHash[$key]->ID; + $this->assertNotEmpty( + RememberLoginHash::get()->byID($ID), + "$key $ID RememberLoginHash is found" + ); + } + + foreach ($unexpected as $key) { + $ID = $this->loginHash[$key]->ID; + $this->assertEmpty( + RememberLoginHash::get()->byID($ID), + "$key RememberLoginHash has been removed" + ); + } + } +} diff --git a/tests/php/Security/RememberLoginHashTest.yml b/tests/php/Security/RememberLoginHashTest.yml new file mode 100644 index 00000000000..8f01780344f --- /dev/null +++ b/tests/php/Security/RememberLoginHashTest.yml @@ -0,0 +1,7 @@ +'SilverStripe\Security\Member': + main: + FirstName: Main + Surname: Test Subject + secondary: + FirstName: Secondary + Surname: Test Subject