Re: [PoC] Federated Authn/z with OAUTHBEARER - Mailing list pgsql-hackers

From Andrey Chudnovsky
Subject Re: [PoC] Federated Authn/z with OAUTHBEARER
Date
Msg-id CACrwV56MTdboeBKVU-b-M2oHLttRxoVm4tJmwVOK2cydL+Spdg@mail.gmail.com
Whole thread Raw
In response to Re: [PoC] Federated Authn/z with OAUTHBEARER  (Jacob Champion <jchampion@timescale.com>)
Responses Re: [PoC] Federated Authn/z with OAUTHBEARER
List pgsql-hackers
Jacob,
Thanks for your feedback.
I think we can focus on the roles and responsibilities of the components first.
Details of the patch can be elaborated. Like "flow type code" is a
mistake on our side, and we will use the term "grant_type" which is
defined by OIDC spec. As well as details of usage of refresh_token.

> Rather than decouple things, I think this proposal drives a spike
> through the client app, libpq, and the server. Please correct me if I've
> misunderstood pieces of the patch, but the following is my view of it:

> What used to be a validator hook on the server side now actively
> participates in the client-side flow for some reason. (I still don't
> understand what the server is supposed to do with that knowledge.
> Changing your authz requirements based on the flow the client wants to
> use seems like a good way to introduce bugs.)

> The client-side hook is now coupled to the application logic: you have
> to know to expect an error from the first PQconnect*() call, then check
> whatever magic your hook has done for you to be able to set up the
> second call to PQconnect*() with the correctly scoped bearer token. So
> if you want to switch between the internal libpq OAuth implementation
> and your own hook, you have to rewrite your app logic.

Basically Yes. We propose an increase of the server side hook responsibility.
From just validating the token, to also return the provider root URL
and required audience. And possibly provide more metadata in the
future.
Which is in our opinion aligned with SASL protocol, where the server
side is responsible for telling the client auth requirements based on
the requested role in the startup packet.

Our understanding is that in the original patch that information came
purely from hba, and we propose extension being able to control that
metadata.
As we see extension as being owned by the identity provider, compared
to HBA which is owned by the server administrator or cloud provider.

This change of the roles is based on the vision of 4 independent actor
types in the ecosystem:
1. Identity Providers (Okta, Google, Microsoft, other OIDC providers).
   - Publish open source extensions for PostgreSQL.
   - Don't have to own the server deployments, and must ensure their
extensions can work in any environment. This is where we think
additional hook responsibility helps.
2. Server Owners / PAAS providers (On premise admins, Cloud providers,
multi-cloud PAAS providers).
   - Install extensions and configure HBA to allow clients to
authenticate with the identity providers of their choice.
3. Client Application Developers (Data Wis, integration tools,
PgAdmin, monitoring tools, e.t.c.)
   - Independent from specific Identity providers or server providers.
Write one code for all identity providers.
   - Rely on application deployment owners to configure which OIDC
provider to use across client and server setups.
4. Application Deployment Owners (End customers setting up applications)
   - The only actor actually aware of which identity provider to use.
Configures the stack based on the Identity and PostgreSQL deployments
they have.

The critical piece of the vision is (3.) above is applications
agnostic of the identity providers. Those applications rely on
properly configured servers and rich driver logic (libpq,
com.postgresql, npgsql) to allow their application to popup auth
windows or do service-to-service authentication with any provider. In
our view that would significantly democratize the deployment of OAUTH
authentication in the community.

In order to allow this separation, we propose:
1. HBA + Extension is the single source of truth of Provider root URL
+ Required Audience for each role. If some backfill for missing OIDC
discovery is needed, the provider-specific extension would be
providing it.
2. Client Application knows which grant_type to use in which scenario.
But can be coded without knowledge of a specific provider. So can't
provide discovery details.
3. Driver (libpq, others) - coordinate the authentication flow based
on client grant_type and identity provider metadata to allow client
applications to use any flow with any provider in a unified way.

Yes, this would require a little more complicated flow between
components than in your original patch. And yes, more complexity comes
with more opportunity to make bugs.
However, I see PG Server and Libpq as the places which can have more
complexity. For the purpose of making work for the community
participants easier and simplify adoption.

Does this make sense to you?


On Tue, Nov 29, 2022 at 1:20 PM Jacob Champion <jchampion@timescale.com> wrote:
>
> On 11/24/22 00:20, mahendrakar s wrote:
> > I had validated Github by skipping the discovery mechanism and letting
> > the provider extension pass on the endpoints. This is just for
> > validation purposes.
> > If it needs to be supported, then need a way to send the discovery
> > document from extension.
>
> Yeah. I had originally bounced around the idea that we could send a
> data:// URL, but I think that opens up problems.
>
> You're supposed to be able to link the issuer URI with the URI you got
> the configuration from, and if they're different, you bail out. If a
> server makes up its own OpenID configuration, we'd have to bypass that
> safety check, and decide what the risks and mitigations are... Not sure
> it's worth it.
>
> Especially if you could just lobby GitHub to, say, provide an OpenID
> config. (Maybe there's a security-related reason they don't.)
>
> --Jacob



pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: predefined role(s) for VACUUM and ANALYZE
Next
From: Ian Lawrence Barwick
Date:
Subject: Re: pg_basebackup: add test about zstd compress option