Re: [oauth] Add TLS support to OAuth tests
| От | Jonathan Gonzalez V. |
|---|---|
| Тема | Re: [oauth] Add TLS support to OAuth tests |
| Дата | |
| Msg-id | 2bd4fd90e49175f83d764b09b733905617edb88d.camel@gmail.com обсуждение исходный текст |
| Ответ на | Re: [oauth] Add TLS support to OAuth tests (Jacob Champion <jacob.champion@enterprisedb.com>) |
| Список | pgsql-hackers |
Hello! > Yep, we need to support back to Python 3.6.8 (which I've tested your > patch with locally). That's indeed good to know, there's a way to know which is the oldest version of Python used? What do you think about adding this information on top of the `oauth_server.py` file? Just as a reminder because I suspected that the lower version will be 3.9 not 3.6 > Comments: > > > Signed-off-by: Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> > > Note that we have no signoff convention here, at least that I'm aware > of. It's not a problem to include it, but this will be replaced by an > `Author:` line in the final commit. Submissions here fall under the > Archive Policy: > > https://www.postgresql.org/about/policies/archives/ Yes, I'm fully aware of this, it's just my default git configuration to add the sign-off, I'll check to remove that, thanks for the point! > > > +my $certdir = dirname(__FILE__) . "/../../../ssl/ssl"; > > +$ENV{PGOAUTHCAFILE} = "$certdir/root+server_ca.crt"; > > I think we should inject this directory from the build system, > instead > of walking across the tree. I wasn't aware of this, will keep in mind for hte future > > > - my $issuer = "http://127.0.0.1:$port"; > > + my $issuer = "https://localhost:$port"; > > Unfortunately this will cause strange bugs [1]: we aren't listening > on > IPv6, but Curl will attempt to contact a DNS hostname on both IPv4 > and > IPv6 simultaneously, leading to intermittent failures. So we'll need > to get the certificate's SANs working... I wasn't aware of this, good learning! I'll keep it in mind for the future, and probably we can work on IPv6 support later. > > server-ip-in-dnsname \ > > + server-ip-localhost \ > > server-single-alt-name \ > > nitpick: Though the naming scheme is not really all that well > defined, > I think this probably belongs with the other -alt-name certs rather > than the -ip- certs. I had my questions if add it to the alt-name too, and now I'm thinking in other test ideas due to this, like using multiple alternative names for the host and issuer, but for sure, for the future not here! > -- > > To avoid making you play fetch-a-rock, I've attached these comments > in > code form as v1.1-0002 (but you're under no obligation to take them > if > you'd prefer to do it a different way :D). I also ran oauth_server.py > through Black. I did the same! just added the comments I mentioned, besides that, all good on my side. Thank you! --- Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> EnterpriseDB
Вложения
В списке pgsql-hackers по дате отправления: