Horton

Thread

Commits

  1. oauth: Add TLS support for oauth_validator tests

  1. [oauth] Add TLS support to OAuth tests

    Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> — 2026-02-27T19:15:59Z

    Hi!!

After some discussion about PGOAUTHCAFILE[1] variable, the need of
having the TLS support for the tests added in a different patch[2].

I'm attaching a patch that add this TLS support making use of the
already certs system in the `src/test/ssl/` directory and just making
the `oauth_server.py` script able to support TLS only, this removes the
plain HTTP support from the server.

My Python skills are old, but I tried to keep the modifications as
simple as possible, even that there's an easy way to have the context
in Python 3.14, I decided to just have a context because the function
HTTPSServer() is only available from Python 3.14 and above, which is
not so widely used yet, in the future for sure will be more simple to
use that function.


[1]
https://www.postgresql.org/message-id/flat/16a91d02795cb991963326a902afa764e4d721db.camel%40gmail.com
[2]
https://www.postgresql.org/message-id/flat/9850AB21-78A7-43CD-94C6-FA8E3BC9F1B3%40yesql.se#11e8d9febb3c3b428caa383f0c3f6acf
-- 
Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>

  2. Re: [oauth] Add TLS support to OAuth tests

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-02-28T00:18:21Z

    On Fri, Feb 27, 2026 at 11:17 AM Jonathan Gonzalez V.
<jonathan.abdiel@gmail.com> wrote:
> I'm attaching a patch that add this TLS support making use of the
> already certs system in the `src/test/ssl/` directory and just making
> the `oauth_server.py` script able to support TLS only, this removes the
> plain HTTP support from the server.

Thank you!

> My Python skills are old, but I tried to keep the modifications as
> simple as possible, even that there's an easy way to have the context
> in Python 3.14, I decided to just have a context because the function
> HTTPSServer() is only available from Python 3.14 and above, which is
> not so widely used yet, in the future for sure will be more simple to
> use that function.

Yep, we need to support back to Python 3.6.8 (which I've tested your
patch with locally).

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/

> +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.

> -  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...

> +[ req ]
> +distinguished_name     = req_distinguished_name
> +prompt                 = no
> +
> +[ req_distinguished_name ]
> +CN = localhost
> +OU = PostgreSQL test suite
> +
> +# For Subject Alternative Names
> +[ v3_req ]
> +subjectAltName = @alt_names

...which will require the v3_req section to be enabled here. (We can
move the CommonName into SANs as well, since Curl is a modern web
client.)

> -# To test against HTTP rather than HTTPS, we need to enable PGOAUTHDEBUG. But
> -# first, check to make sure the client refuses such connections by default.
> -$node->connect_fails(

I think we should keep this test; it's an important safeguard. This is
also a good chance to test the case where certificate verification
fails.

>     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.

--

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.

Thanks!
--Jacob

[1] https://postgr.es/m/CAOYmi%2Bn4EDOOUL27_OqYT2-F2rS6S%2B3mK-ppWb2Ec92UEoUbYA%40mail.gmail.com

  3. Re: [oauth] Add TLS support to OAuth tests

    Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> — 2026-02-28T05:09:43Z

    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

  4. Re: [oauth] Add TLS support to OAuth tests

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-03-02T19:31:31Z

    On Fri, Feb 27, 2026 at 9:09 PM Jonathan Gonzalez V.
<jonathan.abdiel@gmail.com> wrote:
> That's indeed good to know, there's a way to know which is the oldest
> version of Python used?

We have a partial list of minimum dependencies here:

    https://wiki.postgresql.org/wiki/Minimum_Dependency_Versions

> What do you think about adding this information
> on top of the `oauth_server.py` file?

I think if we put it into the repo somewhere (and honestly, we
probably should), it should be centralized rather than per-file. It'll
rot really quickly otherwise.

So I've pulled in part of your -0003, done some more comment-smithing,
and squashed it all down for commit. Attached in v2.

Thanks!
--Jacob

  5. Re: [oauth] Add TLS support to OAuth tests

    Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> — 2026-03-03T11:15:03Z

    Hi!
> > What do you think about adding this information
> > on top of the `oauth_server.py` file?
> 
> I think if we put it into the repo somewhere (and honestly, we
> probably should), it should be centralized rather than per-file.
> It'll
> rot really quickly otherwise.

Yeah, you're right, I couldn't find anything on the repo with the name
of tools or dependencies required or anything like that that contain
the versions, probably for another discussion indeed!

> So I've pulled in part of your -0003, done some more comment-
> smithing,
> and squashed it all down for commit. Attached in v2.

Yeah I saw! nice on putting the `cert_dir` outside in the `Makefile`
and `meson.build` file!

Thanks for everything! I'll wait for this to be committed to come with
a new version of the other patch [1]

[1]
https://www.postgresql.org/message-id/flat/16a91d02795cb991963326a902afa764e4d721db.camel@gmail.com
-- 
Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>
EnterpriseDB

  6. Re: [oauth] Add TLS support to OAuth tests

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-03-06T01:04:31Z

    On Tue, Mar 3, 2026 at 3:15 AM Jonathan Gonzalez V.
<jonathan.abdiel@gmail.com> wrote:
> Thanks for everything! I'll wait for this to be committed to come with
> a new version of the other patch [1]

Committed, thanks!

--Jacob