FIX Better handling of remember me token when login across devices is…

… disabled (#9895)

* BUG Make sure remember me tokens are not invalidated when logging out without the logout_across_devices flag

* Remove unneeded comment

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();
     }
 }

tests/php/Security/RememberLoginHashTest.php

@@ -0,0 +1,75 @@
+<?php
+
+namespace SilverStripe\Security\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Security\Member;
+use SilverStripe\Security\RememberLoginHash;
+
+class RememberLoginHashTest extends SapphireTest
+{
+    protected static $fixture_file = 'RememberLoginHashTest.yml';
+
+    /** @var RememberLoginHash[]  */
+    private $loginHash = [];
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        /** @var Member $main */
+        $main = $this->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"
+            );
+        }
+    }
+}

tests/php/Security/RememberLoginHashTest.yml

@@ -0,0 +1,7 @@
+'SilverStripe\Security\Member':
+  main:
+    FirstName: Main
+    Surname: Test Subject
+  secondary:
+    FirstName: Secondary
+    Surname: Test Subject