Commit d9d18c7c authored by jcnventura's avatar jcnventura Committed by Joao Ventura
Browse files

Issue #3112173 by sleitner, jcnventura, pfrilling, munish.kumar, mrinalini9,...

Issue #3112173 by sleitner, jcnventura, pfrilling, munish.kumar, mrinalini9, solideogloria, sanduhrs, Insasse, Shamsher_Alam, fabianderijk, tomvv: Drupal9 deprecations
parent 436f53cf
......@@ -3,15 +3,12 @@
"type": "drupal-module",
"description": "A pluggable client implementation for the OpenID Connect protocol.",
"keywords": ["Drupal"],
"license": "GPL-2.0+",
"license": "GPL-2.0-or-later",
"homepage": "https://www.drupal.org/project/openid_connect",
"minimum-stability": "dev",
"support": {
"issues": "https://www.drupal.org/project/issues/openid_connect",
"source": "http://cgit.drupalcode.org/openid_connect"
"source": "https://git.drupalcode.org/project/openid_connect"
},
"require": {
"drupal/core": "^8.5"
}
"require": {}
}
name: OpenID Connect
type: module
description: A pluggable client implementation for the OpenID Connect protocol.
core: 8.x
core_version_requirement: ^8.8 || ^9
package: Other
configure: openid_connect.admin_settings
......@@ -5,9 +5,14 @@
* Hook implementations of the OpenID Connect module.
*/
/** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
/** @noinspection PhpUnused */
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Url;
use Drupal\user\UserInterface;
/**
* Implements hook_help().
......@@ -365,7 +370,7 @@ function openid_connect_extract_sub(array $user_data, array $userinfo) {
* The client.
* @param array $tokens
* The tokens as returned from OpenIDConnectClientInterface::retrieveTokens().
* @param string|array &$destination
* @param string|array $destination
* The path to redirect to after authorization.
*
* @return bool
......
services:
openid_connect.openid_connect:
class: Drupal\openid_connect\OpenIDConnect
arguments: ['@config.factory', '@openid_connect.authmap', '@entity_type.manager', '@entity_field.manager', '@current_user', '@user.data', '@email.validator', '@messenger', '@module_handler', '@logger.factory']
arguments: ['@config.factory', '@openid_connect.authmap', '@entity_type.manager', '@entity_field.manager', '@current_user', '@user.data', '@email.validator', '@messenger', '@module_handler', '@logger.factory', '@file_system']
plugin.manager.openid_connect_client.processor:
class: Drupal\openid_connect\Plugin\OpenIDConnectClientManager
......
......@@ -6,11 +6,13 @@ use Drupal\Core\Access\AccessResult;
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use Drupal\Core\Routing\Access\AccessInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\Core\Url;
use Drupal\openid_connect\Plugin\OpenIDConnectClientManager;
use Drupal\openid_connect\Plugin\OpenIDConnectClientInterface;
use Drupal\openid_connect\OpenIDConnect;
use Drupal\openid_connect\OpenIDConnectStateToken;
use Drupal\user\UserInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\RequestStack;
......@@ -45,9 +47,9 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
protected $loggerFactory;
/**
* Drupal\Core\Session\AccountProxy definition.
* Current user service.
*
* @var \Drupal\Core\Session\AccountProxy
* @var \Drupal\Core\Session\AccountProxyInterface
*/
protected $currentUser;
......@@ -66,7 +68,7 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
OpenIDConnect $openid_connect,
RequestStack $request_stack,
LoggerChannelFactoryInterface $logger_factory,
AccountInterface $current_user
AccountProxyInterface $current_user
) {
$this->pluginManager = $plugin_manager;
$this->openIDConnect = $openid_connect;
......@@ -91,7 +93,7 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
/**
* Access callback: Redirect page.
*
* @return bool
* @return \Drupal\Core\Access\AccessResultInterface
* Whether the state token matches the previously created one that is stored
* in the session.
*/
......@@ -141,7 +143,7 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
$client_name,
$configuration
);
if (!$query->get('error') && (!$client || !$query->get('code'))) {
if (!$query->get('error') && (!($client instanceof OpenIDConnectClientInterface) || !$query->get('code'))) {
// In case we don't have an error, but the client could not be loaded or
// there is no state token specified, the URI is probably being visited
// outside of the login flow.
......@@ -159,7 +161,7 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
])) {
// If we have an one of the above errors, that means the user hasn't
// granted the authorization for the claims.
drupal_set_message($this->t('Logging in with @provider has been canceled.', $provider_param), 'warning');
$this->messenger()->addWarning($this->t('Logging in with @provider has been canceled.', $provider_param));
}
else {
// Any other error should be logged. E.g. invalid scope.
......@@ -169,7 +171,7 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
];
$message = 'Authorization failed: @error. Details: @details';
$this->loggerFactory->get('openid_connect_' . $client_name)->error($message, $variables);
drupal_set_message($this->t('Could not authenticate with @provider.', $provider_param), 'error');
$this->messenger()->addError($this->t('Could not authenticate with @provider.', $provider_param));
}
}
else {
......@@ -185,19 +187,19 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
// Respect possible override from OpenID-Connect settings.
$register_override = $this->config('openid_connect.settings')
->get('override_registration_settings');
if ($register === USER_REGISTER_ADMINISTRATORS_ONLY && $register_override) {
$register = USER_REGISTER_VISITORS;
if ($register === UserInterface::REGISTER_ADMINISTRATORS_ONLY && $register_override) {
$register = UserInterface::REGISTER_VISITORS;
}
switch ($register) {
case USER_REGISTER_ADMINISTRATORS_ONLY:
case USER_REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL:
case UserInterface::REGISTER_ADMINISTRATORS_ONLY:
case UserInterface::REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL:
// Skip creating an error message, as completeAuthorization
// already added according messages.
break;
default:
drupal_set_message($this->t('Logging in with @provider could not be completed due to an error.', $provider_param), 'error');
$this->messenger()->addError($this->t('Logging in with @provider could not be completed due to an error.', $provider_param));
break;
}
}
......@@ -205,10 +207,10 @@ class OpenIDConnectRedirectController extends ControllerBase implements AccessIn
elseif ($parameters['op'] === 'connect' && $parameters['connect_uid'] === $this->currentUser->id()) {
$success = $this->openIDConnect->connectCurrentUser($client, $tokens);
if ($success) {
drupal_set_message($this->t('Account successfully connected with @provider.', $provider_param));
$this->messenger()->addMessage($this->t('Account successfully connected with @provider.', $provider_param));
}
else {
drupal_set_message($this->t('Connecting with @provider could not be completed due to an error.', $provider_param), 'error');
$this->messenger()->addError($this->t('Connecting with @provider could not be completed due to an error.', $provider_param));
}
}
}
......
......@@ -193,12 +193,12 @@ class OpenIDConnectAccountsForm extends FormBase implements ContainerInjectionIn
if ($op === 'disconnect') {
$this->authmap->deleteAssociation($form_state->get('account')->id(), $client_name);
$client = $this->pluginManager->getDefinition($client_name);
drupal_set_message($this->t('Account successfully disconnected from @client.', ['@client' => $client['label']]));
$this->messenger()->addMessage($this->t('Account successfully disconnected from @client.', ['@client' => $client['label']]));
return;
}
if ($this->currentUser->id() !== $form_state->get('account')->id()) {
drupal_set_message($this->t("You cannot connect another user's account."), 'error');
$this->messenger()->addError($this->t("You cannot connect another user's account."));
return;
}
......
......@@ -37,7 +37,7 @@ class OpenIDConnectSettingsForm extends ConfigFormBase implements ContainerInjec
/**
* The entity manager.
*
* @var \Drupal\Core\Entity\EntityManager
* @var \Drupal\Core\Entity\EntityFieldManagerInterface
*/
protected $entityFieldManager;
......@@ -328,7 +328,7 @@ class OpenIDConnectSettingsForm extends ConfigFormBase implements ContainerInjec
->getEditable('openid_connect.settings.' . $client_name)
->get('settings');
/* @var \Drupal\openid_connect\Plugin\OpenIDConnectClientInterface $client */
/** @var \Drupal\openid_connect\Plugin\OpenIDConnectClientInterface $client */
$client = $this->pluginManager->createInstance(
$client_name,
$configuration ?: []
......
......@@ -6,6 +6,7 @@ use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Entity\EntityFieldManagerInterface;
use Drupal\Core\Extension\ModuleHandler;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Session\AccountProxyInterface;
......@@ -74,7 +75,7 @@ class OpenIDConnect {
/**
* The module handler.
*
* @var Drupal\Core\Extension\ModuleHandler
* @var \Drupal\Core\Extension\ModuleHandler
*/
protected $moduleHandler;
......@@ -88,10 +89,17 @@ class OpenIDConnect {
/**
* The OpenID Connect logger channel.
*
* @var Drupal\Core\Logger\LoggerChannelInterface
* @var \Drupal\Core\Logger\LoggerChannelInterface
*/
protected $logger;
/**
* File system.
*
* @var \Drupal\Core\File\FileSystemInterface
*/
private $fileSystem;
/**
* Construct an instance of the OpenID Connect service.
*
......@@ -115,6 +123,8 @@ class OpenIDConnect {
* The module handler.
* @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger
* A logger channel factory instance.
* @param \Drupal\Core\File\FileSystemInterface $fileSystem
* The file system service.
*/
public function __construct(
ConfigFactoryInterface $config_factory,
......@@ -126,7 +136,8 @@ class OpenIDConnect {
EmailValidatorInterface $email_validator,
MessengerInterface $messenger,
ModuleHandler $module_handler,
LoggerChannelFactoryInterface $logger
LoggerChannelFactoryInterface $logger,
FileSystemInterface $fileSystem
) {
$this->configFactory = $config_factory;
$this->authmap = $authmap;
......@@ -138,6 +149,7 @@ class OpenIDConnect {
$this->messenger = $messenger;
$this->moduleHandler = $module_handler;
$this->logger = $logger->get('openid_connect');
$this->fileSystem = $fileSystem;
}
/**
......@@ -213,7 +225,7 @@ class OpenIDConnect {
* @param array $tokens
* The tokens as returned from
* OpenIDConnectClientInterface::retrieveTokens().
* @param string|array &$destination
* @param string|array $destination
* The path to redirect to after authorization.
*
* @return bool
......@@ -249,7 +261,7 @@ class OpenIDConnect {
return FALSE;
}
/* @var \Drupal\user\UserInterface $account */
/** @var \Drupal\user\UserInterface|bool $account */
$account = $this->authmap->userLoadBySub($sub, $client->getPluginId());
$context = [
'tokens' => $tokens,
......@@ -279,7 +291,7 @@ class OpenIDConnect {
}
}
if ($account) {
if ($account !== FALSE) {
// An existing account was found. Save user claims.
if ($this->configFactory->get('openid_connect.settings')->get('always_save_userinfo')) {
$context = [
......@@ -307,6 +319,7 @@ class OpenIDConnect {
'mail' => $userinfo['email'],
]);
if ($accounts) {
/** @var \Drupal\user\UserInterface|bool $account */
$account = reset($accounts);
$connect_existing_users = $this->configFactory->get('openid_connect.settings')
->get('connect_existing_users');
......@@ -328,24 +341,24 @@ class OpenIDConnect {
// Respect possible override from OpenID-Connect settings.
$register_override = $this->configFactory->get('openid_connect.settings')
->get('override_registration_settings');
if ($register === USER_REGISTER_ADMINISTRATORS_ONLY && $register_override) {
$register = USER_REGISTER_VISITORS;
if ($register === UserInterface::REGISTER_ADMINISTRATORS_ONLY && $register_override) {
$register = UserInterface::REGISTER_VISITORS;
}
if (empty($account)) {
switch ($register) {
case USER_REGISTER_ADMINISTRATORS_ONLY:
case UserInterface::REGISTER_ADMINISTRATORS_ONLY:
// Deny user registration.
$this->messenger->addError($this->t('Only administrators can register new accounts.'));
return FALSE;
case USER_REGISTER_VISITORS:
case UserInterface::REGISTER_VISITORS:
// Create a new account if register settings is set to visitors or
// override is active.
$account = $this->createUser($sub, $userinfo, $client->getPluginId(), 1);
break;
case USER_REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL:
case UserInterface::REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL:
// Create a new account and inform the user of the pending approval.
$account = $this->createUser($sub, $userinfo, $client->getPluginId(), 0);
$this->messenger->addMessage($this->t('Thank you for applying for an account. Your account is currently pending approval by the site administrator.'));
......@@ -414,7 +427,6 @@ class OpenIDConnect {
throw new \RuntimeException('User not logged in');
}
/* @var \Drupal\openid_connect\Authmap $authmap */
$user_data = $client->decodeIdToken($tokens['id_token']);
$userinfo = $client->retrieveUserInfo($tokens['access_token']);
......@@ -444,7 +456,7 @@ class OpenIDConnect {
return FALSE;
}
/* @var \Drupal\user\UserInterface $account */
/** @var \Drupal\user\UserInterface|bool $account */
$account = $this->authmap->userLoadBySub($sub, $client->getPluginId());
$context = [
'tokens' => $tokens,
......@@ -474,12 +486,12 @@ class OpenIDConnect {
}
}
if ($account && $account->id() !== $this->currentUser->id()) {
if ($account !== FALSE && $account->id() !== $this->currentUser->id()) {
$this->messenger->addError($this->t('Another user is already connected to this @provider account.', $provider_param));
return FALSE;
}
if (!$account) {
if ($account === FALSE) {
$account = $this->userStorage->load($this->currentUser->id());
$this->authmap->createAssociation($account, $client->getPluginId(), $sub);
}
......@@ -682,12 +694,13 @@ class OpenIDConnect {
case 'image':
// Create file object from remote URL.
$basename = explode('?', drupal_basename($claim_value))[0];
$basename = explode('?', $this->fileSystem->basename($claim_value))[0];
$data = file_get_contents($claim_value);
$file = file_save_data(
$data,
'public://user-picture-' . $account->id() . '-' . $basename,
FILE_EXISTS_RENAME
FileSystemInterface::EXISTS_RENAME
);
// Cleanup the old file.
......
......@@ -95,7 +95,7 @@ class OpenIDConnectAuthmap {
->condition('sub', $sub, '=')
->execute();
foreach ($result as $record) {
/* @var \Drupal\user\Entity\User $account */
/** @var \Drupal\user\Entity\User $account */
$account = $this->userStorage->load($record->uid);
if (is_object($account)) {
return $account;
......
......@@ -16,6 +16,7 @@ use Exception;
use GuzzleHttp\ClientInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\RequestStack;
use Drupal\Component\Datetime\TimeInterface;
/**
* Base class for OpenID Connect client plugins.
......@@ -44,6 +45,13 @@ abstract class OpenIDConnectClientBase extends PluginBase implements OpenIDConne
*/
protected $loggerFactory;
/**
* The datetime.time service.
*
* @var \Drupal\Component\Datetime\TimeInterface
*/
protected $dateTime;
/**
* The constructor.
*
......@@ -59,6 +67,8 @@ abstract class OpenIDConnectClientBase extends PluginBase implements OpenIDConne
* The http client.
* @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger_factory
* The logger factory.
* @param \Drupal\Component\Datetime\TimeInterface $datetime_time
* The datetime.time service.
*/
public function __construct(
array $configuration,
......@@ -66,7 +76,8 @@ abstract class OpenIDConnectClientBase extends PluginBase implements OpenIDConne
$plugin_definition,
RequestStack $request_stack,
ClientInterface $http_client,
LoggerChannelFactoryInterface $logger_factory
LoggerChannelFactoryInterface $logger_factory,
TimeInterface $datetime_time = NULL
) {
parent::__construct(
$configuration,
......@@ -77,6 +88,7 @@ abstract class OpenIDConnectClientBase extends PluginBase implements OpenIDConne
$this->requestStack = $request_stack;
$this->httpClient = $http_client;
$this->loggerFactory = $logger_factory;
$this->dateTime = $datetime_time ?: \Drupal::service('datetime.time');
$this->setConfiguration($configuration);
}
......@@ -95,7 +107,8 @@ abstract class OpenIDConnectClientBase extends PluginBase implements OpenIDConne
$plugin_definition,
$container->get('request_stack'),
$container->get('http_client'),
$container->get('logger.factory')
$container->get('logger.factory'),
$container->get('datetime.time')
);
}
......@@ -287,7 +300,7 @@ abstract class OpenIDConnectClientBase extends PluginBase implements OpenIDConne
],
];
/* @var \GuzzleHttp\ClientInterface $client */
/** @var \GuzzleHttp\ClientInterface $client */
$client = $this->httpClient;
try {
$response = $client->post($endpoints['token'], $request_options);
......@@ -299,7 +312,7 @@ abstract class OpenIDConnectClientBase extends PluginBase implements OpenIDConne
'access_token' => isset($response_data['access_token']) ? $response_data['access_token'] : NULL,
];
if (array_key_exists('expires_in', $response_data)) {
$tokens['expire'] = REQUEST_TIME + $response_data['expires_in'];
$tokens['expire'] = $this->dateTime->getRequestTime() + $response_data['expires_in'];
}
if (array_key_exists('refresh_token', $response_data)) {
$tokens['refresh_token'] = $response_data['refresh_token'];
......
......@@ -2,14 +2,15 @@
namespace Drupal\openid_connect\Plugin;
use Drupal\Component\Plugin\ConfigurablePluginInterface;
use Drupal\Component\Plugin\ConfigurableInterface;
use Drupal\Component\Plugin\DependentPluginInterface;
use Drupal\Component\Plugin\PluginInspectionInterface;
use Drupal\Core\Plugin\PluginFormInterface;
/**
* Defines an interface for OpenID Connect client plugins.
*/
interface OpenIDConnectClientInterface extends ConfigurablePluginInterface, PluginFormInterface, PluginInspectionInterface {
interface OpenIDConnectClientInterface extends ConfigurableInterface, DependentPluginInterface, PluginFormInterface, PluginInspectionInterface {
/**
* Returns an array of endpoints.
......
<?php
namespace Drupal\openid_connect\Tests;
use Drupal\simpletest\WebTestBase;
/**
* Provides tests for the settings form.
*
* @group openid_connect
*/
class OpenIDConnectSettingsFormTest extends WebTestBase {
/**
* Modules to enable.
*
* @var array
*/
public static $modules = ['openid_connect'];
/**
* A regular user.
*
* @var \Drupal\user\UserInterface
*/
protected $webUser;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->webUser = $this->drupalCreateUser([
'administer site configuration',
]);
}
/**
* Tests the OpenID connect settings form.
*/
public function testUpdateSettings() {
$this->drupalLogin($this->webUser);
$this->drupalGet('admin/config/services/openid-connect');
// Override the default values.
$edit = [
'always_save_userinfo' => FALSE,
'override_registration_settings' => TRUE,
];
$this->drupalPostForm(NULL, $edit, 'Save configuration', [], [], 'openid-connect-admin-settings');
// Check the config was updated.
$config_factory = $this->container->get('config.factory');
/* @var \Drupal\Core\Config\Config $config */
$config = $config_factory->get('openid_connect.settings');
$user_info = $config->get('always_save_userinfo');
$this->assertFalse($user_info);
$override_registration_settings = $config->get('override_registration_settings');
$this->assertTrue($override_registration_settings);
}
}
......@@ -2,6 +2,8 @@
declare(strict_types = 1);
namespace Drupal\Tests\openid_connect\Unit;
use Drupal\Tests\UnitTestCase;
use Drupal\Core\Database\Connection;
use Drupal\Core\Entity\EntityStorageInterface;
......@@ -166,7 +168,7 @@ class OpenIDConnectAuthmapTest extends UnitTestCase {
/**
* Provide data to the testDeleteAssociationMethod test.
*
* @return array|string[]
* @return array
* Return the client names to test.
*/
public function getDeleteAssociationParameters(): array {
......@@ -245,8 +247,9 @@ class OpenIDConnectAuthmapTest extends UnitTestCase {
* The parameters to pass to the userLoadBySubMethod.
*/
public function getUserLoadBySubParameters(): array {
$test = new stdClass();
$test->uid = self::USER_ID;
$test = (object) [
'uid' => self::USER_ID,
];
$results = [
$test,
];
......@@ -325,9 +328,10 @@ class OpenIDConnectAuthmapTest extends UnitTestCase {
* Data to test the getConnectedAccounts method.
*/
public function getConnectedAccountsParameters(): array {
$record = new stdClass();
$record->client_name = $this->randomMachineName();
$record->sub = $this->randomMachineName();
$record = (object) [
'client_name' => $this->randomMachineName(),
'sub' => $this->randomMachineName(),
];
return [
[[]],
......
......@@ -2,6 +2,8 @@
declare(strict_types = 1);
namespace Drupal\Tests\openid_connect\Unit;
use Drupal\Tests\UnitTestCase;
use Drupal\openid_connect\OpenIDConnectStateToken;
......
......@@ -2,6 +2,8 @@
declare(strict_types = 1);
namespace Drupal\Tests\openid_connect\Unit;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Entity\EntityFieldManagerInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
......@@ -24,10 +26,13 @@ use Drupal\Core\Field\FieldDefinitionInterface;
use Drupal\Core\Entity\EntityTypeRepositoryInterface;
use Drupal\file\Entity\File;
use Drupal\Core\Field\FieldItemListInterface;
use Drupal\Core\File\FileSystemInterface;
use Psr\Log\InvalidArgumentException;
use stdClass;
/**
* Class OpenIDConnectTest.
*
* @coversDefaultClass \Drupal\openid_connect\OpenIDConnect
* @group openid_connect
*/
class OpenIDConnectTest extends UnitTestCase {
......@@ -123,6 +128,13 @@ class OpenIDConnectTest extends UnitTestCase {
*/