diff --git a/src/app/Http/Controllers/API/V4/UsersController.php b/src/app/Http/Controllers/API/V4/UsersController.php --- a/src/app/Http/Controllers/API/V4/UsersController.php +++ b/src/app/Http/Controllers/API/V4/UsersController.php @@ -296,9 +296,12 @@ } $current_user = $this->guard()->user(); + $requires_controller = $request->skus !== null || $request->aliases !== null; + $can_update = $requires_controller ? $current_user->canDelete($user) : $current_user->canUpdate($user); - // TODO: Decide what attributes a user can change on his own profile - if (!$current_user->canUpdate($user)) { + // Only wallet controller can set subscriptions and aliases + // TODO: Consider changes in canUpdate() or introduce isController() + if (!$can_update) { return $this->errorResponse(403); } @@ -306,11 +309,6 @@ return $error_response; } - // Entitlements, only controller can do that - if ($request->skus !== null && !$current_user->canDelete($user)) { - return $this->errorResponse(422, "You have no permission to change entitlements"); - } - DB::beginTransaction(); SkusController::updateEntitlements($user, $request->skus); diff --git a/src/app/Http/Controllers/RelationController.php b/src/app/Http/Controllers/RelationController.php --- a/src/app/Http/Controllers/RelationController.php +++ b/src/app/Http/Controllers/RelationController.php @@ -291,7 +291,9 @@ return $this->errorResponse(404); } - if (!$this->guard()->user()->canUpdate($resource)) { + // Only wallet controller can do this, therefor canDelete() not canUpdate() + // TODO: Consider changes in canUpdate() or introduce isController() + if (!$this->guard()->user()->canDelete($resource)) { return $this->errorResponse(403); } diff --git a/src/tests/Feature/Controller/UsersTest.php b/src/tests/Feature/Controller/UsersTest.php --- a/src/tests/Feature/Controller/UsersTest.php +++ b/src/tests/Feature/Controller/UsersTest.php @@ -693,7 +693,7 @@ $response->assertStatus(404); - // Test access by user not being a wallet controller + // Test access by user not being a wallet controller (controller's config change) $post = ['greylist_enabled' => 1]; $response = $this->actingAs($jack)->post("/api/v4/users/{$john->id}/config", $post); $json = $response->json(); @@ -704,6 +704,10 @@ $this->assertSame("Access denied", $json['message']); $this->assertCount(2, $json); + // Test access by user not being a wallet controller (self config change) + $response = $this->actingAs($jack)->post("/api/v4/users/{$jack->id}/config", $post); + $response->assertStatus(403); + // Test some invalid data $post = ['grey' => 1, 'password_policy' => 'min:1,max:255']; $response = $this->actingAs($john)->post("/api/v4/users/{$john->id}/config", $post); @@ -1159,10 +1163,14 @@ $owner->assignPackage($package_kolab); $owner->assignPackage($package_lite, $user); - // Non-controller cannot update his own entitlements + // Non-controller cannot update his own entitlements, nor aliases $post = ['skus' => []]; $response = $this->actingAs($user)->put("/api/v4/users/{$user->id}", $post); - $response->assertStatus(422); + $response->assertStatus(403); + + $post = ['aliases' => []]; + $response = $this->actingAs($user)->put("/api/v4/users/{$user->id}", $post); + $response->assertStatus(403); // Test updating entitlements $post = [