From 460ab0924e6d5e20ee49a3020838b787db3f6691 Mon Sep 17 00:00:00 2001
From: Duncan McClean <duncan@duncanmcclean.com>
Date: Fri, 20 Dec 2024 15:38:19 +0000
Subject: [PATCH] [5.x] Prevent duplicate roles & groups (#11270)

---
 src/Auth/File/User.php      | 22 +++++++++++-------
 tests/Auth/FileUserTest.php | 45 +++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/src/Auth/File/User.php b/src/Auth/File/User.php
index 8935121f33..4da8ee696e 100644
--- a/src/Auth/File/User.php
+++ b/src/Auth/File/User.php
@@ -171,11 +171,14 @@ public function explicitRoles($roles = null)
 
     public function assignRole($role)
     {
-        $roles = collect(Arr::wrap($role))->map(function ($role) {
-            return is_string($role) ? $role : $role->handle();
-        })->all();
+        $roles = collect($this->get('roles', []))
+            ->merge(Arr::wrap($role))
+            ->map(fn ($role) => is_string($role) ? $role : $role->handle())
+            ->unique()
+            ->values()
+            ->all();
 
-        $this->set('roles', array_merge($this->get('roles', []), $roles));
+        $this->set('roles', $roles);
 
         return $this;
     }
@@ -198,11 +201,14 @@ public function removeRole($role)
 
     public function addToGroup($group)
     {
-        $groups = collect(Arr::wrap($group))->map(function ($group) {
-            return is_string($group) ? $group : $group->handle();
-        })->all();
+        $groups = collect($this->get('groups', []))
+            ->merge(Arr::wrap($group))
+            ->map(fn ($group) => is_string($group) ? $group : $group->handle())
+            ->unique()
+            ->values()
+            ->all();
 
-        $this->set('groups', array_merge($this->get('groups', []), $groups));
+        $this->set('groups', $groups);
 
         return $this;
     }
diff --git a/tests/Auth/FileUserTest.php b/tests/Auth/FileUserTest.php
index 6332c16446..6ef2f75244 100644
--- a/tests/Auth/FileUserTest.php
+++ b/tests/Auth/FileUserTest.php
@@ -9,7 +9,9 @@
 use Statamic\Contracts\Auth\Role as RoleContract;
 use Statamic\Contracts\Auth\UserGroup as UserGroupContract;
 use Statamic\Facades\Role;
+use Statamic\Facades\Role as RoleAPI;
 use Statamic\Facades\UserGroup;
+use Statamic\Facades\UserGroup as UserGroupAPI;
 use Statamic\Support\Arr;
 use Tests\PreventSavingStacheItemsToDisk;
 use Tests\TestCase;
@@ -123,4 +125,47 @@ public function it_gets_permissions_from_a_cache()
         // Doing it a second time should give the same result but without multiple calls.
         $this->assertEquals($expectedPermissions, $user->permissions()->all());
     }
+
+    #[Test]
+    public function it_prevents_saving_duplicate_roles()
+    {
+        $roleA = (new \Statamic\Auth\File\Role)->handle('a');
+        $roleB = (new \Statamic\Auth\File\Role)->handle('b');
+        $roleC = (new \Statamic\Auth\File\Role)->handle('c');
+
+        RoleAPI::shouldReceive('find')->with('a')->andReturn($roleA);
+        RoleAPI::shouldReceive('find')->with('b')->andReturn($roleB);
+        RoleAPI::shouldReceive('find')->with('c')->andReturn($roleC);
+        RoleAPI::shouldReceive('all')->andReturn(collect([$roleA, $roleB])); // the stache calls this when getting a user. unrelated to test.
+
+        $user = $this->createPermissible();
+        $user->assignRole('a');
+
+        $this->assertEquals(['a'], $user->get('roles'));
+
+        $user->assignRole(['a', 'b', 'c']);
+
+        $this->assertEquals(['a', 'b', 'c'], $user->get('roles'));
+    }
+
+    #[Test]
+    public function it_prevents_saving_duplicate_groups()
+    {
+        $groupA = (new \Statamic\Auth\File\UserGroup)->handle('a');
+        $groupB = (new \Statamic\Auth\File\UserGroup)->handle('b');
+        $groupC = (new \Statamic\Auth\File\UserGroup)->handle('c');
+
+        UserGroupAPI::shouldReceive('find')->with('a')->andReturn($groupA);
+        UserGroupAPI::shouldReceive('find')->with('b')->andReturn($groupB);
+        UserGroupAPI::shouldReceive('find')->with('c')->andReturn($groupC);
+
+        $user = $this->createPermissible();
+        $user->addToGroup('a');
+
+        $this->assertEquals(['a'], $user->get('groups'));
+
+        $user->addToGroup(['a', 'b', 'c']);
+
+        $this->assertEquals(['a', 'b', 'c'], $user->get('groups'));
+    }
 }