Page MenuHomePhabricator

D18902.diff
No OneTemporary

D18902.diff

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
@@ -2092,6 +2092,7 @@
'PhabricatorAuthPassword' => 'applications/auth/storage/PhabricatorAuthPassword.php',
'PhabricatorAuthPasswordEditor' => 'applications/auth/editor/PhabricatorAuthPasswordEditor.php',
'PhabricatorAuthPasswordEngine' => 'applications/auth/engine/PhabricatorAuthPasswordEngine.php',
+ 'PhabricatorAuthPasswordException' => 'applications/auth/password/PhabricatorAuthPasswordException.php',
'PhabricatorAuthPasswordPHIDType' => 'applications/auth/phid/PhabricatorAuthPasswordPHIDType.php',
'PhabricatorAuthPasswordQuery' => 'applications/auth/query/PhabricatorAuthPasswordQuery.php',
'PhabricatorAuthPasswordResetTemporaryTokenType' => 'applications/auth/tokentype/PhabricatorAuthPasswordResetTemporaryTokenType.php',
@@ -7387,6 +7388,7 @@
),
'PhabricatorAuthPasswordEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorAuthPasswordEngine' => 'Phobject',
+ 'PhabricatorAuthPasswordException' => 'Exception',
'PhabricatorAuthPasswordPHIDType' => 'PhabricatorPHIDType',
'PhabricatorAuthPasswordQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorAuthPasswordResetTemporaryTokenType' => 'PhabricatorAuthTemporaryTokenType',
diff --git a/src/applications/auth/engine/PhabricatorAuthPasswordEngine.php b/src/applications/auth/engine/PhabricatorAuthPasswordEngine.php
--- a/src/applications/auth/engine/PhabricatorAuthPasswordEngine.php
+++ b/src/applications/auth/engine/PhabricatorAuthPasswordEngine.php
@@ -54,6 +54,79 @@
return $this->upgradeHashers;
}
+ public function checkNewPassword(
+ PhutilOpaqueEnvelope $password,
+ PhutilOpaqueEnvelope $confirm,
+ $can_skip = false) {
+
+ $raw_password = $password->openEnvelope();
+
+ if (!strlen($raw_password)) {
+ if ($can_skip) {
+ throw new PhabricatorAuthPasswordException(
+ pht('You must choose a password or skip this step.'),
+ pht('Required'));
+ } else {
+ throw new PhabricatorAuthPasswordException(
+ pht('You must choose a password.'),
+ pht('Required'));
+ }
+ }
+
+ $min_len = PhabricatorEnv::getEnvConfig('account.minimum-password-length');
+ $min_len = (int)$min_len;
+ if ($min_len) {
+ if (strlen($raw_password) < $min_len) {
+ throw new PhabricatorAuthPasswordException(
+ pht(
+ 'The selected password is too short. Passwords must be a minimum '.
+ 'of %s characters long.',
+ new PhutilNumber($min_len)),
+ pht('Too Short'));
+ }
+ }
+
+ $raw_confirm = $confirm->openEnvelope();
+
+ if (!strlen($raw_confirm)) {
+ throw new PhabricatorAuthPasswordException(
+ pht('You must confirm the selected password.'),
+ null,
+ pht('Required'));
+ }
+
+ if ($raw_password !== $raw_confirm) {
+ throw new PhabricatorAuthPasswordException(
+ pht('The password and confirmation do not match.'),
+ pht('Invalid'),
+ pht('Invalid'));
+ }
+
+ if (PhabricatorCommonPasswords::isCommonPassword($raw_password)) {
+ throw new PhabricatorAuthPasswordException(
+ pht(
+ 'The selected password is very weak: it is one of the most common '.
+ 'passwords in use. Choose a stronger password.'),
+ pht('Very Weak'));
+ }
+
+ if ($this->isRevokedPassword($password)) {
+ throw new PhabricatorAuthPasswordException(
+ pht(
+ 'The password you entered has been revoked. You can not reuse '.
+ 'a password which has been revoked. Choose a new password.'),
+ pht('Revoked'));
+ }
+
+ if (!$this->isUniquePassword($password)) {
+ throw new PhabricatorAuthPasswordException(
+ pht(
+ 'The password you entered is the same as another password '.
+ 'associated with your account. Each password must be unique.'),
+ pht('Not Unique'));
+ }
+ }
+
public function isValidPassword(PhutilOpaqueEnvelope $envelope) {
$this->requireSetup();
diff --git a/src/applications/auth/password/PhabricatorAuthPasswordException.php b/src/applications/auth/password/PhabricatorAuthPasswordException.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/password/PhabricatorAuthPasswordException.php
@@ -0,0 +1,28 @@
+<?php
+
+final class PhabricatorAuthPasswordException
+ extends Exception {
+
+ private $passwordError;
+ private $confirmErorr;
+
+ public function __construct(
+ $message,
+ $password_error,
+ $confirm_error = null) {
+
+ $this->passwordError = $password_error;
+ $this->confirmError = $confirm_error;
+
+ parent::__construct($message);
+ }
+
+ public function getPasswordError() {
+ return $this->passwordError;
+ }
+
+ public function getConfirmError() {
+ return $this->confirmError;
+ }
+
+}
diff --git a/src/applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php b/src/applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php
--- a/src/applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php
+++ b/src/applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php
@@ -58,6 +58,19 @@
$e_password = true;
$e_confirm = true;
+ $content_source = PhabricatorContentSource::newFromRequest($request);
+
+ // NOTE: This test is against $viewer (not $user), so that the error
+ // message below makes sense in the case that the two are different,
+ // and because an admin reusing their own password is bad, while
+ // system agents generally do not have passwords anyway.
+
+ $engine = id(new PhabricatorAuthPasswordEngine())
+ ->setViewer($viewer)
+ ->setContentSource($content_source)
+ ->setObject($viewer)
+ ->setPasswordType($vcs_type);
+
if ($request->isFormPost()) {
if ($request->getBool('remove')) {
if ($vcspassword->getID()) {
@@ -68,71 +81,26 @@
$new_password = $request->getStr('password');
$confirm = $request->getStr('confirm');
- if (!strlen($new_password)) {
- $e_password = pht('Required');
- $errors[] = pht('Password is required.');
- } else {
- $e_password = null;
- }
- if (!strlen($confirm)) {
- $e_confirm = pht('Required');
- $errors[] = pht('You must confirm the new password.');
- } else {
+ $envelope = new PhutilOpaqueEnvelope($new_password);
+ $confirm_envelope = new PhutilOpaqueEnvelope($confirm);
+
+ try {
+ $engine->checkNewPassword($envelope, $confirm_envelope);
+ $e_password = null;
$e_confirm = null;
+ } catch (PhabricatorAuthPasswordException $ex) {
+ $errors[] = $ex->getMessage();
+ $e_password = $ex->getPasswordError();
+ $e_confirm = $ex->getConfirmError();
}
if (!$errors) {
- $envelope = new PhutilOpaqueEnvelope($new_password);
- $content_source = PhabricatorContentSource::newFromRequest($request);
-
- // NOTE: This test is against $viewer (not $user), so that the error
- // message below makes sense in the case that the two are different,
- // and because an admin reusing their own password is bad, while
- // system agents generally do not have passwords anyway.
-
- $engine = id(new PhabricatorAuthPasswordEngine())
- ->setViewer($viewer)
- ->setContentSource($content_source)
- ->setObject($viewer)
- ->setPasswordType($vcs_type);
-
- $same_password = !$engine->isUniquePassword($envelope);
- $revoked_password = $engine->isRevokedPassword($envelope);
-
- if ($new_password !== $confirm) {
- $e_password = pht('Does Not Match');
- $e_confirm = pht('Does Not Match');
- $errors[] = pht('Password and confirmation do not match.');
- } else if ($revoked_password) {
- $e_password = pht('Revoked');
- $e_confirm = pht('Revoked');
- $errors[] = pht(
- 'This password has been revoked. You must choose a new, unique '.
- 'password.');
- } else if ($same_password) {
- $e_password = pht('Not Unique');
- $e_confirm = pht('Not Unique');
- $errors[] = pht(
- 'This password is the same as another password associated '.
- 'with your account. You must use a unique password for '.
- 'VCS access.');
- } else if (
- PhabricatorCommonPasswords::isCommonPassword($new_password)) {
- $e_password = pht('Very Weak');
- $e_confirm = pht('Very Weak');
- $errors[] = pht(
- 'This password is extremely weak: it is one of the most common '.
- 'passwords in use. Choose a stronger password.');
- }
+ $vcspassword
+ ->setPassword($envelope, $user)
+ ->save();
-
- if (!$errors) {
- $vcspassword->setPassword($envelope, $user);
- $vcspassword->save();
-
- return id(new AphrontRedirectResponse())->setURI($panel_uri);
- }
+ return id(new AphrontRedirectResponse())->setURI($panel_uri);
}
}

File Metadata

Mime Type
text/plain
Expires
Thu, Nov 7, 9:10 PM (5 d, 5 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6749458
Default Alt Text
D18902.diff (9 KB)

Event Timeline