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