Page MenuHomePhorge
Files F117766098

D5547.1775231912.diff
No OneTemporary
Actions

Authored By
Unknown
Size
21 KB
Referenced Files
None
Subscribers
None

D5547.1775231912.diff
View Options

diff --git a/src/app/Http/Controllers/API/V4/DomainsController.php b/src/app/Http/Controllers/API/V4/DomainsController.php
--- a/src/app/Http/Controllers/API/V4/DomainsController.php
+++ b/src/app/Http/Controllers/API/V4/DomainsController.php
@@ -103,9 +103,9 @@
public function store(Request $request)
{
$current_user = $this->guard()->user();
- $owner = $current_user->wallet()->owner;
+ $wallet = $current_user->wallet();
- if ($owner->id != $current_user->id) {
+ if (!$wallet || !$wallet->isController($current_user) || !$wallet->owner) {
return $this->errorResponse(403);
}
@@ -126,8 +126,8 @@
// Domain already exists
if ($domain = Domain::withTrashed()->where('namespace', $namespace)->first()) {
// Check if the domain is soft-deleted and belongs to the same user
- $deleteBeforeCreate = $domain->trashed() && ($wallet = $domain->wallet())
- && $wallet->owner && $wallet->owner->id == $owner->id;
+ $deleteBeforeCreate = $domain->trashed() && ($domain_wallet = $domain->wallet())
+ && $domain_wallet->id == $wallet->id;
if (!$deleteBeforeCreate) {
$errors = ['namespace' => self::trans('validation.domainnotavailable')];
@@ -137,7 +137,7 @@
if (
empty($request->package)
- || !($package = Package::withObjectTenantContext($owner)->find($request->package))
+ || !($package = Package::withObjectTenantContext($wallet->owner)->find($request->package))
) {
$errors = ['package' => self::trans('validation.packagerequired')];
return response()->json(['status' => 'error', 'errors' => $errors], 422);
@@ -161,7 +161,7 @@
'type' => Domain::TYPE_EXTERNAL,
]);
- $domain->assignPackage($package, $owner);
+ $domain->assignPackage($package, $wallet->owner);
DB::commit();
diff --git a/src/app/Http/Controllers/API/V4/GroupsController.php b/src/app/Http/Controllers/API/V4/GroupsController.php
--- a/src/app/Http/Controllers/API/V4/GroupsController.php
+++ b/src/app/Http/Controllers/API/V4/GroupsController.php
@@ -65,9 +65,9 @@
public function store(Request $request)
{
$current_user = $this->guard()->user();
- $owner = $current_user->wallet()->owner;
+ $wallet = $current_user->wallet();
- if ($owner->id != $current_user->id) {
+ if (!$wallet || !$wallet->isController($current_user) || !$wallet->owner) {
return $this->errorResponse(403);
}
@@ -81,11 +81,11 @@
$this->deleteBeforeCreate = null;
// Validate group address
- if ($error = self::validateGroupEmail($email, $owner, $this->deleteBeforeCreate)) {
+ if ($error = self::validateGroupEmail($email, $wallet->owner, $this->deleteBeforeCreate)) {
$errors['email'] = $error;
} else {
[, $domainName] = explode('@', $email);
- $rules['name'] = ['required', 'string', new GroupName($owner, $domainName)];
+ $rules['name'] = ['required', 'string', new GroupName($wallet->owner, $domainName)];
}
// Validate the group name
@@ -101,7 +101,7 @@
} else {
foreach ($members as $i => $member) {
if (is_string($member) && !empty($member)) {
- if ($error = self::validateMemberEmail($member, $owner)) {
+ if ($error = self::validateMemberEmail($member, $wallet->owner)) {
$errors['members'][$i] = $error;
} elseif (\strtolower($member) === \strtolower($email)) {
$errors['members'][$i] = self::trans('validation.memberislist');
@@ -129,7 +129,7 @@
$group->members = $members;
$group->save();
- $group->assignToWallet($owner->wallets->first());
+ $group->assignToWallet($wallet);
DB::commit();
diff --git a/src/app/Http/Controllers/API/V4/ResourcesController.php b/src/app/Http/Controllers/API/V4/ResourcesController.php
--- a/src/app/Http/Controllers/API/V4/ResourcesController.php
+++ b/src/app/Http/Controllers/API/V4/ResourcesController.php
@@ -54,15 +54,15 @@
public function store(Request $request)
{
$current_user = $this->guard()->user();
- $owner = $current_user->wallet()->owner;
+ $wallet = $current_user->wallet();
- if ($owner->id != $current_user->id) {
+ if (!$wallet || !$wallet->isController($current_user) || !$wallet->owner) {
return $this->errorResponse(403);
}
$domain = request()->input('domain');
- $rules = ['name' => ['required', 'string', new ResourceName($owner, $domain)]];
+ $rules = ['name' => ['required', 'string', new ResourceName($wallet->owner, $domain)]];
$v = Validator::make($request->all(), $rules);
@@ -78,7 +78,7 @@
$resource->domainName = $domain;
$resource->save();
- $resource->assignToWallet($owner->wallets->first());
+ $resource->assignToWallet($wallet);
DB::commit();
diff --git a/src/app/Http/Controllers/API/V4/SharedFoldersController.php b/src/app/Http/Controllers/API/V4/SharedFoldersController.php
--- a/src/app/Http/Controllers/API/V4/SharedFoldersController.php
+++ b/src/app/Http/Controllers/API/V4/SharedFoldersController.php
@@ -57,13 +57,13 @@
public function store(Request $request)
{
$current_user = $this->guard()->user();
- $owner = $current_user->walletOwner();
+ $wallet = $current_user->wallet();
- if (empty($owner) || $owner->id != $current_user->id) {
+ if (!$wallet || !$wallet->isController($current_user) || !$wallet->owner) {
return $this->errorResponse(403);
}
- if ($error_response = $this->validateFolderRequest($request, null, $owner)) {
+ if ($error_response = $this->validateFolderRequest($request, null, $wallet->owner)) {
return $error_response;
}
@@ -80,7 +80,7 @@
$folder->setAliases($request->aliases);
}
- $folder->assignToWallet($owner->wallets->first());
+ $folder->assignToWallet($wallet);
DB::commit();
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())
@@ -273,6 +274,7 @@
$wallet = $user->wallet();
$isController = $wallet->isController($user);
+ $isOwner = $wallet->user_id == $user->id;
$isDegraded = $user->isDegraded();
$plan = $isController ? $wallet->plan() : null;
@@ -297,11 +299,11 @@
'enableMailfilter' => $isController && config('app.with_mailfilter'),
'enableResources' => $isController && $hasCustomDomain && $hasBeta && \config('app.with_resources'),
'enableRooms' => $hasMeet,
- 'enableSettings' => $isController,
+ 'enableSettings' => $isOwner,
'enableSubscriptions' => $isController && \config('app.with_subscriptions'),
'enableUsers' => $isController,
- 'enableWallets' => $isController && \config('app.with_wallet'),
- 'enableWalletMandates' => $isController,
+ 'enableWallets' => $isOwner && \config('app.with_wallet'),
+ 'enableWalletMandates' => $isOwner,
'enableCompanionapps' => $hasBeta && \config('app.with_companion_app'),
'enableLoginAs' => $isController && \config('app.with_loginas'),
];
@@ -319,9 +321,9 @@
public function store(Request $request)
{
$current_user = $this->guard()->user();
- $owner = $current_user->walletOwner();
+ $wallet = $current_user->wallet();
- if ($owner->id != $current_user->id) {
+ if (!$wallet || !$wallet->isController($current_user) || !$wallet->owner) {
return $this->errorResponse(403);
}
@@ -333,7 +335,7 @@
if (
empty($request->package)
- || !($package = Package::withObjectTenantContext($owner)->find($request->package))
+ || !($package = Package::withObjectTenantContext($current_user)->find($request->package))
) {
$errors = ['package' => self::trans('validation.packagerequired')];
return response()->json(['status' => 'error', 'errors' => $errors], 422);
@@ -355,12 +357,12 @@
$user = User::create([
'email' => $request->email,
'password' => $request->password,
- 'status' => $owner->isRestricted() ? User::STATUS_RESTRICTED : 0,
+ 'status' => $wallet->owner->isRestricted() ? User::STATUS_RESTRICTED : 0,
]);
$this->activatePassCode($user);
- $owner->assignPackage($package, $user);
+ $wallet->owner->assignPackage($package, $user);
if (!empty($settings)) {
$user->setSettings($settings);
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/DomainsTest.php b/src/tests/Feature/Controller/DomainsTest.php
--- a/src/tests/Feature/Controller/DomainsTest.php
+++ b/src/tests/Feature/Controller/DomainsTest.php
@@ -618,7 +618,13 @@
$this->assertSame('The specified domain is not available.', $json['errors']['namespace']);
// Test acting as account controller (not owner)
+ $ned = $this->getTestUser('ned@kolab.org');
+ $domain->forceDelete();
+
+ $response = $this->actingAs($ned)->post("/api/v4/domains", $post);
+ $response->assertStatus(200);
- $this->markTestIncomplete();
+ $domain = Domain::where('namespace', $post['namespace'])->first();
+ $this->assertSame($john->wallets->first()->id, $domain->wallet()->id);
}
}
diff --git a/src/tests/Feature/Controller/GroupsTest.php b/src/tests/Feature/Controller/GroupsTest.php
--- a/src/tests/Feature/Controller/GroupsTest.php
+++ b/src/tests/Feature/Controller/GroupsTest.php
@@ -546,6 +546,16 @@
$response->assertStatus(200);
$this->assertCount(1, Group::where('email', $post['email'])->get());
+
+ // Test that wallet controllers can create groups too
+ $ned = $this->getTestUser('ned@kolab.org');
+ Group::where('email', $post['email'])->delete();
+
+ $response = $this->actingAs($ned)->post("/api/v4/groups", $post);
+ $response->assertStatus(200);
+
+ $group = Group::where('email', $post['email'])->first();
+ $this->assertSame($john->wallets->first()->id, $group->wallet()->id);
}
/**
diff --git a/src/tests/Feature/Controller/ResourcesTest.php b/src/tests/Feature/Controller/ResourcesTest.php
--- a/src/tests/Feature/Controller/ResourcesTest.php
+++ b/src/tests/Feature/Controller/ResourcesTest.php
@@ -513,6 +513,17 @@
$this->assertInstanceOf(Resource::class, $resource);
$this->assertTrue($john->resources()->get()->contains($resource));
+ // Test that a wallet controller can create a resource
+ $ned = $this->getTestUser('ned@kolab.org');
+ $resource->delete();
+
+ $response = $this->actingAs($ned)->post("/api/v4/resources", $post);
+ $json = $response->json();
+
+ $response->assertStatus(200);
+ $resource = Resource::where('name', $post['name'])->first();
+ $this->assertTrue($john->resources()->get()->contains($resource));
+
// Resource name must be unique within a domain
$response = $this->actingAs($john)->post("/api/v4/resources", $post);
$response->assertStatus(422);
diff --git a/src/tests/Feature/Controller/SharedFoldersTest.php b/src/tests/Feature/Controller/SharedFoldersTest.php
--- a/src/tests/Feature/Controller/SharedFoldersTest.php
+++ b/src/tests/Feature/Controller/SharedFoldersTest.php
@@ -565,6 +565,14 @@
$folder = SharedFolder::where('name', $post['name'])->first();
$this->assertSame(['shared+shared/Test/Folder@kolab.org'], $folder->aliases()->pluck('alias')->all());
+
+ // Test wallet controllers can create shared folders
+ $ned = $this->getTestUser('ned@kolab.org');
+ $folder->forceDelete();
+ $response = $this->actingAs($ned)->post("/api/v4/shared-folders", $post);
+ $json = $response->json();
+
+ $response->assertStatus(200);
}
/**
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}");
@@ -992,11 +994,21 @@
$this->assertTrue($code->active);
$this->assertTrue(is_string($user->password) && strlen($user->password) >= 60);
- // Test acting as account controller not owner, which is not yet supported
+ // Test acting as account controller not owner
+ $this->deleteTestUser('john2.doe2@kolab.org');
$john->wallets->first()->addController($user);
+ $post = [
+ 'password' => 'simple123',
+ 'password_confirmation' => 'simple123',
+ 'email' => 'john2.doe2@kolab.org',
+ 'package' => $package_kolab->id,
+ ];
- $response = $this->actingAs($user)->post("/api/v4/users", []);
- $response->assertStatus(403);
+ $response = $this->actingAs($user)->post("/api/v4/users", $post);
+ $response->assertStatus(200);
+
+ $user = User::where('email', 'john2.doe2@kolab.org')->first();
+ $this->assertSame($john->wallets->first()->id, $user->wallet()->id);
// Test that creating a user in a restricted account creates a restricted user
$package_domain = Package::withEnvTenantContext()->where('title', 'domain-hosting')->first();
@@ -1463,11 +1475,11 @@
$this->assertSame($provider, $result['wallets'][0]['provider']);
$this->assertFalse($result['isLocked']);
+ $this->assertFalse($result['statusInfo']['enableWallets']);
+ $this->assertFalse($result['statusInfo']['enableWalletMandates']);
+ $this->assertFalse($result['statusInfo']['enableSettings']);
$this->assertTrue($result['statusInfo']['enableDomains']);
- $this->assertTrue($result['statusInfo']['enableWallets']);
- $this->assertTrue($result['statusInfo']['enableWalletMandates']);
$this->assertTrue($result['statusInfo']['enableUsers']);
- $this->assertTrue($result['statusInfo']['enableSettings']);
$this->assertTrue($result['statusInfo']['enableDistlists']);
$this->assertTrue($result['statusInfo']['enableFolders']);
$this->assertTrue($result['statusInfo']['enableDelegation']);
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
Fri, Apr 3, 3:58 PM (7 h, 34 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18824595
Default Alt Text
D5547.1775231912.diff (21 KB)

Event Timeline