diff --git a/openid_connect.module b/openid_connect.module index ec7a465..158dbdb 100644 --- a/openid_connect.module +++ b/openid_connect.module @@ -12,7 +12,8 @@ use Drupal\user\UserInterface; /** * Implements hook_entity_property_info_alter(). */ -function openid_connect_entity_property_info_alter(&$info) { +function openid_connect_entity_property_info_alter(&$info) +{ $properties = &$info['user']['properties']; if (!isset($properties['timezone'])) { @@ -32,7 +33,8 @@ function openid_connect_entity_property_info_alter(&$info) { /** * Implements hook_user_insert(). */ -function openid_connect_user_insert(EntityInterface $entity) { +function openid_connect_user_insert(EntityInterface $entity) +{ if (isset($entity->openid_connect_client) && isset($entity->openid_connect_sub)) { openid_connect_connect_account($entity, $entity->openid_connect_client, $entity->openid_connect_sub); } @@ -41,7 +43,8 @@ function openid_connect_user_insert(EntityInterface $entity) { /** * Implements hook_user_cancel(). */ -function openid_connect_user_cancel($edit, $account, $method) { +function openid_connect_user_cancel($edit, $account, $method) +{ $authmap = \Drupal::service('openid_connect.authmap'); $authmap->deleteAssociation($account->id()); } @@ -49,7 +52,8 @@ function openid_connect_user_cancel($edit, $account, $method) { /** * Implements hook_ENTITY_TYPE_delete(). */ -function openid_connect_user_delete(EntityInterface $entity) { +function openid_connect_user_delete(EntityInterface $entity) +{ $authmap = \Drupal::service('openid_connect.authmap'); $authmap->deleteAssociation($entity->id()); } @@ -57,7 +61,8 @@ function openid_connect_user_delete(EntityInterface $entity) { /** * Implements hook_user_format_name_alter(). */ -function openid_connect_user_format_name_alter(&$name, $account) { +function openid_connect_user_format_name_alter(&$name, $account) +{ // Ensure that usernames are not displayed if they are email addresses, or if // they are generated names starting with 'oidc_'. $oidc_name = \Drupal::service('user.data')->get('openid_connect', $account->id(), 'oidc_name'); @@ -69,11 +74,11 @@ function openid_connect_user_format_name_alter(&$name, $account) { /** * Implements hook_form_FORM_ID_alter(). */ -function openid_connect_form_user_form_alter(&$form, &$form_state) { +function openid_connect_form_user_form_alter(&$form, &$form_state) +{ if (isset($form['account'])) { $account_form = &$form['account']; - } - else { + } else { $account_form = &$form; } @@ -90,11 +95,11 @@ function openid_connect_form_user_form_alter(&$form, &$form_state) { /** * Implements hook_form_FORM_ID_alter(). */ -function openid_connect_form_user_profile_form_alter(&$form, &$form_state) { +function openid_connect_form_user_profile_form_alter(&$form, &$form_state) +{ if (isset($form['account'])) { $account_form = &$form['account']; - } - else { + } else { $account_form = &$form; } @@ -114,7 +119,8 @@ function openid_connect_form_user_profile_form_alter(&$form, &$form_state) { * @param array $userinfo * An array with information about the user. */ -function openid_connect_save_userinfo(UserInterface $account, array $userinfo) { +function openid_connect_save_userinfo(UserInterface $account, array $userinfo) +{ $properties = \Drupal::entityManager()->getFieldDefinitions('user', 'user'); $properties_skip = _openid_connect_user_properties_to_skip(); foreach ($properties as $property_name => $property) { @@ -174,8 +180,7 @@ function openid_connect_save_userinfo(UserInterface $account, array $userinfo) { break; } - } - // Catch the error if the field does not exist. + } // Catch the error if the field does not exist. catch (\InvalidArgumentException $e) { \Drupal::logger('openid_connect')->error($e->getMessage()); } @@ -198,24 +203,47 @@ function openid_connect_save_userinfo(UserInterface $account, array $userinfo) { * @param \Drupal\user\UserInterface $account * The user account. */ -function openid_connect_login_user(UserInterface $account) { +function openid_connect_login_user(UserInterface $account) +{ user_login_finalize($account); } /** * Save the current path in the session, for redirecting after authorization. */ -function openid_connect_save_destination() { - $current_path = \Drupal::service('path.current')->getPath(); - $path = $current_path == '/user/login' ? '/user' : $current_path; +function openid_connect_save_destination() +{ + $path = $current_path = \Drupal::service('path.current')->getPath(); // The destination could contain query parameters. Ensure that they are // preserved. - $query = \Drupal::request()->getQueryString(); + $query_string = \Drupal::request()->getQueryString(); + + if ($query_string) { + parse_str($query_string, $query); + } else { + $query = []; + } + + // Check if a destination parameter is set, update the destination, and + // alter query parameters. + if (!empty(\Drupal::destination()->get())) { + $path = \Drupal::destination()->get(); + + $request = \Drupal::request(); + $parameter_bag = $request->query; + $parameter_bag->remove('destination'); + + // This has to be http_build_query() because + // \Drupal::request()->getQueryString() returns the original query string + // but here we need the modified query string (without destination). + $query = http_build_query($parameter_bag->all()); + } $_SESSION['openid_connect_destination'] = [ - $path, - ['query' => $query], + 'path' => $path, + 'options' => [ + 'query' => $query], ]; } @@ -234,7 +262,8 @@ function openid_connect_save_destination() { * @return object|false * The user object or FALSE on failure. */ -function openid_connect_create_user($sub, array $userinfo, $client_name, $status = 1) { +function openid_connect_create_user($sub, array $userinfo, $client_name, $status = 1) +{ /** @var \Drupal\user\Entity\User $account */ $account = User::create([ 'name' => openid_connect_generate_username($sub, $userinfo, $client_name), @@ -262,8 +291,9 @@ function openid_connect_create_user($sub, array $userinfo, $client_name, $status * @return string * A unique username. */ -function openid_connect_generate_username($sub, array $userinfo, $client_name) { - $name = 'oidc_' . $client_name . '_' . $sub; +function openid_connect_generate_username($sub, array $userinfo, $client_name) +{ + $name = 'oidc_' . $client_name . '_' . md5($sub); $candidates = ['preferred_username', 'name']; foreach ($candidates as $candidate) { if (!empty($userinfo[$candidate])) { @@ -289,13 +319,14 @@ function openid_connect_generate_username($sub, array $userinfo, $client_name) { * @return bool * TRUE if a user exists with the given name, FALSE otherwise. */ -function openid_connect_username_exists($name) { +function openid_connect_username_exists($name) +{ return db_query( - 'SELECT COUNT(*) FROM {users_field_data} WHERE name = :name', - [ - ':name' => $name, - ] - )->fetchField() > 0; + 'SELECT COUNT(*) FROM {users_field_data} WHERE name = :name', + [ + ':name' => $name, + ] + )->fetchField() > 0; } /** @@ -307,7 +338,8 @@ function openid_connect_username_exists($name) { * @return bool * TRUE if access is granted, FALSE otherwise. */ -function openid_connect_set_password_access($account) { +function openid_connect_set_password_access($account) +{ if ($account->hasPermission('openid connect set own password')) { return TRUE; } @@ -322,7 +354,8 @@ function openid_connect_set_password_access($account) { /** * Returns user properties that can be skipped when mapping user profile info. */ -function _openid_connect_user_properties_to_skip() { +function _openid_connect_user_properties_to_skip() +{ $properties_to_skip = [ 'uid', 'uuid', 'langcode', 'preferred_langcode', 'preferred_admin_langcode', 'name', 'pass', 'mail', 'status', 'created', 'changed', 'access', 'login', @@ -342,7 +375,8 @@ function _openid_connect_user_properties_to_skip() { * @param string $sub * The 'sub' property identifying the external account. */ -function openid_connect_connect_account($account, $client_name, $sub) { +function openid_connect_connect_account($account, $client_name, $sub) +{ /* @var \Drupal\openid_connect\Authmap $authmap */ $authmap = \Drupal::service('openid_connect.authmap'); $authmap->createAssociation($account, $client_name, $sub); @@ -356,7 +390,8 @@ function openid_connect_connect_account($account, $client_name, $sub) { * @param string $client_name * The client machine name. */ -function openid_connect_disconnect_account($account, $client_name) { +function openid_connect_disconnect_account($account, $client_name) +{ /* @var \Drupal\openid_connect\Authmap $authmap */ $authmap = \Drupal::service('openid_connect.authmap'); $authmap->deleteAssociation($account->id(), $client_name); @@ -378,14 +413,13 @@ function openid_connect_disconnect_account($account, $client_name) { * @return string|false * The sub, or FALSE if there was an error. */ -function openid_connect_extract_sub(array $user_data, array $userinfo) { +function openid_connect_extract_sub(array $user_data, array $userinfo) +{ if (!isset($user_data['sub']) && !isset($userinfo['sub'])) { return FALSE; - } - elseif (!isset($user_data['sub'])) { + } elseif (!isset($user_data['sub'])) { return $userinfo['sub']; - } - elseif (isset($userinfo['sub']) && $user_data['sub'] != $userinfo['sub']) { + } elseif (isset($userinfo['sub']) && $user_data['sub'] != $userinfo['sub']) { return FALSE; } return $user_data['sub']; @@ -404,7 +438,8 @@ function openid_connect_extract_sub(array $user_data, array $userinfo) { * @return bool * TRUE on success, FALSE on failure. */ -function openid_connect_complete_authorization($client, array $tokens, &$destination) { +function openid_connect_complete_authorization($client, array $tokens, &$destination) +{ if (\Drupal::currentUser()->isAuthenticated()) { throw new \RuntimeException('User already logged in'); } @@ -463,8 +498,7 @@ function openid_connect_complete_authorization($client, array $tokens, &$destina if (\Drupal::config('openid_connect.settings')->get('always_save_userinfo')) { openid_connect_save_userinfo($account, $userinfo); } - } - else { + } else { // Check whether the e-mail address is valid. if (!\Drupal::service('email.validator')->isValid($userinfo['email'])) { drupal_set_message( @@ -486,8 +520,7 @@ function openid_connect_complete_authorization($client, array $tokens, &$destina if ($connect_existing_users) { // Connect existing user account with this sub. openid_connect_connect_account($account, $client->getPluginId(), $sub); - } - else { + } else { drupal_set_message( t('The e-mail address is already taken: @email', [ @@ -506,7 +539,7 @@ function openid_connect_complete_authorization($client, array $tokens, &$destina // Respect possible override from OpenID-Connect settings. $register_override = \Drupal::config('openid_connect.settings') ->get('override_registration_settings'); - if ($register === USER_REGISTER_ADMINISTRATORS_ONLY && $register_override) { + if ((($register === USER_REGISTER_ADMINISTRATORS_ONLY) || ($register === USER_REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL)) && $register_override) { $register = USER_REGISTER_VISITORS; } @@ -559,7 +592,8 @@ function openid_connect_complete_authorization($client, array $tokens, &$destina * @return bool * TRUE on success, FALSE on failure. */ -function openid_connect_connect_current_user($client, array $tokens) { +function openid_connect_connect_current_user($client, array $tokens) +{ /* @var \Drupal\Core\Session\AccountProxyInterface $user */ $user = \Drupal::currentUser(); if (!$user->isAuthenticated()) { diff --git a/src/Controller/RedirectController.php b/src/Controller/RedirectController.php index 6993605..dd1757d 100644 --- a/src/Controller/RedirectController.php +++ b/src/Controller/RedirectController.php @@ -20,7 +20,8 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; * * @package Drupal\openid_connect\Controller */ -class RedirectController extends ControllerBase implements AccessInterface { +class RedirectController extends ControllerBase implements AccessInterface +{ /** * Drupal\openid_connect\Plugin\OpenIDConnectClientManager definition. @@ -54,11 +55,12 @@ class RedirectController extends ControllerBase implements AccessInterface { * {@inheritdoc} */ public function __construct( - OpenIDConnectClientManager $plugin_manager, - RequestStack $request_stack, - LoggerChannelFactoryInterface $logger_factory, - AccountInterface $current_user - ) { + OpenIDConnectClientManager $plugin_manager, + RequestStack $request_stack, + LoggerChannelFactoryInterface $logger_factory, + AccountInterface $current_user + ) + { $this->pluginManager = $plugin_manager; $this->requestStack = $request_stack; @@ -69,7 +71,8 @@ class RedirectController extends ControllerBase implements AccessInterface { /** * {@inheritdoc} */ - public static function create(ContainerInterface $container) { + public static function create(ContainerInterface $container) + { return new static( $container->get('plugin.manager.openid_connect_client.processor'), $container->get('request_stack'), @@ -85,7 +88,8 @@ class RedirectController extends ControllerBase implements AccessInterface { * Whether the state token matches the previously created one that is stored * in the session. */ - public function access() { + public function access() + { // Confirm anti-forgery state token. This round-trip verification helps to // ensure that the user, not a malicious script, is making the request. $query = $this->requestStack->getCurrentRequest()->query; @@ -105,7 +109,8 @@ class RedirectController extends ControllerBase implements AccessInterface { * @return \Symfony\Component\HttpFoundation\RedirectResponse * The redirect response starting the authentication request. */ - public function authenticate($client_name) { + public function authenticate($client_name) + { $query = $this->requestStack->getCurrentRequest()->query; // Delete the state token, since it's already been confirmed. @@ -113,10 +118,16 @@ class RedirectController extends ControllerBase implements AccessInterface { // Get parameters from the session, and then clean up. $parameters = [ - 'destination' => 'user', + 'destination' => [ + 'path' => 'user', + 'options' => [ + 'query' => [], + ], + ], 'op' => 'login', 'connect_uid' => NULL, ]; + foreach ($parameters as $key => $default) { if (isset($_SESSION['openid_connect_' . $key])) { $parameters[$key] = $_SESSION['openid_connect_' . $key]; @@ -150,8 +161,7 @@ class RedirectController extends ControllerBase implements AccessInterface { // If we have an one of the above errors, that means the user hasn't // granted the authorization for the claims. drupal_set_message(t('Logging in with @provider has been canceled.', $provider_param), 'warning'); - } - else { + } else { // Any other error should be logged. E.g. invalid scope. $variables = [ '@error' => $query->get('error'), @@ -161,8 +171,7 @@ class RedirectController extends ControllerBase implements AccessInterface { $this->loggerFactory->get('openid_connect_' . $client_name)->error($message, $variables); drupal_set_message(t('Could not authenticate with @provider.', $provider_param), 'error'); } - } - else { + } else { // Process the login or connect operations. $tokens = $client->retrieveTokens($query->get('code')); if ($tokens) { @@ -173,25 +182,54 @@ class RedirectController extends ControllerBase implements AccessInterface { if (!$success && $register !== USER_REGISTER_ADMINISTRATORS_ONLY) { drupal_set_message(t('Logging in with @provider could not be completed due to an error.', $provider_param), 'error'); } - } - elseif ($parameters['op'] === 'connect' && $parameters['connect_uid'] === $this->currentUser->id()) { + } elseif ($parameters['op'] === 'connect' && $parameters['connect_uid'] === $this->currentUser->id()) { $success = openid_connect_connect_current_user($client, $tokens); if ($success) { drupal_set_message($this->t('Account successfully connected with @provider.', $provider_param)); - } - else { + } else { drupal_set_message($this->t('Connecting with @provider could not be completed due to an error.', $provider_param), 'error'); } } } } + // Support destination query parameter and ensure no /user/login redirect. + if ($destination['path'] == '/user/login') { + $destination['path'] = 'user'; + + // Use the "destination" query parameter if one is set. + if (isset($destination['options']['query']['destination'])) { + + $destination_parts = []; + if ($destination['options']['query']['destination']) { + $destination_parts = UrlHelper::parse($destination['options']['query']['destination']); + } + + $empty_path = empty($destination_parts['path']); + if ($empty_path || $destination_parts['path'] != 'user/login') { + $destination = [ + // Empty destination query parameter means the front page, ie. '/'. + 'path' => (!$empty_path) ? $destination_parts['path'] : '', + 'options' => [ + 'query' => [], + ], + ]; + // Include the query options if any. For example from a views' filters. + if (!empty($destination_parts['query'])) { + $destination['options']['query'] = $destination_parts['query']; + } + // Include the fragment ... not likely unless the options are altered. + if (!empty($destination_parts['fragment'])) { + $destination['options']['fragment'] = $destination_parts['fragment']; + } + } + } + } + // It's possible to set 'options' in the redirect destination. if (is_array($destination)) { - $query = !empty($destination[1]['query']) ? '?' . $destination[1]['query'] : ''; - $redirect = Url::fromUri('internal:/' . ltrim($destination[0], '/') . $query)->toString(); - } - else { + $redirect = Url::fromUri('internal:/' . ltrim($destination['path'], '/'), $destination['options'])->toString(); + } else { $redirect = Url::fromUri('internal:/' . ltrim($destination, '/'))->toString(); } return new RedirectResponse($redirect);