@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Include specific details about why the username is invalid
Summary: Inspired by T16386
Test Plan:
For each of the following flows, enter various invalid usernames and see sensible output:
- https://phorge.localhost/auth/register
- The "rename user" workflow in http://phorge.localhost/people/manage/1/
- http://phorge.localhost/people/new/standard/
The `PhabricatorUserEditor.php` class change can't be tested - it's a low-level failsafe that should be unreachable because higher-level code should validate usernames.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno, aklapper
Differential Revision: https://we.phorge.it/D26567
+26
-12
+1
-1
src/applications/auth/controller/PhabricatorAuthRegisterController.php
+1
-1
src/applications/auth/controller/PhabricatorAuthRegisterController.php
···
293
293
$errors[] = pht('Username is required.');
294
294
} else if (!PhabricatorUser::validateUsername($value_username)) {
295
295
$e_username = pht('Invalid');
296
-
$errors[] = PhabricatorUser::describeValidUsername();
296
+
$errors[] = PhabricatorUser::describeValidUsername($value_username);
297
297
} else {
298
298
$e_username = null;
299
299
}
+5
-3
src/applications/people/controller/PhabricatorPeopleNewController.php
+5
-3
src/applications/people/controller/PhabricatorPeopleNewController.php
···
62
62
63
63
$user->setRealName($request->getStr('realname'));
64
64
65
-
if (!strlen($user->getUsername())) {
65
+
$username = $user->getUsername();
66
+
67
+
if (!strlen($username)) {
66
68
$errors[] = pht('Username is required.');
67
69
$e_username = pht('Required');
68
-
} else if (!PhabricatorUser::validateUsername($user->getUsername())) {
69
-
$errors[] = PhabricatorUser::describeValidUsername();
70
+
} else if (!PhabricatorUser::validateUsername($username)) {
71
+
$errors[] = PhabricatorUser::describeValidUsername($username);
70
72
$e_username = pht('Invalid');
71
73
} else {
72
74
$e_username = null;
+3
-2
src/applications/people/editor/PhabricatorUserEditor.php
+3
-2
src/applications/people/editor/PhabricatorUserEditor.php
···
43
43
}
44
44
}
45
45
46
-
if (!PhabricatorUser::validateUsername($user->getUsername())) {
47
-
$valid = PhabricatorUser::describeValidUsername();
46
+
$name = $user->getUsername();
47
+
if (!PhabricatorUser::validateUsername($name)) {
48
+
$valid = PhabricatorUser::describeValidUsername($name);
48
49
throw new Exception(pht('Username is invalid! %s', $valid));
49
50
}
50
51
+16
-5
src/applications/people/storage/PhabricatorUser.php
+16
-5
src/applications/people/storage/PhabricatorUser.php
···
528
528
}
529
529
}
530
530
531
-
public static function describeValidUsername() {
531
+
public static function describeValidUsername($bad_username) {
532
+
if (strlen($bad_username) > self::MAXIMUM_USERNAME_LENGTH) {
533
+
return pht(
534
+
'Usernames must have no more than %s characters.',
535
+
new PhutilNumber(self::MAXIMUM_USERNAME_LENGTH));
536
+
}
537
+
if (strpos($bad_username, ' ') !== false) {
538
+
return pht('Usernames cannot contain spaces. Please replace them '.
539
+
'with underscores, hyphens, periods, or some other way of separating '.
540
+
'words.');
541
+
}
542
+
if (substr($bad_username, -1) === '.') {
543
+
return pht('Usernames cannot end with a period.');
544
+
}
532
545
return pht(
533
-
'Usernames must contain only numbers, letters, period, underscore, and '.
534
-
'hyphen, and can not end with a period. They must have no more than %d '.
535
-
'characters.',
536
-
new PhutilNumber(self::MAXIMUM_USERNAME_LENGTH));
546
+
'Usernames must contain only numbers, Latin letters, period, '.
547
+
'underscore and hyphen.');
537
548
}
538
549
539
550
public static function validateUsername($username) {
+1
-1
src/applications/people/xaction/PhabricatorUserUsernameTransaction.php
+1
-1
src/applications/people/xaction/PhabricatorUserUsernameTransaction.php
···
87
87
$xaction);
88
88
} else if (!PhabricatorUser::validateUsername($new)) {
89
89
$errors[] = $this->newInvalidError(
90
-
PhabricatorUser::describeValidUsername(),
90
+
PhabricatorUser::describeValidUsername($new),
91
91
$xaction);
92
92
} else if ($this->generateOldValue($object) === $new) {
93
93
$errors[] = $this->newInvalidError(