Page MenuHomePhorge

D2815.1775152190.diff
No OneTemporary

Authored By
Unknown
Size
26 KB
Referenced Files
None
Subscribers
None

D2815.1775152190.diff

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,11 @@
'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')
+ ->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,11 @@
'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')
+ ->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) {
@@ -106,49 +104,14 @@
);
}
- // see if all recipients and their domains are opt-outs
- $enabled = false;
-
+ // See if the recipient opted-out of the feature
+ $enabled = true;
if ($recipient) {
- $setting = Setting::where(
- [
- 'object_id' => $this->recipientID,
- 'object_type' => $this->recipientType,
- 'key' => 'greylist_enabled'
- ]
- )->first();
-
- if (!$setting) {
- $domain = $recipient->domain();
-
- if ($domain) {
- $setting = Setting::where(
- [
- 'object_id' => $recipient->domain()->id,
- 'object_type' => \App\Domain::class,
- 'key' => 'greylist_enabled'
- ]
- )->first();
-
- if (!$setting) {
- $enabled = true;
- } else {
- if ($setting->{'value'} !== 'false') {
- $enabled = true;
- }
- }
- } else {
- $enabled = true;
- }
- } else {
- if ($setting->{'value'} !== 'false') {
- $enabled = true;
- }
- }
- } else {
- $enabled = true;
+ $enabled = $recipient->getSetting('greylist_enabled') !== 'false';
}
+ // FIXME: Shouldn't we bail-out (return early) if there's no $recipient?
+
// the following block is to maintain statistics and state ...
$entries = Connect::where(
[
@@ -228,8 +191,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 +214,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 +246,6 @@
private function recipientFromRequest()
{
- $recipient = null;
-
$recipients = \App\Utils::findObjectsByRecipientAddress($this->request['recipient']);
if (sizeof($recipients) > 1) {
diff --git a/src/app/Policy/Greylist/Setting.php b/src/app/Policy/Greylist/Setting.php
deleted file mode 100644
--- a/src/app/Policy/Greylist/Setting.php
+++ /dev/null
@@ -1,17 +0,0 @@
-<?php
-
-namespace App\Policy\Greylist;
-
-use Illuminate\Database\Eloquent\Model;
-
-class Setting extends Model
-{
- protected $table = 'greylist_settings';
-
- protected $fillable = [
- 'object_id',
- 'object_type',
- 'key',
- 'value'
- ];
-}
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,7 @@
// TODO: Should we store the default value somewhere in config?
- $config['greylisting'] = $this->getSetting('greylist_enabled') !== 'false';
+ $config['greylist_enabled'] = $this->getSetting('greylist_enabled') !== 'false';
return $config;
}
@@ -30,7 +32,7 @@
$errors = [];
foreach ($config as $key => $value) {
- if ($key == 'greylisting') {
+ if ($key == 'greylist_enabled') {
$this->setSetting('greylist_enabled', $value ? 'true' : 'false');
} 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_drop_greylist_settings_table.php b/src/database/migrations/2021_08_25_120000_drop_greylist_settings_table.php
new file mode 100644
--- /dev/null
+++ b/src/database/migrations/2021_08_25_120000_drop_greylist_settings_table.php
@@ -0,0 +1,43 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+// phpcs:ignore
+class DropGreylistSettingsTable extends Migration
+{
+ /**
+ * Run the migrations.
+ *
+ * @return void
+ */
+ public function up()
+ {
+ Schema::dropIfExists('greylist_settings');
+ }
+
+ /**
+ * Reverse the migrations.
+ *
+ * @return void
+ */
+ public function down()
+ {
+ // FIXME: Should we really create the table?
+
+ Schema::create(
+ 'greylist_settings',
+ function (Blueprint $table) {
+ $table->bigIncrements('id');
+ $table->bigInteger('object_id');
+ $table->string('object_type', 16);
+ $table->string('key', 64);
+ $table->text('value');
+ $table->timestamps();
+
+ $table->index(['object_id', 'object_type', 'key'], 'ook_idx');
+ }
+ );
+ }
+}
diff --git a/src/resources/vue/Admin/User.vue b/src/resources/vue/Admin/User.vue
--- a/src/resources/vue/Admin/User.vue
+++ b/src/resources/vue/Admin/User.vue
@@ -312,10 +312,10 @@
<div class="card-text">
<form class="read-only short">
<div class="row plaintext">
- <label for="greylisting" class="col-sm-4 col-form-label">{{ $t('user.greylisting') }}</label>
+ <label for="greylist_enabled" class="col-sm-4 col-form-label">{{ $t('user.greylisting') }}</label>
<div class="col-sm-8">
- <span class="form-control-plaintext" id="greylisting">
- <span v-if="user.config.greylisting" class="text-success">{{ $t('form.enabled') }}</span>
+ <span class="form-control-plaintext" id="greylist_enabled">
+ <span v-if="user.config.greylist_enabled" class="text-success">{{ $t('form.enabled') }}</span>
<span v-else class="text-danger">{{ $t('form.disabled') }}</span>
</span>
</div>
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
@@ -177,9 +177,9 @@
<div class="tab-pane" id="settings" role="tabpanel" aria-labelledby="tab-settings">
<form @submit.prevent="submitSettings" class="card-body">
<div class="row checkbox mb-3">
- <label for="greylisting" class="col-sm-4 col-form-label">{{ $t('user.greylisting') }}</label>
+ <label for="greylist_enabled" class="col-sm-4 col-form-label">{{ $t('user.greylisting') }}</label>
<div class="col-sm-8 pt-2">
- <input type="checkbox" id="greylisting" name="greylisting" value="1" class="form-check-input d-block mb-2" :checked="user.config.greylisting">
+ <input type="checkbox" id="greylist_enabled" name="greylist_enabled" value="1" class="form-check-input d-block mb-2" :checked="user.config.greylist_enabled">
<small id="greylisting-hint" class="text-muted">
{{ $t('user.greylisting-text') }}
</small>
@@ -350,7 +350,7 @@
},
submitSettings() {
this.$root.clearFormValidation($('#settings form'))
- let post = { greylisting: $('#greylisting').prop('checked') ? 1 : 0 }
+ let post = { greylist_enabled: $('#greylist_enabled').prop('checked') ? 1 : 0 }
axios.post('/api/v4/users/' + this.user_id + '/config', post)
.then(response => {
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
@@ -288,6 +288,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->setSetting('greylist_enabled', 'false');
$page = new UserPage($ned->id);
$browser->click('@user-users tbody tr:nth-child(4) td:first-child a')
@@ -358,6 +359,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/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
@@ -245,7 +245,7 @@
$this->assertTrue(is_array($json['statusInfo']));
$this->assertTrue(is_array($json['settings']));
$this->assertTrue(is_array($json['aliases']));
- $this->assertTrue($json['config']['greylisting']);
+ $this->assertTrue($json['config']['greylist_enabled']);
$this->assertSame([], $json['skus']);
// Values below are tested by Unit tests
$this->assertArrayHasKey('isDeleted', $json);
@@ -478,14 +478,14 @@
$john->setSetting('greylist_enabled', null);
// Test unknown user id
- $post = ['greylisting' => 1];
+ $post = ['greylist_enabled' => 1];
$response = $this->actingAs($john)->post("/api/v4/users/123/config", $post);
$json = $response->json();
$response->assertStatus(404);
// Test access by user not being a wallet controller
- $post = ['greylisting' => 1];
+ $post = ['greylist_enabled' => 1];
$response = $this->actingAs($jack)->post("/api/v4/users/{$john->id}/config", $post);
$json = $response->json();
@@ -510,7 +510,7 @@
$this->assertNull($john->fresh()->getSetting('greylist_enabled'));
// Test some valid data
- $post = ['greylisting' => 1];
+ $post = ['greylist_enabled' => 1];
$response = $this->actingAs($john)->post("/api/v4/users/{$john->id}/config", $post);
$response->assertStatus(200);
@@ -523,7 +523,7 @@
$this->assertSame('true', $john->fresh()->getSetting('greylist_enabled'));
// Test some valid data
- $post = ['greylisting' => 0];
+ $post = ['greylist_enabled' => 0];
$response = $this->actingAs($john)->post("/api/v4/users/{$john->id}/config", $post);
$response->assertStatus(200);
diff --git a/src/tests/Feature/Stories/GreylistTest.php b/src/tests/Feature/Stories/GreylistTest.php
--- a/src/tests/Feature/Stories/GreylistTest.php
+++ b/src/tests/Feature/Stories/GreylistTest.php
@@ -14,7 +14,6 @@
class GreylistTest extends TestCase
{
private $clientAddress;
- private $instance;
private $requests = [];
private $net;
@@ -24,20 +23,17 @@
$this->setUpTest();
$this->useServicesUrl();
- $this->instance = $this->generateInstanceId();
$this->clientAddress = '212.103.80.148';
$this->net = \App\IP4Net::getNet($this->clientAddress);
DB::delete("DELETE FROM greylist_connect WHERE sender_domain = 'sender.domain';");
- DB::delete("DELETE FROM greylist_settings;");
DB::delete("DELETE FROM greylist_whitelist WHERE sender_domain = 'sender.domain';");
}
public function tearDown(): void
{
DB::delete("DELETE FROM greylist_connect WHERE sender_domain = 'sender.domain';");
- DB::delete("DELETE FROM greylist_settings;");
DB::delete("DELETE FROM greylist_whitelist WHERE sender_domain = 'sender.domain';");
parent::tearDown();
@@ -218,161 +214,6 @@
$this->assertFalse($request->shouldDefer());
}
- public function testDomainDisabled()
- {
- $setting = Greylist\Setting::create(
- [
- 'object_id' => $this->domainHosted->id,
- 'object_type' => \App\Domain::class,
- 'key' => 'greylist_enabled',
- 'value' => 'false'
- ]
- );
-
- $request = new Greylist\Request(
- [
- 'sender' => 'someone@sender.domain',
- 'recipient' => $this->domainOwner->email,
- 'client_address' => $this->clientAddress
- ]
- );
-
- $this->assertFalse($request->shouldDefer());
- }
-
- public function testDomainEnabled()
- {
- $connect = Greylist\Connect::create(
- [
- 'sender_local' => 'someone',
- 'sender_domain' => 'sender.domain',
- 'recipient_hash' => hash('sha256', $this->domainOwner->email),
- 'recipient_id' => $this->domainOwner->id,
- 'recipient_type' => \App\User::class,
- 'connect_count' => 1,
- 'net_id' => \App\IP4Net::getNet('212.103.80.148')->id,
- 'net_type' => \App\IP4Net::class
- ]
- );
-
- $setting = Greylist\Setting::create(
- [
- 'object_id' => $this->domainHosted->id,
- 'object_type' => \App\Domain::class,
- 'key' => 'greylist_enabled',
- 'value' => 'true'
- ]
- );
-
- $request = new Greylist\Request(
- [
- 'sender' => 'someone@sender.domain',
- 'recipient' => $this->domainOwner->email,
- 'client_address' => $this->clientAddress
- ]
- );
-
- $this->assertTrue($request->shouldDefer());
-
- $connect->created_at = \Carbon\Carbon::now()->subMinutes(6);
- $connect->save();
-
- $this->assertFalse($request->shouldDefer());
- }
-
- public function testDomainDisabledUserDisabled()
- {
- $connect = Greylist\Connect::create(
- [
- 'sender_local' => 'someone',
- 'sender_domain' => 'sender.domain',
- 'recipient_hash' => hash('sha256', $this->domainOwner->email),
- 'recipient_id' => $this->domainOwner->id,
- 'recipient_type' => \App\User::class,
- 'connect_count' => 1,
- 'net_id' => $this->net->id,
- 'net_type' => \App\IP4Net::class
- ]
- );
-
- $settingDomain = Greylist\Setting::create(
- [
- 'object_id' => $this->domainHosted->id,
- 'object_type' => \App\Domain::class,
- 'key' => 'greylist_enabled',
- 'value' => 'false'
- ]
- );
-
- $settingUser = Greylist\Setting::create(
- [
- 'object_id' => $this->domainOwner->id,
- 'object_type' => \App\User::class,
- 'key' => 'greylist_enabled',
- 'value' => 'false'
- ]
- );
-
- $request = new Greylist\Request(
- [
- 'sender' => 'someone@sender.domain',
- 'recipient' => $this->domainOwner->email,
- 'client_address' => $this->clientAddress
- ]
- );
-
- $this->assertFalse($request->shouldDefer());
- }
-
- public function testDomainDisabledUserEnabled()
- {
- $connect = Greylist\Connect::create(
- [
- 'sender_local' => 'someone',
- 'sender_domain' => 'sender.domain',
- 'recipient_hash' => hash('sha256', $this->domainOwner->email),
- 'recipient_id' => $this->domainOwner->id,
- 'recipient_type' => \App\User::class,
- 'connect_count' => 1,
- 'net_id' => $this->net->id,
- 'net_type' => \App\IP4Net::class
- ]
- );
-
- $settingDomain = Greylist\Setting::create(
- [
- 'object_id' => $this->domainHosted->id,
- 'object_type' => \App\Domain::class,
- 'key' => 'greylist_enabled',
- 'value' => 'false'
- ]
- );
-
- $settingUser = Greylist\Setting::create(
- [
- 'object_id' => $this->domainOwner->id,
- 'object_type' => \App\User::class,
- 'key' => 'greylist_enabled',
- 'value' => 'true'
- ]
- );
-
- $request = new Greylist\Request(
- [
- 'sender' => 'someone@sender.domain',
- 'recipient' => $this->domainOwner->email,
- 'client_address' => $this->clientAddress
- ]
- );
-
- $this->assertTrue($request->shouldDefer());
-
- $connect->created_at = \Carbon\Carbon::now()->subMinutes(6);
- $connect->save();
-
- $this->assertFalse($request->shouldDefer());
- }
-
public function testInvalidDomain()
{
$connect = Greylist\Connect::create(
@@ -440,14 +281,7 @@
]
);
- $setting = Greylist\Setting::create(
- [
- 'object_id' => $this->domainOwner->id,
- 'object_type' => \App\User::class,
- 'key' => 'greylist_enabled',
- 'value' => 'false'
- ]
- );
+ $this->domainOwner->setSetting('greylist_enabled', 'false');
$request = new Greylist\Request(
[
@@ -459,7 +293,6 @@
$this->assertFalse($request->shouldDefer());
-
// Ensure we also find the setting by alias
$aliases = $this->domainOwner->aliases()->orderBy('alias')->get();
$request = new Greylist\Request(
@@ -488,14 +321,7 @@
]
);
- $setting = Greylist\Setting::create(
- [
- 'object_id' => $this->domainOwner->id,
- 'object_type' => \App\User::class,
- 'key' => 'greylist_enabled',
- 'value' => 'true'
- ]
- );
+ $this->domainOwner->setSetting('greylist_enabled', 'true');
$request = new Greylist\Request(
[
@@ -537,14 +363,7 @@
]
);
- Greylist\Setting::create(
- [
- 'object_id' => $user->id,
- 'object_type' => \App\User::class,
- 'key' => 'greylist_enabled',
- 'value' => 'false'
- ]
- );
+ $user->setSetting('greylist_enabled', 'false');
if ($user->email == $this->domainOwner->email) {
continue;
@@ -586,14 +405,7 @@
]
);
- Greylist\Setting::create(
- [
- 'object_id' => $user->id,
- 'object_type' => \App\User::class,
- 'key' => 'greylist_enabled',
- 'value' => ($user->id == $this->jack->id) ? 'true' : 'false'
- ]
- );
+ $user->setSetting('greylist_enabled', ($user->id == $this->jack->id) ? 'true' : 'false');
if ($user->email == $this->domainOwner->email) {
continue;
@@ -614,17 +426,4 @@
}
}
}
-
- private function generateInstanceId()
- {
- $instance = [];
-
- for ($x = 0; $x < 3; $x++) {
- for ($y = 0; $y < 3; $y++) {
- $instance[] = substr('01234567889', rand(0, 9), 1);
- }
- }
-
- return implode('.', $instance);
- }
}
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
@@ -355,6 +355,27 @@
}
/**
+ * Test User::getConfig() and setConfig() methods
+ */
+ public function testConfigTrait(): void
+ {
+ $john = $this->getTestUser('john@kolab.org');
+ $john->setSetting('greylist_enabled', null);
+
+ $this->assertSame(['greylist_enabled' => true], $john->getConfig());
+
+ $result = $john->setConfig(['greylist_enabled' => false, 'unknown' => false]);
+
+ $this->assertSame(['greylist_enabled' => false], $john->getConfig());
+ $this->assertSame('false', $john->getSetting('greylist_enabled'));
+
+ $result = $john->setConfig(['greylist_enabled' => true]);
+
+ $this->assertSame(['greylist_enabled' => true], $john->getConfig());
+ $this->assertSame('true', $john->getSetting('greylist_enabled'));
+ }
+
+ /**
* Test User::hasSku() method
*/
public function testHasSku(): void

File Metadata

Mime Type
text/plain
Expires
Thu, Apr 2, 5:49 PM (3 d, 4 h ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18820119
Default Alt Text
D2815.1775152190.diff (26 KB)

Event Timeline