On October 17, 2019 5:33 pm, Thomas Lamprecht wrote:
> On 10/17/19 3:14 PM, Fabian Grünbichler wrote:
>> based on idea & RFC by Tim Marx, incorporating feedback by Thomas
>> Lamprecht. this will be extended to support API tokens in the
>> Authorization header as well, so make it generic.
>>
>> Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
>> ---
>>
>> Notes:
>> semi-independent, could also leave extract_auth_cookie as alias/wrapper
>> to
>> avoid a change in PMG. but since we need to change other method
>> signatures
>> anyway for the token part, we could change this as well.
>>
>> as-is, needs a versioned breaks/depends on pve-manager and some part of
>> PMG?
>>
>> PVE/APIServer/AnyEvent.pm | 5 ++++-
>> PVE/APIServer/Formatter.pm | 12 ++++++------
>> 2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/PVE/APIServer/AnyEvent.pm b/PVE/APIServer/AnyEvent.pm
>> index 9aba27d..c0b90ab 100644
>> --- a/PVE/APIServer/AnyEvent.pm
>> +++ b/PVE/APIServer/AnyEvent.pm
>> @@ -1232,7 +1232,10 @@ sub unshift_read_header {
>> } elsif ($path =~ m/^\Q$base_uri\E/) {
>> my $token = $r->header('CSRFPreventionToken');
>> my $cookie = $r->header('Cookie');
>> - my $ticket =
>> PVE::APIServer::Formatter::extract_auth_cookie($cookie,
>> $self->{cookie_name});
>> + my $auth_header = $r->header('Authorization');
>> + my $ticket =
>> PVE::APIServer::Formatter::extract_auth_value($cookie, $self->{cookie_name});
>> + $ticket =
>> PVE::APIServer::Formatter::extract_auth_value($auth_header,
>> $self->{cookie_name})
>> + if !$ticket;
>
> can we do this a bit more separate like:
>
> if (!$ticket && (my $auth_header = $r->header('Authorization')) {
> $ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{cookie_name});
> }
this would then (with the next patch) become:
my $api_token;
if (!$ticket && (my $auth_header = $r->header('Authorization')) {
$ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
$self->{cookie_name});
if (!$ticket) {
$api_token =
PVE::APIServer::Formatter::extract_auth_value($auth_header,
$self->{apitoken_name});
}
}
>
> or if you prefer:
>
> if (!$ticket) {
> my $auth_header = $r->header('Authorization');
> $ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{cookie_name});
> }
my $api_token;
if (!$ticket) {
my $auth_header = $r->header('Authorization');
$ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
$self->{cookie_name});
if (!$ticket) {
$api_token =
PVE::APIServer::Formatter::extract_auth_value($auth_header,
$self->{apitoken_name});
}
}
>
> makes it slightly cleaner/easier to read, IMO
which makes it harder to parse IMHO, but if you still prefer it I will
rewrite it with your suggestion for v2? we could of course also add
comments, like so:
# check actual cookie
my $ticket = PVE::APIServer::Formatter::extract_auth_value($cookie,
$self->{cookie_name});
# fallback to cookie in 'Authorization' header
$ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
$self->{cookie_name})
if !$ticket;
# fallback to API token
my $api_token = PVE::APIServer::Formatter::extract_auth_value($auth_header,
$self->{apitoken_name})
if !$ticket;
>
>>
>> my ($rel_uri, $format) = &$split_abs_uri($path,
>> $self->{base_uri});
>> if (!$format) {
>> diff --git a/PVE/APIServer/Formatter.pm b/PVE/APIServer/Formatter.pm
>> index 0c459bd..def1932 100644
>> --- a/PVE/APIServer/Formatter.pm
>> +++ b/PVE/APIServer/Formatter.pm
>> @@ -75,16 +75,16 @@ sub get_login_formatter {
>>
>> # some helper functions
>>
>> -sub extract_auth_cookie {
>> - my ($cookie, $cookie_name) = @_;
>> +sub extract_auth_value {
>> + my ($header, $key) = @_;
>>
>> - return undef if !$cookie;
>> + return undef if !$header;
>>
>> - my $ticket = ($cookie =~ /(?:^|\s)\Q$cookie_name\E=([^;]*)/)[0];
>> + my $value = ($header =~ /(?:^|\s)\Q$key\E(?:=| )([^;]*)/)[0];
>>
>> - $ticket = uri_unescape($ticket) if $ticket;
>> + $value = uri_unescape($value) if $value;
>>
>> - return $ticket;
>> + return $value;
>> }
>>
>> sub create_auth_cookie {
>>
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
