Claiming existing Drupal account upon first OpenID authentication

If you are in control of the OpenID provider, you are also able to spoof the e-mail-address on that account.
So, this is by design.

I'll set this feature request back to "Normal" priority. Feel free to provide a patch that suits your needs.

Most probably, this feature can be back-ported from the 8.x version of the module, which has a whole lot of more hooks during the authentication process and even provides the required connect tab within the user account.

Unfortunately, we're kind of busy at the time being and can't provide your feature back-port at once. But we'll happily review contrib patches and see what we can do to add your missing feature in the 7.x version.

Looks good to me at first sight. I'll do some proper testing over the weekend and set it to RTBC, if it runs without side effects.

It's working as expected. Setting to RTBC, so we can add it to the next release of the 7.x version.

Unfortunately this patch can mistakenly re-enable disabled settings on the admin form. Patch to follow.

Patch :-)

See #2834095: openid_connect_always_save_userinfo doesn't update from configuration screen - the intention was that variables always get set, even when defaulted. Additional problem with change made in comment 10's patch here, was that deleting variables when they were unticked was not correct, because they then default to enabled (ticked)!

So this patch just restores what was done in #2834095: openid_connect_always_save_userinfo doesn't update from configuration screen, as there's no real need to treat variables differently if they default differently, nor avoid saving variables.

#15 LGTM. Reviewed it and tried it out, and it worked pretty well for the use case of allowing an existing user to be associated based on e-mail address for cases in which you trust the IDP.

Did we want to add a note to the UI that informs admins why it might be unsafe to enable the option if the IDP is not 100% trusted?

This is perfect. I use it for a site that has Domain Access module running with two domains. Logging in and out works as well as creation of new accounts. Additionally, if I clone the OpenID Connect plugin*, any of the domains can act as the server or the client.

I think in this context the concern @GuyPaddock mentions does not apply, because there is only one database and it is fully under admin's control.

* as mentioned here https://www.drupal.org/project/openid_connect/issues/2273615

In the context I am mentioning above (Oauth2 server + Domain access module + two OpenID connect clients all running on the same drupal installation) I see a POD exception shown below, after this sequence of steps:

1. open two tabs in a browser
2. load site with domain A into one tab and site with domain B into another
3. clear all cookies for the sites
4. create an account on site with domain A by using the normal drupal registration facility
5. on site with domain B use OpenID connect login button to trigger login using the new account of the domain A site
6. result state: the same user is logged in both sites
7. log off the user on the domain A site
8. use OpenID Connect button on the domain A site to trigger login against the site B
9. the error shown below is thrown

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '56' for key 'authname': INSERT INTO {authmap} (uid, module, authname) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 56 [:db_insert_placeholder_1] => openid_connect_generic [:db_insert_placeholder_2] => 56 ) in user_set_authmaps() (line 2094 of .../modules/user/user.module).

Considering that the table schema prevents duplicates in the authname column, one solution could be to set the authname to some combination of the user ID and (say) the domain name, or the module name.

Using the latest patch I am unable to save the new setting, ticking and saving the pages returns the form with the checkbox unticked again.

The actual functionality of the connecting of users works great however (after manual calling a variable_set)

If anyone is experiencing the same problem as Rob Holmes, it's because the version 7.x-1.0-beta8 has a bug where the settings are not saved correctly because of the following lines of code: (openid_connect.admin.inc - line 154)

if (isset($form_state['values'][$variable]) && empty($form_state['values'][$variable])) {
  variable_set($variable, $form_state['values'][$variable]);
}

See how variables are only set when the form state values both contains $variable and does not contain $variable? It will never work...

This is not relevant to this issue however, because this issue is about version 7.x-1.x-dev. It already got fixed in 7.x-1.x-dev.
The issue where this was solved is: https://www.drupal.org/project/openid_connect/issues/2834095

#15 didn't apply, so I rerolled it.

I just used patch #21 and can confirm it works great connecting existing user accounts.

I think issue #18 should probably be it's own issue.

