[pmg-devel] [PATCH pmg-api] fix #2013 spamreport: remove ticket if authmode is ldap

[pmg-devel] [PATCH pmg-api] fix #2013 spamreport: remove ticket if authmode is ldap

[Stoiko Ivanov]

Currently the 'authmode' setting for the spamquarantine is not used
anywhere. According to documentation setting it to 'ldap' should allow
access to the quarantine only with ldap credentials.

This patch addresses the issue by not generating a quarantineticket,
and adapting all links accordingly if the authmode is 'ldap'.

tested by changing the authmode and running
`pmgqm send -receiver <email-address> -debug 1`

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/CLI/pmgqm.pm | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/PMG/CLI/pmgqm.pm b/src/PMG/CLI/pmgqm.pm
index 39253db..1e21bf0 100755
--- a/src/PMG/CLI/pmgqm.pm
+++ b/src/PMG/CLI/pmgqm.pm
@@ -70,8 +70,12 @@ sub get_item_data {
     $item->{file} = $ref->{file};
 
     my $basehref = "$data->{protocol_fqdn_port}/quarantine";
-    my $ticket = uri_escape($data->{ticket});
-    $item->{href} = "$basehref?ticket=$ticket&cselect=$item->{id}&date=$item->{date}";
+    if ($data->{authmode} ne 'ldap') {
+	my $ticket = uri_escape($data->{ticket});
+	$item->{href} = "$basehref?ticket=$ticket&cselect=$item->{id}&date=$item->{date}";
+    } else {
+	$item->{href} = "$basehref?cselect=$item->{id}&date=$item->{date}";
+    }
 
     return $item;
 }
