Page MenuHomePhorge
Files F117804960

D5547.1775271052.diff
No OneTemporary
Actions

Authored By
Unknown
Size
7 KB
Referenced Files
None
Subscribers
None

D5547.1775271052.diff
View Options

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
@@ -241,6 +241,7 @@
$response['skus'] = Entitlement::objectEntitlementsSummary($user);
$response['config'] = $user->getConfig(true);
$response['aliases'] = $user->aliases()->pluck('alias')->all();
+ $response['canDelete'] = $this->guard()->user()->canDelete($user);
$code = $user->verificationcodes()->where('active', true)
->where('expires_at', '>', Carbon::now())
diff --git a/src/app/User.php b/src/app/User.php
--- a/src/app/User.php
+++ b/src/app/User.php
@@ -207,9 +207,11 @@
return true;
}
- // Other wallet controllers can remove users but not the account owner
- if ($object instanceof self && $object->id == $wallet->user_id) {
- return false;
+ // Other wallet controllers can remove users, but not the account owner nor themselves
+ if ($object instanceof self) {
+ if ($object->id == $wallet->user_id || $object->id == $this->id) {
+ return false;
+ }
}
return $wallet->isController($this);
diff --git a/src/resources/js/app.js b/src/resources/js/app.js
--- a/src/resources/js/app.js
+++ b/src/resources/js/app.js
@@ -111,23 +111,6 @@
hasSKU(name) {
return this.authInfo.statusInfo.skus && this.authInfo.statusInfo.skus.indexOf(name) != -1
},
- isController(wallet_id) {
- if (wallet_id && this.authInfo) {
- let i
- for (i = 0; i < this.authInfo.wallets.length; i++) {
- if (wallet_id == this.authInfo.wallets[i].id) {
- return true
- }
- }
- for (i = 0; i < this.authInfo.accounts.length; i++) {
- if (wallet_id == this.authInfo.accounts[i].id) {
- return true
- }
- }
- }
-
- return false
- },
isDegraded() {
return this.authInfo && this.authInfo.isAccountDegraded
},
diff --git a/src/resources/vue/User/Info.vue b/src/resources/vue/User/Info.vue
--- a/src/resources/vue/User/Info.vue
+++ b/src/resources/vue/User/Info.vue
@@ -6,7 +6,7 @@
<div class="card-body">
<div class="card-title" v-if="user_id === 'new'">{{ $t('user.new') }}</div>
<div class="card-title" v-else>{{ $t($route.name == 'settings' ? 'dashboard.myaccount' : 'user.title') }}
- <btn v-if="isController" icon="trash-can" class="btn-outline-danger button-delete float-end" @click="$refs.deleteWarning.show()">
+ <btn v-if="user.canDelete" icon="trash-can" class="btn-outline-danger button-delete float-end" @click="$refs.deleteWarning.show()">
{{ $t(isSelf ? 'user.profile-delete' : 'user.delete') }}
</btn>
</div>
@@ -367,7 +367,7 @@
}
opts.maildelivery = this.$t('policies.mailDelivery')
}
- if ((this.isController || this.isSelf) && this.$root.authInfo.statusInfo.enableDelegation) {
+ if ((this.isController || this.isSelf) && this.$root.hasPermission('delegation')) {
opts.delegation = this.$t('user.delegation')
}
return opts
@@ -407,6 +407,13 @@
if (this.user_id !== 'new') {
axios.get('/api/v4/users/' + this.user_id, { loader: true })
.then(response => {
+ // A controller cannot edit the account owner
+ if (this.user_id != this.$root.authInfo.id && this.user_id == response.data.wallet.user_id) {
+ // TODO: Instead of an error page we should consider a read-only page
+ this.$root.errorPage(403)
+ return
+ }
+
this.user = { ...response.data, ...response.data.settings }
this.status = response.data.statusInfo
this.passwordLinkCode = this.user.passwordLinkCode
diff --git a/src/tests/Browser/UsersTest.php b/src/tests/Browser/UsersTest.php
--- a/src/tests/Browser/UsersTest.php
+++ b/src/tests/Browser/UsersTest.php
@@ -711,7 +711,7 @@
});
// Test that controller user (Ned) can see all the users
- $this->browse(static function (Browser $browser) {
+ $this->browse(function (Browser $browser) use ($john) {
$browser->visit('/logout')
->on(new Home())
->submitLogon('ned@kolab.org', 'simple123', true)
@@ -720,7 +720,21 @@
$browser->assertElementsCount('tbody tr', 4);
});
- // TODO: Test the delete action in details
+ // Test that controller cannot edit/delete the account owner
+ $browser->visit('/user/' . $john->id)
+ ->assertErrorPage(403);
+
+ // Test that controller cannot delete himself
+ $ned = $this->getTestUser('ned@kolab.org');
+ $browser->visit('/user/' . $ned->id)
+ ->on(new UserInfo())
+ ->assertMissing('button.button-delete');
+
+ // Test that controller can delete any other user in controlled wallet
+ $jack = $this->getTestUser('jack@kolab.org');
+ $browser->visit('/user/' . $jack->id)
+ ->on(new UserInfo())
+ ->assertSeeIn('button.button-delete', 'Delete user');
});
// TODO: Test what happens with the logged in user session after he's been deleted by another 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
@@ -124,10 +124,12 @@
$response = $this->actingAs($user3)->delete("api/v4/users/{$user3->id}");
$response->assertStatus(403);
- // Test that wallet controller cannot remove the account owner
+ // Test that wallet controller cannot remove the account owner, nor himself
$user1->wallets()->first()->addController($user3);
$response = $this->actingAs($user3)->delete("api/v4/users/{$user1->id}");
$response->assertStatus(403);
+ $response = $this->actingAs($user3)->delete("api/v4/users/{$user3->id}");
+ $response->assertStatus(403);
// Test that wallet controller can delete other non-owner users
$response = $this->actingAs($user3)->delete("api/v4/users/{$user2->id}");
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
@@ -258,7 +258,7 @@
$this->assertFalse($jack->canDelete($admin));
// Normal user - John's wallet controller
- $this->assertTrue($ned->canDelete($ned));
+ $this->assertFalse($ned->canDelete($ned));
$this->assertFalse($ned->canDelete($john));
$this->assertTrue($ned->canDelete($jack));
$this->assertTrue($ned->canDelete($domain));

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 4, 2:50 AM (4 h, 31 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18827836
Default Alt Text
D5547.1775271052.diff (7 KB)

Event Timeline