Hi,
Currently authentication modules(eg cas or ldapauth) implement hook_auth() and hook_user() to interact with Drupal core.
BUT
They don't provide any hooks themselves to allow other modules to interact with themselves.
Ldapauth provides two somewhat inflexible 'hooks' for transforming the user and for filtering users
by checking for two functions! in a configuration file. A nice idea BUT anything that goes in there is per host, NOT per site.
The proper way to implement this interaction would be to provide hooks for altering the user and for filtering the user.
I think all authentication modules should provide a way to accomplish this type of filtering and transformation,
so I am submitting patches to the ones I use: cas & ldapauth.
Use cases:
1) hook_user_transform($op, &$username)
- if cas returns uid:irstudio:password:200810101432:192.168.2.22 instead of just irstudio
i don't want to hack the cas module, but I would rather make another module that just implements hook_user_transform and do:
switch ($op) { case 'cas': $useparts = explode($username) ; return $userparts[1]; }
2) hook_auth_filter($op, $username)
- Suppose that I want to use cas authentication, but only allow people in my department into my Drupal site.
(if you are familiar with PAM in linux then i am referring to requisite authentication modules
where if one requisite module fails, then stop and deny. So, cas might succeed, but if any filter fails, then deny anyway)
- e.g. an implementation of hook_auth_filter might do:
if (($personid = department_get_user_personid($username) === FALSE) { return FALSE; }
Works like a charm on my sites.
Now I just have to whip up a patch for ldap_integration as well.
P.S.
Eventually, it might be nice to get the filter hook into core, so the blocked user stuff could just be the user.modules implementation of the auth_filter hook. So, then you wouldn't have to check for blocked users, inactive users, etc. just invoke hook auth filter and be done with it ;)
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | cas-transform-filter-hooks-coder-d6.diff | 47.07 KB | treksler |
| #2 | cas-transform-filter-hooks-2.diff | 3.44 KB | treksler |
| cas-transform-filter-hooks.diff | 3.03 KB | treksler |
Comments
Comment #1
metzlerd commentedOk I like the concept of the filter hook. I'd be willing to support that. I agree with your assessment of the LDAP module. It didn't work for me either, although I'd like to rename the function the term requisite doesn't mean much to me. Let's refactor a bit though. Since the invoke_requisite function isn't really generic (it does tests on the return data) let's just create a simple function cas_invoke_auth_filter which calls all the auth filters and returns on the first false. No need to make it take a hook as a parameter again, since it only operates on true/false values.
I'm not so sure about the concept of the transformation hook. I believe it can be dealt with in the hook_user that is part of drupal core. and that CAS invokes at login. If you trap the user login hook and make any data modifications by calling user_save directly, isn't this more flexible? CAS doesn't make any pretense of modifying user accounts. It only creates new ones, and there may be need for such modules to continually update the user->name field for example to keep up with name changes, etc. What am I missing with regard to this approach?
Dave
Comment #2
treksler commentedHey
1) filter
So, I simplified the functions a bit to have just cas_invoke_auth_filter().
I tested it on my devel site here, and looks like it works fine.
2) transform
I matched the naming scheme with the filter hook. So it is cas_invoke_auth_transform() in this patch.
I hear what you are saying about CAS only making new user accounts
and not modifying them.
This patch does not modify the user account.
It just maps the return value of phpCAS::getUser() into a Drupal username via a hook.
This mapping has to happen before we can filter based on blocked users or Drupal access rules, or the filter hook, or run user save.
So, the user insert hook is way too late even user load is way to late, and login user hook is waaay too late because that fires after login.
This is why I have put it where it is,
i.e. after we get contol back from cas and before anything else happens
------------
Use Case:
------------
a) The user types in 'risto' as the username on the CAS login page and is redirected back to the Drupal site
b) phpCAS::getUser() returns 'risto:risto.treksler:192.168.2.1' (for example)
c) Drupal CAS module would now check for blocked users, authmaps, denied users, filter hook ;)
and even make a user with username 'risto:risto.treksler:192.168.2.1'
BUT
What if i need the username to be 'risto.treksler' (i.e. the second 'field' returned by cas)
because that's what is in my department database, and in the Drupal database?
What if 'risto.treksler' is a blocked user? He would get in, because CAS returned 'risto:risto.treksler:192.168.2.1'
which won't match 'risto.treksler' during the check for blocked users
Unless I can hook in there and alter whatever string CAS returns,
right after the call to phpCAS::getUser(),
things will go wrong.
So, I really don't see another way to do it.
I thought about rolling transform into the filter function (return FALSE OR return the transformed name)
BUT
LDAP_Integration runs the 'transform hook' before the LDAP bind (ldapauth.module line 580 - ldapauth_transform_login_name)
and it runs the 'filter hook' after the bind (ldapauth.module line 627 - ldapauth_user_filter)
whereas
we need to run both after the cas check (we can't run anything before or during, because it happens on the cas server ;)
Thus, these hooks need to be separate, so both modules can put them where they need to and both benefit from them.
Anyway,
I hope you see why both hooks are needed (at least) for our setup here,
and probably for some other people too, who might otherwise have to hack the CAS module to make things work.
maybe it should be called something else?
hook_external_login_name_alter ??
Cheers,
Risto
Comment #3
metzlerd commentedActually you point out a good reason why we're not done with the blocked users issue. On some of my sites, we let people edit the user->name field, since that's what typically displays for user posts.
Which means that you can't count on a link between the cas user name and the external auth database name. In such cases your patch will NOT block "blocked users". I think we need to rethink this strategy. I didn't realize how blocking was tied to your transform. Hmmmm....
I think I'm getting this need, but let me gel for a bit. I need to run now, family calls. Will type more later.
Comment #4
metzlerd commentedOk I get it now. I'm on board. Given the size of this, could you create a patch against HEAD for me? Feel free to add the coder reformatting changes there as well.
Comment #5
metzlerd commentedReviewed and committed to 5.x code tree. Would love to have a HEAD patch to commit :).
Comment #6
treksler commentedok will roll one up
Comment #7
treksler commentedOK,
This should do it for Drupal 6.
I should verify it still by porting my other module to Drupal 6, so I can check that the hooks work,
but I don't see why they wouldn't work.
Risto
Comment #8
joe.murray commentedI recently implemented a SSO with netOffice. Because its permissioning and roles where not nicely modularized but integrated into code throughout the project, I decided for better or worse to implement the single sign on as a simultaneous sign on to both Drupal and netOffice, allowing each to retain its own permissioning, etc. This required me to hack core by putting in a module_invoke call to sess_regenerate in session.inc to let netOffice know about the successful sign-in/sign-out of the session, which it tracks as well. I'm too much of a newbie to propose a change to core so I haven't posted the module. But it seems related to this discussion of SSO hooks.
Comment #9
treksler commentedWhat about a regenerate_id hook in session.inc
ok so why couldn't you just use the login hook and the logout hooks?
Comment #10
metzlerd commentedApplied to head as well.
Thanks so much Risto for these great patches.
Dave
Comment #11
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.