Skip to content

Commit

Permalink
Merge pull request #36 from City-of-Helsinki/UHF-9466
Browse files Browse the repository at this point in the history
UHF-9466: Exclude Tunnistamo users from User Expire, automatic phpcs fixes
  • Loading branch information
tuutti authored Apr 30, 2024
2 parents 18aa1b6 + c9bfede commit e968d82
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 9 deletions.
19 changes: 17 additions & 2 deletions helfi_tunnistamo.module
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
declare(strict_types=1);

use Drupal\Component\Uuid\Uuid;
use Drupal\Core\Database\Query\AlterableInterface;
use Drupal\Core\Database\Query\SelectInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\helfi_tunnistamo\Plugin\OpenIDConnectClient\Tunnistamo;
use Drupal\openid_connect\Entity\OpenIDConnectClientEntity;
Expand Down Expand Up @@ -36,7 +38,7 @@ function helfi_tunnistamo_form_user_form_alter(&$form, FormStateInterface $form_
return;
}

if (!\Drupal::service('externalauth.authmap')->getAll($account->id())) {
if (!Drupal::service('externalauth.authmap')->getAll($account->id())) {
return;
}
// Prevent users from changing the email if they have used openid_connect
Expand All @@ -63,7 +65,7 @@ function helfi_tunnistamo_create_email(array $userinfo) : string {
*/
function helfi_tunnistamo_openid_connect_userinfo_alter(
array &$userinfo,
array $context
array $context,
) : void {
$plugin = OpenIDConnectClientEntity::load($context['plugin_id'])?->getPlugin();

Expand All @@ -84,3 +86,16 @@ function helfi_tunnistamo_openid_connect_userinfo_alter(
}

}

/**
* Implements hook_query_TAG_alter().
*
* Exclude Tunnistamo users from User Expire feature.
*/
function helfi_tunnistamo_query_expired_users_alter(AlterableInterface $query) : void {
assert($query instanceof SelectInterface);
// Only load users that have no 'authmap' entry. This should exclude
// all Tunnistamo users.
$query->leftJoin('authmap', 'a', 'a.uid = users_field_data.uid');
$query->condition('a.uid', operator: 'IS NULL');
}
2 changes: 1 addition & 1 deletion src/Event/RedirectUrlEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class RedirectUrlEvent extends Event {
public function __construct(
private Url $redirectUrl,
private Request $request,
private Tunnistamo $client
private Tunnistamo $client,
) {
}

Expand Down
2 changes: 1 addition & 1 deletion src/EventSubscriber/HttpExceptionSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class HttpExceptionSubscriber extends HttpExceptionSubscriberBase {
public function __construct(
private EntityTypeManagerInterface $entityTypeManager,
private OpenIDConnectSession $session,
private AccountProxyInterface $accountProxy
private AccountProxyInterface $accountProxy,
) {
}

Expand Down
6 changes: 3 additions & 3 deletions src/Plugin/OpenIDConnectClient/Tunnistamo.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static function create(
ContainerInterface $container,
array $configuration,
$plugin_id,
$plugin_definition
$plugin_definition,
) : self {
$instance = parent::create($container, $configuration, $plugin_id, $plugin_definition);
$instance->eventDispatcher = $container->get('event_dispatcher');
Expand Down Expand Up @@ -99,7 +99,7 @@ private function isDebugLogEnabled(): bool {
*/
protected function getRedirectUrl(
array $route_parameters = [],
array $options = []
array $options = [],
): Url {
$url = parent::getRedirectUrl($route_parameters, $options);
/** @var \Drupal\helfi_tunnistamo\Event\RedirectUrlEvent $urlEvent */
Expand Down Expand Up @@ -150,7 +150,7 @@ public function getEndpoints(): array {
*/
public function buildConfigurationForm(
array $form,
FormStateInterface $form_state
FormStateInterface $form_state,
): array {
$form = parent::buildConfigurationForm($form, $form_state);

Expand Down
3 changes: 2 additions & 1 deletion tests/src/Kernel/KernelTestBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ protected function setUp(): void {
$this->installConfig('helfi_tunnistamo');
$this->installEntitySchema('action');
$this->installEntitySchema('user');
$this->installSchema('externalauth', ['authmap']);

}

Expand All @@ -66,7 +67,7 @@ protected function setupEndpoints(
?string $authorization = 'https://localhost/authorization',
?string $token = 'https://localhost/token',
?string $userinfo = 'https://localhost/userinfo',
?string $endSession = 'https://localhost/endsession'
?string $endSession = 'https://localhost/endsession',
) : void {
$this->container->get('kernel')->rebuildContainer();
$this->setPluginConfiguration('environment_url', $environmentUrl);
Expand Down
79 changes: 79 additions & 0 deletions tests/src/Kernel/UserExpireTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php

declare(strict_types=1);

namespace Drupal\Tests\helfi_tunnistamo\Kernel;

use Drupal\helfi_api_base\Features\FeatureManager;
use Drupal\helfi_api_base\UserExpire\UserExpireManager;
use Drupal\Tests\user\Traits\UserCreationTrait;
use Drupal\user\Entity\User;

/**
* Tests API Base's user expiration feature with Tunnistamo.
*
* @group helfi_tunnistamo
*/
class UserExpireTest extends KernelTestBase {

use UserCreationTrait;

/**
* {@inheritdoc}
*/
protected function setUp() : void {
parent::setUp();

/** @var \Drupal\helfi_api_base\Features\FeatureManager $featureManager */
$featureManager = $this->container->get(FeatureManager::class);
$featureManager->enableFeature(FeatureManager::USER_EXPIRE);
}

/**
* Gets the SUT.
*
* @return \Drupal\helfi_api_base\UserExpire\UserExpireManager
* The SUT.
*/
public function getSut() : UserExpireManager {
return $this->container->get(UserExpireManager::class);
}

/**
* Tests the expired users.
*/
public function testTunnistamoUsers() : void {
/** @var \Drupal\user\UserInterface[] $users */
$users = [
'1' => $this->createUser(),
'2' => $this->createUser(),
];

foreach ($users as $user) {
// Make sure users have never logged in.
$this->assertEquals(0, $user->getLastAccessedTime());
$this->assertTrue($user->getCreatedTime() > 0);
// Set access time over the threshold.
$user->setLastAccessTime(strtotime('-7 months'))
->save();
}
// Make sure both users are marked as expired.
$expired = $this->getSut()->getExpiredUserIds();
$this->assertEquals([1 => 1, 2 => 2], $expired);

/** @var \Drupal\externalauth\ExternalAuthInterface $externalAuth */
$externalAuth = $this->container->get('externalauth.externalauth');
$externalAuth->linkExistingAccount('123', 'openid_connect.tunnistamo', $users['2']);

// Make sure user 2 is not marked as expired after logging in using
// Tunnistamo.
$expired = $this->getSut()->getExpiredUserIds();
$this->assertArrayNotHasKey(2, $expired);

$this->getSut()->cancelExpiredUsers();

$this->assertTrue(User::load(1)->isBlocked());
$this->assertFalse(User::load(2)->isBlocked());
}

}
1 change: 0 additions & 1 deletion tests/src/Kernel/UserInfoAlterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class UserInfoAlterTest extends KernelTestBase {
protected function setUp() : void {
parent::setUp();

$this->installSchema('externalauth', ['authmap']);
$this->installSchema('user', ['users_data']);
$this->installConfig('user');
}
Expand Down

0 comments on commit e968d82

Please sign in to comment.