Index: src/__phutil_library_map__.php =================================================================== --- src/__phutil_library_map__.php +++ 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', Index: src/applications/auth/controller/PhabricatorAuthRegisterController.php =================================================================== --- src/applications/auth/controller/PhabricatorAuthRegisterController.php +++ 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; Index: src/applications/people/editor/PhabricatorUserEditor.php =================================================================== --- src/applications/people/editor/PhabricatorUserEditor.php +++ 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()); } Index: src/applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php =================================================================== --- /dev/null +++ src/applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php @@ -0,0 +1,68 @@ + 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); + } + +} Index: src/applications/people/storage/PhabricatorUserEmail.php =================================================================== --- src/applications/people/storage/PhabricatorUserEmail.php +++ 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; Index: src/applications/settings/panel/PhabricatorSettingsPanelEmailAddresses.php =================================================================== --- src/applications/settings/panel/PhabricatorSettingsPanelEmailAddresses.php +++ 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(); }