On 03/01/2016 08:18 AM, Pavel Stehule wrote:
> 2016-03-01 16:52 GMT+01:00 Joe Conway:
> On 03/01/2016 02:09 AM, Pavel Stehule wrote:
> > > On 01/06/2016 10:36 AM, Tom Lane wrote:
> > >> I think a design that was actually somewhat robust would require two
> > >> hooks, one at check_role and one at assign_role, wherein the first one
> > >> would do any potentially-failing work and package all required info into
> > >> a blob that could be passed through to the assign hook.
>
> > I see following issues:
> >
> > 1. Missing the possibility to pass custom data from SetRoleCheck_hook to
> > SetRoleAssign_hook. Tom mentioned it in his comment.
>
> You can pass the data between the two plugin hook functions in your
> extension itself, so I see no need to try to pass custom data through
> the backend. Do any of the other hooks even do that?
>
> I don't know about it, but these hooks are specific. is it ensured a
> order of calls of these hooks?
> > 2. Missing little bit more comments and an explanation why and when to
> > use these hooks.
>
> Doesn't look all that different from existing user hooks to me, but
> sure, I'll add a bit more to the comments.
> some like "I think the main point was to have two hooks. The
> potentially-failing
> work could be done during the check_role() hook and the collected info
> could be used during the assign_role() hook."
Ok, I added a comment similar to that at the check_role() function hook
call site. Updated patch attached.
I still don't see any point in trying to pass data back from the hooks
as the extension can maintain that state just fine, although it looks
like it would be pretty trivial to do using a new void* member added to
role_auth_extra. Tom (or anyone else), any comment on that?
I do however find myself wishing I could pass the action down from
set_config_option() to at least the assign_role() hook, but that seems
more invasive than I'd like.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
