浏览代码

Prevent last admin demotion - Closes #331

Bubka 1 年之前
父节点
当前提交
1bc55f5535

+ 13 - 7
app/Api/v1/Controllers/UserManagerController.php

@@ -102,9 +102,10 @@ class UserManagerController extends Controller
         Log::info(sprintf('User ID #%s created by user ID #%s', $user->id, $request->user()->id));
 
         if ($validated['is_admin']) {
-            $user->promoteToAdministrator();
-            $user->save();
-            Log::notice(sprintf('User ID #%s set as administrator at creation by user ID #%s', $user->id, $request->user()->id));
+            if ($user->promoteToAdministrator()) {
+                $user->save();
+                Log::notice(sprintf('User ID #%s set as administrator at creation by user ID #%s', $user->id, $request->user()->id));
+            }
         }
 
         $user->refresh();
@@ -192,12 +193,17 @@ class UserManagerController extends Controller
     {
         $this->authorize('promote', $user);
 
-        $user->promoteToAdministrator($request->validated('is_admin'));
-        $user->save();
+        if ($user->promoteToAdministrator($request->validated('is_admin')))
+        {
+            $user->save();
+            Log::info(sprintf('User ID #%s set is_admin=%s for User ID #%s', $request->user()->id, $user->isAdministrator(), $user->id));
 
-        Log::info(sprintf('User ID #%s set is_admin=%s for User ID #%s', $request->user()->id, $user->isAdministrator(), $user->id));
+            return new UserManagerResource($user);
+        }
 
-        return new UserManagerResource($user);
+        return response()->json([
+            'message' => __('errors.cannot_demote_the_only_admin'),
+        ], 403);
     }
 
     /**

+ 27 - 2
app/Models/User.php

@@ -77,6 +77,13 @@ class User extends Authenticatable implements WebAuthnAuthenticatable
         'groups_count'       => 'integer',
     ];
 
+    /**
+     * These are extra user-defined events observers may subscribe to.
+     */
+    protected $observables = [
+        'demoting'
+    ];
+
     /**
      * Scope a query to only include admin users.
      *
@@ -100,12 +107,30 @@ class User extends Authenticatable implements WebAuthnAuthenticatable
 
     /**
      * Grant administrator permissions to the user.
+     */
+    public function promoteToAdministrator(bool $promote = true): bool
+    {
+        if ($promote == false && $this->fireModelEvent('demoting') === false) {
+            return false;
+        }
+
+        $this->is_admin = $promote;
+
+        return true;
+    }
+
+    /**
+     * Say if the user is the only registered administrator
      *
      * @return void
      */
-    public function promoteToAdministrator(bool $promote = true)
+    public function isLastAdministrator()
     {
-        $this->is_admin = $promote;
+        $admins = User::admins()->get();
+
+        $toto = $admins->contains($this->id) && $admins->count() === 1;
+
+        return $toto;
     }
 
     /**

+ 17 - 4
app/Observers/UserObserver.php

@@ -31,6 +31,21 @@ class UserObserver
         //
     }
 
+    /**
+     * Handle the User "demoting" event.
+     */
+    public function demoting(User $user) : bool
+    {
+        // Prevent demotion of the only administrator
+        if ($user->isLastAdministrator()) {
+            Log::notice(sprintf('Demotion of user ID #%s denied, cannot demote the only administrator', $user->id));
+
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * Handle the User "deleting" event.
      */
@@ -39,10 +54,8 @@ class UserObserver
         Log::info(sprintf('Deletion of User ID #%s requested by User ID #%s', $user->id, Auth::user()->id ?? 'unknown'));
 
         // Prevent deletion of the only administrator
-        $isLastAdmin = $user->isAdministrator() && User::admins()->count() == 1;
-
-        if ($isLastAdmin) {
-            Log::notice(sprintf('Deletion of user ID #%s refused, cannot delete the only administrator', $user->id));
+        if ($user->isLastAdministrator()) {
+            Log::notice(sprintf('Deletion of user ID #%s denied, cannot delete the only administrator', $user->id));
 
             return false;
         }

+ 8 - 2
resources/js/views/admin/users/Manage.vue

@@ -100,12 +100,18 @@
             }
         }
 
-        userService.promote(managedUser.value.info.id, { 'is_admin': isAdmin }).then(response => {
+        userService.promote(managedUser.value.info.id, { 'is_admin': isAdmin }, { returnError: true }).then(response => {
             managedUser.value.info.is_admin = response.data.info.is_admin
             notify.success({ text: trans('admin.user_role_updated') })
         })
         .catch(error => {
-            notify.error(error)
+            if( error.response.status === 403 ) {
+                notify.alert({ text: error.response.data.message })
+                managedUser.value.info.is_admin = true
+            }
+            else {
+                notify.error(error.response)
+            }
         })
     }
 

+ 1 - 0
resources/lang/en/errors.php

@@ -57,6 +57,7 @@ return [
     'unauthorized' => 'Unauthorized',
     'unauthorized_legend' => 'You do not have permissions to view this resource or to perform this action',
     'cannot_delete_the_only_admin' => 'Cannot delete the only admin account',
+    'cannot_demote_the_only_admin' => 'Cannot demote the only admin account',
     'error_during_data_fetching' => '💀 Something went wrong during data fetching',
     'check_failed_try_later' => 'Check failed, please retry later',
     'sso_disabled' => 'SSO is disabled',

+ 35 - 0
tests/Api/v1/Controllers/UserManagerControllerTest.php

@@ -483,4 +483,39 @@ class UserManagerControllerTest extends FeatureTestCase
 
         $response->assertExactJson($resources->response($request)->getData(true));
     }
+
+    /**
+     * @test
+     */
+    public function test_demote_returns_UserManagerResource() : void
+    {
+        $anotherAdmin = User::factory()->administrator()->create();
+
+        $path    = '/api/v1/users/' . $anotherAdmin->id . '/promote';
+        $request = Request::create($path, 'PUT');
+
+        $response = $this->actingAs($this->admin, 'api-guard')
+            ->json('PATCH', $path, [
+                'is_admin' => false,
+            ]);
+
+        $anotherAdmin->refresh();
+        $resources = UserManagerResource::make($anotherAdmin);
+
+        $response->assertExactJson($resources->response($request)->getData(true));
+    }
+
+    /**
+     * @test
+     */
+    public function test_demote_the_only_admin_returns_forbidden() : void
+    {
+        $this->assertTrue(User::admins()->count() == 1);
+
+        $this->actingAs($this->admin, 'api-guard')
+            ->json('PATCH', '/api/v1/users/' . $this->admin->id . '/promote', [
+                'is_admin' => false,
+            ])
+            ->assertForbidden();
+    }
 }