Re: Proposal: Support custom authentication methods using hooks

Hi,

On Wed, Mar 16, 2022 at 8:02 AM Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> Greetings,
>
> * samay sharma (smilingsamay(at)gmail(dot)com) wrote:
> > On Fri, Mar 4, 2022 at 11:15 AM Jacob Champion <pchampion(at)vmware(dot)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
>