Page MenuHomePhabricator
Files F15397179

D8308.id19757.diff
No OneTemporary
Actions

D8308.id19757.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2175,6 +2175,7 @@
'PhabricatorUserCustomFieldStringIndex' => 'applications/people/storage/PhabricatorUserCustomFieldStringIndex.php',
'PhabricatorUserDAO' => 'applications/people/storage/PhabricatorUserDAO.php',
'PhabricatorUserEditor' => 'applications/people/editor/PhabricatorUserEditor.php',
+ 'PhabricatorUserEditorTestCase' => 'applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php',
'PhabricatorUserEmail' => 'applications/people/storage/PhabricatorUserEmail.php',
'PhabricatorUserLog' => 'applications/people/storage/PhabricatorUserLog.php',
'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php',
@@ -4997,6 +4998,7 @@
'PhabricatorUserCustomFieldStringIndex' => 'PhabricatorCustomFieldStringIndexStorage',
'PhabricatorUserDAO' => 'PhabricatorLiskDAO',
'PhabricatorUserEditor' => 'PhabricatorEditor',
+ 'PhabricatorUserEditorTestCase' => 'PhabricatorTestCase',
'PhabricatorUserEmail' => 'PhabricatorUserDAO',
'PhabricatorUserLog' => 'PhabricatorUserDAO',
'PhabricatorUserPreferences' => 'PhabricatorUserDAO',
diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
--- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php
+++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
@@ -200,8 +200,11 @@
if (!strlen($value_email)) {
$e_email = pht('Required');
$errors[] = pht('Email is required.');
- } else if (!PhabricatorUserEmail::isAllowedAddress($value_email)) {
+ } else if (!PhabricatorUserEmail::isValidAddress($value_email)) {
$e_email = pht('Invalid');
+ $errors[] = PhabricatorUserEmail::describeValidAddresses();
+ } else if (!PhabricatorUserEmail::isAllowedAddress($value_email)) {
+ $e_email = pht('Disallowed');
$errors[] = PhabricatorUserEmail::describeAllowedAddresses();
} else {
$e_email = null;
diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php
--- a/src/applications/people/editor/PhabricatorUserEditor.php
+++ b/src/applications/people/editor/PhabricatorUserEditor.php
@@ -570,6 +570,10 @@
// user friendly errors for us, but we omit the courtesy checks on some
// pathways like administrative scripts for simplicity.
+ if (!PhabricatorUserEmail::isValidAddress($email->getAddress())) {
+ throw new Exception(PhabricatorUserEmail::describeValidAddresses());
+ }
+
if (!PhabricatorUserEmail::isAllowedAddress($email->getAddress())) {
throw new Exception(PhabricatorUserEmail::describeAllowedAddresses());
}
diff --git a/src/applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php b/src/applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php
@@ -0,0 +1,68 @@
+<?php
+
+final class PhabricatorUserEditorTestCase extends PhabricatorTestCase {
+
+ protected function getPhabricatorTestCaseConfiguration() {
+ return array(
+ self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => true,
+ );
+ }
+
+ public function testRegistrationEmailOK() {
+ $env = PhabricatorEnv::beginScopedEnv();
+ $env->overrideEnvConfig('auth.email-domains', array('example.com'));
+
+ $this->registerUser(
+ 'PhabricatorUserEditorTestCaseOK',
+ 'PhabricatorUserEditorTestCase@example.com');
+ }
+
+ public function testRegistrationEmailInvalid() {
+ $env = PhabricatorEnv::beginScopedEnv();
+ $env->overrideEnvConfig('auth.email-domains', array('example.com'));
+
+ $prefix = str_repeat('a', PhabricatorUserEmail::MAX_ADDRESS_LENGTH);
+ $email = $prefix.'@evil.com@example.com';
+
+ try {
+ $this->registerUser(
+ 'PhabricatorUserEditorTestCaseInvalid',
+ $email);
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $this->assertEqual(true, ($caught instanceof Exception));
+ }
+
+ public function testRegistrationEmailDomain() {
+ $env = PhabricatorEnv::beginScopedEnv();
+ $env->overrideEnvConfig('auth.email-domains', array('example.com'));
+
+ $caught = null;
+ try {
+ $this->registerUser(
+ 'PhabricatorUserEditorTestCaseDomain',
+ 'PhabricatorUserEditorTestCase@whitehouse.gov');
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $this->assertEqual(true, ($caught instanceof Exception));
+ }
+
+ private function registerUser($username, $email) {
+ $user = id(new PhabricatorUser())
+ ->setUsername($username)
+ ->setRealname($username);
+
+ $email = id(new PhabricatorUserEmail())
+ ->setAddress($email)
+ ->setIsVerified(0);
+
+ id(new PhabricatorUserEditor())
+ ->setActor($user)
+ ->createNewUser($user, $email);
+ }
+
+}
diff --git a/src/applications/people/storage/PhabricatorUserEmail.php b/src/applications/people/storage/PhabricatorUserEmail.php
--- a/src/applications/people/storage/PhabricatorUserEmail.php
+++ b/src/applications/people/storage/PhabricatorUserEmail.php
@@ -12,6 +12,8 @@
protected $isPrimary;
protected $verificationCode;
+ const MAX_ADDRESS_LENGTH = 128;
+
public function getVerificationURI() {
return '/emailverify/'.$this->getVerificationCode().'/';
}
@@ -30,7 +32,33 @@
/**
* @task restrictions
*/
+ public static function isValidAddress($address) {
+ if (strlen($address) > self::MAX_ADDRESS_LENGTH) {
+ return false;
+ }
+
+ return true;
+ }
+
+
+ /**
+ * @task restrictions
+ */
+ public static function describeValidAddresses() {
+ return pht(
+ 'The maximum length of an email address is %d character(s).',
+ new PhutilNumber(self::MAX_ADDRESS_LENGTH));
+ }
+
+
+ /**
+ * @task restrictions
+ */
public static function isAllowedAddress($address) {
+ if (!self::isValidAddress($address)) {
+ return false;
+ }
+
$allowed_domains = PhabricatorEnv::getEnvConfig('auth.email-domains');
if (!$allowed_domains) {
return true;
diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelEmailAddresses.php b/src/applications/settings/panel/PhabricatorSettingsPanelEmailAddresses.php
--- a/src/applications/settings/panel/PhabricatorSettingsPanelEmailAddresses.php
+++ b/src/applications/settings/panel/PhabricatorSettingsPanelEmailAddresses.php
@@ -173,8 +173,11 @@
if (!strlen($email)) {
$e_email = pht('Required');
$errors[] = pht('Email is required.');
- } else if (!PhabricatorUserEmail::isAllowedAddress($email)) {
+ } else if (!PhabricatorUserEmail::isValidAddress($email)) {
$e_email = pht('Invalid');
+ $errors[] = PhabricatorUserEmail::describeValidAddresses();
+ } else if (!PhabricatorUserEmail::isAllowedAddress($email)) {
+ $e_email = pht('Disallowed');
$errors[] = PhabricatorUserEmail::describeAllowedAddresses();
}

File Metadata

Mime Type
text/plain
Expires
Mon, Mar 17, 6:39 PM (6 d, 20 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7708726
Default Alt Text
D8308.id19757.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility