From c704db94800243cbf1c4eb9a2a8c1e260e9eb75e Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Tue, 11 Apr 2023 13:07:01 +0100
Subject: [PATCH] Issue #3311563 by stefanos.petrakis, rpayanm, smustgrave,
longwave, Wim Leers: Safeguarding against UnblockUser::execute()'s method
unblocking the anonymous user
---
core/modules/user/src/AccountForm.php | 2 +-
core/modules/user/src/Entity/User.php | 5 +-
.../user/src/UserAccessControlHandler.php | 2 +-
.../src/Kernel/UserAccountFormFieldsTest.php | 23 ++++++++--
.../UserAccountFormPasswordResetTest.php | 11 +++--
.../src/Kernel/UserAnonymousActivateTest.php | 46 +++++++++++++++++++
.../src/Unit/UserAccessControlHandlerTest.php | 16 +++----
7 files changed, 84 insertions(+), 21 deletions(-)
create mode 100644 core/modules/user/tests/src/Kernel/UserAnonymousActivateTest.php
diff --git a/core/modules/user/src/AccountForm.php b/core/modules/user/src/AccountForm.php
index 1cbd8704c1c9..e51b0f9ea0d1 100644
--- a/core/modules/user/src/AccountForm.php
+++ b/core/modules/user/src/AccountForm.php
@@ -72,7 +72,7 @@ public function form(array $form, FormStateInterface $form_state) {
$language_interface = \Drupal::languageManager()->getCurrentLanguage();
// Check for new account.
- $register = $account->isAnonymous();
+ $register = $account->isNew();
// For a new account, there are 2 sub-cases:
// $self_register: A user creates their own, new, account
diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php
index 1df5d88dce65..9a653b191892 100644
--- a/core/modules/user/src/Entity/User.php
+++ b/core/modules/user/src/Entity/User.php
@@ -305,6 +305,9 @@ public function isBlocked() {
* {@inheritdoc}
*/
public function activate() {
+ if ($this->isAnonymous()) {
+ throw new \LogicException('The anonymous user account should remain blocked at all times.');
+ }
$this->get('status')->value = 1;
return $this;
}
@@ -370,7 +373,7 @@ public function isAuthenticated() {
* {@inheritdoc}
*/
public function isAnonymous() {
- return $this->id() == 0;
+ return $this->id() === 0 || $this->id() === '0';
}
/**
diff --git a/core/modules/user/src/UserAccessControlHandler.php b/core/modules/user/src/UserAccessControlHandler.php
index a231d5214de9..46b97cfa2e1e 100644
--- a/core/modules/user/src/UserAccessControlHandler.php
+++ b/core/modules/user/src/UserAccessControlHandler.php
@@ -101,7 +101,7 @@ protected function checkFieldAccess($operation, FieldDefinitionInterface $field_
case 'name':
// Allow view access to anyone with access to the entity.
// The username field is editable during the registration process.
- if ($operation == 'view' || ($items && $items->getEntity()->isAnonymous())) {
+ if ($operation == 'view' || ($items && $items->getEntity()->isNew())) {
return AccessResult::allowed()->cachePerPermissions();
}
// Allow edit access for the own user name if the permission is
diff --git a/core/modules/user/tests/src/Kernel/UserAccountFormFieldsTest.php b/core/modules/user/tests/src/Kernel/UserAccountFormFieldsTest.php
index 1aabea79a8a5..0eac0422d089 100644
--- a/core/modules/user/tests/src/Kernel/UserAccountFormFieldsTest.php
+++ b/core/modules/user/tests/src/Kernel/UserAccountFormFieldsTest.php
@@ -4,6 +4,8 @@
use Drupal\Core\Form\FormState;
use Drupal\KernelTests\KernelTestBase;
+use Drupal\user\Entity\User;
+use Drupal\user\UserInterface;
/**
* Verifies that the field order in user account forms is compatible with
@@ -20,6 +22,11 @@ class UserAccountFormFieldsTest extends KernelTestBase {
*/
protected static $modules = ['system', 'user', 'field'];
+ /**
+ * @var \Drupal\user\UserInterface
+ */
+ protected UserInterface $user;
+
/**
* Tests the root user account form section in the "Configure site" form.
*/
@@ -72,6 +79,10 @@ public function testUserRegistrationForm() {
public function testUserEditForm() {
// Install default configuration; required for AccountFormController.
$this->installConfig(['user']);
+ $this->installEntitySchema('user');
+
+ $this->user = User::create(['name' => 'test']);
+ $this->user->save();
$form = $this->buildAccountForm('default');
@@ -127,13 +138,15 @@ protected function assertFieldOrder(array $elements): void {
protected function buildAccountForm($operation) {
// @see HtmlEntityFormController::getFormObject()
$entity_type = 'user';
- $fields = [];
if ($operation != 'register') {
- $fields['uid'] = 2;
+ // Use an existing user.
+ $entity = $this->user;
+ }
+ else {
+ $entity = $this->container->get('entity_type.manager')
+ ->getStorage($entity_type)
+ ->create();
}
- $entity = $this->container->get('entity_type.manager')
- ->getStorage($entity_type)
- ->create($fields);
// @see EntityFormBuilder::getForm()
return $this->container->get('entity.form_builder')->getForm($entity, $operation);
diff --git a/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php b/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php
index 703775bdd564..1bee4d3893a9 100644
--- a/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php
+++ b/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php
@@ -87,13 +87,14 @@ public function testPasswordResetToken() {
protected function buildAccountForm($operation) {
// @see HtmlEntityFormController::getFormObject()
$entity_type = 'user';
- $fields = [];
if ($operation != 'register') {
- $fields['uid'] = $this->user->id();
+ $entity = $this->user;
+ }
+ else {
+ $entity = $this->container->get('entity_type.manager')
+ ->getStorage($entity_type)
+ ->create();
}
- $entity = $this->container->get('entity_type.manager')
- ->getStorage($entity_type)
- ->create($fields);
// @see EntityFormBuilder::getForm()
return $this->container->get('entity.form_builder')->getForm($entity, $operation);
diff --git a/core/modules/user/tests/src/Kernel/UserAnonymousActivateTest.php b/core/modules/user/tests/src/Kernel/UserAnonymousActivateTest.php
new file mode 100644
index 000000000000..2aa2046e0f97
--- /dev/null
+++ b/core/modules/user/tests/src/Kernel/UserAnonymousActivateTest.php
@@ -0,0 +1,46 @@
+<?php
+
+namespace Drupal\Tests\user\Kernel;
+
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * Tests unblocking the anonymous user account.
+ *
+ * @group user
+ */
+class UserAnonymousActivateTest extends KernelTestBase {
+
+ /**
+ * Modules to enable.
+ *
+ * @var array
+ */
+ protected static $modules = ['user'];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+ $this->container->get('module_handler')->loadInclude('user', 'install');
+ $this->installEntitySchema('user');
+ user_install();
+ }
+
+ /**
+ * Tests that the anonymous user cannot be activated.
+ */
+ public function testAnonymousActivate() {
+ $accountAnon = \Drupal::entityTypeManager()->getStorage('user')->load(0);
+
+ // Test that the anonymous user is blocked.
+ $this->assertTrue($accountAnon->isBlocked());
+
+ // Test that the anonymous user cannot be activated.
+ $this->expectException(\LogicException::class);
+ $this->expectExceptionMessage('The anonymous user account should remain blocked at all times.');
+ $accountAnon->activate();
+ }
+
+}
diff --git a/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php b/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php
index 333d7bcff3ac..de7482a3ba2e 100644
--- a/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php
+++ b/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php
@@ -28,28 +28,28 @@ class UserAccessControlHandlerTest extends UnitTestCase {
/**
* The mock user account with view access.
*
- * @var \Drupal\Core\Session\AccountInterface
+ * @var \Drupal\user\UserInterface
*/
protected $viewer;
/**
* The mock user account with 'view user email addresses' permission.
*
- * @var \Drupal\Core\Session\AccountInterface
+ * @var \Drupal\user\UserInterface
*/
protected $emailViewer;
/**
* The mock user account that is able to change their own account name.
*
- * @var \Drupal\Core\Session\AccountInterface
+ * @var \Drupal\user\UserInterface
*/
protected $owner;
/**
* The mock administrative test user.
*
- * @var \Drupal\Core\Session\AccountInterface
+ * @var \Drupal\user\UserInterface
*/
protected $admin;
@@ -73,7 +73,7 @@ protected function setUp(): void {
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
- $this->viewer = $this->createMock('\Drupal\Core\Session\AccountInterface');
+ $this->viewer = $this->createMock('\Drupal\user\UserInterface');
$this->viewer
->expects($this->any())
->method('hasPermission')
@@ -83,7 +83,7 @@ protected function setUp(): void {
->method('id')
->willReturn(1);
- $this->owner = $this->createMock('\Drupal\Core\Session\AccountInterface');
+ $this->owner = $this->createMock('\Drupal\user\UserInterface');
$this->owner
->expects($this->any())
->method('hasPermission')
@@ -97,13 +97,13 @@ protected function setUp(): void {
->method('id')
->willReturn(2);
- $this->admin = $this->createMock('\Drupal\Core\Session\AccountInterface');
+ $this->admin = $this->createMock('\Drupal\user\UserInterface');
$this->admin
->expects($this->any())
->method('hasPermission')
->willReturn(TRUE);
- $this->emailViewer = $this->createMock('\Drupal\Core\Session\AccountInterface');
+ $this->emailViewer = $this->createMock('\Drupal\user\UserInterface');
$this->emailViewer
->expects($this->any())
->method('hasPermission')
--
GitLab