I'm using OpenID Connect for a site that uses Google as the provider, but needs to restrict users to certain domains. For this and other purposes, It would be useful to allow modules to block Drupal login or perform other actions after the user has been authorized by the provider but before they've been logged into Drupal. I suggest an openid_connect_pre_login hook similar to the existing openid_connect_post_authorize hook. I've got a version of this working on my site. I'll generate a patch file and upload it shortly.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

illeace created an issue. See original summary.

illeace’s picture

Here's the promised patch file.

othermachines’s picture

Status: Active » Needs review
othermachines’s picture

I think this is a great feature. Patch applies cleanly and works as advertised.

A couple of comments:

- Do you think $account should also be passed in as an argument? Whether the account exists may be useful info in some circumstances. (Although I can't think of any off the top of my head.)

- In the API example, can we use a value we know will exist, like $userinfo['email']? I was a little confused by what $userinfo['hd'] was supposed to be.

Thanks a lot for this!

illeace’s picture

Both suggestions seem reasonable to me. The revised patch now passes $account as a parameter to the hook, and I've update the example in the API file to be a little more generic.

othermachines’s picture

Patch applies. I further tested it by implementing a hook with the example code and it all checks out.

Great work. I would love to see this committed!

othermachines’s picture

@illeace I came across an issue unrelated to your patch while testing: #2590875: Missing $userinfo check results in account being created with no user information. A side effect of this problem when your patch is applied is that $userinfo['email'] is empty in your watchdog message:

Login denied for via pre-login hook.

 
If you have time to pop over there and test the other patch it would be much appreciated.

Aron Novak’s picture

Status: Needs review » Reviewed & tested by the community

I did a review and tested the patch, it's especially helpful if you'd like to perform an additional validation before login, for instance against the organization inside GitHub for the user trying to login. The patch worked without any issues, however comment https://www.drupal.org/node/2559543#comment-10446399 is still valid, but it's outside of the scope here.

ayduns’s picture

Also tested this with a custom module implementing the hook to check the email domain. Works well. My testing did not trigger the issues reported with empty $userinfo['email'] - the watchdog messages contained the user email. Would be great to get this committed.

hugovk’s picture

jcnventura’s picture

Thanks!

  • jcnventura authored 3169d81 on 7.x-1.x
    Issue #2559543 by illeace, othermachines, Aron Novak, ayduns, jcnventura...
jcnventura’s picture

For consistency with the D8 version, the hook is now called hook_openid_connect_pre_authorize. Those using this patch will need to rename their existing hook functions. That was the only change I did.

jcnventura’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.