[pmg-devel] [PATCH pmg-api] fix #3924: ldap: skip non-smtp proxyAddresses

[pmg-devel] [PATCH pmg-api] fix #3924: ldap: skip non-smtp proxyAddresses

[Markus Frank]

When an entry in proxyAddresses starts with SIP (or everthing
that is not smtp) no e-mail gets listed on the webpage.
Therefore no quarantine notification messages are sent.

To change this, every entry with another protocol than smtp,
should be skipped.

removed "SMTP" because of lowercase function is called before.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 src/PMG/LDAPCache.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/PMG/LDAPCache.pm b/src/PMG/LDAPCache.pm
index df61454..8172329 100755
--- a/src/PMG/LDAPCache.pm
+++ b/src/PMG/LDAPCache.pm
@@ -159,12 +159,17 @@ sub queryusers {
 	    next if !$user->{attributes}->{$attr};
 	    foreach my $mail (@{$user->{attributes}->{$attr}}) {
 		$mail = lc($mail);
+
+		# Check if proxyAddresses entry starts with anything
+		# other than smtp and skip if true
+		next if ($mail =~ m/^(?!.*smtp).*[\:\$]/gs);
+
 		# Test if the Line starts with one of the following lines:
-		# proxyAddresses: [smtp|SMTP]:
+		# proxyAddresses: [smtp]:
 		# and also discard this starting string, so that $mail is only the
 		# address without any other characters...
 
-		$mail =~ s/^(smtp|SMTP)[\:\$]//gs;
+		$mail =~ s/^smtp[\:\$]//gs;
 
 		if ($mail !~ m/[\{\}\\\/]/ && $mail =~ m/^\S+\@\S+$/) {
 		    $umails->{$mail} = 1;
-- 
2.30.2

Re: [pmg-devel] [PATCH pmg-api] fix #3924: ldap: skip non-smtp proxyAddresses

[Thomas Lamprecht]

On 18.03.22 13:37, Markus Frank wrote:
> When an entry in proxyAddresses starts with SIP (or everthing
> that is not smtp) no e-mail gets listed on the webpage.
> Therefore no quarantine notification messages are sent.
> 

1. that statement doesn't really tells me any rationale or the actual
   problem, also, which webpage? do you mean the web-interface (GUI)?

2. this loops not only over proxyAddresses but *all* mail attributes
   (user configurable, default: mail, userPrincipalName, proxyAddresses,
   othermailbox, mailAlternativeAddress) so unconditionally skipping all
   that doesn't starts with smtp seems odd

3. the bug report shows that there is a smtp address, it just isn't on
   the first line, should this rather cope with such multi-line stuff
   and better extract the (any?) mail addresses there?

> To change this, every entry with another protocol than smtp,
> should be skipped.
> 
> removed "SMTP" because of lowercase function is called before.

nit: could be a separate patch as its rather unrelated with the
skipping.

> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  src/PMG/LDAPCache.pm | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PMG/LDAPCache.pm b/src/PMG/LDAPCache.pm
> index df61454..8172329 100755
> --- a/src/PMG/LDAPCache.pm
> +++ b/src/PMG/LDAPCache.pm
> @@ -159,12 +159,17 @@ sub queryusers {
>  	    next if !$user->{attributes}->{$attr};
>  	    foreach my $mail (@{$user->{attributes}->{$attr}}) {
>  		$mail = lc($mail);
> +
> +		# Check if proxyAddresses entry starts with anything

but it's not only 'proxyAddresses' that's accessed here (see 2. above).

> +		# other than smtp and skip if true
> +		next if ($mail =~ m/^(?!.*smtp).*[\:\$]/gs);

iff we'd do it this way a negated match (`!~` vs. `=~`) could be favorable over a
negative-lookahead readability wise.

> +
>  		# Test if the Line starts with one of the following lines:
> -		# proxyAddresses: [smtp|SMTP]:
> +		# proxyAddresses: [smtp]:
>  		# and also discard this starting string, so that $mail is only the
>  		# address without any other characters...
>  
> -		$mail =~ s/^(smtp|SMTP)[\:\$]//gs;
> +		$mail =~ s/^smtp[\:\$]//gs;
>  
>  		if ($mail !~ m/[\{\}\\\/]/ && $mail =~ m/^\S+\@\S+$/) {
>  		    $umails->{$mail} = 1;