Page MenuHomePhorge
Files F117917053

D4692.1775412914.diff
No OneTemporary
Actions

Authored By
Unknown
Size
17 KB
Referenced Files
None
Subscribers
None

D4692.1775412914.diff
View Options

diff --git a/src/app/Listeners/SqlDebug.php b/src/app/Listeners/SqlDebug.php
--- a/src/app/Listeners/SqlDebug.php
+++ b/src/app/Listeners/SqlDebug.php
@@ -68,6 +68,8 @@
$serialized = array_map(function ($entry) use ($ipv) {
if ($entry instanceof \DateTime) {
return $entry->format('Y-m-d h:i:s');
+ } elseif (is_bool($entry)) {
+ return $entry ? 'true' : 'false';
} elseif ($ipv && is_string($entry) && strlen($entry) == ($ipv == 6 ? 16 : 4)) {
// binary IP address? use HEX representation
return '0x' . bin2hex($entry);
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
@@ -6,6 +6,7 @@
class Request
{
+ protected $connect;
protected $header;
protected $netID;
protected $netType;
@@ -19,6 +20,11 @@
protected $whitelist;
protected $request = [];
+ /**
+ * Class constructor
+ *
+ * @param array $request Request input
+ */
public function __construct($request)
{
$this->request = $request;
@@ -30,14 +36,17 @@
}
}
- public function headerGreylist()
+ /**
+ * Get request state headers (Received-Greylist) - after self::shouldDefer() call
+ */
+ public function headerGreylist(): string
{
if ($this->whitelist) {
if ($this->whitelist->sender_local) {
return sprintf(
- "Received-Greylist: sender %s whitelisted since %s",
+ "Received-Greylist: sender %s whitelisted since %s (UTC)",
$this->sender,
- $this->whitelist->created_at->toDateString()
+ $this->whitelist->created_at->toDateTimeString()
);
}
@@ -49,20 +58,23 @@
);
}
- $connect = $this->findConnectsCollection()->orderBy('created_at')->first();
-
- if ($connect) {
+ if ($this->connect) {
return sprintf(
"Received-Greylist: greylisted from %s until %s.",
- $connect->created_at,
- $this->timestamp
+ $this->connect->created_at->toDateTimeString(),
+ $this->timestamp->toDateTimeString()
);
}
return "Received-Greylist: no opinion here";
}
- public function shouldDefer()
+ /**
+ * Connection check regarding greylisting policy
+ *
+ * @return bool True if the message should be put off, False otherwise
+ */
+ public function shouldDefer(): bool
{
list($this->netID, $this->netType) = \App\Utils::getNetFromAddress($this->request['client_address']);
@@ -82,11 +94,13 @@
$this->senderLocal = substr($this->senderLocal, 0, 255);
}
- // Purge all old information if we have no recent entries
- $noEntry = false;
- if (!$this->findConnectsCollectionRecent()->exists()) {
+ // Get the most recent entry
+ $connect = $this->findConnectsCollection()->first();
+
+ // Purge all old information if we have no recent entry
+ if ($connect && $connect->updated_at < $this->timestamp->copy()->subDays(7)) {
$this->findConnectsCollection()->delete();
- $noEntry = true;
+ $connect = null;
}
// See if the recipient opted-out of the feature
@@ -100,80 +114,52 @@
// the following block is to maintain statistics and state ...
// determine if the sender domain is a whitelist from this network
- $this->whitelist = Whitelist::where(
- [
- 'sender_domain' => $this->senderDomain,
- 'net_id' => $this->netID,
- 'net_type' => $this->netType
- ]
- )->first();
+ $this->whitelist = Whitelist::where('sender_domain', $this->senderDomain)
+ ->where('net_id', $this->netID)
+ ->where('net_type', $this->netType)
+ ->first();
+
+ $cutoffDate = $this->timestamp->copy()->subDays(7)->startOfDay();
- $cutoffDate = $this->timestamp->copy()->subDays(7);
+ DB::beginTransaction();
+
+ // Whitelist older than a month, delete it
+ if ($this->whitelist && $this->whitelist->updated_at < $this->timestamp->copy()->subMonthsWithoutOverflow(1)) {
+ $this->whitelist->delete();
+ $this->whitelist = null;
+ }
+ $all = Connect::where('sender_domain', $this->senderDomain)
+ ->where('net_id', $this->netID)
+ ->where('net_type', $this->netType)
+ ->where('updated_at', '>=', $cutoffDate->toDateTimeString());
+
+ // "Touch" the whitelist if exists
if ($this->whitelist) {
- if ($this->whitelist->updated_at < $this->timestamp->copy()->subMonthsWithoutOverflow(1)) {
- $this->whitelist->delete();
- } else {
+ // For performance reasons do not update timestamp more than once per 1 minute
+ // FIXME: Such granularity should be good enough, right? It might be a problem
+ // if you wanted to compare this timestamp with mail log entries.
+ if ($this->whitelist->updated_at < $this->timestamp->copy()->subMinute()) {
$this->whitelist->updated_at = $this->timestamp;
$this->whitelist->save(['timestamps' => false]);
-
- Connect::where(
- [
- 'sender_domain' => $this->senderDomain,
- 'net_id' => $this->netID,
- 'net_type' => $this->netType,
- 'greylisting' => true
- ]
- )
- ->whereDate('updated_at', '>=', $cutoffDate)
- ->update(
- [
- 'greylisting' => false,
- 'updated_at' => $this->timestamp
- ]
- );
-
- return false;
}
- } else {
- $count = Connect::where(
- [
- 'sender_domain' => $this->senderDomain,
- 'net_id' => $this->netID,
- 'net_type' => $this->netType
- ]
- )
- ->whereDate('updated_at', '>=', $cutoffDate)
- ->limit(4)->count();
+ $all->where('greylisting', true)
+ ->update(['greylisting' => false, 'updated_at' => $this->timestamp]);
+
+ $enabled = false;
+ } elseif ($all->count() >= 4) {
// Automatically create a whitelist if we have at least 5 (4 existing plus this) messages from the sender
- if ($count >= 4) {
- $this->whitelist = Whitelist::create(
- [
- 'sender_domain' => $this->senderDomain,
- 'net_id' => $this->netID,
- 'net_type' => $this->netType,
- 'created_at' => $this->timestamp,
- 'updated_at' => $this->timestamp
- ]
- );
+ $this->whitelist = Whitelist::create([
+ 'sender_domain' => $this->senderDomain,
+ 'net_id' => $this->netID,
+ 'net_type' => $this->netType,
+ 'created_at' => $this->timestamp,
+ 'updated_at' => $this->timestamp
+ ]);
- Connect::where(
- [
- 'sender_domain' => $this->senderDomain,
- 'net_id' => $this->netID,
- 'net_type' => $this->netType,
- 'greylisting' => true
- ]
- )
- ->whereDate('updated_at', '>=', $cutoffDate)
- ->update(
- [
- 'greylisting' => false,
- 'updated_at' => $this->timestamp
- ]
- );
- }
+ $all->where('greylisting', true)
+ ->update(['greylisting' => false, 'updated_at' => $this->timestamp]);
}
// TODO: determine if the sender (individual) is a whitelist
@@ -181,13 +167,14 @@
// TODO: determine if the sender is a penpal of any of the recipients. First recipient wins.
if (!$enabled) {
+ DB::commit();
return false;
}
$defer = true;
- // Retrieve the entry for the sender/recipient/net combination
- if (!$noEntry && ($connect = $this->findConnectsCollection()->first())) {
+ // Update/Create an entry for the sender/recipient/net combination
+ if ($connect) {
$connect->connect_count += 1;
// TODO: The period of time for which the greylisting persists is configurable.
@@ -199,8 +186,7 @@
$connect->save();
} else {
- Connect::create(
- [
+ $connect = Connect::create([
'sender_local' => $this->senderLocal,
'sender_domain' => $this->senderDomain,
'net_id' => $this->netID,
@@ -210,35 +196,28 @@
'recipient_type' => $this->recipientType,
'created_at' => $this->timestamp,
'updated_at' => $this->timestamp
- ]
- );
+ ]);
}
+ $this->connect = $connect;
+
+ DB::commit();
+
return $defer;
}
- private function findConnectsCollection()
+ protected function findConnectsCollection()
{
- $collection = Connect::where(
- [
+ return Connect::where([
'sender_local' => $this->senderLocal,
'sender_domain' => $this->senderDomain,
'recipient_hash' => $this->recipientHash,
'net_id' => $this->netID,
'net_type' => $this->netType,
- ]
- );
-
- return $collection;
- }
-
- private function findConnectsCollectionRecent()
- {
- return $this->findConnectsCollection()
- ->where('updated_at', '>=', $this->timestamp->copy()->subDays(7));
+ ]);
}
- private function recipientFromRequest()
+ protected function recipientFromRequest()
{
$recipients = \App\Utils::findObjectsByRecipientAddress($this->request['recipient']);
@@ -265,7 +244,7 @@
return $recipient;
}
- public function senderFromRequest()
+ protected function senderFromRequest()
{
return \App\Utils::normalizeAddress($this->request['sender']);
}
diff --git a/src/tests/Feature/Controller/PolicyTest.php b/src/tests/Feature/Controller/PolicyTest.php
new file mode 100644
--- /dev/null
+++ b/src/tests/Feature/Controller/PolicyTest.php
@@ -0,0 +1,87 @@
+<?php
+
+namespace Tests\Feature\Controller;
+
+use App\Domain;
+use App\Policy\Greylist;
+use Illuminate\Support\Facades\DB;
+use Tests\TestCase;
+
+class PolicyTest extends TestCase
+{
+ private $clientAddress;
+ private $net;
+ private $testUser;
+ private $testDomain;
+
+ public function setUp(): void
+ {
+ parent::setUp();
+
+ $this->clientAddress = '212.103.80.148';
+ $this->net = \App\IP4Net::getNet($this->clientAddress);
+ $this->testDomain = $this->getTestDomain('test.domain', [
+ 'type' => Domain::TYPE_EXTERNAL,
+ 'status' => Domain::STATUS_ACTIVE | Domain::STATUS_CONFIRMED | Domain::STATUS_VERIFIED
+ ]);
+ $this->testUser = $this->getTestUser('john@test.domain');
+
+ Greylist\Connect::where('sender_domain', 'sender.domain')->delete();
+ Greylist\Whitelist::where('sender_domain', 'sender.domain')->delete();
+
+ $this->useServicesUrl();
+ }
+
+ public function tearDown(): void
+ {
+ $this->deleteTestUser($this->testUser->email);
+ $this->deleteTestDomain($this->testDomain->namespace);
+
+ Greylist\Connect::where('sender_domain', 'sender.domain')->delete();
+ Greylist\Whitelist::where('sender_domain', 'sender.domain')->delete();
+
+ parent::tearDown();
+ }
+
+ /**
+ * Test greylist policy webhook
+ *
+ * @group data
+ * @group greylist
+ */
+ public function testGreylist()
+ {
+ // Note: Only basic tests here. More detailed policy handler tests are in another place
+
+ // Test 403 response
+ $post = [
+ 'sender' => 'someone@sender.domain',
+ 'recipient' => $this->testUser->email,
+ 'client_address' => $this->clientAddress,
+ 'client_name' => 'some.mx'
+ ];
+
+ $response = $this->post('/api/webhooks/policy/greylist', $post);
+
+ $response->assertStatus(403);
+
+ $json = $response->json();
+
+ $this->assertEquals('DEFER_IF_PERMIT', $json['response']);
+ $this->assertEquals("Greylisted for 5 minutes. Try again later.", $json['reason']);
+
+ // Test 200 response
+ $connect = Greylist\Connect::where('sender_domain', 'sender.domain')->first();
+ $connect->created_at = \Carbon\Carbon::now()->subMinutes(6);
+ $connect->save();
+
+ $response = $this->post('/api/webhooks/policy/greylist', $post);
+
+ $response->assertStatus(200);
+
+ $json = $response->json();
+
+ $this->assertEquals('DUNNO', $json['response']);
+ $this->assertMatchesRegularExpression('/^Received-Greylist: greylisted from/', $json['prepend'][0]);
+ }
+}
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
@@ -2,12 +2,12 @@
namespace Tests\Feature\Stories;
+use App\Domain;
use App\Policy\Greylist;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
/**
- * @group slow
* @group data
* @group greylist
*/
@@ -20,20 +20,24 @@
{
parent::setUp();
- $this->setUpTest();
- $this->useServicesUrl();
$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_whitelist WHERE sender_domain = 'sender.domain';");
+ $this->domainHosted = $this->getTestDomain('test.domain', [
+ 'type' => Domain::TYPE_EXTERNAL,
+ 'status' => Domain::STATUS_ACTIVE | Domain::STATUS_CONFIRMED | Domain::STATUS_VERIFIED
+ ]);
+
+ $this->domainOwner = $this->getTestUser('john@test.domain');
+
+ Greylist\Connect::where('sender_domain', 'sender.domain')->delete();
+ Greylist\Whitelist::where('sender_domain', 'sender.domain')->delete();
}
public function tearDown(): void
{
- DB::delete("DELETE FROM greylist_connect WHERE sender_domain = 'sender.domain';");
- DB::delete("DELETE FROM greylist_whitelist WHERE sender_domain = 'sender.domain';");
+ Greylist\Connect::where('sender_domain', 'sender.domain')->delete();
+ Greylist\Whitelist::where('sender_domain', 'sender.domain')->delete();
parent::tearDown();
}
@@ -253,11 +257,12 @@
$this->assertFalse($request->shouldDefer());
// Ensure we also find the setting by alias
- $aliases = $this->domainOwner->aliases()->orderBy('alias')->pluck('alias')->all();
+ $this->domainOwner->setAliases(['alias1@test2.domain2']);
+
$request = new Greylist\Request(
[
'sender' => 'someone@sender.domain',
- 'recipient' => $aliases[0],
+ 'recipient' => 'alias1@test2.domain2',
'client_address' => $this->clientAddress
]
);
@@ -298,8 +303,13 @@
$this->assertFalse($request->shouldDefer());
}
+ /**
+ * @group slow
+ */
public function testMultipleUsersAllDisabled()
{
+ $this->setUpTest();
+
$request = new Greylist\Request(
[
'sender' => 'someone@sender.domain',
@@ -340,8 +350,13 @@
}
}
+ /**
+ * @group slow
+ */
public function testMultipleUsersAnyEnabled()
{
+ $this->setUpTest();
+
$request = new Greylist\Request(
[
'sender' => 'someone@sender.domain',
@@ -388,16 +403,14 @@
public function testControllerNew()
{
- $data = [
+ $request = new Greylist\Request([
'sender' => 'someone@sender.domain',
'recipient' => $this->domainOwner->email,
'client_address' => $this->clientAddress,
'client_name' => 'some.mx'
- ];
-
- $response = $this->post('/api/webhooks/policy/greylist', $data);
+ ]);
- $response->assertStatus(403);
+ $this->assertTrue($request->shouldDefer());
}
public function testControllerNotNew()
@@ -418,15 +431,13 @@
$connect->created_at = \Carbon\Carbon::now()->subMinutes(6);
$connect->save();
- $data = [
+ $request = new Greylist\Request([
'sender' => 'someone@sender.domain',
'recipient' => $this->domainOwner->email,
'client_address' => $this->clientAddress,
'client_name' => 'some.mx'
- ];
-
- $response = $this->post('/api/webhooks/policy/greylist', $data);
+ ]);
- $response->assertStatus(200);
+ $this->assertFalse($request->shouldDefer());
}
}

File Metadata

Mime Type
text/plain
Expires
Sun, Apr 5, 6:15 PM (9 h, 12 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18833180
Default Alt Text
D4692.1775412914.diff (17 KB)

Event Timeline