mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 14:00:56 +01:00
Allow administrators to get a list of users who don't have MFA configured
Summary: Fixes T12400. Adds a "Has MFA" filter to People so you can figure out who you need to harass before turning on "require MFA". When you run this as a non-admin, you don't currently actually hit the exception: the query just doesn't work. I think this is probably okay, but if we add more of these it might be better to make the "this didn't work" more explicit since it could be confusing in some weird edge cases (like, an administrator sending a non-administrator a link which they expect will show the non-administrator some interesting query results, but they actually just get no constraint). The exception is more of a fail-safe in case we make application changes in the future and don't remember this weird special case. Test Plan: - As an administrator and non-administrator, used People and Conduit to query MFA, no-MFA, and don't-care-about-MFA. These queries worked for an admin and didn't work for a non-admin. - Viewed the list as an administrator, saw MFA users annotated. - Viewed config help, clicked link as an admin, ended up in the right place. {F4093033} {F4093034} Reviewers: chad Reviewed By: chad Maniphest Tasks: T12400 Differential Revision: https://secure.phabricator.com/D17500
This commit is contained in:
parent
fd69dfaa9a
commit
d6d3ad6f80
6 changed files with 86 additions and 19 deletions
|
@ -3763,6 +3763,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php',
|
'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php',
|
||||||
'PhabricatorSearchCheckboxesField' => 'applications/search/field/PhabricatorSearchCheckboxesField.php',
|
'PhabricatorSearchCheckboxesField' => 'applications/search/field/PhabricatorSearchCheckboxesField.php',
|
||||||
'PhabricatorSearchConfigOptions' => 'applications/search/config/PhabricatorSearchConfigOptions.php',
|
'PhabricatorSearchConfigOptions' => 'applications/search/config/PhabricatorSearchConfigOptions.php',
|
||||||
|
'PhabricatorSearchConstraintException' => 'applications/search/exception/PhabricatorSearchConstraintException.php',
|
||||||
'PhabricatorSearchController' => 'applications/search/controller/PhabricatorSearchController.php',
|
'PhabricatorSearchController' => 'applications/search/controller/PhabricatorSearchController.php',
|
||||||
'PhabricatorSearchCustomFieldProxyField' => 'applications/search/field/PhabricatorSearchCustomFieldProxyField.php',
|
'PhabricatorSearchCustomFieldProxyField' => 'applications/search/field/PhabricatorSearchCustomFieldProxyField.php',
|
||||||
'PhabricatorSearchDAO' => 'applications/search/storage/PhabricatorSearchDAO.php',
|
'PhabricatorSearchDAO' => 'applications/search/storage/PhabricatorSearchDAO.php',
|
||||||
|
@ -9072,6 +9073,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorSearchBaseController' => 'PhabricatorController',
|
'PhabricatorSearchBaseController' => 'PhabricatorController',
|
||||||
'PhabricatorSearchCheckboxesField' => 'PhabricatorSearchField',
|
'PhabricatorSearchCheckboxesField' => 'PhabricatorSearchField',
|
||||||
'PhabricatorSearchConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
'PhabricatorSearchConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
||||||
|
'PhabricatorSearchConstraintException' => 'Exception',
|
||||||
'PhabricatorSearchController' => 'PhabricatorSearchBaseController',
|
'PhabricatorSearchController' => 'PhabricatorSearchBaseController',
|
||||||
'PhabricatorSearchCustomFieldProxyField' => 'PhabricatorSearchField',
|
'PhabricatorSearchCustomFieldProxyField' => 'PhabricatorSearchField',
|
||||||
'PhabricatorSearchDAO' => 'PhabricatorLiskDAO',
|
'PhabricatorSearchDAO' => 'PhabricatorLiskDAO',
|
||||||
|
|
|
@ -66,6 +66,21 @@ EOTEXT
|
||||||
,
|
,
|
||||||
PhabricatorEnv::getDoclink('Configuring Encryption')));
|
PhabricatorEnv::getDoclink('Configuring Encryption')));
|
||||||
|
|
||||||
|
$require_mfa_description = $this->deformat(pht(<<<EOTEXT
|
||||||
|
By default, Phabricator allows users to add multi-factor authentication to
|
||||||
|
their accounts, but does not require it. By enabling this option, you can
|
||||||
|
force all users to add at least one authentication factor before they can use
|
||||||
|
their accounts.
|
||||||
|
|
||||||
|
Administrators can query a list of users who do not have MFA configured in
|
||||||
|
{nav People}:
|
||||||
|
|
||||||
|
- **[[ %s | %s ]]**
|
||||||
|
EOTEXT
|
||||||
|
,
|
||||||
|
'/people/?mfa=false',
|
||||||
|
pht('List of Users Without MFA')));
|
||||||
|
|
||||||
return array(
|
return array(
|
||||||
$this->newOption('security.alternate-file-domain', 'string', null)
|
$this->newOption('security.alternate-file-domain', 'string', null)
|
||||||
->setLocked(true)
|
->setLocked(true)
|
||||||
|
@ -132,13 +147,7 @@ EOTEXT
|
||||||
->setLocked(true)
|
->setLocked(true)
|
||||||
->setSummary(
|
->setSummary(
|
||||||
pht('Require all users to configure multi-factor authentication.'))
|
pht('Require all users to configure multi-factor authentication.'))
|
||||||
->setDescription(
|
->setDescription($require_mfa_description)
|
||||||
pht(
|
|
||||||
'By default, Phabricator allows users to add multi-factor '.
|
|
||||||
'authentication to their accounts, but does not require it. '.
|
|
||||||
'By enabling this option, you can force all users to add '.
|
|
||||||
'at least one authentication factor before they can use their '.
|
|
||||||
'accounts.'))
|
|
||||||
->setBoolOptions(
|
->setBoolOptions(
|
||||||
array(
|
array(
|
||||||
pht('Multi-Factor Required'),
|
pht('Multi-Factor Required'),
|
||||||
|
|
|
@ -18,6 +18,7 @@ final class PhabricatorPeopleQuery
|
||||||
private $nameLike;
|
private $nameLike;
|
||||||
private $nameTokens;
|
private $nameTokens;
|
||||||
private $namePrefixes;
|
private $namePrefixes;
|
||||||
|
private $isEnrolledInMultiFactor;
|
||||||
|
|
||||||
private $needPrimaryEmail;
|
private $needPrimaryEmail;
|
||||||
private $needProfile;
|
private $needProfile;
|
||||||
|
@ -100,6 +101,11 @@ final class PhabricatorPeopleQuery
|
||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function withIsEnrolledInMultiFactor($enrolled) {
|
||||||
|
$this->isEnrolledInMultiFactor = $enrolled;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
public function needPrimaryEmail($need) {
|
public function needPrimaryEmail($need) {
|
||||||
$this->needPrimaryEmail = $need;
|
$this->needPrimaryEmail = $need;
|
||||||
return $this;
|
return $this;
|
||||||
|
@ -337,6 +343,13 @@ final class PhabricatorPeopleQuery
|
||||||
$this->nameLike);
|
$this->nameLike);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($this->isEnrolledInMultiFactor !== null) {
|
||||||
|
$where[] = qsprintf(
|
||||||
|
$conn,
|
||||||
|
'user.isEnrolledInMultiFactor = %d',
|
||||||
|
(int)$this->isEnrolledInMultiFactor);
|
||||||
|
}
|
||||||
|
|
||||||
return $where;
|
return $where;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -18,7 +18,7 @@ final class PhabricatorPeopleSearchEngine
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function buildCustomSearchFields() {
|
protected function buildCustomSearchFields() {
|
||||||
return array(
|
$fields = array(
|
||||||
id(new PhabricatorSearchStringListField())
|
id(new PhabricatorSearchStringListField())
|
||||||
->setLabel(pht('Usernames'))
|
->setLabel(pht('Usernames'))
|
||||||
->setKey('usernames')
|
->setKey('usernames')
|
||||||
|
@ -84,18 +84,36 @@ final class PhabricatorPeopleSearchEngine
|
||||||
pht(
|
pht(
|
||||||
'Pass true to find only users awaiting administrative approval, '.
|
'Pass true to find only users awaiting administrative approval, '.
|
||||||
'or false to omit these users.')),
|
'or false to omit these users.')),
|
||||||
id(new PhabricatorSearchDateField())
|
);
|
||||||
|
|
||||||
|
$viewer = $this->requireViewer();
|
||||||
|
if ($viewer->getIsAdmin()) {
|
||||||
|
$fields[] = id(new PhabricatorSearchThreeStateField())
|
||||||
|
->setLabel(pht('Has MFA'))
|
||||||
|
->setKey('mfa')
|
||||||
|
->setOptions(
|
||||||
|
pht('(Show All)'),
|
||||||
|
pht('Show Only Users With MFA'),
|
||||||
|
pht('Hide Users With MFA'))
|
||||||
|
->setDescription(
|
||||||
|
pht(
|
||||||
|
'Pass true to find only users who are enrolled in MFA, or false '.
|
||||||
|
'to omit these users.'));
|
||||||
|
}
|
||||||
|
|
||||||
|
$fields[] = id(new PhabricatorSearchDateField())
|
||||||
->setKey('createdStart')
|
->setKey('createdStart')
|
||||||
->setLabel(pht('Joined After'))
|
->setLabel(pht('Joined After'))
|
||||||
->setDescription(
|
->setDescription(
|
||||||
pht('Find user accounts created after a given time.')),
|
pht('Find user accounts created after a given time.'));
|
||||||
id(new PhabricatorSearchDateField())
|
|
||||||
|
$fields[] = id(new PhabricatorSearchDateField())
|
||||||
->setKey('createdEnd')
|
->setKey('createdEnd')
|
||||||
->setLabel(pht('Joined Before'))
|
->setLabel(pht('Joined Before'))
|
||||||
->setDescription(
|
->setDescription(
|
||||||
pht('Find user accounts created before a given time.')),
|
pht('Find user accounts created before a given time.'));
|
||||||
|
|
||||||
);
|
return $fields;
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function getDefaultFieldOrder() {
|
protected function getDefaultFieldOrder() {
|
||||||
|
@ -151,6 +169,19 @@ final class PhabricatorPeopleSearchEngine
|
||||||
$query->withIsApproved(!$map['needsApproval']);
|
$query->withIsApproved(!$map['needsApproval']);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (idx($map, 'mfa') !== null) {
|
||||||
|
$viewer = $this->requireViewer();
|
||||||
|
if (!$viewer->getIsAdmin()) {
|
||||||
|
throw new PhabricatorSearchConstraintException(
|
||||||
|
pht(
|
||||||
|
'The "Has MFA" query constraint may only be used by '.
|
||||||
|
'administrators, to prevent attackers from using it to target '.
|
||||||
|
'weak accounts.'));
|
||||||
|
}
|
||||||
|
|
||||||
|
$query->withIsEnrolledInMultiFactor($map['mfa']);
|
||||||
|
}
|
||||||
|
|
||||||
if ($map['createdStart']) {
|
if ($map['createdStart']) {
|
||||||
$query->withDateCreatedAfter($map['createdStart']);
|
$query->withDateCreatedAfter($map['createdStart']);
|
||||||
}
|
}
|
||||||
|
@ -254,6 +285,12 @@ final class PhabricatorPeopleSearchEngine
|
||||||
$item->addIcon('fa-envelope-o', pht('Mailing List'));
|
$item->addIcon('fa-envelope-o', pht('Mailing List'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($viewer->getIsAdmin()) {
|
||||||
|
if ($user->getIsEnrolledInMultiFactor()) {
|
||||||
|
$item->addIcon('fa-lock', pht('Has MFA'));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if ($viewer->getIsAdmin()) {
|
if ($viewer->getIsAdmin()) {
|
||||||
$user_id = $user->getID();
|
$user_id = $user->getID();
|
||||||
if ($is_approval) {
|
if ($is_approval) {
|
||||||
|
|
|
@ -331,6 +331,8 @@ final class PhabricatorApplicationSearchController
|
||||||
'query parameters and correct errors.');
|
'query parameters and correct errors.');
|
||||||
} catch (PhutilSearchQueryCompilerSyntaxException $ex) {
|
} catch (PhutilSearchQueryCompilerSyntaxException $ex) {
|
||||||
$exec_errors[] = $ex->getMessage();
|
$exec_errors[] = $ex->getMessage();
|
||||||
|
} catch (PhabricatorSearchConstraintException $ex) {
|
||||||
|
$exec_errors[] = $ex->getMessage();
|
||||||
}
|
}
|
||||||
|
|
||||||
// The engine may have encountered additional errors during rendering;
|
// The engine may have encountered additional errors during rendering;
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class PhabricatorSearchConstraintException
|
||||||
|
extends Exception {}
|
Loading…
Reference in a new issue