Looking at #16, I think that using openid_connect when you don't trust the IDP is already a bad idea. The scenario where all your users use an IDP, but the admin users do not is possible to exploit if you now allow the module to create connections to existing accounts.

I'd prefer that the description of the new option be extended with:

Enabling this option, will allow login to existing accounts, which may be a security risk if the identity provider doesn't have in place adequate procedures to guaranteeing that users really own the e-mail addresses provided.

Sounds reasonable :-) Here's an updated patch then. I've tweaked that messaging slightly as I felt the wording could be improved.

The patch works for me, and it's better than the custom solution I was working with before.

Hi,

The patch above does not work anymore in version 7.x-1.0.

Appreciate if we can have the patch to enable 'automatically connect existing user' in this version.

Thanks

Sorry for the really minor nitpick. But from what I remember, this is flagged as a coding standard violation.

+++ b/includes/openid_connect.admin.inc
@@ -55,6 +55,16 @@ function openid_connect_admin_form($form, &$form_state) {
+    '#description' => t('If disabled, authentication will fail for existing email addresses. Enabling this option will allow logging into existing accounts, which may be a security risk if the identity provider doesn\'t have in place adequate procedures to guarantee that users really own the e-mail addresses used.'),

Please, don't use \' to escape the apostrophe when using single quotes. Either skip the contraction (write does not instead of doesn't) or use double quotes around the entire string.

Excellent! I need this module on an existing user base. Here's a patch rerolled for 7.x-1.0.

Adding the patch without checking the new checkbox preserves original behaviour.

Checking the "Automatically connect existing users" creates an authmap entry for the existing account. I also removed the contraction mentioned in issue #29.

Not sure if I can RTBC a patch I rerolled myself, but I will surely start using this in production now.

Thanks!

Tested, works as advertised!

Maybe we should just take this patch since a year has passed and nobody fixed the nitpick.

What do you mean? The nitpick was fixed in #30 with doesn\'t -> does not

Oh, I didn't realize that since this is still RTBC. Thanks,

This looks like it made to the module, but it is not working for me. I can only login if I manually connect the accounts while logged in as a drupal user. I would like to be able to create a drupal user on the fly if the user is authenticated via oauth.

Any ideas?

@mcanada, Nothing was ever committed. Did you apply a patch, and which one? Are you using 7.x?

@solideogloria I did not appy any patches. What I mean is that I see the option on the settings (see picture attached) but it doesn't seem to work.

I would like to be able to create a drupal user on the fly

That's not what this option is supposed to do. This option never creates accounts. It claims an existing account that was already created programmatically or by some other login or process. If you want to create an account, you'll have to use a custom module to register an account in the code. At least, that's my understanding.

The option doesn't say "automatically connect non-existing users". It is for connecting OpenID users if there is an existing Drupal account that hasn't been bound to the OpenID user yet.

In my case, we sync information over from Activity Directory in the code and register Drupal accounts for users that didn't have one yet. Then, when they log in via OpenID, we needed to connect them to the new Drupal account.

Otherwise, it could be that you need to download a patch or that have have some configuration that needs to be changed.

If you can, use xDebug to step through the code and see what isn't working. Or, share any error messages you see. I'm not on Drupal 7 anymore, but I was using this feature without any problems for years.

@solideogloria Thank you. That makes sense (I think).

I guess what I still don't understand is how the below works

It is for connecting OpenID users if there is an existing Drupal account that hasn't been bound to the OpenID user yet.

What is the criteria to match a Drupal account to the OpenID user? Name, email?

From your screenshot, you can see that it's by email address. At least, that's what it is when I use it. That's why is says that without this option, if the email address is already in use, the user won't be able to log in.

@solideogloria yes that is what it says, but it doesn't work. I logged in with my provider and created a d9 user with the same email address. It did not match the accounts.

You'll have to do some more digging then. I use it with Azure AD and it works. Does your provider have the UserPrincipleName (UPN) set to the email address?

The best thing you can do is get xDebug working and use breakpoints to find the source of the issue by stepping through the code.

Committed, thanks!