Skip to content

Commit 4b73c73

Browse files
thorstenthorsten
committed
Merge branch '4.0' into 'main'
2 parents 4964d17 + 44cd20f commit 4b73c73

5 files changed

Lines changed: 199 additions & 6 deletions

File tree

‎phpmyfaq/src/phpMyFAQ/Helper/RegistrationHelper.php‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use phpMyFAQ\Strings;
2424
use phpMyFAQ\Translation;
2525
use phpMyFAQ\User;
26+
use phpMyFAQ\User\UserData;
2627
use phpMyFAQ\Utils;
2728
use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
2829

@@ -42,17 +43,31 @@ public function __construct(Configuration $configuration)
4243
}
4344

4445
/**
45-
* Creates a new user account, sends mail and returns success or
46+
* Creates a new user account and saves the user data.
47+
* If user generation was successful, account activation is sent via an email
4648
* error message as an array.
4749
* The password will be automatically generated and sent by email
48-
* as soon if admin switch user to "active"
50+
* as soon if admin switches user to "active"
4951
*
5052
* @throws Exception|TransportExceptionInterface
5153
*/
5254
public function createUser(string $userName, string $fullName, string $email, bool $isVisible): array
5355
{
5456
$user = new User($this->configuration);
5557

58+
// Check if email already exists in the userdata table (even if the username is different)
59+
if (!empty($email)) {
60+
if (!$user->userdata instanceof UserData) {
61+
$user->userdata = new UserData($this->configuration);
62+
}
63+
if ($user->userdata->emailExists($email)) {
64+
return [
65+
'registered' => false,
66+
'error' => User::ERROR_USER_EMAIL_NOT_UNIQUE
67+
];
68+
}
69+
}
70+
5671
if (!$user->createUser($userName, '')) {
5772
return [
5873
'registered' => false,
@@ -99,14 +114,14 @@ public function createUser(string $userName, string $fullName, string $email, bo
99114
}
100115

101116
/**
102-
* Returns true, if the hostname of the given email address is allowed.
117+
* Returns true if the hostname of the given email address is allowed.
103118
* otherwise false.
104119
*/
105120
public function isDomainAllowed(string $email): bool
106121
{
107122
$whitelistedDomains = $this->configuration->get('security.domainWhiteListForRegistrations');
108123

109-
if (Strings::strlen($whitelistedDomains) === 0) {
124+
if ($whitelistedDomains === null || Strings::strlen(trim((string) $whitelistedDomains)) === 0) {
110125
return true;
111126
}
112127

‎phpmyfaq/src/phpMyFAQ/User.php‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ class User
6262

6363
final public const string ERROR_USER_LOGIN_NOT_UNIQUE = 'The Login name already exists.';
6464

65+
final public const string ERROR_USER_EMAIL_NOT_UNIQUE = 'The email address already exists.';
66+
6567
final public const string ERROR_USER_LOGIN_INVALID = 'The chosen login is invalid. A valid login has at least ' .
6668
'four characters. Only letters, numbers and underscore _ are allowed. The first letter must be a letter. ';
6769

@@ -358,14 +360,24 @@ public function createUser(string $login, string $pass = '', string $domain = ''
358360
throw new Exception(self::ERROR_USER_LOGIN_NOT_UNIQUE);
359361
}
360362

363+
// If $login is an email address, check if it already exists in the userdata table
364+
if ($this->isEmailAddress($login)) {
365+
if (!$this->userdata instanceof UserData) {
366+
$this->userdata = new UserData($this->configuration);
367+
}
368+
if ($this->userdata->emailExists($login)) {
369+
throw new Exception(self::ERROR_USER_EMAIL_NOT_UNIQUE);
370+
}
371+
}
372+
361373
// set user-ID
362374
if (0 === $userId) {
363375
$this->userId = $this->configuration->getDb()->nextId(Database::getTablePrefix() . 'faquser', 'user_id');
364376
} else {
365377
$this->userId = $userId;
366378
}
367379

368-
// create user entry
380+
// create a user entry
369381
$insert = sprintf(
370382
"INSERT INTO %sfaquser (user_id, login, session_timestamp, member_since) VALUES (%d, '%s', %d, '%s')",
371383
Database::getTablePrefix(),
@@ -785,7 +797,7 @@ public function getUserVisibilityByEmail(string $email): bool
785797

786798
/**
787799
* Returns true on success.
788-
* This will change a users' status to active, and send an email with a new password.
800+
* This will change a users' status to active and send an email with a new password.
789801
*
790802
* @throws Exception|TransportExceptionInterface
791803
*/
@@ -999,4 +1011,14 @@ public function getWebAuthnKeys(): string
9991011

10001012
return '';
10011013
}
1014+
1015+
/**
1016+
* Checks if a string is a valid email address.
1017+
*
1018+
* @param string $string String to check
1019+
*/
1020+
private function isEmailAddress(string $string): bool
1021+
{
1022+
return filter_var($string, FILTER_VALIDATE_EMAIL) !== false;
1023+
}
10021024
}

‎phpmyfaq/src/phpMyFAQ/User/UserData.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,4 +288,26 @@ public function delete(int $userId): bool
288288

289289
return true;
290290
}
291+
292+
/**
293+
* Checks if an email address already exists in the user data table.
294+
* Returns true if the email exists, false otherwise.
295+
*
296+
* @param string $email Email address to check
297+
*/
298+
public function emailExists(string $email): bool
299+
{
300+
if (empty($email)) {
301+
return false;
302+
}
303+
304+
$select = sprintf(
305+
"SELECT user_id FROM %sfaquserdata WHERE email = '%s'",
306+
Database::getTablePrefix(),
307+
$this->configuration->getDb()->escape($email)
308+
);
309+
310+
$res = $this->configuration->getDb()->query($select);
311+
return $this->configuration->getDb()->numRows($res) > 0;
312+
}
291313
}

‎tests/phpMyFAQ/User/UserDataTest.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,4 +92,30 @@ public function testDelete(): void
9292
$result = $this->userData->delete(1);
9393
$this->assertTrue($result);
9494
}
95+
96+
public function testEmailExistsReturnsTrueWhenEmailExists(): void
97+
{
98+
$this->database->method('escape')->willReturn('test@example.com');
99+
$this->database->method('query')->willReturn(true);
100+
$this->database->method('numRows')->willReturn(1);
101+
102+
$result = $this->userData->emailExists('test@example.com');
103+
$this->assertTrue($result);
104+
}
105+
106+
public function testEmailExistsReturnsFalseWhenEmailDoesNotExist(): void
107+
{
108+
$this->database->method('escape')->willReturn('nonexistent@example.com');
109+
$this->database->method('query')->willReturn(true);
110+
$this->database->method('numRows')->willReturn(0);
111+
112+
$result = $this->userData->emailExists('nonexistent@example.com');
113+
$this->assertFalse($result);
114+
}
115+
116+
public function testEmailExistsReturnsFalseForEmptyEmail(): void
117+
{
118+
$result = $this->userData->emailExists('');
119+
$this->assertFalse($result);
120+
}
95121
}

