From affcc9283c2e1a8d0c47c51273132509f9cb59ca Mon Sep 17 00:00:00 2001 From: tuutti Date: Thu, 25 Apr 2024 11:41:30 +0300 Subject: [PATCH 1/5] UHF-9466: Exclude Tunnistamo users from User Expire, automatic phpcs fixes --- helfi_tunnistamo.module | 19 ++++- src/Event/RedirectUrlEvent.php | 2 +- .../HttpExceptionSubscriber.php | 2 +- src/Plugin/OpenIDConnectClient/Tunnistamo.php | 6 +- tests/src/Kernel/KernelTestBase.php | 3 +- tests/src/Kernel/UserExpireTest.php | 79 +++++++++++++++++++ 6 files changed, 103 insertions(+), 8 deletions(-) create mode 100644 tests/src/Kernel/UserExpireTest.php diff --git a/helfi_tunnistamo.module b/helfi_tunnistamo.module index 9dee38c..bf3aa72 100644 --- a/helfi_tunnistamo.module +++ b/helfi_tunnistamo.module @@ -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; @@ -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 @@ -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(); @@ -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'); +} diff --git a/src/Event/RedirectUrlEvent.php b/src/Event/RedirectUrlEvent.php index 3e35c49..58c0cda 100644 --- a/src/Event/RedirectUrlEvent.php +++ b/src/Event/RedirectUrlEvent.php @@ -27,7 +27,7 @@ final class RedirectUrlEvent extends Event { public function __construct( private Url $redirectUrl, private Request $request, - private Tunnistamo $client + private Tunnistamo $client, ) { } diff --git a/src/EventSubscriber/HttpExceptionSubscriber.php b/src/EventSubscriber/HttpExceptionSubscriber.php index 3bac0f0..d9d6fdb 100644 --- a/src/EventSubscriber/HttpExceptionSubscriber.php +++ b/src/EventSubscriber/HttpExceptionSubscriber.php @@ -29,7 +29,7 @@ final class HttpExceptionSubscriber extends HttpExceptionSubscriberBase { public function __construct( private EntityTypeManagerInterface $entityTypeManager, private OpenIDConnectSession $session, - private AccountProxyInterface $accountProxy + private AccountProxyInterface $accountProxy, ) { } diff --git a/src/Plugin/OpenIDConnectClient/Tunnistamo.php b/src/Plugin/OpenIDConnectClient/Tunnistamo.php index 0cf3dc9..7e6d911 100644 --- a/src/Plugin/OpenIDConnectClient/Tunnistamo.php +++ b/src/Plugin/OpenIDConnectClient/Tunnistamo.php @@ -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'); @@ -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 */ @@ -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); diff --git a/tests/src/Kernel/KernelTestBase.php b/tests/src/Kernel/KernelTestBase.php index 23d43a0..925669e 100644 --- a/tests/src/Kernel/KernelTestBase.php +++ b/tests/src/Kernel/KernelTestBase.php @@ -42,6 +42,7 @@ protected function setUp(): void { $this->installConfig('helfi_tunnistamo'); $this->installEntitySchema('action'); $this->installEntitySchema('user'); + $this->installSchema('externalauth', ['authmap']); } @@ -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); diff --git a/tests/src/Kernel/UserExpireTest.php b/tests/src/Kernel/UserExpireTest.php new file mode 100644 index 0000000..52afe48 --- /dev/null +++ b/tests/src/Kernel/UserExpireTest.php @@ -0,0 +1,79 @@ +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()); + } + +} From 9baa2b250bcdb418103c87514fab33c06e1c991d Mon Sep 17 00:00:00 2001 From: tuutti Date: Thu, 25 Apr 2024 11:45:52 +0300 Subject: [PATCH 2/5] UHF-9466: Phpstan fixes --- tests/src/Kernel/UserExpireTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Kernel/UserExpireTest.php b/tests/src/Kernel/UserExpireTest.php index 52afe48..27d1b17 100644 --- a/tests/src/Kernel/UserExpireTest.php +++ b/tests/src/Kernel/UserExpireTest.php @@ -24,7 +24,7 @@ class UserExpireTest extends KernelTestBase { protected function setUp() : void { parent::setUp(); - /** @var \Drupal\helfi_api_base\Features\FeatureManager $manager */ + /** @var \Drupal\helfi_api_base\Features\FeatureManager $featureManager */ $featureManager = $this->container->get(FeatureManager::class); $featureManager->enableFeature(FeatureManager::USER_EXPIRE); } From 3b494ad16b084df9fc16ca6f9d057ebb4db85b65 Mon Sep 17 00:00:00 2001 From: tuutti Date: Thu, 25 Apr 2024 11:48:43 +0300 Subject: [PATCH 3/5] UHF-9466: Require latest api-base --- composer.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/composer.json b/composer.json index 5f8b336..ce2c6f0 100644 --- a/composer.json +++ b/composer.json @@ -8,6 +8,9 @@ "drupal/helfi_api_base": "*", "drupal/openid_connect": "^3.0" }, + "conflict": { + "drupal/helfi_api_base": "<=2.7.1" + }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0", "drupal/coder": "^8.3" From e9d8e7d62839f453d15d73f3ade863572f6fee44 Mon Sep 17 00:00:00 2001 From: tuutti Date: Thu, 25 Apr 2024 11:50:11 +0300 Subject: [PATCH 4/5] UHF-9466: Reverted conflict --- composer.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/composer.json b/composer.json index ce2c6f0..5f8b336 100644 --- a/composer.json +++ b/composer.json @@ -8,9 +8,6 @@ "drupal/helfi_api_base": "*", "drupal/openid_connect": "^3.0" }, - "conflict": { - "drupal/helfi_api_base": "<=2.7.1" - }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0", "drupal/coder": "^8.3" From c9bfede1303616bf3299c691d75bbcdd397d153d Mon Sep 17 00:00:00 2001 From: tuutti Date: Tue, 30 Apr 2024 08:55:49 +0300 Subject: [PATCH 5/5] UHF-9466: Fixed UserInfoAlterTest --- tests/src/Kernel/UserInfoAlterTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/src/Kernel/UserInfoAlterTest.php b/tests/src/Kernel/UserInfoAlterTest.php index 4a21428..d1ff81a 100644 --- a/tests/src/Kernel/UserInfoAlterTest.php +++ b/tests/src/Kernel/UserInfoAlterTest.php @@ -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'); }