diff --git a/src/app/Backends/LDAP.php b/src/app/Backends/LDAP.php --- a/src/app/Backends/LDAP.php +++ b/src/app/Backends/LDAP.php @@ -8,7 +8,12 @@ class LDAP { - /** @const array UserSettings used by the backend */ + /** @const array Group settings used by the backend */ + public const GROUP_SETTINGS = [ + 'sender_policy', + ]; + + /** @const array User settings used by the backend */ public const USER_SETTINGS = [ 'first_name', 'last_name', @@ -570,7 +575,7 @@ if (empty($domain)) { self::throwException( $ldap, - "Failed to update group {$group->email} in LDAP (" . __LINE__ . ")" + "Failed to update group {$group->email} in LDAP (group not found)" ); } @@ -704,6 +709,10 @@ */ private static function setGroupAttributes($ldap, Group $group, &$entry) { + $settings = $group->getSettings(['sender_policy']); + + $entry['kolaballowsmtpsender'] = json_decode($settings['sender_policy'] ?: '[]', true); + $validMembers = []; $domain = $group->domain(); diff --git a/src/app/Domain.php b/src/app/Domain.php --- a/src/app/Domain.php +++ b/src/app/Domain.php @@ -386,16 +386,6 @@ ); } - /** - * Any (additional) properties of this domain. - * - * @return \Illuminate\Database\Eloquent\Relations\HasMany - */ - public function settings() - { - return $this->hasMany('App\DomainSetting', 'domain_id'); - } - /** * Suspend this domain. * diff --git a/src/app/Group.php b/src/app/Group.php --- a/src/app/Group.php +++ b/src/app/Group.php @@ -4,6 +4,8 @@ use App\Traits\BelongsToTenantTrait; use App\Traits\EntitleableTrait; +use App\Traits\GroupConfigTrait; +use App\Traits\SettingsTrait; use App\Traits\UuidIntKeyTrait; use App\Wallet; use Illuminate\Database\Eloquent\Model; @@ -22,6 +24,8 @@ { use BelongsToTenantTrait; use EntitleableTrait; + use GroupConfigTrait; + use SettingsTrait; use SoftDeletes; use UuidIntKeyTrait; diff --git a/src/app/GroupSetting.php b/src/app/GroupSetting.php new file mode 100644 --- /dev/null +++ b/src/app/GroupSetting.php @@ -0,0 +1,30 @@ +belongsTo(\App\Group::class, 'group_id', 'id'); + } +} 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 @@ -142,7 +142,7 @@ } // Only owner (or admin) has access to the domain - if (!$this->guard()->user()->canRead($domain)) { + if (!$this->guard()->user()->canUpdate($domain)) { return $this->errorResponse(403); } 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 @@ -87,6 +87,37 @@ return response()->json($result); } + /** + * Set the group configuration. + * + * @param int $id Group identifier + * + * @return \Illuminate\Http\JsonResponse|void + */ + public function setConfig($id) + { + $group = Group::find($id); + + if (!$this->checkTenant($group)) { + return $this->errorResponse(404); + } + + if (!$this->guard()->user()->canUpdate($group)) { + return $this->errorResponse(403); + } + + $errors = $group->setConfig(request()->input()); + + if (!empty($errors)) { + return response()->json(['status' => 'error', 'errors' => $errors], 422); + } + + return response()->json([ + 'status' => 'success', + 'message' => \trans('app.distlist-setconfig-success'), + ]); + } + /** * Display information of a group specified by $id. * @@ -111,6 +142,9 @@ $response = array_merge($response, self::groupStatuses($group)); $response['statusInfo'] = self::statusInfo($group); + // Group configuration, e.g. sender_policy + $response['config'] = $group->getConfig(); + return response()->json($response); } 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 @@ -144,7 +144,7 @@ return $this->errorResponse(404); } - if (!$this->guard()->user()->canRead($user)) { + if (!$this->guard()->user()->canUpdate($user)) { return $this->errorResponse(403); } diff --git a/src/app/Observers/GroupSettingObserver.php b/src/app/Observers/GroupSettingObserver.php new file mode 100644 --- /dev/null +++ b/src/app/Observers/GroupSettingObserver.php @@ -0,0 +1,51 @@ +key, LDAP::GROUP_SETTINGS)) { + \App\Jobs\Group\UpdateJob::dispatch($groupSetting->group_id); + } + } + + /** + * Handle the group setting "updated" event. + * + * @param \App\GroupSetting $groupSetting Settings object + * + * @return void + */ + public function updated(GroupSetting $groupSetting) + { + if (in_array($groupSetting->key, LDAP::GROUP_SETTINGS)) { + \App\Jobs\Group\UpdateJob::dispatch($groupSetting->group_id); + } + } + + /** + * Handle the group setting "deleted" event. + * + * @param \App\GroupSetting $groupSetting Settings object + * + * @return void + */ + public function deleted(GroupSetting $groupSetting) + { + if (in_array($groupSetting->key, LDAP::GROUP_SETTINGS)) { + \App\Jobs\Group\UpdateJob::dispatch($groupSetting->group_id); + } + } +} diff --git a/src/app/Providers/AppServiceProvider.php b/src/app/Providers/AppServiceProvider.php --- a/src/app/Providers/AppServiceProvider.php +++ b/src/app/Providers/AppServiceProvider.php @@ -47,6 +47,7 @@ \App\Domain::observe(\App\Observers\DomainObserver::class); \App\Entitlement::observe(\App\Observers\EntitlementObserver::class); \App\Group::observe(\App\Observers\GroupObserver::class); + \App\GroupSetting::observe(\App\Observers\GroupSettingObserver::class); \App\OpenVidu\Connection::observe(\App\Observers\OpenVidu\ConnectionObserver::class); \App\PackageSku::observe(\App\Observers\PackageSkuObserver::class); \App\PlanPackage::observe(\App\Observers\PlanPackageObserver::class); diff --git a/src/app/Tenant.php b/src/app/Tenant.php --- a/src/app/Tenant.php +++ b/src/app/Tenant.php @@ -71,16 +71,6 @@ return $this->hasMany('App\Discount'); } - /** - * Any (additional) settings of this tenant. - * - * @return \Illuminate\Database\Eloquent\Relations\HasMany - */ - public function settings() - { - return $this->hasMany('App\TenantSetting'); - } - /** * SignupInvitations assigned to this tenant. * diff --git a/src/app/Traits/GroupConfigTrait.php b/src/app/Traits/GroupConfigTrait.php new file mode 100644 --- /dev/null +++ b/src/app/Traits/GroupConfigTrait.php @@ -0,0 +1,70 @@ +getSetting('sender_policy'); + + $config['sender_policy'] = array_filter( + $sp ? json_decode($sp, true) : [], + function ($item) { + // remove the special "-" entry, it's an implementation detail + return $item !== '-'; + } + ); + + return $config; + } + + /** + * A helper to update a group configuration. + * + * @param array $config An array of configuration options + * + * @return array A list of input validation errors + */ + public function setConfig(array $config): array + { + $errors = []; + + foreach ($config as $key => $value) { + // validate and save SMTP sender policy entries + if ($key === 'sender_policy') { + if (!is_array($value)) { + $value = (array) $value; + } + + foreach ($value as $i => $v) { + if (!is_string($v)) { + $errors[$key][$i] = \trans('validation.sp-entry-invalid'); + } + } + + if (empty($errors[$key])) { + // remove empty entries, and '-' entry + $value = array_filter($value, function ($item) { + return strlen($item) > 0 && $item !== '-'; + }); + + if (!empty($value)) { + $value[] = '-'; // Block anyone not on the list + } + + $this->setSetting($key, json_encode($value)); + } + } else { + $errors[$key] = \trans('validation.invalid-config-parameter'); + } + } + + return $errors; + } +} diff --git a/src/app/Traits/SettingsTrait.php b/src/app/Traits/SettingsTrait.php --- a/src/app/Traits/SettingsTrait.php +++ b/src/app/Traits/SettingsTrait.php @@ -109,6 +109,16 @@ } } + /** + * Any (additional) properties of this domain. + * + * @return \Illuminate\Database\Eloquent\Relations\HasMany + */ + public function settings() + { + return $this->hasMany(self::class . 'Setting'); + } + /** * Create or update a setting. * diff --git a/src/app/User.php b/src/app/User.php --- a/src/app/User.php +++ b/src/app/User.php @@ -601,16 +601,6 @@ return $matchFound; } - /** - * Any (additional) properties of this user. - * - * @return \Illuminate\Database\Eloquent\Relations\HasMany - */ - public function settings() - { - return $this->hasMany('App\UserSetting', 'user_id'); - } - /** * Suspend this domain. * diff --git a/src/app/Wallet.php b/src/app/Wallet.php --- a/src/app/Wallet.php +++ b/src/app/Wallet.php @@ -409,16 +409,6 @@ } } - /** - * Any (additional) properties of this wallet. - * - * @return \Illuminate\Database\Eloquent\Relations\HasMany - */ - public function settings() - { - return $this->hasMany('App\WalletSetting'); - } - /** * Retrieve the transactions against this wallet. * diff --git a/src/database/migrations/2021_11_05_100000_create_group_settings_table.php b/src/database/migrations/2021_11_05_100000_create_group_settings_table.php new file mode 100644 --- /dev/null +++ b/src/database/migrations/2021_11_05_100000_create_group_settings_table.php @@ -0,0 +1,44 @@ +bigIncrements('id'); + $table->bigInteger('group_id'); + $table->string('key'); + $table->text('value'); + $table->timestamp('created_at')->useCurrent(); + $table->timestamp('updated_at')->useCurrent(); + + $table->foreign('group_id')->references('id')->on('groups') + ->onDelete('cascade')->onUpdate('cascade'); + + $table->unique(['group_id', 'key']); + } + ); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists('group_settings'); + } +} diff --git a/src/resources/lang/en/app.php b/src/resources/lang/en/app.php --- a/src/resources/lang/en/app.php +++ b/src/resources/lang/en/app.php @@ -49,6 +49,7 @@ 'distlist-delete-success' => 'Distribution list deleted successfully.', 'distlist-suspend-success' => 'Distribution list suspended successfully.', 'distlist-unsuspend-success' => 'Distribution list unsuspended successfully.', + 'distlist-setconfig-success' => 'Distribution list settings updated successfully.', 'domain-create-success' => 'Domain created successfully.', 'domain-delete-success' => 'Domain deleted successfully.', diff --git a/src/resources/lang/en/ui.php b/src/resources/lang/en/ui.php --- a/src/resources/lang/en/ui.php +++ b/src/resources/lang/en/ui.php @@ -58,6 +58,10 @@ 'list-empty' => "There are no distribution lists in this account.", 'new' => "New distribution list", 'recipients' => "Recipients", + 'sender-policy' => "Sender Access List", + 'sender-policy-text' => "With this list you can specify who can send mail to the distribution list." + . " You can put a complete email address (jane@kolab.org), domain (kolab.org) or suffix (.org) that the sender email address is compared to." + . " If the list is empty, mail from anyone is allowed.", ], 'domain' => [ diff --git a/src/resources/lang/en/validation.php b/src/resources/lang/en/validation.php --- a/src/resources/lang/en/validation.php +++ b/src/resources/lang/en/validation.php @@ -139,6 +139,7 @@ 'memberislist' => 'A recipient cannot be the same as the list address.', 'listmembersrequired' => 'At least one recipient is required.', 'spf-entry-invalid' => 'The entry format is invalid. Expected a domain name starting with a dot.', + 'sp-entry-invalid' => 'The entry format is invalid. Expected an email, domain, or part of it.', 'invalid-config-parameter' => 'The requested configuration parameter is not supported.', /* diff --git a/src/resources/vue/Admin/Distlist.vue b/src/resources/vue/Admin/Distlist.vue --- a/src/resources/vue/Admin/Distlist.vue +++ b/src/resources/vue/Admin/Distlist.vue @@ -22,7 +22,7 @@
- +
{{ member }}
 @@ -41,6 +41,31 @@
+ +
+
+
+
+
+
+ +
+ + {{ list.config.sender_policy && list.config.sender_policy.length ? list.config.sender_policy.join(', ') : $t('form.none') }} + +
+
+
+
+
+
+
@@ -48,7 +73,7 @@ export default { data() { return { - list: { members: [] } + list: { members: [], config: {} } } }, created() { diff --git a/src/resources/vue/Admin/Domain.vue b/src/resources/vue/Admin/Domain.vue --- a/src/resources/vue/Admin/Domain.vue +++ b/src/resources/vue/Admin/Domain.vue @@ -66,7 +66,7 @@
- {{ domain.config && domain.config.spf_whitelist.length ? domain.config.spf_whitelist.join(', ') : 'none' }} + {{ domain.config && domain.config.spf_whitelist.length ? domain.config.spf_whitelist.join(', ') : $t('form.none') }}
diff --git a/src/resources/vue/Distlist/Info.vue b/src/resources/vue/Distlist/Info.vue --- a/src/resources/vue/Distlist/Info.vue +++ b/src/resources/vue/Distlist/Info.vue @@ -12,27 +12,57 @@
{{ $t('distlist.new') }}
-
-
- -
- {{ $root.distlistStatusText(list) }} -
+ +
+
+ +
+ +
+ {{ $root.distlistStatusText(list) }} +
+
+
+ +
+ +
+
+
+ +
+ +
+
+ +
-
- -
- -
+
+
+
+ +
+ + + {{ $t('distlist.sender-policy-text') }} + +
+
+ +
-
- -
- -
-
- - +
@@ -51,7 +81,7 @@ data() { return { list_id: null, - list: { members: [] }, + list: { members: [], config: {} }, status: {} } }, @@ -99,6 +129,15 @@ this.$toast.success(response.data.message) this.$router.push({ name: 'distlists' }) }) + }, + submitSettings() { + this.$root.clearFormValidation($('#settings form')) + let post = this.list.config + + axios.post('/api/v4/groups/' + this.list_id + '/config', post) + .then(response => { + this.$toast.success(response.data.message) + }) } } } diff --git a/src/routes/api.php b/src/routes/api.php --- a/src/routes/api.php +++ b/src/routes/api.php @@ -54,8 +54,6 @@ } ); - - Route::group( [ 'domain' => \config('app.website_domain'), @@ -78,6 +76,7 @@ Route::apiResource('groups', API\V4\GroupsController::class); Route::get('groups/{id}/status', 'API\V4\GroupsController@status'); + Route::post('groups/{id}/config', 'API\V4\GroupsController@setConfig'); Route::apiResource('packages', API\V4\PackagesController::class); Route::apiResource('skus', API\V4\SkusController::class); diff --git a/src/tests/Browser/Admin/DistlistTest.php b/src/tests/Browser/Admin/DistlistTest.php --- a/src/tests/Browser/Admin/DistlistTest.php +++ b/src/tests/Browser/Admin/DistlistTest.php @@ -63,6 +63,7 @@ $group->assignToWallet($user->wallets->first()); $group->members = ['test1@gmail.com', 'test2@gmail.com']; $group->save(); + $group->setConfig(['sender_policy' => ['test1.com', 'test2.com']]); $distlist_page = new DistlistPage($group->id); $user_page = new UserPage($user->id); @@ -87,6 +88,13 @@ ->assertSeeIn('.row:nth-child(3) label', 'Recipients') ->assertSeeIn('.row:nth-child(3) #members', $group->members[0]) ->assertSeeIn('.row:nth-child(3) #members', $group->members[1]); + }) + ->assertElementsCount('ul.nav-tabs', 1) + ->assertSeeIn('ul.nav-tabs .nav-link', 'Settings') + ->with('@distlist-settings form', function (Browser $browser) { + $browser->assertElementsCount('.row', 1) + ->assertSeeIn('.row:nth-child(1) label', 'Sender Access List') + ->assertSeeIn('.row:nth-child(1) #sender_policy', 'test1.com, test2.com'); }); // Test invalid group identifier diff --git a/src/tests/Browser/Components/ListInput.php b/src/tests/Browser/Components/ListInput.php --- a/src/tests/Browser/Components/ListInput.php +++ b/src/tests/Browser/Components/ListInput.php @@ -34,9 +34,9 @@ */ public function assert($browser) { - $browser->assertVisible($this->selector()) - ->assertVisible('@input') - ->assertVisible('@add-btn'); + $browser->assertVisible($this->selector) + ->assertVisible("{$this->selector} @input") + ->assertVisible("{$this->selector} @add-btn"); } /** diff --git a/src/tests/Browser/DistlistTest.php b/src/tests/Browser/DistlistTest.php --- a/src/tests/Browser/DistlistTest.php +++ b/src/tests/Browser/DistlistTest.php @@ -125,7 +125,9 @@ ->click('button.create-list') ->on(new DistlistInfo()) ->assertSeeIn('#distlist-info .card-title', 'New distribution list') - ->with('@form', function (Browser $browser) { + ->assertSeeIn('@nav #tab-general', 'General') + ->assertMissing('@nav #tab-settings') + ->with('@general', function (Browser $browser) { // Assert form content $browser->assertMissing('#status') ->assertSeeIn('div.row:nth-child(1) label', 'Email') @@ -140,7 +142,7 @@ }) // Test error conditions ->type('#email', 'group-test@kolabnow.com') - ->click('button[type=submit]') + ->click('@general button[type=submit]') ->waitFor('#email + .invalid-feedback') ->assertSeeIn('#email + .invalid-feedback', 'The specified domain is not available.') ->assertFocused('#email') @@ -153,7 +155,7 @@ $browser->addListEntry('test1@gmail.com') ->addListEntry('test2@gmail.com'); }) - ->click('button[type=submit]') + ->click('@general button[type=submit]') ->assertToast(Toast::TYPE_SUCCESS, 'Distribution list created successfully.') ->on(new DistlistList()) ->assertElementsCount('@table tbody tr', 1); @@ -162,7 +164,7 @@ $browser->click('@table tr:nth-child(1) a') ->on(new DistlistInfo()) ->assertSeeIn('#distlist-info .card-title', 'Distribution list') - ->with('@form', function (Browser $browser) { + ->with('@general', function (Browser $browser) { // Assert form content $browser->assertSeeIn('div.row:nth-child(1) label', 'Status') ->assertSeeIn('div.row:nth-child(1) span.text-danger', 'Not Ready') @@ -180,7 +182,7 @@ ->with(new ListInput('#members'), function (Browser $browser) { $browser->addListEntry('invalid address'); }) - ->click('button[type=submit]') + ->click('@general button[type=submit]') ->waitFor('#members + .invalid-feedback') ->assertSeeIn('#members + .invalid-feedback', 'The specified email address is invalid.') ->assertVisible('#members .input-group:nth-child(4) input.is-invalid') @@ -189,7 +191,7 @@ ->with(new ListInput('#members'), function (Browser $browser) { $browser->removeListEntry(3)->removeListEntry(2); }) - ->click('button[type=submit]') + ->click('@general button[type=submit]') ->assertToast(Toast::TYPE_SUCCESS, 'Distribution list updated successfully.') ->assertMissing('.invalid-feedback') ->on(new DistlistList()) @@ -251,6 +253,53 @@ // TODO: Test all group statuses on the list } + /** + * Test distribution list settings + */ + public function testSettings(): void + { + $john = $this->getTestUser('john@kolab.org'); + $this->addDistlistEntitlement($john); + $group = $this->getTestGroup('group-test@kolab.org'); + $group->assignToWallet($john->wallets->first()); + $group->status = Group::STATUS_NEW | Group::STATUS_ACTIVE; + $group->save(); + + $this->browse(function ($browser) use ($group) { + // Test auto-refresh + $browser->visit('/distlist/' . $group->id) + ->on(new DistlistInfo()) + ->assertSeeIn('@nav #tab-general', 'General') + ->assertSeeIn('@nav #tab-settings', 'Settings') + ->click('@nav #tab-settings') + ->with('@settings form', function (Browser $browser) { + // Assert form content + $browser->assertSeeIn('div.row:nth-child(1) label', 'Sender Access List') + ->assertVisible('div.row:nth-child(1) .list-input') + ->with(new ListInput('#sender-policy'), function (Browser $browser) { + $browser->assertListInputValue([]) + ->assertValue('@input', ''); + }) + ->assertSeeIn('button[type=submit]', 'Submit'); + }) + // Test error handling + ->with(new ListInput('#sender-policy'), function (Browser $browser) { + $browser->addListEntry('test.com'); + }) + ->click('@settings button[type=submit]') + ->assertToast(Toast::TYPE_SUCCESS, 'Distribution list settings updated successfully.') + ->assertMissing('.invalid-feedback') + ->refresh() + ->on(new DistlistInfo()) + ->click('@nav #tab-settings') + ->with('@settings form', function (Browser $browser) { + $browser->with(new ListInput('#sender-policy'), function (Browser $browser) { + $browser->assertListInputValue(['test.com']) + ->assertValue('@input', ''); + }); + }); + }); + } /** * Register the beta + distlist entitlements for the user diff --git a/src/tests/Browser/Pages/Admin/Distlist.php b/src/tests/Browser/Pages/Admin/Distlist.php --- a/src/tests/Browser/Pages/Admin/Distlist.php +++ b/src/tests/Browser/Pages/Admin/Distlist.php @@ -51,7 +51,7 @@ return [ '@app' => '#app', '@distlist-info' => '#distlist-info', - '@distlist-config' => '#distlist-config', + '@distlist-settings' => '#distlist-settings', ]; } } diff --git a/src/tests/Browser/Pages/DistlistInfo.php b/src/tests/Browser/Pages/DistlistInfo.php --- a/src/tests/Browser/Pages/DistlistInfo.php +++ b/src/tests/Browser/Pages/DistlistInfo.php @@ -25,7 +25,7 @@ */ public function assert($browser) { - $browser->waitFor('@form') + $browser->waitFor('@general') ->waitUntilMissing('.app-loader'); } @@ -38,7 +38,9 @@ { return [ '@app' => '#app', - '@form' => '#distlist-info form', + '@general' => '#general', + '@nav' => 'ul.nav-tabs', + '@settings' => '#settings', '@status' => '#status-box', ]; } diff --git a/src/tests/Browser/Reseller/DistlistTest.php b/src/tests/Browser/Reseller/DistlistTest.php --- a/src/tests/Browser/Reseller/DistlistTest.php +++ b/src/tests/Browser/Reseller/DistlistTest.php @@ -63,6 +63,7 @@ $group->assignToWallet($user->wallets->first()); $group->members = ['test1@gmail.com', 'test2@gmail.com']; $group->save(); + $group->setConfig(['sender_policy' => ['test1.com', 'test2.com']]); $distlist_page = new DistlistPage($group->id); $user_page = new UserPage($user->id); @@ -87,6 +88,13 @@ ->assertSeeIn('.row:nth-child(3) label', 'Recipients') ->assertSeeIn('.row:nth-child(3) #members', $group->members[0]) ->assertSeeIn('.row:nth-child(3) #members', $group->members[1]); + }) + ->assertElementsCount('ul.nav-tabs', 1) + ->assertSeeIn('ul.nav-tabs .nav-link', 'Settings') + ->with('@distlist-settings form', function (Browser $browser) { + $browser->assertElementsCount('.row', 1) + ->assertSeeIn('.row:nth-child(1) label', 'Sender Access List') + ->assertSeeIn('.row:nth-child(1) #sender_policy', 'test1.com, test2.com'); }); // Test invalid group identifier diff --git a/src/tests/Browser/Reseller/WalletTest.php b/src/tests/Browser/Reseller/WalletTest.php --- a/src/tests/Browser/Reseller/WalletTest.php +++ b/src/tests/Browser/Reseller/WalletTest.php @@ -213,7 +213,7 @@ $browser->waitUntilMissing('.app-loader') ->assertElementsCount('table tbody tr', 10) ->assertMissing('table td.email') - ->assertSeeIn('#transactions-loader button', 'Load more'); + ->assertSeeIn('.more-loader button', 'Load more'); foreach ($pages[0] as $idx => $transaction) { $selector = 'table tbody tr:nth-child(' . ($idx + 1) . ')'; @@ -225,10 +225,10 @@ } // Load the next page - $browser->click('#transactions-loader button') + $browser->click('.more-loader button') ->waitUntilMissing('.app-loader') ->assertElementsCount('table tbody tr', 12) - ->assertMissing('#transactions-loader button'); + ->assertMissing('.more-loader button'); $debitEntry = null; foreach ($pages[1] as $idx => $transaction) { diff --git a/src/tests/Feature/Backends/LDAPTest.php b/src/tests/Feature/Backends/LDAPTest.php --- a/src/tests/Feature/Backends/LDAPTest.php +++ b/src/tests/Feature/Backends/LDAPTest.php @@ -127,6 +127,7 @@ $group = $this->getTestGroup('group@kolab.org', [ 'members' => ['member1@testldap.com', 'member2@testldap.com'] ]); + $group->setSetting('sender_policy', '["test.com"]'); // Create the group LDAP::createGroup($group); @@ -142,6 +143,7 @@ 'groupofuniquenames', 'kolabgroupofuniquenames' ], + 'kolaballowsmtpsender' => 'test.com', 'uniquemember' => [ 'uid=member1@testldap.com,ou=People,ou=kolab.org,' . $root_dn, 'uid=member2@testldap.com,ou=People,ou=kolab.org,' . $root_dn, @@ -155,11 +157,13 @@ // Update members $group->members = ['member3@testldap.com']; $group->save(); + $group->setSetting('sender_policy', '["test.com","-"]'); LDAP::updateGroup($group); // TODO: Should we force this to be always an array? $expected['uniquemember'] = 'uid=member3@testldap.com,ou=People,ou=kolab.org,' . $root_dn; + $expected['kolaballowsmtpsender'] = ['test.com', '-']; $ldap_group = LDAP::getGroup($group->email); @@ -172,11 +176,13 @@ // Update members (add non-existing local member, expect it to be aot-removed from the group) $group->members = ['member3@testldap.com', 'member-local@kolab.org']; $group->save(); + $group->setSetting('sender_policy', null); LDAP::updateGroup($group); // TODO: Should we force this to be always an array? $expected['uniquemember'] = 'uid=member3@testldap.com,ou=People,ou=kolab.org,' . $root_dn; + $expected['kolaballowsmtpsender'] = null; $ldap_group = LDAP::getGroup($group->email); @@ -186,9 +192,9 @@ $this->assertSame(['member3@testldap.com'], $group->fresh()->members); - // We called save() twice, so we expect two update obs, this is making sure - // that there's no job executed by the LDAP backend - Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 2); + // We called save() twice, and setSettings() three times, + // this is making sure that there's no job executed by the LDAP backend + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 5); // Delete the domain LDAP::deleteGroup($group); @@ -345,6 +351,24 @@ LDAP::updateDomain($domain); } + /** + * Test handling update of a non-existing group + * + * @group ldap + */ + public function testUpdateGroupException(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessageMatches('/group not found/'); + + $group = new Group([ + 'email' => 'test@testldap.com', + 'status' => Group::STATUS_NEW | Group::STATUS_ACTIVE, + ]); + + LDAP::updateGroup($group); + } + /** * Test handling update of a non-existing user * 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 @@ -117,6 +117,83 @@ $this->assertSame($group->email, $json[0]['email']); } + /** + * Test group config update (POST /api/v4/groups//config) + */ + public function testSetConfig(): void + { + $john = $this->getTestUser('john@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + $group = $this->getTestGroup('group-test@kolab.org'); + $group->assignToWallet($john->wallets->first()); + + // Test unknown group id + $post = ['sender_policy' => []]; + $response = $this->actingAs($john)->post("/api/v4/groups/123/config", $post); + $json = $response->json(); + + $response->assertStatus(404); + + // Test access by user not being a wallet controller + $post = ['sender_policy' => []]; + $response = $this->actingAs($jack)->post("/api/v4/groups/{$group->id}/config", $post); + $json = $response->json(); + + $response->assertStatus(403); + + $this->assertSame('error', $json['status']); + $this->assertSame("Access denied", $json['message']); + $this->assertCount(2, $json); + + // Test some invalid data + $post = ['test' => 1]; + $response = $this->actingAs($john)->post("/api/v4/groups/{$group->id}/config", $post); + $response->assertStatus(422); + + $json = $response->json(); + + $this->assertSame('error', $json['status']); + $this->assertCount(2, $json); + $this->assertCount(1, $json['errors']); + $this->assertSame('The requested configuration parameter is not supported.', $json['errors']['test']); + + $group->refresh(); + + $this->assertNull($group->getSetting('test')); + $this->assertNull($group->getSetting('sender_policy')); + + // Test some valid data + $post = ['sender_policy' => ['domain.com']]; + $response = $this->actingAs($john)->post("/api/v4/groups/{$group->id}/config", $post); + + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(2, $json); + $this->assertSame('success', $json['status']); + $this->assertSame('Distribution list settings updated successfully.', $json['message']); + + $this->assertSame(['sender_policy' => $post['sender_policy']], $group->fresh()->getConfig()); + + // Test input validation + $post = ['sender_policy' => [5]]; + $response = $this->actingAs($john)->post("/api/v4/groups/{$group->id}/config", $post); + $response->assertStatus(422); + + $json = $response->json(); + + $this->assertCount(2, $json); + $this->assertSame('error', $json['status']); + $this->assertCount(1, $json['errors']); + $this->assertSame( + 'The entry format is invalid. Expected an email, domain, or part of it.', + $json['errors']['sender_policy'][0] + ); + + $this->assertSame(['sender_policy' => ['domain.com']], $group->fresh()->getConfig()); + } + /** * Test fetching group data/profile (GET /api/v4/groups/) */ @@ -128,6 +205,7 @@ $group = $this->getTestGroup('group-test@kolab.org'); $group->assignToWallet($john->wallets->first()); + $group->setSetting('sender_policy', '["test"]'); // Test unauthorized access to a profile of other user $response = $this->get("/api/v4/groups/{$group->id}"); @@ -155,6 +233,7 @@ $this->assertArrayHasKey('isSuspended', $json); $this->assertArrayHasKey('isActive', $json); $this->assertArrayHasKey('isLdapReady', $json); + $this->assertSame(['sender_policy' => ['test']], $json['config']); } /** diff --git a/src/tests/Feature/GroupTest.php b/src/tests/Feature/GroupTest.php --- a/src/tests/Feature/GroupTest.php +++ b/src/tests/Feature/GroupTest.php @@ -42,6 +42,30 @@ $result->assignToWallet($user->wallets->first()); } + /** + * Test Group::getConfig() and setConfig() methods + */ + public function testConfigTrait(): void + { + $group = $this->getTestGroup('group-test@kolabnow.com'); + + $group->setSetting('sender_policy', '["test","-"]'); + + $this->assertSame(['sender_policy' => ['test']], $group->getConfig()); + + $result = $group->setConfig(['sender_policy' => [], 'unknown' => false]); + + $this->assertSame(['sender_policy' => []], $group->getConfig()); + $this->assertSame('[]', $group->getSetting('sender_policy')); + $this->assertSame(['unknown' => "The requested configuration parameter is not supported."], $result); + + $result = $group->setConfig(['sender_policy' => ['test']]); + + $this->assertSame(['sender_policy' => ['test']], $group->getConfig()); + $this->assertSame('["test","-"]', $group->getSetting('sender_policy')); + $this->assertSame([], $result); + } + /** * Test group status assignment and is*() methods */ @@ -189,6 +213,64 @@ $this->assertSame($result->id, $group->id); } + /** + * Tests for GroupSettingsTrait functionality and GroupSettingObserver + */ + public function testSettings(): void + { + Queue::fake(); + Queue::assertNothingPushed(); + + $group = $this->getTestGroup('group-test@kolabnow.com'); + + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 0); + + // Add a setting + $group->setSetting('unknown', 'test'); + + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 0); + + // Add a setting that is synced to LDAP + $group->setSetting('sender_policy', '[]'); + + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 1); + + // Note: We test both current group as well as fresh group object + // to make sure cache works as expected + $this->assertSame('test', $group->getSetting('unknown')); + $this->assertSame('[]', $group->fresh()->getSetting('sender_policy')); + + Queue::fake(); + + // Update a setting + $group->setSetting('unknown', 'test1'); + + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 0); + + // Update a setting that is synced to LDAP + $group->setSetting('sender_policy', '["-"]'); + + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 1); + + $this->assertSame('test1', $group->getSetting('unknown')); + $this->assertSame('["-"]', $group->fresh()->getSetting('sender_policy')); + + Queue::fake(); + + // Delete a setting (null) + $group->setSetting('unknown', null); + + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 0); + + // Delete a setting that is synced to LDAP + $group->setSetting('sender_policy', null); + + Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 1); + + $this->assertSame(null, $group->getSetting('unknown')); + $this->assertSame(null, $group->fresh()->getSetting('sender_policy')); + } + /** * Tests for Group::suspend() */