Page MenuHomePhorge
Files F117755614

D5715.1775203834.diff
No OneTemporary
Actions

Authored By
Unknown
Size
5 KB
Referenced Files
None
Subscribers
None

D5715.1775203834.diff
View Options

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
@@ -321,9 +321,8 @@
return $this->errorResponse(404);
}
- // Only wallet controller can do this, therefor canDelete() not canUpdate()
- // TODO: Consider changes in canUpdate() or introduce isController()
- if (!$this->guard()->user()->canDelete($resource)) {
+ // Only wallet controllers can do this
+ if (!$this->guard()->user()->canUpdateConfig($resource)) {
return $this->errorResponse(403);
}
diff --git a/src/app/User.php b/src/app/User.php
--- a/src/app/User.php
+++ b/src/app/User.php
@@ -292,6 +292,37 @@
return $this->canDelete($object);
}
+ /**
+ * Check if current user can update config of another object.
+ *
+ * @param mixed $object A user|domain|wallet|group object
+ *
+ * @return bool True if he can, False otherwise
+ */
+ public function canUpdateConfig($object): bool
+ {
+ if (!is_object($object) || !method_exists($object, 'wallet')) {
+ return false;
+ }
+
+ $wallet = $object->wallet();
+ if (!$wallet) {
+ return false;
+ }
+
+ // Wallet owner can do everything
+ if ($wallet->user_id == $this->id) {
+ return true;
+ }
+
+ // Other wallet controllers can update config of another user, except the wallet controller
+ if ($object instanceof self && $object->id == $wallet->user_id) {
+ return false;
+ }
+
+ return $wallet->isController($this);
+ }
+
/**
* Contacts (global addressbook) for this user.
*
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
@@ -87,6 +87,8 @@
$user->setSettings(['plan_id' => null]);
$folder = $this->getTestSharedFolder('folder-mail@kolab.org');
$folder->setAliases([]);
+ $this->getTestUser('jack@kolab.org')->settings()->whereIn('key', ['greylist_enabled'])->delete();
+ $this->getTestUser('ned@kolab.org')->settings()->whereIn('key', ['greylist_enabled'])->delete();
parent::tearDown();
}
@@ -849,6 +851,16 @@
$this->assertSame('true', $john->getSetting('guam_enabled'));
$this->assertSame('min:10,max:255,upper,lower,digit,special', $john->getSetting('password_policy'));
$this->assertSame('6', $john->getSetting('max_password_age'));
+
+ // Another account controller can update other user's config
+ $post = ['greylist_enabled' => 1];
+ $response = $this->actingAs($ned)->post("/api/v4/users/{$jack->id}/config", $post);
+ $response->assertStatus(200);
+
+ // Another account controller can update his own config
+ $post = ['greylist_enabled' => 1];
+ $response = $this->actingAs($ned)->post("/api/v4/users/{$ned->id}/config", $post);
+ $response->assertStatus(200);
}
/**
diff --git a/src/tests/Feature/UserTest.php b/src/tests/Feature/UserTest.php
--- a/src/tests/Feature/UserTest.php
+++ b/src/tests/Feature/UserTest.php
@@ -407,6 +407,56 @@
$this->assertFalse($ned->canUpdate($admin));
}
+ /**
+ * Test User::canUpdateConfig() method
+ */
+ public function testCanUpdateConfig(): void
+ {
+ $john = $this->getTestUser('john@kolab.org');
+ $ned = $this->getTestUser('ned@kolab.org');
+ $jack = $this->getTestUser('jack@kolab.org');
+ $reseller1 = $this->getTestUser('reseller@' . \config('app.domain'));
+ $admin = $this->getTestUser('jeroen@jeroen.jeroen');
+ $domain = $this->getTestDomain('kolab.org');
+
+ // Admin
+ $this->assertFalse($admin->canUpdateConfig($john));
+ $this->assertFalse($admin->canUpdateConfig($jack));
+ $this->assertFalse($admin->canUpdateConfig($domain));
+ $this->assertFalse($admin->canUpdateConfig($reseller1));
+ $this->assertTrue($admin->canUpdateConfig($admin));
+
+ // Reseller
+ $this->assertFalse($reseller1->canUpdateConfig($john));
+ $this->assertFalse($reseller1->canUpdateConfig($jack));
+ $this->assertFalse($reseller1->canUpdateConfig($domain));
+ $this->assertTrue($reseller1->canUpdateConfig($reseller1));
+ $this->assertFalse($reseller1->canUpdateConfig($admin));
+
+ // Normal user - account owner
+ $this->assertTrue($john->canUpdateConfig($john));
+ $this->assertTrue($john->canUpdateConfig($ned));
+ $this->assertTrue($john->canUpdateConfig($jack));
+ $this->assertTrue($john->canUpdateConfig($domain));
+ $this->assertFalse($john->canUpdateConfig($reseller1));
+ $this->assertFalse($john->canUpdateConfig($admin));
+
+ // Normal user - a non-owner and non-controller
+ $this->assertFalse($jack->canUpdateConfig($jack));
+ $this->assertFalse($jack->canUpdateConfig($john));
+ $this->assertFalse($jack->canUpdateConfig($domain));
+ $this->assertFalse($jack->canUpdateConfig($reseller1));
+ $this->assertFalse($jack->canUpdateConfig($admin));
+
+ // Normal user - John's wallet controller
+ $this->assertTrue($ned->canUpdateConfig($ned));
+ $this->assertTrue($ned->canUpdateConfig($jack));
+ $this->assertTrue($ned->canUpdateConfig($domain));
+ $this->assertFalse($ned->canUpdateConfig($john));
+ $this->assertFalse($ned->canUpdateConfig($reseller1));
+ $this->assertFalse($ned->canUpdateConfig($admin));
+ }
+
/**
* Test user created/creating/updated observers
*/

File Metadata

Mime Type
text/plain
Expires
Fri, Apr 3, 8:10 AM (13 h, 27 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18823154
Default Alt Text
D5715.1775203834.diff (5 KB)

Event Timeline