From 3e15237f90bbe4e91b8e32358fc2a514603716ef Mon Sep 17 00:00:00 2001
From: denkfabrik-li <274324701+denkfabrik-li@users.noreply.github.com>
Date: Fri, 28 Aug 2026 00:39:50 +0200
Subject: [PATCH] Refuse self-deactivation over the API however the boolean is
 written

Api\UsersController::update() compares the validated value strictly:

    if ($user->is($actor) && ($validated['active'] ?? true) === false) {

The `boolean` rule accepts 0 and "0" as well as false, and it does not
cast. `0 === false` is false, so the refusal never fires -- and the
model's own `boolean` cast then stores as false exactly the value the
guard had just decided was not a deactivation.

Measured against main, with a second administrator present so that
guardLastAdministrator is not what answers:

    {"active": false}  -> 422, still active
    {"active": 0}      -> 200, active is now false
    {"active": "0"}    -> 200, active is now false

The method's own docblock says it is "Refused with a 422 if the change
would leave the installation with no active administrator, or if you
would be deactivating yourself", and the web screen does refuse. This is
the API half of that sentence.

RolesController::guardScopeRemoval documents the rule this breaks, in the
same words: callers resolve the flag with Request::boolean() and hand the
same value to the guard and to the write, deliberately, because reading
the validated array and comparing it strictly "would let a request
through here that the model's `boolean` cast then stores as false anyway
-- the guard and the write disagreeing about one value is exactly the
shape this guard exists to prevent".

So read it once, with Request::boolean(), and give that one value to both.

Not changed: the validation rule. It stays `boolean`, so the accepted
inputs are the same as before -- what changes is that one of them stops
meaning two different things on its way through. Nor anything about
deactivating somebody else: all three forms still work, and there are
tests saying so.

Six cases from two datasets. Two measured red against the unfixed
controller (2 failed / 4 passed): 0 and "0" on yourself. `false` was
already refused, and the three "somebody else" cases are green either way
-- they guard against the fix over-refusing, not against the bug.

Full suite passes (2054 passed / 2 skipped), PHPStan level 8 clean.
---
 .../Http/Controllers/Api/UsersController.php  | 16 ++++++++--
 tests/Feature/Api/UsersTest.php               | 29 +++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/app/Modules/Identity/Http/Controllers/Api/UsersController.php b/app/Modules/Identity/Http/Controllers/Api/UsersController.php
index df1163f6..73953871 100644
--- a/app/Modules/Identity/Http/Controllers/Api/UsersController.php
+++ b/app/Modules/Identity/Http/Controllers/Api/UsersController.php
@@ -176,15 +176,27 @@ class UsersController extends Controller
             'assigned_clients.*' => ['integer', Rule::in($this->accounts->assignableClientIds($actor))],
         ]);
 
+        // Read through Request::boolean() rather than off the validated
+        // array, for the reason RolesController::guardScopeRemoval spells
+        // out: the `boolean` rule accepts 0 and "0" as well as false but
+        // does not cast, so a strict comparison lets through a value the
+        // model's own `boolean` cast then stores as false anyway. The same
+        // value goes to the guard and to the write.
+        $deactivating = array_key_exists('active', $validated) && ! $request->boolean('active');
+
         // The same refusal the web screen makes, and for the same reason:
         // locking yourself out is never what was meant.
-        if ($user->is($actor) && ($validated['active'] ?? true) === false) {
+        if ($user->is($actor) && $deactivating) {
             throw ValidationException::withMessages([
                 'active' => __('You cannot deactivate your own account.'),
             ]);
         }
 
-        $attributes = array_intersect_key($validated, array_flip(['name', 'email', 'active', 'password']));
+        $attributes = array_intersect_key($validated, array_flip(['name', 'email', 'password']));
+
+        if (array_key_exists('active', $validated)) {
+            $attributes['active'] = $request->boolean('active');
+        }
 
         if (array_key_exists('role_id', $validated)) {
             $attributes['role_id'] = (int) $validated['role_id'];
diff --git a/tests/Feature/Api/UsersTest.php b/tests/Feature/Api/UsersTest.php
index c4a001a1..66c3f9d4 100644
--- a/tests/Feature/Api/UsersTest.php
+++ b/tests/Feature/Api/UsersTest.php
@@ -351,6 +351,35 @@ test('a caller cannot deactivate or delete their own account', function () {
         ->assertJsonPath('errors.user.0', 'You cannot delete your own account.');
 });
 
+test('self-deactivation is refused however the boolean is written', function (mixed $active) {
+    // A second administrator, so the last-administrator guard is not what
+    // refuses this: with only one, that guard answers first and the refusal
+    // under test here is never reached.
+    User::factory()->create();
+
+    $this->withToken($this->token)->patchJson("/api/v1/users/{$this->admin->id}", ['active' => $active])
+        ->assertStatus(422)
+        ->assertJsonPath('errors.active.0', 'You cannot deactivate your own account.');
+
+    expect($this->admin->refresh()->active)->toBeTrue();
+})->with([
+    'false' => [false],
+    'zero' => [0],
+    'the string zero' => ['0'],
+]);
+
+test('deactivating somebody else still works in every one of those forms', function (mixed $active) {
+    $user = User::factory()->role(SystemRole::Uploader)->create();
+
+    $this->withToken($this->token)->patchJson("/api/v1/users/{$user->id}", ['active' => $active])->assertOk();
+
+    expect($user->refresh()->active)->toBeFalse();
+})->with([
+    'false' => [false],
+    'zero' => [0],
+    'the string zero' => ['0'],
+]);
+
 /*
 |--------------------------------------------------------------------------
 | Deletion and its content