Re: Proposal: Support custom authentication methods using hooks - Mailing list pgsql-hackers
| From | samay sharma |
|---|---|
| Subject | Re: Proposal: Support custom authentication methods using hooks |
| Date | |
| Msg-id | CAJxrbyzEaYm_r0jO6pknxeKCau0jm-KqJsRL5ogfQ8hpgydDWw@mail.gmail.com Whole thread Raw |
| In response to | Re: Proposal: Support custom authentication methods using hooks (Stephen Frost <sfrost@snowman.net>) |
| Responses |
Re: Proposal: Support custom authentication methods using hooks
(Stephen Frost <sfrost@snowman.net>)
Re: Proposal: Support custom authentication methods using hooks (Peter Eisentraut <peter.eisentraut@enterprisedb.com>) |
| List | pgsql-hackers |
Hi,
On Wed, Mar 16, 2022 at 8:02 AM Stephen Frost <sfrost@snowman.net> wrote:
Greetings,
* samay sharma (smilingsamay@gmail.com) wrote:
> On Fri, Mar 4, 2022 at 11:15 AM Jacob Champion <pchampion@vmware.com> wrote:
> > On Thu, 2022-03-03 at 11:12 +0100, Peter Eisentraut wrote:
> > > At the moment, it is not possible to judge whether the hook interface
> > > you have chosen is appropriate.
> > >
> > > I suggest you actually implement the Azure provider, then make the hook
> > > interface, and then show us both and we can see what to do with it.
> >
> > To add a data point here, I've rebased my OAUTHBEARER experiment [1] on
> > top of this patchset. (That should work with Azure's OIDC provider, and
> > if it doesn't, I'd like to know why.)
>
> Firstly, thanks for doing this. It helps to have another data point and the
> feedback you provided is very valuable. I've looked to address it with the
> patchset attached to this email.
>
> This patch-set adds the following:
>
> * Allow multiple custom auth providers to be registered (Addressing
> feedback from Aleksander and Andrew)
> * Modify the test extension to use SCRAM to exchange secrets (Based on
> Andres's suggestion)
> * Add support for custom auth options to configure provider's behavior (by
> exposing a new hook) (Required by OAUTHBEARER)
> * Allow custom auth methods to use usermaps. (Required by OAUTHBEARER)
That we're having to extend this quite a bit to work for the proposed
OAUTH patches and that it still doesn't do anything for the client side
(note that the OAUTHBEARER patches are still patching libpq to add
support directly and surely will still be even with this latest patch
set ...) makes me seriously doubt that this is the right direction to be
going in.
I think I should have clarified this in my previous email. There is nothing specific to OAUTHBEARER in these changes. Having auth options and using usermaps is something which any auth method could require. I had not added them yet, but I'm pretty sure this would have been requested.
> > After the port, here are the changes I still needed to carry in the
> > backend to get the tests passing:
> >
> > - I needed to add custom HBA options to configure the provider.
>
> Could you try to rebase your patch to use the options hook and let me know
> if it satisfies your requirements?
>
> Please let me know if there's any other feedback.
How about- if we just added OAUTH support directly into libpq and the
backend, would that work with Azure's OIDC provider? If not, why not?
Overall, Azure AD implements OIDC, so this could be doable. However, we'll have to explore further to see if the current implementation would satisfy all the requirements.
If it does, then what's the justification for trying to allow custom
backend-only authentication methods?
The only goal of this patch wasn't to enable support for Azure AD. That's just one client. Users might have a need to add or change auth methods in the future and providing that extensibility so we don't need to have core changes for each one of them would be useful. I know there isn't alignment on this yet, but if we'd like to move certain auth methods out of core into extensions, then this might provide a good framework for that.
Regards,
Samay
Thanks,
Stephen
pgsql-hackers by date: