From 498dcfeb80fcf2917b09d544ce04e7a4b997084d Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Sun, 13 Feb 2022 23:15:17 -0800 Subject: [PATCH] Require OATHAuth for membership in specified user groups Users in groups listed in $wgOATHRequiredForGroups (default none) must have two-factor authentication enabled otherwise their membership in those groups will be disabled. This is done using the UserEffectiveGroups hook, which allows dynamically adding or removing user groups. If a user doesn't have 2FA enabled, it will appear to them as if they aren't a member of the group at all. Special:Preferences will show which groups are disabled. In the future it would be good to have a hook into PermissionsError to show this as well. The UserGetRights hook is used to ensure the user still has the "oathauth-enable" user right in case it was only granted to them as part of the user group they are disabled from. On the outside, Special:ListUsers will still show the user as a member of the group. The API list=users&prop=groups|groupmemberships will show inconsistent informaiton, groups will remove disabled groups while groupmemberships will not. This functionality was somewhat already available with $wgOATHExclusiveRights, except that implementation has flaws outlined at T150562#6078263 and haven't been resolved in I69af6a58e4 for over a year now. If this works out, it's expected that will be deprecated/removed. Bug: T150562 Change-Id: I07ebddafc6f2233ccec216fa8ac6e996553499fb --- extension.json | 10 +++- i18n/en.json | 4 +- i18n/qqq.json | 4 +- src/Hook/HookHandler.php | 101 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 113 insertions(+), 6 deletions(-) diff --git a/extension.json b/extension.json index 04a5ceec..7aa7770e 100644 --- a/extension.json +++ b/extension.json @@ -44,7 +44,9 @@ "AuthChangeFormFields": "main", "LoadExtensionSchemaUpdates": "\\MediaWiki\\Extension\\OATHAuth\\Hook\\LoadExtensionSchemaUpdates\\UpdateTables::callback", "GetPreferences": "main", - "getUserPermissionsErrors": "main" + "getUserPermissionsErrors": "main", + "UserEffectiveGroups": "main", + "UserGetRights": "main" }, "HookHandlers": { "main": { @@ -52,7 +54,8 @@ "services": [ "OATHUserRepository", "PermissionManager", - "MainConfig" + "MainConfig", + "UserGroupManager" ] } }, @@ -74,6 +77,9 @@ }, "OATHExclusiveRights": { "value": [] + }, + "OATHRequiredForGroups": { + "value": [] } }, "ResourceModules": { diff --git a/i18n/en.json b/i18n/en.json index a1a96215..e9003cbe 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -83,5 +83,7 @@ "oathauth-totp-disable-warning": "You will no longer be able to use the authentication device registered with this account. All scratch-tokens associated with this account will be invalidated.", "oathauth-invalidrequest": "Invalid request", "oathauth-verify-enabled": "{{GENDER:$1|$1}} has two-factor authentication enabled.", - "oathauth-verify-disabled": "{{GENDER:$1|$1}} does not have two-factor authentication enabled." + "oathauth-verify-disabled": "{{GENDER:$1|$1}} does not have two-factor authentication enabled.", + "oathauth-prefs-disabledgroups": "Disabled {{PLURAL:$1|group|groups}}:", + "oathauth-prefs-disabledgroups-help": "{{GENDER:$2|Your membership}} in {{PLURAL:$1|this group|these groups}} is disabled until you enable [[Special:Manage Two-factor authentication|two-factor authentication]]." } diff --git a/i18n/qqq.json b/i18n/qqq.json index 5941d50d..72e9ded5 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -95,5 +95,7 @@ "oathauth-totp-disable-warning": "TOTP specific warning message when disabling/switching to alternative 2FA method", "oathauth-invalidrequest": "Generic error message that is displayed when request cannot be processed due to an unpredicted reason", "oathauth-verify-enabled": "Notice that a user has 2FA enabled, shown on success at [[Special:VerifyOATHForUser]].\n$1 - Name of user", - "oathauth-verify-disabled": "Notice that a user does not have 2FA enabled, shown on success at [[Special:VerifyOATHForUser]].\n$1 - Name of user" + "oathauth-verify-disabled": "Notice that a user does not have 2FA enabled, shown on success at [[Special:VerifyOATHForUser]].\n$1 - Name of user", + "oathauth-prefs-disabledgroups": "Label on Special:Preferences for groups in which the user's membership has been disabled for a lack of two-factor authentication.\n$1 - Number of groups", + "oathauth-prefs-disabledgroups-help": "Help message shown on Special:Preferences for the {{mw-msg|oathauth-prefs-disabledgroups}} field.\n$1 - Number of groups\n$2 - User name for GENDER" } diff --git a/src/Hook/HookHandler.php b/src/Hook/HookHandler.php index bfbcd8ff..f7fdb802 100644 --- a/src/Hook/HookHandler.php +++ b/src/Hook/HookHandler.php @@ -7,20 +7,27 @@ use MediaWiki\Auth\AuthenticationRequest; use MediaWiki\Extension\OATHAuth\OATHAuth; use MediaWiki\Extension\OATHAuth\OATHUserRepository; use MediaWiki\Permissions\Hook\GetUserPermissionsErrorsHook; +use MediaWiki\Permissions\Hook\UserGetRightsHook; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Preferences\Hook\GetPreferencesHook; use MediaWiki\SpecialPage\Hook\AuthChangeFormFieldsHook; +use MediaWiki\User\Hook\UserEffectiveGroupsHook; +use MediaWiki\User\UserGroupManager; use OOUI\ButtonWidget; use OOUI\HorizontalLayout; use OOUI\LabelWidget; +use RequestContext; use SpecialPage; use Title; use User; +use UserGroupMembership; class HookHandler implements AuthChangeFormFieldsHook, GetPreferencesHook, - getUserPermissionsErrorsHook + getUserPermissionsErrorsHook, + UserEffectiveGroupsHook, + UserGetRightsHook { /** * @var OATHUserRepository @@ -32,6 +39,11 @@ class HookHandler implements */ private $permissionManager; + /** + * @var UserGroupManager + */ + private $userGroupManager; + /** * @var Config */ @@ -41,11 +53,13 @@ class HookHandler implements * @param OATHUserRepository $userRepo * @param PermissionManager $permissionManager * @param Config $config + * @param UserGroupManager $userGroupManager */ - public function __construct( $userRepo, $permissionManager, $config ) { + public function __construct( $userRepo, $permissionManager, $config, $userGroupManager ) { $this->userRepo = $userRepo; $this->permissionManager = $permissionManager; $this->config = $config; + $this->userGroupManager = $userGroupManager; } /** @@ -123,9 +137,73 @@ class HookHandler implements 'section' => 'personal/info', ]; + $dbGroups = $this->userGroupManager->getUserGroups( $user ); + $disabledGroups = $this->getDisabledGroups( $user, $dbGroups ); + if ( $module === null && $disabledGroups ) { + $context = RequestContext::getMain(); + $list = []; + foreach ( $disabledGroups as $disabledGroup ) { + $list[] = UserGroupMembership::getLink( $disabledGroup, $context, 'html' ); + } + $info = $context->getLanguage()->commaList( $list ); + $disabledInfo = [ 'oathauth-disabledgroups' => [ + // @phan-suppress-next-line SecurityCheck-XSS T183174 + 'type' => 'info', + 'label-message' => [ 'oathauth-prefs-disabledgroups', + \Message::numParam( count( $disabledGroups ) ) ], + 'help-message' => [ 'oathauth-prefs-disabledgroups-help', + \Message::numParam( count( $disabledGroups ) ), $user->getName() ], + 'default' => $info, + 'raw' => true, + 'section' => 'personal/info', + ] ]; + // Insert right after "Member of groups" + $preferences = wfArrayInsertAfter( $preferences, $disabledInfo, 'usergroups' ); + } + return true; } + /** + * Return the groups that this user is supposed to be in, but are disabled + * because 2FA isn't enabled + * + * @param User $user + * @param string[] $groups All groups the user is supposed to be in + * @return string[] Groups the user should be disabled in + */ + private function getDisabledGroups( User $user, array $groups ): array { + $requiredGroups = $this->config->get( 'OATHRequiredForGroups' ); + // Bail early if: + // * No configured restricted groups + // * The user is not in any of the restricted groups + $intersect = array_intersect( $groups, $requiredGroups ); + if ( !$requiredGroups || !$intersect ) { + return []; + } + + $oathUser = $this->userRepo->findByUser( $user ); + if ( $oathUser->getModule() === null ) { + // Not enabled, strip the groups + return $intersect; + } else { + return []; + } + } + + /** + * Remove groups if 2FA is required for them and it's not enabled + * + * @param User $user User to get groups for + * @param string[] &$groups Current effective groups + */ + public function onUserEffectiveGroups( $user, &$groups ) { + $disabledGroups = $this->getDisabledGroups( $user, $groups ); + if ( $disabledGroups ) { + $groups = array_diff( $groups, $disabledGroups ); + } + } + /** * @param Title $title * @param User $user @@ -150,4 +228,23 @@ class HookHandler implements } return true; } + + /** + * If a user has groups disabled for not having 2FA enabled, make sure they + * have "oathauth-enable" so they can turn it on + * + * @param User $user User to get rights for + * @param string[] &$rights Current rights + */ + public function onUserGetRights( $user, &$rights ) { + if ( in_array( 'oathauth-enable', $rights ) ) { + return; + } + + $dbGroups = $this->userGroupManager->getUserGroups( $user ); + if ( $this->getDisabledGroups( $user, $dbGroups ) ) { + // Has some disabled groups, add oathauth-enable + $rights[] = 'oathauth-enable'; + } + } }