diff --git a/app/Api/v1/Controllers/UserManagerController.php b/app/Api/v1/Controllers/UserManagerController.php index 0231faac..d27b57a1 100644 --- a/app/Api/v1/Controllers/UserManagerController.php +++ b/app/Api/v1/Controllers/UserManagerController.php @@ -102,9 +102,10 @@ public function store(UserManagerStoreRequest $request) 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 @@ public function promote(UserManagerPromoteRequest $request, User $user) { $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); } /** diff --git a/app/Models/User.php b/app/Models/User.php index fe693e20..52c5d6e7 100644 --- a/app/Models/User.php +++ b/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 @@ public function isAdministrator() /** * 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; } /** diff --git a/app/Observers/UserObserver.php b/app/Observers/UserObserver.php index b5f78f1c..40ac1d5f 100644 --- a/app/Observers/UserObserver.php +++ b/app/Observers/UserObserver.php @@ -31,6 +31,21 @@ public function updated(User $user) : void // } + /** + * 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 @@ public function deleting(User $user) : bool 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; } diff --git a/resources/js/views/admin/users/Manage.vue b/resources/js/views/admin/users/Manage.vue index 510546e8..8b7cdbd8 100644 --- a/resources/js/views/admin/users/Manage.vue +++ b/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) + } }) } diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 817b355e..f959785b 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -57,6 +57,7 @@ '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', diff --git a/tests/Api/v1/Controllers/UserManagerControllerTest.php b/tests/Api/v1/Controllers/UserManagerControllerTest.php index 9048199a..13365e7a 100644 --- a/tests/Api/v1/Controllers/UserManagerControllerTest.php +++ b/tests/Api/v1/Controllers/UserManagerControllerTest.php @@ -483,4 +483,39 @@ public function test_promote_returns_UserManagerResource() : void $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(); + } }