Page MenuHomePhorge
Files F117988109

D5379.1775530094.diff
No OneTemporary
Actions

Authored By
Unknown
Size
8 KB
Referenced Files
None
Subscribers
None

D5379.1775530094.diff
View Options

diff --git a/src/app/Policy/Mailfilter.php b/src/app/Policy/Mailfilter.php
--- a/src/app/Policy/Mailfilter.php
+++ b/src/app/Policy/Mailfilter.php
@@ -18,6 +18,8 @@
public const CODE_REJECT = 460;
public const CODE_ERROR = 500;
+ protected static $debugid = '';
+
/**
* SMTP Content Filter
*
@@ -47,9 +49,13 @@
// then we'd send body in another request, but only if needed. For example, a text/plain
// message from same domain sender does not include an iTip, nor needs a footer injection.
+ self::$debugid = dechex((int) str_replace('.', '', explode(' ', microtime())[0]));
+ self::debug("Processing message from {$request->sender} to {$request->recipient}...");
+
// Email with multiple recipients, which we don't handle at the moment.
// Likely an outgoing email, so we just accept.
if (str_contains($request->recipient, ",")) {
+ self::debug('Multiple recipients', self::CODE_ACCEPT_EMPTY);
return response('', self::CODE_ACCEPT_EMPTY);
}
@@ -58,6 +64,7 @@
// Not a local recipient, so e.g. an outgoing email
if (empty($user)) {
+ self::debug('Unknown recipient', self::CODE_ACCEPT_EMPTY);
return response('', self::CODE_ACCEPT_EMPTY);
}
@@ -65,6 +72,7 @@
$modules = self::getModulesConfig($user);
if (empty($modules)) {
+ self::debug('All modules disabled', self::CODE_ACCEPT_EMPTY);
return response('', self::CODE_ACCEPT_EMPTY);
}
@@ -74,6 +82,7 @@
if (count($files) == 1) {
$file = $files[array_key_first($files)];
if (!$file->isValid()) {
+ self::debug('Invalid file upload', self::CODE_ERROR);
return response('Invalid file upload', self::CODE_ERROR);
}
@@ -85,23 +94,31 @@
// Initialize mail parser
$parser = new MailParser($stream);
$parser->setRecipient($user);
+ $parser->setDebugPrefix('<' . self::$debugid . '> Mailfilter: ');
if ($sender = $request->sender) {
$parser->setSender($sender);
}
+ self::debug("Message-ID: " . ($parser->getMessageId() ?? 'unset'));
+
// Execute modules
foreach ($modules as $module => $config) {
+ $module_name = str_replace('Module', '', \class_basename($module));
+ self::debug("Executing module {$module_name}...");
+
$engine = new $module($config);
$result = $engine->handle($parser);
if ($result) {
if ($result->getStatus() == Result::STATUS_REJECT) {
+ self::debug("Rejected by {$module_name}", self::CODE_REJECT);
// FIXME: Better code? Should we use custom header instead?
return response('', self::CODE_REJECT);
}
if ($result->getStatus() == Result::STATUS_DISCARD) {
+ self::debug("Discarded by {$module_name}", self::CODE_DISCARD);
// FIXME: Better code? Should we use custom header instead?
return response('', self::CODE_DISCARD);
}
@@ -124,12 +141,38 @@
fclose($stream);
});
+ self::debug('Message modified', self::CODE_ACCEPT);
+
return $response;
}
+ self::debug('Message intact', self::CODE_ACCEPT_EMPTY);
+
return response('', self::CODE_ACCEPT_EMPTY);
}
+ /**
+ * Log a debug message
+ */
+ protected static function debug(string $message, $status = null): void
+ {
+ $debug = 'Mailfilter:';
+
+ if (self::$debugid !== '') {
+ $debug = '<' . self::$debugid . '> ' . $debug;
+ }
+
+ if ($message !== '') {
+ $debug .= ' ' . $message;
+ }
+
+ if ($status) {
+ $debug .= " [{$status}]";
+ }
+
+ \Log::debug($debug);
+ }
+
/**
* Get list of enabled mail filter modules with their configuration
*/
diff --git a/src/app/Policy/Mailfilter/MailParser.php b/src/app/Policy/Mailfilter/MailParser.php
--- a/src/app/Policy/Mailfilter/MailParser.php
+++ b/src/app/Policy/Mailfilter/MailParser.php
@@ -7,7 +7,7 @@
class MailParser
{
protected $stream;
-
+ protected string $debug_prefix = '';
protected int $start = 0;
protected ?int $end;
protected ?int $bodyPosition;
@@ -24,6 +24,7 @@
'content-transfer-encoding',
'content-type',
'from',
+ 'message-id',
'subject',
];
@@ -43,6 +44,18 @@
$this->parseHeaders();
}
+ /**
+ * Log a debug message
+ */
+ public function debug(string $message): void
+ {
+ if ($this->debug_prefix) {
+ $message = $this->debug_prefix . $message;
+ }
+
+ \Log::debug($message);
+ }
+
/**
* Get mail header
*/
@@ -110,6 +123,14 @@
return $this->bodyPosition;
}
+ /**
+ * Get mail Message-Id header
+ */
+ public function getMessageId(): ?string
+ {
+ return $this->getHeader('message-id');
+ }
+
/**
* Returns email address of the recipient
*/
@@ -267,6 +288,14 @@
$this->modified = true;
}
+ /**
+ * Set debug line prefix for log entries
+ */
+ public function setDebugPrefix(string $prefix): void
+ {
+ $this->debug_prefix = $prefix;
+ }
+
/**
* Set header value
*
diff --git a/src/app/Policy/Mailfilter/Modules/ExternalSenderModule.php b/src/app/Policy/Mailfilter/Modules/ExternalSenderModule.php
--- a/src/app/Policy/Mailfilter/Modules/ExternalSenderModule.php
+++ b/src/app/Policy/Mailfilter/Modules/ExternalSenderModule.php
@@ -18,6 +18,8 @@
$user = $parser->getUser();
if ($this->isExternalSender($sender, $user)) {
+ $parser->debug("{$sender} is an external sender");
+
$subject = $parser->getHeader('subject');
// Update the subject with a prefix
@@ -28,6 +30,8 @@
}
$parser->setHeader('Subject', $subject);
+ } else {
+ $parser->debug("{$sender} is an internal sender");
}
return null;
diff --git a/src/app/Policy/Mailfilter/Modules/ItipModule.php b/src/app/Policy/Mailfilter/Modules/ItipModule.php
--- a/src/app/Policy/Mailfilter/Modules/ItipModule.php
+++ b/src/app/Policy/Mailfilter/Modules/ItipModule.php
@@ -35,6 +35,7 @@
$itip = self::getItip($parser);
if ($itip === null) {
+ $parser->debug("No iTip in the message");
return null; // do nothing
}
@@ -43,6 +44,7 @@
$vobject = $this->parseICal($itip);
if ($vobject === null) {
+ $parser->debug("No supported iCalendar data in the iTip");
return null; // do nothing
}
@@ -52,16 +54,22 @@
// FIXME: If $vobject->METHOD is empty fallback to 'method' param from the Content-Type header?
// rfc5545#section-3.7.2 says if one is specified the other must be too
// @phpstan-ignore-next-line
- switch (\strtoupper((string) $vobject->METHOD)) {
+ $method = \strtoupper((string) $vobject->METHOD);
+ switch ($method) {
case 'REQUEST':
+ $parser->debug("Parsing an iTip REQUEST...");
$handler = new ItipModule\RequestHandler($vobject, $this->type, $this->uid);
break;
case 'CANCEL':
+ $parser->debug("Parsing an iTip CANCEL...");
$handler = new ItipModule\CancelHandler($vobject, $this->type, $this->uid);
break;
case 'REPLY':
+ $parser->debug("Parsing an iTip REPLY...");
$handler = new ItipModule\ReplyHandler($vobject, $this->type, $this->uid);
break;
+ default:
+ $parser->debug("Unsupported iTip method: {$method}. iTip ignored.");
}
// FIXME: Should we handle (any?) errors silently and just deliver the message to Inbox as a fallback?
diff --git a/src/tests/Feature/Controller/PolicyTest.php b/src/tests/Feature/Controller/PolicyTest.php
--- a/src/tests/Feature/Controller/PolicyTest.php
+++ b/src/tests/Feature/Controller/PolicyTest.php
@@ -272,7 +272,7 @@
->assertNoContent(204);
// Test returning (modified) mail content
- $john->setConfig(['externalsender_policy' => true]);
+ $john->setConfig(['externalsender_policy' => true, 'itip_policy' => true]);
$url = '/api/webhooks/policy/mail/filter?recipient=john@kolab.org&sender=jack@external.tld';
$content = $this->call('POST', $url, [], [], [], $headers, $post)
->assertStatus(200)

File Metadata

Mime Type
text/plain
Expires
Tue, Apr 7, 2:48 AM (3 h, 12 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18840725
Default Alt Text
D5379.1775530094.diff (8 KB)

Event Timeline