On 11/23/22 01:58, mahendrakar s wrote: > We validated on libpq handling OAuth natively with different flows > with different OIDC certified providers. > > Flows: Device Code, Client Credentials and Refresh Token. > Providers: Microsoft, Google and Okta.
Great, thank you! > Also validated with OAuth provider Github. (How did you get discovery working? I tried this and had to give up eventually.) > We propose using OpenID Connect (OIDC) as the protocol, instead of > OAuth, as it is: > - Discovery mechanism to bridge the differences and provide metadata. > - Stricter protocol and certification process to reliably identify > which providers can be supported. > - OIDC is designed for authentication, while the main purpose of OAUTH is to > authorize applications on behalf of the user. How does this differ from the previous proposal? The OAUTHBEARER SASL mechanism already relies on OIDC for discovery. (I think that decision is confusing from an architectural and naming standpoint, but I don't think they really had an alternative...) > Github is not OIDC certified, so won’t be supported with this proposal. > However, it may be supported in the future through the ability for the > extension to provide custom discovery document content. Right. > OpenID configuration has a well-known discovery mechanism > for the provider configuration URI which is > defined in OpenID Connect. It allows libpq to fetch > metadata about provider (i.e endpoints, supported grants, response types, > etc). Sure, but this is already how the original PoC works. The test suite implements an OIDC provider, for instance. Is there something different to this that I'm missing? > In the attached patch (based on V2 patch in the thread and does not > contain Samay's changes): > - Provider can configure issuer url and scope through the options hook.) > - Server passes on an open discovery url and scope to libpq. > - Libpq handles OAuth flow based on the flow_type sent in the > connection string [1]. > - Added callbacks to notify a structure to client tools if OAuth flow > requires user interaction. > - Pg backend uses hooks to validate bearer token. Thank you for the sample! > Note that authentication code flow with PKCE for GUI clients is not > implemented yet. > > Proposed next steps: > - Broaden discussion to reach agreement on the approach. High-level thoughts on this particular patch (I assume you're not looking for low-level implementation comments yet): 0) The original hook proposal upthread, I thought, was about allowing libpq's flow implementation to be switched out by the application. I don't see that approach taken here. It's fine if that turned out to be a bad idea, of course, but this patch doesn't seem to match what we were talking about. 1) I'm really concerned about the sudden explosion of flows. We went from one flow (Device Authorization) to six. It's going to be hard enough to validate that *one* flow is useful and can be securely deployed by end users; I don't think we're going to be able to maintain six, especially in combination with my statement that iddawc is not an appropriate dependency for us. I'd much rather give applications the ability to use their own OAuth code, and then maintain within libpq only the flows that are broadly useful. This ties back to (0) above. 2) Breaking the refresh token into its own pseudoflow is, I think, passing the buck onto the user for something that's incredibly security sensitive. The refresh token is powerful; I don't really want it to be printed anywhere, let alone copy-pasted by the user. Imagine the phishing opportunities. If we want to support refresh tokens, I believe we should be developing a plan to cache and secure them within the client. They should be used as an accelerator for other flows, not as their own flow. 3) I don't like the departure from the OAUTHBEARER mechanism that's presented here. For one, since I can't see a sample plugin that makes use of the "flow type" magic numbers that have been added, I don't really understand why the extension to the mechanism is necessary. For two, if we think OAUTHBEARER is insufficient, the people who wrote it would probably like to hear about it. Claiming support for a spec, and then implementing an extension without review from the people who wrote the spec, is not something I'm personally interested in doing. 4) The test suite is still broken, so it's difficult to see these things in practice for review purposes. > - Implement libpq changes without iddawc This in particular will be much easier with a functioning test suite, and with a smaller number of flows. > - Prototype GUI flow with pgAdmin Cool! Thanks, --Jacob
