Page MenuHomePhorge
Files F117922240

D2815.1775434616.diff
No OneTemporary
Actions

Authored By
Unknown
Size
16 KB
Referenced Files
None
Subscribers
None

D2815.1775434616.diff
View Options

diff --git a/src/app/Http/Controllers/API/V4/PolicyController.php b/src/app/Http/Controllers/API/V4/PolicyController.php
--- a/src/app/Http/Controllers/API/V4/PolicyController.php
+++ b/src/app/Http/Controllers/API/V4/PolicyController.php
@@ -19,8 +19,6 @@
{
$data = \request()->input();
- list($local, $domainName) = explode('@', $data['recipient']);
-
$request = new \App\Policy\Greylist\Request($data);
$shouldDefer = $request->shouldDefer();
diff --git a/src/app/IP4Net.php b/src/app/IP4Net.php
--- a/src/app/IP4Net.php
+++ b/src/app/IP4Net.php
@@ -20,21 +20,12 @@
'updated_at'
];
- public static function getNet($ip, $mask = 32)
+ public static function getNet($ip)
{
- $query = "
- SELECT id FROM ip4nets
- WHERE INET_ATON(net_number) <= INET_ATON(?)
- AND INET_ATON(net_broadcast) >= INET_ATON(?)
- ORDER BY INET_ATON(net_number), net_mask DESC LIMIT 1
- ";
-
- $results = DB::select($query, [$ip, $ip]);
-
- if (sizeof($results) == 0) {
- return null;
- }
-
- return \App\IP4Net::find($results[0]->id);
+ $where = 'INET_ATON(net_number) <= INET_ATON(?) and INET_ATON(net_broadcast) >= INET_ATON(?)';
+ return IP4Net::whereRaw($where, [$ip, $ip])
+ ->orderByRaw('INET_ATON(net_number), net_mask DESC')
+ ->limit(1)
+ ->first();
}
}
diff --git a/src/app/IP6Net.php b/src/app/IP6Net.php
--- a/src/app/IP6Net.php
+++ b/src/app/IP6Net.php
@@ -20,21 +20,12 @@
'updated_at'
];
- public static function getNet($ip, $mask = 128)
+ public static function getNet($ip)
{
- $query = "
- SELECT id FROM ip6nets
- WHERE INET6_ATON(net_number) <= INET6_ATON(?)
- AND INET6_ATON(net_broadcast) >= INET6_ATON(?)
- ORDER BY INET6_ATON(net_number), net_mask DESC LIMIT 1
- ";
-
- $results = DB::select($query, [$ip, $ip]);
-
- if (sizeof($results) == 0) {
- return null;
- }
-
- return \App\IP6Net::find($results[0]->id);
+ $where = 'INET6_ATON(net_number) <= INET6_ATON(?) and INET6_ATON(net_broadcast) >= INET6_ATON(?)';
+ return IP6Net::whereRaw($where, [$ip, $ip])
+ ->orderByRaw('INET6_ATON(net_number), net_mask DESC')
+ ->limit(1)
+ ->first();
}
}
diff --git a/src/app/Policy/Greylist/Request.php b/src/app/Policy/Greylist/Request.php
--- a/src/app/Policy/Greylist/Request.php
+++ b/src/app/Policy/Greylist/Request.php
@@ -64,8 +64,6 @@
public function shouldDefer()
{
- $deferIfPermit = true;
-
list($this->netID, $this->netType) = \App\Utils::getNetFromAddress($this->request['client_address']);
if (!$this->netID) {
@@ -124,7 +122,7 @@
if ($domain) {
$setting = Setting::where(
[
- 'object_id' => $recipient->domain()->id,
+ 'object_id' => $domain->id,
'object_type' => \App\Domain::class,
'key' => 'greylist_enabled'
]
@@ -228,8 +226,21 @@
->orderBy('updated_at')
->first();
- if (!$connect) {
- $connect = Connect::create(
+ $deferIfPermit = true;
+
+ if ($connect) {
+ $connect->connect_count += 1;
+
+ // TODO: The period of time for which the greylisting persists is configurable.
+ if ($connect->created_at < $this->timestamp->copy()->subMinutes(5)) {
+ $deferIfPermit = false;
+
+ $connect->greylisting = false;
+ }
+
+ $connect->save();
+ } else {
+ Connect::create(
[
'sender_local' => $this->senderLocal,
'sender_domain' => $this->senderDomain,
@@ -238,24 +249,12 @@
'recipient_hash' => $this->recipientHash,
'recipient_id' => $this->recipientID,
'recipient_type' => $this->recipientType,
- 'connect_count' => 0,
'created_at' => $this->timestamp,
'updated_at' => $this->timestamp
]
);
}
- $connect->connect_count += 1;
-
- // TODO: The period of time for which the greylisting persists is configurable.
- if ($connect->created_at < $this->timestamp->copy()->subMinutes(5)) {
- $deferIfPermit = false;
-
- $connect->greylisting = false;
- }
-
- $connect->save();
-
return $deferIfPermit;
}
@@ -282,8 +281,6 @@
private function recipientFromRequest()
{
- $recipient = null;
-
$recipients = \App\Utils::findObjectsByRecipientAddress($this->request['recipient']);
if (sizeof($recipients) > 1) {
diff --git a/src/app/Traits/UserConfigTrait.php b/src/app/Traits/UserConfigTrait.php
--- a/src/app/Traits/UserConfigTrait.php
+++ b/src/app/Traits/UserConfigTrait.php
@@ -2,6 +2,8 @@
namespace App\Traits;
+use App\Policy\Greylist;
+
trait UserConfigTrait
{
/**
@@ -13,7 +15,11 @@
// TODO: Should we store the default value somewhere in config?
- $config['greylisting'] = $this->getSetting('greylist_enabled') !== 'false';
+ $config['greylisting'] = Greylist\Setting::where('key', 'greylist_enabled')
+ ->where('object_id', $this->id)
+ ->where('object_type', \App\User::class)
+ ->where('value', 'false')
+ ->exists() ? false : true;
return $config;
}
@@ -31,7 +37,23 @@
foreach ($config as $key => $value) {
if ($key == 'greylisting') {
- $this->setSetting('greylist_enabled', $value ? 'true' : 'false');
+ $setting_value = $value ? 'true' : 'false';
+ $setting = Greylist\Setting::where('key', 'greylist_enabled')
+ ->where('object_id', $this->id)
+ ->where('object_type', \App\User::class)
+ ->first();
+
+ if (!$setting) {
+ Greylist\Setting::create([
+ 'key' => 'greylist_enabled',
+ 'object_id' => $this->id,
+ 'object_type' => \App\User::class,
+ 'value' => $setting_value,
+ ]);
+ } else {
+ $setting->value = $setting_value;
+ $setting->save();
+ }
} else {
$errors[$key] = \trans('validation.invalid-config-parameter');
}
diff --git a/src/app/Utils.php b/src/app/Utils.php
--- a/src/app/Utils.php
+++ b/src/app/Utils.php
@@ -46,28 +46,12 @@
public static function countryForIP($ip)
{
if (strpos($ip, ':') === false) {
- $query = "
- SELECT country FROM ip4nets
- WHERE INET_ATON(net_number) <= INET_ATON(?)
- AND INET_ATON(net_broadcast) >= INET_ATON(?)
- ORDER BY INET_ATON(net_number), net_mask DESC LIMIT 1
- ";
+ $net = \App\IP4Net::getNet($ip);
} else {
- $query = "
- SELECT id FROM ip6nets
- WHERE INET6_ATON(net_number) <= INET6_ATON(?)
- AND INET6_ATON(net_broadcast) >= INET6_ATON(?)
- ORDER BY INET6_ATON(net_number), net_mask DESC LIMIT 1
- ";
+ $net = \App\IP6Net::getNet($ip);
}
- $nets = \Illuminate\Support\Facades\DB::select($query, [$ip, $ip]);
-
- if (sizeof($nets) > 0) {
- return $nets[0]->country;
- }
-
- return 'CH';
+ return $net && $net->country ? $net->country : 'CH';
}
/**
diff --git a/src/database/migrations/2021_08_25_120000_greylist_settings_migrate.php b/src/database/migrations/2021_08_25_120000_greylist_settings_migrate.php
new file mode 100644
--- /dev/null
+++ b/src/database/migrations/2021_08_25_120000_greylist_settings_migrate.php
@@ -0,0 +1,43 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+// phpcs:ignore
+class GreylistSettingsMigrate extends Migration
+{
+ /**
+ * Run the migrations.
+ *
+ * @return void
+ */
+ public function up()
+ {
+ $settings = DB::table('user_settings')->where('key', 'greylist_enabled')->get();
+
+ foreach ($settings as $setting) {
+ DB::table('greylist_settings')->insert([
+ 'key' => $setting->key,
+ 'value' => $setting->value,
+ 'object_id' => $setting->user_id,
+ 'object_type' => \App\User::class,
+ 'created_at' => $setting->created_at,
+ 'updated_at' => $setting->updated_at,
+ ]);
+ }
+
+ DB::table('user_settings')->where('key', 'greylist_enabled')->delete();
+ }
+
+ /**
+ * Reverse the migrations.
+ *
+ * @return void
+ */
+ public function down()
+ {
+ // do nothing
+ }
+}
diff --git a/src/tests/Browser/Admin/UserTest.php b/src/tests/Browser/Admin/UserTest.php
--- a/src/tests/Browser/Admin/UserTest.php
+++ b/src/tests/Browser/Admin/UserTest.php
@@ -85,6 +85,7 @@
public function testUserInfo(): void
{
$this->browse(function (Browser $browser) {
+ \App\Policy\Greylist\Setting::where('key', 'greylist_enabled')->delete();
$jack = $this->getTestUser('jack@kolab.org');
$page = new UserPage($jack->id);
@@ -197,7 +198,7 @@
$wallet->save();
$group = $this->getTestGroup('group-test@kolab.org');
$group->assignToWallet($john->wallets->first());
- $john->setSetting('greylist_enabled', null);
+ \App\Policy\Greylist\Setting::where('key', 'greylist_enabled')->delete();
// Click the managed-by link on Jack's page
$browser->click('@user-info #manager a')
@@ -322,7 +323,7 @@
]);
$page = new UserPage($ned->id);
- $ned->setSetting('greylist_enabled', 'false');
+ $ned->setConfig(['greylisting' => false]);
$browser->click('@user-users tbody tr:nth-child(4) td:first-child a')
->on($page);
diff --git a/src/tests/Browser/Reseller/UserTest.php b/src/tests/Browser/Reseller/UserTest.php
--- a/src/tests/Browser/Reseller/UserTest.php
+++ b/src/tests/Browser/Reseller/UserTest.php
@@ -175,6 +175,8 @@
*/
public function testUserInfo2(): void
{
+ \App\Policy\Greylist\Setting::where('key', 'greylist_enabled')->delete();
+
$this->browse(function (Browser $browser) {
$john = $this->getTestUser('john@kolab.org');
$page = new UserPage($john->id);
@@ -288,6 +290,7 @@
// Now we go to Ned's info page, he's a controller on John's wallet
$this->browse(function (Browser $browser) {
$ned = $this->getTestUser('ned@kolab.org');
+ $ned->setConfig(['greylisting' => false]);
$page = new UserPage($ned->id);
$browser->click('@user-users tbody tr:nth-child(4) td:first-child a')
@@ -358,6 +361,15 @@
$browser->assertElementsCount('table tbody tr', 0)
->assertSeeIn('table tfoot tr td', 'There are no distribution lists in this account.');
});
+
+ // Assert Settings tab
+ $browser->assertSeeIn('@nav #tab-settings', 'Settings')
+ ->click('@nav #tab-settings')
+ ->whenAvailable('@user-settings form', function (Browser $browser) {
+ $browser->assertElementsCount('.row', 1)
+ ->assertSeeIn('.row:first-child label', 'Greylisting')
+ ->assertSeeIn('.row:first-child .text-danger', 'disabled');
+ });
});
}
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
@@ -341,7 +341,7 @@
public function testUserSettings(): void
{
$john = $this->getTestUser('john@kolab.org');
- $john->setSetting('greylist_enabled', null);
+ \App\Policy\Greylist\Setting::where('key', 'greylist_enabled')->delete();
$this->browse(function (Browser $browser) use ($john) {
$browser->visit('/user/' . $john->id)
@@ -358,7 +358,7 @@
});
});
- $this->assertSame('false', $john->fresh()->getSetting('greylist_enabled'));
+ $this->assertFalse($john->getConfig()['greylisting']);
}
/**
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
@@ -6,6 +6,7 @@
use App\Domain;
use App\Http\Controllers\API\V4\UsersController;
use App\Package;
+use App\Policy\Greylist;
use App\Sku;
use App\User;
use App\Wallet;
@@ -66,7 +67,7 @@
$wallet->discount()->dissociate();
$wallet->settings()->whereIn('key', ['mollie_id', 'stripe_id'])->delete();
$wallet->save();
- $user->settings()->whereIn('key', ['greylist_enabled'])->delete();
+ Greylist\Setting::where('key', 'greylist_enabled')->delete();
$user->status |= User::STATUS_IMAP_READY;
$user->save();
@@ -475,7 +476,7 @@
$jack = $this->getTestUser('jack@kolab.org');
$john = $this->getTestUser('john@kolab.org');
- $john->setSetting('greylist_enabled', null);
+ Greylist\Setting::where('key', 'greylist_enabled')->delete();
// Test unknown user id
$post = ['greylisting' => 1];
@@ -507,7 +508,7 @@
$this->assertCount(1, $json['errors']);
$this->assertSame('The requested configuration parameter is not supported.', $json['errors']['grey']);
- $this->assertNull($john->fresh()->getSetting('greylist_enabled'));
+ $this->assertTrue($john->getConfig()['greylisting']);
// Test some valid data
$post = ['greylisting' => 1];
@@ -520,7 +521,7 @@
$this->assertSame('success', $json['status']);
$this->assertSame('User settings updated successfully.', $json['message']);
- $this->assertSame('true', $john->fresh()->getSetting('greylist_enabled'));
+ $this->assertTrue($john->getConfig()['greylisting']);
// Test some valid data
$post = ['greylisting' => 0];
@@ -533,7 +534,7 @@
$this->assertSame('success', $json['status']);
$this->assertSame('User settings updated successfully.', $json['message']);
- $this->assertSame('false', $john->fresh()->getSetting('greylist_enabled'));
+ $this->assertFalse($john->getConfig()['greylisting']);
}
/**
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
@@ -354,6 +354,34 @@
$this->assertNotContains($domain->namespace, $domains);
}
+ /**
+ * Test User::getConfig() and setConfig() methods
+ */
+ public function testConfigTrait(): void
+ {
+ $john = $this->getTestUser('john@kolab.org');
+ \App\Policy\Greylist\Setting::where('key', 'greylist_enabled')->delete();
+
+ $this->assertSame(['greylisting' => true], $john->getConfig());
+
+ $result = $john->setConfig(['greylisting' => false, 'unknown' => false]);
+
+ $this->assertSame(['greylisting' => false], $john->getConfig());
+
+ $setting = \App\Policy\Greylist\Setting::where('key', 'greylist_enabled')
+ ->where('object_id', $john->id)
+ ->where('object_type', User::class)
+ ->first();
+
+ $this->assertSame('false', $setting->value);
+
+ $result = $john->setConfig(['greylisting' => true]);
+
+ $this->assertSame(['greylisting' => true], $john->getConfig());
+
+ $this->assertSame('true', $setting->fresh()->value);
+ }
+
/**
* Test User::hasSku() method
*/

File Metadata

Mime Type
text/plain
Expires
Mon, Apr 6, 12:16 AM (12 h, 26 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18821377
Default Alt Text
D2815.1775434616.diff (16 KB)

Event Timeline