Claim mapping for 'email' results in error "property type not implemented: 'email'"

The email address is a very special user property, as it is one of the properties that requires to be unique for all Drupal users.

I anticipate this is the reason, why emails haven't been allowed for property mapping so far.

If we're going to do so now, we should add additional tests for whether the new email address is in use by another user, and raise according exceptions if so.

@Mario Steinitz good point!

raise according exceptions

Not sure if an exception is required. I implemented it as a Drupal log message.

I implemented it as a Drupal log message.

Well... For some projects it can be pretty critical I think. So here is a version with an exception.

Patch to use in version 1.0.0.

Thanks for working on this. It seems a good improvement. However, 1.x will not get any new features, and this is a new feature. The patch needs to be for version 2.x.

1. +++ b/src/OpenIDConnect.php
   @@ -653,6 +653,32 @@ class OpenIDConnect {
   +                    $account->set($property_name, $userinfo[$claim]);
   

   By this time, $property_name is 'mail', might as well not use the variable.

2. +++ b/src/OpenIDConnect.php
   @@ -653,6 +653,32 @@ class OpenIDConnect {
   +                    $this->$this->logger->critical(
   

   The double $this is surely an error...

   Also, although this is an error, it surely is not critical. It does not threaten the integrity of the site.

3. +++ b/src/OpenIDConnect.php
   @@ -653,6 +653,32 @@ class OpenIDConnect {
   +                      'Could not save user info for account %account. Email address %email is already used in account %account_by_mail. An exception will be thrown.',
   

   Let's not reveal the username of the other account. Just use the following message: 'Could not save user info. Email address %email is already used in another account'.

   And simplify the replacement variables, as we only need email now.

4. +++ b/src/OpenIDConnect.php
   @@ -653,6 +653,32 @@ class OpenIDConnect {
   +                    throw new \RuntimeException('Cannot set account email address in openid_connect_save_userinfo because it is already used in another account.');
   

   Use same message as above, to simplify the life of translators. Or better yet, do not throw an exception at all. Just because we can't map a field should not be a reason for the user to not be able to save the other claims. Please delete this line.

Here there is patch with improvements suggested by @jcnventura :)

It's for 1.x version.

It probably applies already to 2.x. That part of the code hasn't changed much,

Patch works for me with version 2 of the module.

Nice to know. I'll test this and probably commit, and implement the change I requested in #7.2, and change the logger message from critical to warning.

Honestly, it's a user that most likely is trying to use OpenID when he already has another account in the system. Hardly the stuff should be above a warning. It maybe should even be a notice.