* [pve-devel] [PATCH access-control] fix #3513: pass configured proxy to OpenID
@ 2021-07-13 8:09 Fabian Grünbichler
2021-08-25 16:31 ` Thomas Lamprecht
2021-11-03 10:32 ` [pve-devel] applied: " Thomas Lamprecht
0 siblings, 2 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2021-07-13 8:09 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@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..
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'});
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH access-control] fix #3513: pass configured proxy to OpenID
2021-07-13 8:09 [pve-devel] [PATCH access-control] fix #3513: pass configured proxy to OpenID Fabian Grünbichler
@ 2021-08-25 16:31 ` Thomas Lamprecht
2021-09-06 8:52 ` Fabian Grünbichler
2021-11-03 10:32 ` [pve-devel] applied: " Thomas Lamprecht
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2021-08-25 16:31 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 13/07/2021 10:09, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@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..
> 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'});
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH access-control] fix #3513: pass configured proxy to OpenID
2021-08-25 16:31 ` Thomas Lamprecht
@ 2021-09-06 8:52 ` Fabian Grünbichler
0 siblings, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2021-09-06 8:52 UTC (permalink / raw)
To: Proxmox VE development discussion, Thomas Lamprecht
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@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'});
>>
>>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] applied: [PATCH access-control] fix #3513: pass configured proxy to OpenID
2021-07-13 8:09 [pve-devel] [PATCH access-control] fix #3513: pass configured proxy to OpenID Fabian Grünbichler
2021-08-25 16:31 ` Thomas Lamprecht
@ 2021-11-03 10:32 ` Thomas Lamprecht
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2021-11-03 10:32 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 13.07.21 10:09, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@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..
>
> src/PVE/API2/OpenId.pm | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>
applied, thanks! just to be sure I made a followup to only change the env if the config
variable actually exists:
--8<--
commit afda4f1a83a5c0bbd798facd27c0cb74759968ff
Author: Thomas Lamprecht <t.lamprecht@proxmox.com>
Date: Wed Nov 3 11:30:05 2021 +0100
openid: proxy: only set env var if DC-config property exists
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index 9080865..4fc0be8 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -98,7 +98,7 @@ __PACKAGE__->register_method ({
my ($param) = @_;
my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
- local $ENV{all_proxy} = $dcconf->{http_proxy};
+ local $ENV{all_proxy} = $dcconf->{http_proxy} if exists $dcconf->{http_proxy};
my $realm = extract_param($param, 'realm');
my $redirect_url = extract_param($param, 'redirect-url');
@@ -153,7 +153,7 @@ __PACKAGE__->register_method ({
my $res;
eval {
my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
- local $ENV{all_proxy} = $dcconf->{http_proxy};
+ local $ENV{all_proxy} = $dcconf->{http_proxy} if exists $dcconf->{http_proxy};
my ($realm, $private_auth_state) = PVE::RS::OpenId::verify_public_auth_state(
$openid_state_path, $param->{'state'});
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-03 10:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 8:09 [pve-devel] [PATCH access-control] fix #3513: pass configured proxy to OpenID Fabian Grünbichler
2021-08-25 16:31 ` Thomas Lamprecht
2021-09-06 8:52 ` Fabian Grünbichler
2021-11-03 10:32 ` [pve-devel] applied: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox