mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-22 14:52:41 +01:00
Migrate VCS passwords to new shared password infrastructure
Summary: Ref T13043. Migrate VCS passwords away from their dedicated table to new the new shared infrastructure. Future changes will migrate account passwords and remove the old table. Test Plan: - Ran migrations. - Cloned with the same password that was configured before the migrations (worked). - Cloned with a different, invalid password (failed). - Changed password. - Cloned with old password (failed). - Cloned with new password (worked). - Deleted password in web UI. - Cloned with old password (failed). - Set password to the same password as it currently is set to (worked, no "unique" collision). - Set password to account password. !!This (incorrectly) works for now until account passwords migrate, since the uniqueness check can't see them yet.!! - Set password to a new unique password. - Cloned (worked). - Revoked the password with `bin/auth revoke`. - Verified web UI shows "no password set". - Verified that pull no longer works. - Verified that I can no longer select the revoked password. - Verified that accounts do not interact: - Tried to set account B to account A's password (worked). - Tried to set account B to a password revoked on account A (worked). - Spot checked the `password` and `passwordtransaction` tables for saniity. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13043 Differential Revision: https://secure.phabricator.com/D18898
This commit is contained in:
parent
bb12f4bab7
commit
dd8f588ac5
4 changed files with 70 additions and 38 deletions
5
resources/sql/autopatches/20180120.auth.03.vcsdata.sql
Normal file
5
resources/sql/autopatches/20180120.auth.03.vcsdata.sql
Normal file
|
@ -0,0 +1,5 @@
|
|||
INSERT INTO {$NAMESPACE}_auth.auth_password
|
||||
(objectPHID, phid, passwordType, passwordHash, isRevoked,
|
||||
dateCreated, dateModified)
|
||||
SELECT userPHID, '', 'vcs', passwordHash, 0, dateCreated, dateModified
|
||||
FROM {$NAMESPACE}_repository.repository_vcspassword;
|
22
resources/sql/autopatches/20180120.auth.04.vcsphid.php
Normal file
22
resources/sql/autopatches/20180120.auth.04.vcsphid.php
Normal file
|
@ -0,0 +1,22 @@
|
|||
<?php
|
||||
|
||||
// Populate VCS passwords (which we copied from the old "VCS Password" table
|
||||
// in the last migration) with new PHIDs.
|
||||
|
||||
$table = new PhabricatorAuthPassword();
|
||||
$conn = $table->establishConnection('w');
|
||||
|
||||
foreach (new LiskMigrationIterator($table) as $row) {
|
||||
if ($row->getPHID()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$new_phid = $row->generatePHID();
|
||||
|
||||
queryfx(
|
||||
$conn,
|
||||
'UPDATE %T SET phid = %s WHERE id = %d',
|
||||
$table->getTableName(),
|
||||
$new_phid,
|
||||
$row->getID());
|
||||
}
|
|
@ -715,30 +715,19 @@ final class DiffusionServeController extends DiffusionController {
|
|||
return null;
|
||||
}
|
||||
|
||||
$password_entry = id(new PhabricatorRepositoryVCSPassword())
|
||||
->loadOneWhere('userPHID = %s', $user->getPHID());
|
||||
if (!$password_entry) {
|
||||
// User doesn't have a password set.
|
||||
$request = $this->getRequest();
|
||||
$content_source = PhabricatorContentSource::newFromRequest($request);
|
||||
|
||||
$engine = id(new PhabricatorAuthPasswordEngine())
|
||||
->setViewer($user)
|
||||
->setContentSource($content_source)
|
||||
->setPasswordType(PhabricatorAuthPassword::PASSWORD_TYPE_VCS)
|
||||
->setObject($user);
|
||||
|
||||
if (!$engine->isValidPassword($password)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (!$password_entry->comparePassword($password, $user)) {
|
||||
// Password doesn't match.
|
||||
return null;
|
||||
}
|
||||
|
||||
// If the user's password is stored using a less-than-optimal hash, upgrade
|
||||
// them to the strongest available hash.
|
||||
|
||||
$hash_envelope = new PhutilOpaqueEnvelope(
|
||||
$password_entry->getPasswordHash());
|
||||
if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) {
|
||||
$password_entry->setPassword($password, $user);
|
||||
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
||||
$password_entry->save();
|
||||
unset($unguarded);
|
||||
}
|
||||
|
||||
return $user;
|
||||
}
|
||||
|
||||
|
|
|
@ -35,13 +35,20 @@ final class DiffusionSetPasswordSettingsPanel extends PhabricatorSettingsPanel {
|
|||
$request,
|
||||
'/settings/');
|
||||
|
||||
$vcspassword = id(new PhabricatorRepositoryVCSPassword())
|
||||
->loadOneWhere(
|
||||
'userPHID = %s',
|
||||
$user->getPHID());
|
||||
if (!$vcspassword) {
|
||||
$vcspassword = id(new PhabricatorRepositoryVCSPassword());
|
||||
$vcspassword->setUserPHID($user->getPHID());
|
||||
$vcs_type = PhabricatorAuthPassword::PASSWORD_TYPE_VCS;
|
||||
|
||||
$vcspasswords = id(new PhabricatorAuthPasswordQuery())
|
||||
->setViewer($viewer)
|
||||
->withObjectPHIDs(array($user->getPHID()))
|
||||
->withPasswordTypes(array($vcs_type))
|
||||
->withIsRevoked(false)
|
||||
->execute();
|
||||
if ($vcspasswords) {
|
||||
$vcspassword = head($vcspasswords);
|
||||
} else {
|
||||
$vcspassword = PhabricatorAuthPassword::initializeNewPassword(
|
||||
$user,
|
||||
$vcs_type);
|
||||
}
|
||||
|
||||
$panel_uri = $this->getPanelURI('?saved=true');
|
||||
|
@ -77,23 +84,32 @@ final class DiffusionSetPasswordSettingsPanel extends PhabricatorSettingsPanel {
|
|||
|
||||
if (!$errors) {
|
||||
$envelope = new PhutilOpaqueEnvelope($new_password);
|
||||
$content_source = PhabricatorContentSource::newFromRequest($request);
|
||||
|
||||
try {
|
||||
// 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.
|
||||
// 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.
|
||||
|
||||
$same_password = $viewer->comparePassword($envelope);
|
||||
} catch (PhabricatorPasswordHasherUnavailableException $ex) {
|
||||
// If we're missing the hasher, just let the user continue.
|
||||
$same_password = false;
|
||||
}
|
||||
$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');
|
||||
|
|
Loading…
Reference in a new issue