@@ -229,6 +233,8 @@ __PACKAGE__->register_method ({
 	    $protocol_fqdn_port .= ":$port";
 	}
 
+	my $authmode = $cfg->get ('spamquar', 'authmode') // 'ticket';
+
 	my $global_data = {
 	    protocol => $protocol,
 	    port => $port,
@@ -238,6 +244,7 @@ __PACKAGE__->register_method ({
 	    timespan => $timespan,
 	    items => [],
 	    protocol_fqdn_port => $protocol_fqdn_port,
+	    authmode => $authmode,
 	};
 
 	my $mailfrom = $cfg->get ('spamquar', 'mailfrom') //
@@ -306,9 +313,13 @@ __PACKAGE__->register_method ({
 		$mailcount = 0;
 
 		$data->{pmail} = $creceiver;
-		$data->{ticket} = PMG::Ticket::assemble_quarantine_ticket($data->{pmail});
-		my $esc_ticket = uri_escape($data->{ticket});
-		$data->{managehref} = "$protocol_fqdn_port/quarantine?ticket=${esc_ticket}";
+		$data->{managehref} = "$protocol_fqdn_port/quarantine";
+		if ($data->{authmode} ne 'ldap') {
+		    $data->{ticket} = PMG::Ticket::assemble_quarantine_ticket($data->{pmail});
+		    my $esc_ticket = uri_escape($data->{ticket});
+		    $data->{managehref} .= "?ticket=${esc_ticket}";
+		}
+
 	    }
 
 	    push @{$data->{items}}, get_item_data($data, $ref);
-- 
2.20.1

Re: [pmg-devel] [PATCH pmg-api] fix #2013 spamreport: remove ticket if authmode is ldap

[Dominik Csapak]

Looks good and works like intended, setting the authmode to
ldap does not include the ticket anymore

just want to comment that this is now the opposite behaviour
of pmg <= 4 where setting authmode to ldap would not
change the template, but not accept quarantine tickets anymore
(which we should *probably* also do, since there may be
some valid tickets around; but this can be a separate patch)

Reviewed-By: Dominik Csapak <d.csapak@proxmox.com>
Tested-By: Dominik Csapak <d.csapak@proxmox.com>

On 5/17/21 4:02 PM, Stoiko Ivanov wrote:
> Currently the 'authmode' setting for the spamquarantine is not used
> anywhere. According to documentation setting it to 'ldap' should allow
> access to the quarantine only with ldap credentials.
> 
> This patch addresses the issue by not generating a quarantineticket,
> and adapting all links accordingly if the authmode is 'ldap'.
> 
> tested by changing the authmode and running
> `pmgqm send -receiver <email-address> -debug 1`
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>   src/PMG/CLI/pmgqm.pm | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PMG/CLI/pmgqm.pm b/src/PMG/CLI/pmgqm.pm
> index 39253db..1e21bf0 100755
> --- a/src/PMG/CLI/pmgqm.pm
> +++ b/src/PMG/CLI/pmgqm.pm
> @@ -70,8 +70,12 @@ sub get_item_data {
>       $item->{file} = $ref->{file};
>   
>       my $basehref = "$data->{protocol_fqdn_port}/quarantine";
> -    my $ticket = uri_escape($data->{ticket});
> -    $item->{href} = "$basehref?ticket=$ticket&cselect=$item->{id}&date=$item->{date}";
> +    if ($data->{authmode} ne 'ldap') {
> +	my $ticket = uri_escape($data->{ticket});
> +	$item->{href} = "$basehref?ticket=$ticket&cselect=$item->{id}&date=$item->{date}";
> +    } else {
> +	$item->{href} = "$basehref?cselect=$item->{id}&date=$item->{date}";
> +    }
>   
>       return $item;
>   }
> @@ -229,6 +233,8 @@ __PACKAGE__->register_method ({
>   	    $protocol_fqdn_port .= ":$port";
>   	}
>   
> +	my $authmode = $cfg->get ('spamquar', 'authmode') // 'ticket';
> +
>   	my $global_data = {
>   	    protocol => $protocol,
>   	    port => $port,
> @@ -238,6 +244,7 @@ __PACKAGE__->register_method ({
>   	    timespan => $timespan,
>   	    items => [],
>   	    protocol_fqdn_port => $protocol_fqdn_port,
> +	    authmode => $authmode,
>   	};
>   
>   	my $mailfrom = $cfg->get ('spamquar', 'mailfrom') //
> @@ -306,9 +313,13 @@ __PACKAGE__->register_method ({
>   		$mailcount = 0;
>   
>   		$data->{pmail} = $creceiver;
> -		$data->{ticket} = PMG::Ticket::assemble_quarantine_ticket($data->{pmail});
> -		my $esc_ticket = uri_escape($data->{ticket});
> -		$data->{managehref} = "$protocol_fqdn_port/quarantine?ticket=${esc_ticket}";
> +		$data->{managehref} = "$protocol_fqdn_port/quarantine";
> +		if ($data->{authmode} ne 'ldap') {
> +		    $data->{ticket} = PMG::Ticket::assemble_quarantine_ticket($data->{pmail});
> +		    my $esc_ticket = uri_escape($data->{ticket});
> +		    $data->{managehref} .= "?ticket=${esc_ticket}";
> +		}
> +
>   	    }
>   
>   	    push @{$data->{items}}, get_item_data($data, $ref);
> 

[pmg-devel] applied: [PATCH pmg-api] fix #2013 spamreport: remove ticket if authmode is ldap

[Thomas Lamprecht]

On 17.05.21 16:02, Stoiko Ivanov wrote:
> Currently the 'authmode' setting for the spamquarantine is not used
> anywhere. According to documentation setting it to 'ldap' should allow
> access to the quarantine only with ldap credentials.
> 
> This patch addresses the issue by not generating a quarantineticket,
> and adapting all links accordingly if the authmode is 'ldap'.
> 
> tested by changing the authmode and running
> `pmgqm send -receiver <email-address> -debug 1`
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/CLI/pmgqm.pm | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
>

applied, with Dominiks R-b, T-b tags, thanks!

His comment was left unanswered though, just as a reminder:

> just want to comment that this is now the opposite behaviour
> of pmg <= 4 where setting authmode to ldap would not
> change the template, but not accept quarantine tickets anymore
> (which we should *probably* also do, since there may be
> some valid tickets around; but this can be a separate patch) 

(no hard feelings regarding this from my side, as said, just a reminder
to ensure it did not get lost, or was discussed off-list and the result of
that did not made it to the list or the like)