Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikejw created an issue. See original summary.

mikejw’s picture

And here is the patch.

mikejw’s picture

Issue summary: View changes
Status: Active » Needs review
mikejw’s picture

Ok, have added in the ability for hooks to return an account so that you could then link it to another account by some mechanism.

yannickoo’s picture

Thank you for this patch mikejw, it works super nice in combination with the implemented hook so I can automatically connect users:

function MODULE_openid_connect_pre_login($tokens, $account, $userinfo, $name, $sub) {
  /** @var \Drupal\user\UserStorageInterface $user_storage */
  $user_storage = \Drupal::service('entity_type.manager')->getStorage('user');

  // Check for existing user.
  $matching_users = $user_storage->loadByProperties([
    'mail' => $userinfo['email'],
  ]);

  if (count($matching_users) === 1) {
    $user = reset($matching_users);
    // Connect the user account.
    openid_connect_connect_account($user, $name, $sub);

    return $user;
  }
  elseif (count($matching_users) === 0) {
    return TRUE;
  }

  return FALSE;
}
yannickoo’s picture

FileSize
3.58 KB
1.03 KB

It seems like you forgot to pass $sub in openid_connect_connect_current_user which results in

Too few arguments to function MODULE_user_openid_connect_pre_login(), 4 passed and exactly 5 expected in MODULE_user_openid_connect_pre_login()
harora’s picture

#4 works but patch has use statement which is already there in code.
Here is the new patch.

yannickoo’s picture

Oh that's true, we need to re-roll the patch. Unfortunately your patch does not include the missing $sub variable which I noticed in #6.

hugovk’s picture

hugovk’s picture

This patch isn't applying for me from composer.json against OpenID Connect 8.x-1.0-beta3 (released 21 June 2017):

  - Applying patches for drupal/openid_connect
    https://www.drupal.org/files/issues/openid_connect-2867260-pre-login-hook-8.patch (Add pre_login hook to restrict domain)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/openid_connect-2867260-pre-login-hook-8.patch


  [Exception]
  Cannot apply patch Add pre_login hook to restrict domain (https://www.drupal.org/files/issues/openid_connect-2867260-pre-login-hook-8.patch)!

When I try and apply it manually I get:

$ patch -p1 < openid_connect-2867260-pre-login-hook-8.patch
patching file openid_connect.module
Hunk #1 FAILED at 425.
Hunk #2 succeeded at 535 (offset -25 lines).
1 out of 2 hunks FAILED -- saving rejects to file openid_connect.module.rej

Was the latest patch against latest 8.x-1.0-beta3? Does it work against the latest one for others?

Thanks!

hugovk’s picture

By the way, I implemented it differently, by creating a new plugin module which inherits from the provided Google plugin. The main code is in modules/openid_connect_newprovider/src/Plugin/OpenIDConnectClient/NewProvider.php:

<?php

namespace Drupal\openid_connect_myprovider\Plugin\OpenIDConnectClient;

use Drupal\Core\Form\FormStateInterface;
use Drupal\openid_connect\Plugin\OpenIDConnectClient\Google;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;

/**
 * My Provider OpenID Connect client.
 *
 * Implements an OpenID Connect Client plugin for My Provider.
 *
 * @OpenIDConnectClient(
 *   id = "myprovider",
 *   label = @Translation("My Provider")
 * )
 */
class MyProvider extends Google {

  /**
   * {@inheritdoc}
   */
  public function endsWith($haystack, $needle) {
    $length = strlen($needle);

    return $length === 0 ||
      (substr($haystack, -$length) === $needle);
  }

  /**
   * {@inheritdoc}
   */
  public function retrieveUserInfo($access_token) {
    $userinfo = parent::retrieveUserInfo($access_token);
    if ($userinfo) {
      if (!$this->endsWith($userinfo['email'], "@example.com")) {
        $variables = [
          '@email' => $userinfo['email'],
        ];
        $this->loggerFactory->get('openid_connect_' . $this->pluginId)
          ->error('Email address not allowed: @email', $variables);
        throw new AccessDeniedHttpException();
      }
    }

    return $userinfo;
  }

}

With also a modules/openid_connect_myprovider/config/install/openid_connect.settings.myprovider.yml:

enabled: false
settings:
  client_id:
  client_secret:
sanduhrs’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
2.14 KB

Although, there is duplicate code in there, it seems reasonable to apply for the time being.

sanduhrs’s picture

Status: Reviewed & tested by the community » Fixed

Committed, thanks.

  • sanduhrs committed 75c0982 on 8.x-1.x
    Issue #2867260 by yannickoo, mikejw, harora, sanduhrs: Add pre_login...

  • sanduhrs committed cbc8b2a on 8.x-1.x
    Issue #2867260 by sanduhrs: rename pre_login to pre_authorize hook
    

Status: Fixed » Closed (fixed)

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

jibran’s picture

+++ b/openid_connect.module
@@ -432,6 +432,26 @@ function openid_connect_complete_authorization($client, array $tokens, &$destina
   $account = $authmap->userLoadBySub($sub, $client->getPluginId());

This can return false and result in fatal.

The website encountered an unexpected error. Please try again later.
TypeError: Argument 2 passed to snsw_openid_connect_openid_connect_pre_authorize() must be an instance of Drupal\user\UserInterface, boolean given in snsw_openid_connect_openid_connect_pre_authorize() (line 60 of modules/custom/snsw_openid_connect/snsw_openid_connect.module).

Created #2940867: hook_openid_connect_pre_authorize can result in fatal for this.

figover’s picture

I am facing the issue.

when linkedin return to drupal.

The error is coming

The website encountered an unexpected error. Please try again later.

I checked in error log

TypeError: Argument 1 passed to openid_connect_extract_sub() must be of the type array, null given,