Skip to content

Commit

Permalink
Add functionality to limit MFA to specific user groups
Browse files Browse the repository at this point in the history
  • Loading branch information
scott-nz committed Oct 29, 2020
1 parent c8e1004 commit 3881b50
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 1 deletion.
33 changes: 32 additions & 1 deletion src/Extension/SiteConfigExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
use SilverStripe\Forms\CompositeField;
use SilverStripe\Forms\DateField;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\ListboxField;
use SilverStripe\Forms\OptionsetField;
use SilverStripe\ORM\DataExtension;
use SilverStripe\ORM\FieldType\DBField;
use SilverStripe\Security\Group;
use SilverStripe\View\Requirements;

/**
Expand All @@ -34,6 +36,10 @@ class SiteConfigExtension extends DataExtension
'MFAGracePeriodExpires' => 'Date',
];

private static $many_many = [
'MFAGroupRestrictions' => Group::class
];

private static $defaults = [
'MFARequired' => false,
];
Expand Down Expand Up @@ -63,7 +69,32 @@ public function updateCMSFields(FieldList $fields)
));
$mfaGraceEnd->addExtraClass('mfa-settings__grace-period');

$mfaOptions = CompositeField::create($mfaOptions, $mfaGraceEnd)
$mapFn = function ($groups = []) {
$map = [];
foreach ($groups as $group) {
// Listboxfield values are escaped, use ASCII char instead of »
$map[$group->ID] = $group->getBreadcrumbs(' > ');
}
asort($map);
return $map;
};
$groupsMap = $mapFn(Group::get());

$mfaGroupRestrict = ListboxField::create(
"MFAGroupRestrictions",
_t(__CLASS__ . '.MFA_GROUP_RESTRICTIONS', "MFA Groups")
)
->setSource($groupsMap)
->setAttribute(
'data-placeholder',
_t(__CLASS__ . '.MFA_GROUP_RESTRICTIONS_PLACEHOLDER', 'Click to select group')
)->setDescription(_t(
__CLASS__ . '.MFA_GROUP_RESTRICTIONS_DESCRIPTION',
'MFA will only be enabled for members of these selected groups. ' .
'If no groups are selected, MFA will be enabled for all users'
));

$mfaOptions = CompositeField::create($mfaOptions, $mfaGraceEnd, $mfaGroupRestrict)
->setTitle(DBField::create_field(
'HTMLFragment',
_t(__CLASS__ . '.MULTI_FACTOR_AUTHENTICATION', 'Multi-factor authentication (MFA)')
Expand Down
23 changes: 23 additions & 0 deletions src/Service/EnforcementManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ public function shouldRedirectToMFA(Member $member): bool
return false;
}

if (!$this->isUserInMFAEnabledGroup($member)) {
return false;
}

if ($member->RegisteredMFAMethods()->exists()) {
return true;
}
Expand Down Expand Up @@ -266,4 +270,23 @@ protected function isEnabled(): bool

return true;
}

protected function isUserInMFAEnabledGroup(Member $member)
{
/** @var SiteConfig&SiteConfigExtension $siteConfig */
$siteConfig = SiteConfig::current_site_config();

$groups = $siteConfig->MFAGroupRestrictions();

// If no groups are set in the Site Config MFAGroupRestrictions field, MFA is enabled for all users
if ($groups->count() === 0) {
return true;
}
foreach ($groups as $group) {
if ($member->inGroup($group)) {
return true;
}
}
return false;
}
}
39 changes: 39 additions & 0 deletions tests/php/Authenticator/LoginHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use SilverStripe\MFA\Store\StoreInterface;
use SilverStripe\MFA\Tests\Stub\BasicMath\Method;
use SilverStripe\ORM\FieldType\DBDatetime;
use SilverStripe\Security\Group;
use SilverStripe\Security\Member;
use SilverStripe\Security\Security;
use SilverStripe\Security\SecurityToken;
Expand Down Expand Up @@ -537,6 +538,44 @@ public function testGetBackURL()
$this->assertSame('foobar', $handler->getBackURL());
}


public function testMFAGroupRestriction()
{
$config = SiteConfig::current_site_config();

/** @var Group $group */
$group = $this->objFromFixture(Group::class, 'admingroup');
$config->MFAGroupRestrictions()->add($group);

// Test that MFA is required for a member of a group that has been set in SiteConfig
/** @var Member&MemberExtension $member */
$member = $this->objFromFixture(Member::class, 'guy');

$this->autoFollowRedirection = false;
$response = $this->doLogin($member, 'Password123');
$this->autoFollowRedirection = true;

$this->assertSame(302, $response->getStatusCode());
$this->assertStringEndsWith(
Controller::join_links(Security::login_url(), 'default/mfa'),
$response->getHeader('location')
);

// Test that MFA is not required for a member that does not belong to any of the selected groups
/** @var Member&MemberExtension $member */
$member = $this->objFromFixture(Member::class, 'colin');

$this->autoFollowRedirection = false;
$response = $this->doLogin($member, 'Password123');
$this->autoFollowRedirection = true;

$this->assertSame(302, $response->getStatusCode());
$this->assertStringEndsWith(
Security::login_url(),
$response->getHeader('location')
);
}

public function methodlessMemberFixtureProvider()
{
return [['guy', 'carla']];
Expand Down
2 changes: 2 additions & 0 deletions tests/php/Authenticator/LoginHandlerTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ SilverStripe\Security\Member:
Groups: =>SilverStripe\Security\Group.admingroup
colin:
Email: colin@example.com
Password: Password123
PasswordExpiry: 2030-01-01
RegisteredMFAMethods: =>SilverStripe\MFA\Model\RegisteredMethod.colin-math
DefaultRegisteredMethodID: =>SilverStripe\MFA\Model\RegisteredMethod.colin-math
Groups: =>SilverStripe\Security\Group.contentgroup
Expand Down

0 comments on commit 3881b50

Please sign in to comment.