Re: [PoC] Federated Authn/z with OAUTHBEARER

> On 20 Dec 2024, at 02:00, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> wrote:

> v40 also contains:

A few small comments on v40 rather than saving up for a longer email:

+ ereport(LOG, errmsg("Internal error in OAuth validator module"));
Tiny nitpick, the errmsg() should start with lowercase 'i'.

+ libpq_append_conn_error(conn, "no custom OAuth flows are available, and libpq was not built using --with-libcurl");
Since we at some point will move away from autoconf I think we should avoid
such implementation details in error messages. How about "was not built with
libcurl support"?

+ * Find the start of the .well-known prefix. IETF rules state this must be
+ * at the beginning of the path component, but OIDC defined it at the end
+ * instead, so we have to search for it anywhere.
I was looking for a reference for OIDC defining the WK prefix placement but I
could only find it deferring to RFC5785 like how RFC8414 does. Can you inject
a document reference for this?

+ if (strcmp(conn->oauth_issuer_id, discovery_issuer) != 0)
Shouldn't the scheme component really be compared case-insensitive, or has it
been normalized at this point? Not sure how much it matters in practice but if
not perhaps we should add a TODO marker there?

Support for oauth seems to be missing from pg_hba_file_rules() which should be
added in hbafuncs.c:get_hba_options().

--
Daniel Gustafsson