Horton

Re: [PoC] Federated Authn/z with OAUTHBEARER

Jacob Champion <jchampion@timescale.com>

View thread
From: Jacob Champion <jchampion@timescale.com>
To: Andrey Chudnovsky <achudnovskij@gmail.com>
Cc: mahendrakar s <mahendrakarforpg@gmail.com>, "hlinnaka@iki.fi" <hlinnaka@iki.fi>, "michael@paquier.xyz" <michael@paquier.xyz>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "smilingsamay@gmail.com" <smilingsamay@gmail.com>
Date: 2022-12-09T00:41:21Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. meson: Fix install-quiet after clean

  2. oauth: Run Autoconf tests with correct compiler flags

  3. Link libpq with libdl if the platform needs that.

  4. Doc: correct spelling of meson switch.

  5. oauth: Correct SSL dependency for libpq-oauth.a

  6. oauth: Fix Autoconf build on macOS

  7. oauth: Move the builtin flow into a separate module

  8. Remove a stray "pgrminclude" annotation

  9. oauth: Simplify copy of PGoauthBearerRequest

  10. oauth: Improve validator docs on interruptibility

  11. oauth: Disallow synchronous DNS in libcurl

  12. oauth: Fix postcondition for set_timer on macOS

  13. oauth: Use IPv4-only issuer in oauth_validator tests

  14. Work around OAuth/EVFILT_TIMER quirk on NetBSD.

  15. oauth: Fix incorrect const markers in struct

  16. Add missing entry to oauth_validator test .gitignore

  17. cirrus: Temporarily fix libcurl link error

  18. Add support for OAUTHBEARER SASL mechanism

  19. libpq: Handle asynchronous actions during SASL

  20. require_auth: prepare for multiple SASL mechanisms

  21. Move PG_MAX_AUTH_TOKEN_LENGTH to libpq/auth.h

  22. Make SASL max message length configurable

  23. jsonapi: fully initialize dummy lexer

  24. common/jsonapi: support libpq as a client

  25. Remove fe_memutils from libpgcommon_shlib

  26. Revert ECPG's use of pnstrdup()

  27. Explicitly require password for SCRAM exchange

  28. Refactor SASL exchange to return tri-state status

On Wed, Dec 7, 2022 at 3:22 PM Andrey Chudnovsky
<achudnovskij@gmail.com> wrote:
> 
>> I think it's okay to have the extension and HBA collaborate to
>> provide discovery information. Your proposal goes further than
>> that, though, and makes the server aware of the chosen client flow.
>> That appears to be an architectural violation: why does an OAuth
>> resource server need to know the client flow at all?
> 
> Ok. It may have left there from intermediate iterations. We did 
> consider making extension drive the flow for specific grant_type,
> but decided against that idea. For the same reason you point to. Is
> it correct that your main concern about use of grant_type was that 
> it's propagated to the server? Then yes, we will remove sending it
> to the server.

Okay. Yes, that was my primary concern.

>> Ideally, yes, but that only works if all identity providers
>> implement the same flows in compatible ways. We're already seeing
>> instances where that's not the case and we'll necessarily have to
>> deal with that up front.
> 
> Yes, based on our analysis OIDC spec is detailed enough, that 
> providers implementing that one, can be supported with generic code
> in libpq / client. Github specifically won't fit there though.
> Microsoft Azure AD, Google, Okta (including Auth0) will. 
> Theoretically discovery documents can be returned from the extension 
> (server-side) which is provider specific. Though we didn't plan to 
> prioritize that.

As another example, Google's device authorization grant is incompatible
with the spec (which they co-authored). I want to say I had problems
with Azure AD not following that spec either, but I don't remember
exactly what they were. I wouldn't be surprised to find more tiny
departures once we get deeper into implementation.

>> That seems to be restating the goal of OAuth and OIDC. Can you
>> explain how the incompatible change allows you to accomplish this
>> better than standard implementations?
> 
> Do you refer to passing grant_type to the server? Which we will get 
> rid of in the next iteration. Or other incompatible changes as well?

Just the grant type, yeah.

>> Why? I claim that standard OAUTHBEARER can handle all of that.
>> What does your proposed architecture (the third diagram) enable
>> that my proposed hook (the second diagram) doesn't?
> 
> The hook proposed on the 2nd diagram effectively delegates all Oauth 
> flows implementations to the client. We propose libpq takes care of
> pulling OpenId discovery and coordination. Which is effectively
> Diagram 1 + more flows + server hook providing root url/audience.
> 
> Created the diagrams with all components for 3 flows: [snip]

(I'll skip ahead to your later mail on this.)

>> (For a future conversation: they need to set up authorization,
>> too, with custom scopes or some other magic. It's not enough to
>> check who the token belongs to; even if Postgres is just using the
>> verified email from OpenID as an authenticator, you have to also
>> know that the user authorized the token -- and therefore the client
>> -- to access Postgres on their behalf.)
> 
> My understanding is that metadata in the tokens is provider
> specific, so server side hook would be the right place to handle
> that. Plus I can envision for some providers it can make sense to
> make a remote call to pull some information.

The server hook is the right place to check the scopes, yes, but I think
the DBA should be able to specify what those scopes are to begin with.
The provider of the extension shouldn't be expected by the architecture
to hardcode those decisions, even if Azure AD chooses to short-circuit
that choice and provide magic instead.

On 12/7/22 20:25, Andrey Chudnovsky wrote:
> That being said, the Diagram 2 would look like this with our proposal:
> [snip]
> 
> With the application taking care of all Token acquisition logic. While
> the server-side hook is participating in the pre-authentication reply.
> 
> That is definitely a required scenario for the long term and the
> easiest to implement in the client core.> And if we can do at least that flow in PG16 it will be a strong
> foundation to provide more support for specific grants in libpq going
> forward.

Agreed.
> Does the diagram above look good to you? We can then start cleaning up
> the patch to get that in first.

I maintain that the hook doesn't need to hand back artifacts to the
client for a second PQconnect call. It can just use those artifacts to
obtain the access token and hand that right back to libpq. (I think any
requirement that clients be rewritten to call PQconnect twice will
probably be a sticking point for adoption of an OAuth patch.)

That said, now that your proposal is also compatible with OAUTHBEARER, I
can pony up some code to hopefully prove my point. (I don't know if I'll
be able to do that by the holidays though.)

Thanks!
--Jacob