Re: [PoC] Federated Authn/z with OAUTHBEARER

On 08/06/2021 19:37, Jacob Champion wrote:
> We've been working on ways to expand the list of third-party auth
> methods that Postgres provides. Some example use cases might be "I want
> to let anyone with a Google account read this table" or "let anyone who
> belongs to this GitHub organization connect as a superuser".

Cool!

> The iddawc dependency for client-side OAuth was extremely helpful to
> develop this proof of concept quickly, but I don't think it would be an
> appropriate component to build a real feature on. It's extremely
> heavyweight -- it incorporates a huge stack of dependencies, including
> a logging framework and a web server, to implement features we would
> probably never use -- and it's fairly difficult to debug in practice.
> If a device authorization flow were the only thing that libpq needed to
> support natively, I think we should just depend on a widely used HTTP
> client, like libcurl or neon, and implement the minimum spec directly
> against the existing test suite.

You could punt and let the application implement that stuff. I'm
imagining that the application code would look something like this:

conn = PQconnectStartParams(...);
for (;;)
{
status = PQconnectPoll(conn)
switch (status)
{
case CONNECTION_SASL_TOKEN_REQUIRED:
/* open a browser for the user, get token */
token = open_browser()
PQauthResponse(token);
break;
...
}
}

It would be nice to have a simple default implementation, though, for
psql and all the other client applications that come with PostgreSQL itself.

> If you've read this far, thank you for your interest, and I hope you
> enjoy playing with it!

A few small things caught my eye in the backend oauth_exchange function:

> + /* Handle the client's initial message. */
> + p = strdup(input);

this strdup() should be pstrdup().

In the same function, there are a bunch of reports like this:

> ereport(ERROR,
> + (errcode(ERRCODE_PROTOCOL_VIOLATION),
> + errmsg("malformed OAUTHBEARER message"),
> + errdetail("Comma expected, but found character \"%s\".",
> + sanitize_char(*p))));

I don't think the double quotes are needed here, because sanitize_char
will return quotes if it's a single character. So it would end up
looking like this: ... found character "'x'".

- Heikki