[pve-devel] [PATCH access-control] fix #3513: pass configured proxy to OpenID

[Fabian Grünbichler]

On August 25, 2021 6:31 pm, Thomas Lamprecht wrote:
> On 13/07/2021 10:09, Fabian Grünbichler wrote:
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
>> ---
>> seemed like the easiest way to fix this - but we could also change the
>> proxmox-openid-rs API to take the proxy as parameter..
>> 
> 
> seems OK in general, but do we only want to set it in case it actually has
> a value? Not sure if non-existing and existing but empty makes any difference
> here - e.g., a behavior that one could possibly imagine is that it would override
> another source/default for a proxy to force no-proxy...
> 
> Mostly just asking if you thought about that, it's probably just a very vague and
> theoretical issue..

Yeah I think this would be rather theoretical, as our API services run 
without that env variable set by default, so a user would have to 
edit/override the systemd unit to set the env variable instead of using 
our built-in mechanism..

but alas,

local %ENV;
if (my $http_proxy = $dcconf->{http_proxy}) {
  $ENV{all_proxy} = $http_proxy;
}

should work just as well but still preserve a fallback to pre-existing 
env variable

> 
>>  src/PVE/API2/OpenId.pm | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
>> index 22423ba..9080865 100644
>> --- a/src/PVE/API2/OpenId.pm
>> +++ b/src/PVE/API2/OpenId.pm
>> @@ -97,6 +97,9 @@ __PACKAGE__->register_method ({
>>      code => sub {
>>  	my ($param) = @_;
>>  
>> +	my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
>> +	local $ENV{all_proxy} = $dcconf->{http_proxy};
>> +
>>  	my $realm = extract_param($param, 'realm');
>>  	my $redirect_url = extract_param($param, 'redirect-url');
>>  
>> @@ -149,6 +152,9 @@ __PACKAGE__->register_method ({
>>  
>>  	my $res;
>>  	eval {
>> +	    my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
>> +	    local $ENV{all_proxy} = $dcconf->{http_proxy};
>> +
>>  	    my ($realm, $private_auth_state) = PVE::RS::OpenId::verify_public_auth_state(
>>  		$openid_state_path, $param->{'state'});
>>  
>> 
> 
>