‎tests/phpMyFAQ/UserTest.php‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<?php
2+
3+
namespace phpMyFAQ;
4+
5+
use Exception;
6+
use phpMyFAQ\Database\Sqlite3;
7+
use phpMyFAQ\User\UserData;
8+
use PHPUnit\Framework\MockObject\MockObject;
9+
use PHPUnit\Framework\TestCase;
10+
use ReflectionClass;
11+
12+
class UserTest extends TestCase
13+
{
14+
private Configuration|MockObject $configuration;
15+
private Sqlite3|MockObject $database;
16+
private User $user;
17+
private UserData|MockObject $userData;
18+
19+
protected function setUp(): void
20+
{
21+
$this->configuration = $this->createMock(Configuration::class);
22+
$this->database = $this->createMock(Sqlite3::class);
23+
$this->userData = $this->createMock(UserData::class);
24+
25+
$this->configuration->method('getDb')->willReturn($this->database);
26+
$this->configuration->method('get')->willReturnMap([
27+
['security.permLevel', 'basic'],
28+
]);
29+
30+
$this->user = new User($this->configuration);
31+
$this->user->userdata = $this->userData;
32+
}
33+
34+
public function testIsEmailAddressReturnsTrueForValidEmail(): void
35+
{
36+
$reflection = new ReflectionClass($this->user);
37+
$method = $reflection->getMethod('isEmailAddress');
38+
39+
$result = $method->invoke($this->user, 'test@example.com');
40+
$this->assertTrue($result);
41+
}
42+
43+
public function testIsEmailAddressReturnsFalseForInvalidEmail(): void
44+
{
45+
$reflection = new ReflectionClass($this->user);
46+
$method = $reflection->getMethod('isEmailAddress');
47+
48+
$result = $method->invoke($this->user, 'invalid-email');
49+
$this->assertFalse($result);
50+
}
51+
52+
public function testCreateUserThrowsExceptionWhenEmailAlreadyExistsAsLogin(): void
53+
{
54+
$this->expectException(Exception::class);
55+
$this->expectExceptionMessage(User::ERROR_USER_EMAIL_NOT_UNIQUE);
56+
57+
// Mock that login is valid
58+
$this->database->method('escape')->willReturn('test@example.com');
59+
60+
// Mock that getUserByLogin returns false (login doesn't exist as login)
61+
$this->database->method('query')->willReturn(true);
62+
$this->database->method('numRows')->willReturn(0);
63+
64+
// Mock that email exists in userdata
65+
$this->userData->method('emailExists')->willReturn(true);
66+
67+
$this->user->createUser('test@example.com');
68+
}
69+
70+
public function testCreateUserDoesNotCheckEmailWhenLoginIsNotEmail(): void
71+
{
72+
// Mock database operations to simulate login not existing
73+
$this->database->method('escape')->willReturn('username');
74+
$this->database->method('query')->willReturn(true);
75+
$this->database->method('numRows')->willReturn(0);
76+
77+
// emailExists should never be called for non-email logins
78+
$this->userData->expects($this->never())->method('emailExists');
79+
80+
// This will throw an exception because no auth container is set up,
81+
// but that's expected - we just want to verify emailExists wasn't called
82+
try {
83+
$this->user->createUser('username');
84+
} catch (Exception $e) {
85+
// Expected - ignore this exception as we're only testing the email check logic
86+
}
87+
}
88+
89+
public function testCreateUserThrowsExceptionWhenLoginNotUnique(): void
90+
{
91+
$this->expectException(Exception::class);
92+
$this->expectExceptionMessage(User::ERROR_USER_LOGIN_NOT_UNIQUE);
93+
94+
// Mock that login exists
95+
$this->database->method('escape')->willReturn('existinguser');
96+
$this->database->method('query')->willReturn(true);
97+
$this->database->method('numRows')->willReturn(1);
98+
$this->database->method('fetchArray')->willReturn([
99+
'user_id' => 1,
100+
'login' => 'existinguser',
101+
'account_status' => 'active',
102+
'is_superadmin' => false,
103+
'auth_source' => 'local'
104+
]);
105+
106+
$this->user->createUser('existinguser');
107+
}
108+
}

0 commit comments

Comments
 (0)