Skip to content

Commit

Permalink
Merge pull request #37 from City-of-Helsinki/UHF-9515
Browse files Browse the repository at this point in the history
UHF-9515: Disable external user password on login and deploy
  • Loading branch information
hyrsky authored Jun 18, 2024
2 parents e968d82 + 96bd451 commit 881201f
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 5 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ $config['openid_connect.client.tunnistamo']['settings']['environment_url'] = 'ht

See https://helsinkisolutionoffice.atlassian.net/wiki/spaces/HEL/pages/8283226135/Helfi-tunnistamo+moduuli for more information.

## Preventing local user login

Drupal account is created once a user has authenticated through the OpenID provider. The account cannot log without the OpenID authentication if its password is set to null. For additional safeguards, we set the password to null in [post deploy hook](https://github.com/City-of-Helsinki/drupal-module-helfi-api-base/blob/main/documentation/deploy-hooks.md) and during login.

## Contact

Slack: #helfi-drupal (http://helsinkicity.slack.com/)
13 changes: 13 additions & 0 deletions helfi_tunnistamo.module
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ function helfi_tunnistamo_openid_connect_post_authorize(UserInterface $account,
}
$plugin->mapRoles($account, $context);
$plugin->setUserPreferredAdminLanguage($account);

// Once user logs in with openid connect, set user password to null.
// This prevents the user from logging in with the local user in the
// future.
if ($account->getPassword()) {
$account
->setPassword(NULL)
->save();
}
}

/**
Expand All @@ -45,6 +54,10 @@ function helfi_tunnistamo_form_user_form_alter(&$form, FormStateInterface $form_
// to log in.
$form['account']['mail']['#type'] = 'item';
$form['account']['mail']['#markup'] = $account->getEmail();

// External users password cannot be changed.
$form['account']['pass']['#access'] = FALSE;
$form['account']['current_pass']['#access'] = FALSE;
}

/**
Expand Down
12 changes: 7 additions & 5 deletions helfi_tunnistamo.services.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
services:
helfi_tunnistamo.http_exception_subscriber:
class: Drupal\helfi_tunnistamo\EventSubscriber\HttpExceptionSubscriber
arguments: ['@entity_type.manager', '@openid_connect.session', '@current_user']
tags:
- { name: event_subscriber }
_defaults:
autowire: true
autoconfigure: true

logger.channel.helfi_tunnistamo:
parent: logger.channel_base
arguments: [ 'helfi_tunnistamo' ]

Drupal\helfi_tunnistamo\EventSubscriber\HttpExceptionSubscriber: ~
Drupal\helfi_tunnistamo\EventSubscriber\DisableExternalUsersPasswordSubscriber: ~
59 changes: 59 additions & 0 deletions src/EventSubscriber/DisableExternalUsersPasswordSubscriber.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

declare(strict_types=1);

namespace Drupal\helfi_tunnistamo\EventSubscriber;

use Drupal\Core\Database\Connection;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\helfi_api_base\EventSubscriber\DeployHookEventSubscriberBase;
use Symfony\Contracts\EventDispatcher\Event;

/**
* Sets tunnistamo users' password to NULL.
*
* This should prevent given users from logging in using password.
*/
final class DisableExternalUsersPasswordSubscriber extends DeployHookEventSubscriberBase {

/**
* Constructs a new instance.
*
* @param \Drupal\Core\Database\Connection $database
* Database connection.
* @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager
* The entity type manager.
*/
public function __construct(
private readonly Connection $database,
private readonly EntityTypeManagerInterface $entityTypeManager,
) {
}

/**
* {@inheritdoc}
*/
public function onPostDeploy(Event $event) : void {
// Query tunnistamo users that have their passwords set.
$query = $this->database->select('authmap', 'am');
$query->leftJoin('users_field_data', 'ufd', 'ufd.uid = am.uid');
$query
->fields('am', ['uid'])
->condition('ufd.pass', NULL, 'IS NOT NULL')
// Make sure we have an upper bound.
->range(0, 50);

$storage = $this->entityTypeManager->getStorage('user');
foreach ($query->execute()->fetchCol() as $id) {
/** @var \Drupal\user\UserInterface $account */
$account = $storage->load($id);

// Set user password to null. This prevents the user
// from logging in with the local user in the future.
$account
->setPassword(NULL)
->save();
}
}

}
2 changes: 2 additions & 0 deletions src/EventSubscriber/HttpExceptionSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\helfi_tunnistamo\Plugin\OpenIDConnectClient\Tunnistamo;
use Drupal\openid_connect\OpenIDConnectSession;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;

/**
Expand All @@ -28,6 +29,7 @@ final class HttpExceptionSubscriber extends HttpExceptionSubscriberBase {
*/
public function __construct(
private EntityTypeManagerInterface $entityTypeManager,
#[Autowire(service: 'openid_connect.session')]
private OpenIDConnectSession $session,
private AccountProxyInterface $accountProxy,
) {
Expand Down
52 changes: 52 additions & 0 deletions tests/src/Kernel/DisableExternalUsersPasswordSubscriberTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

declare(strict_types=1);

namespace Drupal\Tests\helfi_tunnistamo\Kernel;

use Drupal\helfi_api_base\Event\PostDeployEvent;
use Drupal\Tests\user\Traits\UserCreationTrait;
use Drupal\user\Entity\User;

/**
* Tests disable local users subscriber.
*
* @group helfi_tunnistamo
*/
class DisableExternalUsersPasswordSubscriberTest extends KernelTestBase {

use UserCreationTrait;

/**
* Tests that deploy prevents external users from logging in with password.
*/
public function testSubscriber() : void {
/** @var \Drupal\externalauth\Authmap $authmap */
$authmap = $this->container->get('externalauth.authmap');
$external = $this->createUser(values: ['pass' => '123']);
$local = $this->createUser(values: ['pass' => '123']);

// Add external auth to user.
$authmap->save($external, 'test_provider', 'test_authname');

// User login is enabled.
$this->assertNotEmpty($external->getPassword());
$this->assertNotEmpty($local->getPassword());

$this->triggerEvent();

// User password is disabled.
$this->assertEmpty(User::load($external->id())->getPassword());
$this->assertNotEmpty(User::load($local->id())->getPassword());
}

/**
* Triggers the post deploy event.
*/
private function triggerEvent() : void {
/** @var \Symfony\Contracts\EventDispatcher\EventDispatcherInterface $service */
$service = $this->container->get('event_dispatcher');
$service->dispatch(new PostDeployEvent());
}

}

0 comments on commit 881201f

Please sign in to comment.