From e365e38c127cfb3e6b40cd60780476449048ad73 Mon Sep 17 00:00:00 2001
From: dakkar <dakkar@thenautilus.net>
Date: Sun, 9 Feb 2025 13:34:08 +0000
Subject: skip rewriting when it's not useful

---
 Changes                              |  2 ++
 lib/Sietima/Role/NoSpoof.pm          | 14 ++++++-----
 lib/Sietima/Role/NoSpoof/DMARC.pm    | 45 +++++++++++++++++++-----------------
 t/tests/sietima/role/nospoof.t       | 18 ++++++++++++++-
 t/tests/sietima/role/nospoof/dmarc.t |  2 ++
 5 files changed, 53 insertions(+), 28 deletions(-)

diff --git a/Changes b/Changes
index 1ebe9a3..6a7e403 100644
--- a/Changes
+++ b/Changes
@@ -1,4 +1,6 @@
 {{$NEXT}}
+ - NoSpoof / NoSpoof::DMARC will no longer rewrite "from" addresses
+   belonging to the same domain as the list itself
 
 1.1.2     2023-03-31 16:51:00+01:00 Europe/London
  - new role NoSpoof::DMARC, which replaces the From only when needed
diff --git a/lib/Sietima/Role/NoSpoof.pm b/lib/Sietima/Role/NoSpoof.pm
index ba703cb..5c419df 100644
--- a/lib/Sietima/Role/NoSpoof.pm
+++ b/lib/Sietima/Role/NoSpoof.pm
@@ -14,10 +14,10 @@ use namespace::clean;
 =head1 DESCRIPTION
 
 A L<< C<Sietima> >> list with this role applied will replace the
-`From` address with its own L<<
+C<From> address with its own L<<
 C<post_address>|Sietima::Role::WithPostAddress >> (this is a
 "sub-role" of L<< C<WithPostAddress>|Sietima::Role::WithPostAddress
->>).
+>>) I<if> the C<From> is on a different domain.
 
 This will make the list DMARC-compliant.
 
@@ -29,11 +29,13 @@ around munge_mail => sub ($orig,$self,$incoming_mail) {
     my $sender = $self->post_address->address;
     my ($from) = Email::Address->parse($incoming_mail->header_str('From'));
 
-    $from->address($sender);
+    if ($from->host ne $self->post_address->host) {
+        $from->address($sender);
 
-    $incoming_mail->header_str_set(
-        From => $from,
-    );
+        $incoming_mail->header_str_set(
+            From => $from,
+        );
+    }
 
     return $self->$orig($incoming_mail);
 };
diff --git a/lib/Sietima/Role/NoSpoof/DMARC.pm b/lib/Sietima/Role/NoSpoof/DMARC.pm
index de021da..4a6cedf 100644
--- a/lib/Sietima/Role/NoSpoof/DMARC.pm
+++ b/lib/Sietima/Role/NoSpoof/DMARC.pm
@@ -18,7 +18,8 @@ A L<< C<Sietima> >> list with this role applied will replace the
 C<From> address with its own L<<
 C<post_address>|Sietima::Role::WithPostAddress >> (this is a
 "sub-role" of L<< C<WithPostAddress>|Sietima::Role::WithPostAddress
->>) I<if> the originating address's DMARC policy requires it.
+>>) I<if> the C<From> is on a different domain and the originating
+address's DMARC policy requires it.
 
 This will make the list DMARC-compliant while minimising the changes
 to the messages.
@@ -55,36 +56,38 @@ around munge_mail => sub ($orig,$self,$incoming_mail) {
     my ($from) = Email::Address->parse($incoming_mail->header_str('From'));
     my $from_domain = $from->host;
 
-    my $dmarc = Mail::DMARC::PurePerl->new(
-        resolver => $self->dmarc_resolver,
-    );
-    $dmarc->header_from($from_domain);
+    if ($from_domain ne $self->post_address->host) {
+        my $dmarc = Mail::DMARC::PurePerl->new(
+            resolver => $self->dmarc_resolver,
+        );
+        $dmarc->header_from($from_domain);
 
-    if (my $policy = $dmarc->discover_policy) {
-        # sp applies to sub-domains, defaults to p; p applies to the
-        # domain itself, and is required
-        my $relevant_value = $dmarc->is_subdomain
-            ? ( $policy->sp // $policy->p )
-            : $policy->p;
+        if (my $policy = $dmarc->discover_policy) {
+            # sp applies to sub-domains, defaults to p; p applies to
+            # the domain itself, and is required
+            my $relevant_value = $dmarc->is_subdomain
+                ? ( $policy->sp // $policy->p )
+                : $policy->p;
 
-        if ($relevant_value ne 'none') {
-            $incoming_mail->header_str_set(
-                'Original-From' => $from,
-            );
+            if ($relevant_value ne 'none') {
+                $incoming_mail->header_str_set(
+                    'Original-From' => $from,
+                );
 
-            $from->address($sender);
+                $from->address($sender);
 
-            $incoming_mail->header_str_set(
-                From => $from,
-            );
+                $incoming_mail->header_str_set(
+                    From => $from,
+                );
 
-            return $self->$orig($incoming_mail);
+                return $self->$orig($incoming_mail);
+            }
         }
     }
 
     $incoming_mail->header_str_set(
         Sender => $sender,
-    );
+    ) if $sender ne $from->address;
 
     return $self->$orig($incoming_mail);
 
diff --git a/t/tests/sietima/role/nospoof.t b/t/tests/sietima/role/nospoof.t
index 6f30635..b0ec622 100644
--- a/t/tests/sietima/role/nospoof.t
+++ b/t/tests/sietima/role/nospoof.t
@@ -10,6 +10,10 @@ my $s = make_sietima(
     ],
 );
 
+my $return_path = $s->return_path;
+my $return_path_address = $return_path->address;
+my $return_path_host = $return_path->host;
+
 test_sending(
     sietima => $s,
     mail => {
@@ -17,7 +21,19 @@ test_sending(
     },
     mails => [
         object {
-            call [ header_str => 'from' ] => '"a user" <'.$s->return_path->address.'>';
+            call [ header_str => 'from' ] => qq{"a user" <$return_path_address>};
+        },
+    ],
+);
+
+test_sending(
+    sietima => $s,
+    mail => {
+        from => qq{a user <one\@$return_path_host>},
+    },
+    mails => [
+        object {
+            call [ header_str => 'from' ] => qq{"a user" <one\@$return_path_host>};
         },
     ],
 );
diff --git a/t/tests/sietima/role/nospoof/dmarc.t b/t/tests/sietima/role/nospoof/dmarc.t
index ddbd76a..620268b 100644
--- a/t/tests/sietima/role/nospoof/dmarc.t
+++ b/t/tests/sietima/role/nospoof/dmarc.t
@@ -64,4 +64,6 @@ test_rewriting    'foo@sub.q-q-pol.com';
 
 test_no_rewriting 'foo@example.com';
 
+test_no_rewriting 'foo@' . $s->post_address->host;
+
 done_testing;
-- 
cgit v1.2.3