From 8ddedb038ea0866917ea7b39a853a6ab21a2373c Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 27 Jan 2023 11:38:10 +1300 Subject: [PATCH 1/3] NEW Allow admins to require password reset for members This came from silverstripe/silverstripe-security-extensions --- src/Security/Member.php | 49 ++++++++- tests/php/Security/MemberTest.php | 160 +++++++++++++++++++++++++++++- tests/php/Security/MemberTest.yml | 14 +++ 3 files changed, 221 insertions(+), 2 deletions(-) diff --git a/src/Security/Member.php b/src/Security/Member.php index 1e3262943..c408d7e67 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -13,6 +13,7 @@ use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\TestMailer; +use SilverStripe\Forms\CheckboxField; use SilverStripe\Forms\ConfirmedPasswordField; use SilverStripe\Forms\DropdownField; use SilverStripe\Forms\FieldList; @@ -398,7 +399,40 @@ class Member extends DataObject return null; } - public function isPasswordExpired() + /** + * Used to get the value for the reset password on next login checkbox + */ + public function getRequiresPasswordChangeOnNextLogin(): bool + { + return $this->isPasswordExpired(); + } + + /** + * Set password expiry to "now" to require a change of password next log in + * + * @param int|null $dataValue 1 is checked, 0/null is not checked {@see CheckboxField::dataValue} + */ + public function saveRequiresPasswordChangeOnNextLogin(?int $dataValue): static + { + if (!$this->canEdit()) { + return $this; + } + + $currentValue = $this->PasswordExpiry; + $currentDate = $this->dbObject('PasswordExpiry'); + + if ($dataValue && (!$currentValue || $currentDate->inFuture())) { + // Only alter future expiries - this way an admin could see how long ago a password expired still + $this->PasswordExpiry = DBDatetime::now()->Rfc2822(); + } elseif (!$dataValue && $this->isPasswordExpired()) { + // Only unset if the expiry date is in the past + $this->PasswordExpiry = null; + } + + return $this; + } + + public function isPasswordExpired(): bool { if (!$this->PasswordExpiry) { return false; @@ -1363,6 +1397,19 @@ class Member extends DataObject if ($permissionsTab) { $permissionsTab->addExtraClass('readonly'); } + + $currentUser = Security::getCurrentUser(); + // We can allow an admin to require a user to change their password. But: + // - Don't show a read only field if the user cannot edit this record + // - Don't show if a user views their own profile (just let them reset their own password) + if ($currentUser && ($currentUser->ID !== $this->ID) && $this->canEdit()) { + $requireNewPassword = CheckboxField::create( + 'RequiresPasswordChangeOnNextLogin', + _t(__CLASS__ . '.RequiresPasswordChangeOnNextLogin', 'Requires password change on next log in') + ); + $fields->insertAfter('Password', $requireNewPassword); + $fields->dataFieldByName('Password')->addExtraClass('form-field--no-divider mb-0 pb-0'); + } }); return parent::getCMSFields(); diff --git a/tests/php/Security/MemberTest.php b/tests/php/Security/MemberTest.php index 18aa85174..7c31075fe 100644 --- a/tests/php/Security/MemberTest.php +++ b/tests/php/Security/MemberTest.php @@ -7,6 +7,9 @@ use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\FunctionalTest; +use SilverStripe\Forms\CheckboxField; +use SilverStripe\Forms\FieldList; +use SilverStripe\Forms\Form; use SilverStripe\Forms\ListboxField; use SilverStripe\i18n\i18n; use SilverStripe\ORM\DataObject; @@ -402,6 +405,161 @@ class MemberTest extends FunctionalTest $this->assertFalse($member->isPasswordExpired()); } + public function testAdminCanRequirePasswordChangeOnNextLogIn() + { + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'someone'); + $this->logInWithPermission('ADMIN'); + $field = $targetMember->getCMSFields()->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $this->assertNotNull($field); + $this->assertInstanceOf(CheckboxField::class, $field, 'The field should be an instance of ' . CheckboxField::class); + } + + public function testUserCannotRequireTheirOwnPasswordChangeOnNextLogIn() + { + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'someone'); + $this->logInAs($targetMember); + $field = $targetMember->getCMSFields()->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $this->assertNull($field); + } + + public function testUserCannotRequireOthersToPasswordChangeOnNextLogIn() + { + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'anyone'); + $this->logInAs('someone'); + $field = $targetMember->getCMSFields()->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $this->assertNull($field); + } + + public function testCheckingRequiresPasswordChangeOnNextLoginWillSetPasswordExpiryToNow() + { + $mockDate = '2019-03-02 00:00:00'; + DBDateTime::set_mock_now($mockDate); + + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'someone'); + + $this->assertNull($targetMember->PasswordExpiry); + + $this->logInWithPermission('ADMIN'); + $fields = $targetMember->getCMSFields(); + $form = new Form(null, 'SomeForm', $fields, new FieldList()); + $field = $fields->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $field->setValue(1); + $form->saveInto($targetMember); + + $this->assertEquals($mockDate, $targetMember->PasswordExpiry); + } + + public function testCheckingPasswordChangeUpdatesFutureExpiriesToNow() + { + $mockDate = '2019-03-02 00:00:00'; + DBDateTime::set_mock_now($mockDate); + + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'willexpire'); + + $this->assertTrue($targetMember->dbObject('PasswordExpiry')->inFuture()); + + $this->logInWithPermission('ADMIN'); + $fields = $targetMember->getCMSFields(); + $form = new Form(null, 'SomeForm', $fields, new FieldList()); + $field = $fields->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $field->setValue(1); + $form->saveInto($targetMember); + + $this->assertEquals($mockDate, $targetMember->PasswordExpiry); + } + + public function testCheckingPasswordChangeDoesNotAlterPastDates() + { + $mockDate = '2019-03-02 00:00:00'; + DBDateTime::set_mock_now($mockDate); + + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'expired'); + $originalValue = $targetMember->PasswordExpiry; + + $this->assertTrue($targetMember->dbObject('PasswordExpiry')->inPast()); + + $this->logInWithPermission('ADMIN'); + $fields = $targetMember->getCMSFields(); + $form = new Form(null, 'SomeForm', $fields, new FieldList()); + $field = $fields->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $field->setValue(1); + $form->saveInto($targetMember); + + $this->assertEquals($originalValue, $targetMember->PasswordExpiry); + } + + public function testSavingUncheckedPasswordChangeNullsPastDates() + { + $mockDate = '2019-03-02 00:00:00'; + DBDateTime::set_mock_now($mockDate); + + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'expired'); + + $this->logInWithPermission('ADMIN'); + $fields = $targetMember->getCMSFields(); + $form = new Form(null, 'SomeForm', $fields, new FieldList()); + $field = $fields->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $field->setValue(0); + $form->saveInto($targetMember); + + $this->assertNull($targetMember->PasswordExpiry); + } + + public function testSavingUncheckedPasswordChangeDoesNotAlterFutureDates() + { + $mockDate = '2019-03-02 00:00:00'; + DBDateTime::set_mock_now($mockDate); + + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'willexpire'); + $originalValue = $targetMember->PasswordExpiry; + + $this->logInWithPermission('ADMIN'); + $fields = $targetMember->getCMSFields(); + $form = new Form(null, 'SomeForm', $fields, new FieldList()); + $field = $fields->dataFieldByName('RequiresPasswordChangeOnNextLogin'); + $field->setValue(0); + $form->saveInto($targetMember); + + $this->assertNotNull($targetMember->PasswordExpiry); + $this->assertEquals($originalValue, $targetMember->PasswordExpiry); + } + + public function testSavingChangePasswordOnNextLoginIsNotPossibleIfTheCurrentMemberCannotEditTheMemberBeingSaved() + { + /** @var Member&MemberExtension $targetMember */ + $targetMember = $this->objFromFixture(Member::class, 'expired'); + $originalValue = $targetMember->PasswordExpiry; + + $this->logInAs('someone'); + $fields = $targetMember->saveRequiresPasswordChangeOnNextLogin(0); + + $this->assertEquals($originalValue, $targetMember->PasswordExpiry); + } + + public function testGetRequiresPasswordChangeOnNextLogin() + { + $this->assertTrue( + $this->objFromFixture(Member::class, 'expired')->getRequiresPasswordChangeOnNextLogin(), + 'PasswordExpiry date in the past should require a change' + ); + $this->assertFalse( + $this->objFromFixture(Member::class, 'willexpire')->getRequiresPasswordChangeOnNextLogin(), + 'PasswordExpiry date in the past should NOT require a change' + ); + $this->assertFalse( + $this->objFromFixture(Member::class, 'someone')->getRequiresPasswordChangeOnNextLogin(), + 'PasswordExpiry is NULL should NOT require a change' + ); + } + public function testInGroups() { /** @var Member $staffmember */ @@ -869,7 +1027,7 @@ class MemberTest extends FunctionalTest public function testMap_in_groupsReturnsAll() { $members = Member::map_in_groups(); - $this->assertEquals(13, $members->count(), 'There are 12 members in the mock plus a fake admin'); + $this->assertEquals(17, $members->count(), 'There are 16 members in the mock plus a fake admin'); } /** diff --git a/tests/php/Security/MemberTest.yml b/tests/php/Security/MemberTest.yml index 0f9ed3c91..0711914e6 100644 --- a/tests/php/Security/MemberTest.yml +++ b/tests/php/Security/MemberTest.yml @@ -57,6 +57,20 @@ Surname: User Email: noexpiry@silverstripe.com Password: 1nitialPassword + someone: + FirstName: 'Someone' + Email: 'someone@example.com' + anyone: + FirstName: 'Anyone' + Email: 'anyone@example.com' + expired: + Firstname: 'Expired' + Email: 'expired@example.com' + PasswordExpiry: '2018-01-01' + willexpire: + Firstname: 'William' + Email: 'william@example.com' + PasswordExpiry: '3018-01-01' staffmember: Email: staffmember@test.com Groups: '=>SilverStripe\Security\Group.staffgroup' From fecb7ba4d8341e21c8d95d6cfe7bbf1095281645 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 30 Jan 2023 12:51:38 +1300 Subject: [PATCH 2/3] NEW Add sudo mode service --- _config/security.yml | 2 + .../SessionAuthenticationHandler.php | 12 ++- src/Security/SudoMode/SudoModeService.php | 52 ++++++++++++ .../SudoMode/SudoModeServiceInterface.php | 31 +++++++ .../Security/SudoMode/SudoModeServiceTest.php | 84 +++++++++++++++++++ 5 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 src/Security/SudoMode/SudoModeService.php create mode 100644 src/Security/SudoMode/SudoModeServiceInterface.php create mode 100644 tests/php/Security/SudoMode/SudoModeServiceTest.php diff --git a/_config/security.yml b/_config/security.yml index 1dfd8ef92..b7d029973 100644 --- a/_config/security.yml +++ b/_config/security.yml @@ -43,6 +43,8 @@ SilverStripe\Core\Injector\Injector: Authenticators: cms: '%$SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator' SilverStripe\Security\IdentityStore: '%$SilverStripe\Security\AuthenticationHandler' + SilverStripe\Security\SudoMode\SudoModeServiceInterface: + class: SilverStripe\Security\SudoMode\SudoModeService SilverStripe\Security\PasswordExpirationMiddleware: default_redirect: Security/changepassword diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index c2fbd399c..97ad8e6ab 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -6,8 +6,11 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\Cookie; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\Session; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Security\AuthenticationHandler; use SilverStripe\Security\Member; +use SilverStripe\Security\SudoMode\SudoModeServiceInterface; /** * Authenticate a member passed on a session cookie @@ -72,13 +75,20 @@ class SessionAuthenticationHandler implements AuthenticationHandler { static::regenerateSessionId(); $request = $request ?: Controller::curr()->getRequest(); - $request->getSession()->set($this->getSessionVariable(), $member->ID); + $session = $request->getSession(); + $session->set($this->getSessionVariable(), $member->ID); // This lets apache rules detect whether the user has logged in // @todo make this a setting on the authentication handler if (Member::config()->get('login_marker_cookie')) { Cookie::set(Member::config()->get('login_marker_cookie'), 1, 0); } + + // Activate sudo mode on login so the user doesn't have to reauthenticate for sudo + // actions until the sudo mode timeout expires + /** @var SudoModeServiceInterface $service */ + $service = Injector::inst()->get(SudoModeServiceInterface::class); + $service->activate($session); } /** diff --git a/src/Security/SudoMode/SudoModeService.php b/src/Security/SudoMode/SudoModeService.php new file mode 100644 index 000000000..33deb10bf --- /dev/null +++ b/src/Security/SudoMode/SudoModeService.php @@ -0,0 +1,52 @@ +get(self::SUDO_MODE_SESSION_KEY); + // Not activated at all + if (!$lastActivated) { + return false; + } + + // Activated within the last "lifetime" window + $nowTimestamp = DBDatetime::now()->getTimestamp(); + + return $lastActivated > ($nowTimestamp - $this->getLifetime() * 60); + } + + public function activate(Session $session): bool + { + $session->set(self::SUDO_MODE_SESSION_KEY, DBDatetime::now()->getTimestamp()); + return true; + } + + public function getLifetime(): int + { + return (int) static::config()->get('lifetime_minutes'); + } +} diff --git a/src/Security/SudoMode/SudoModeServiceInterface.php b/src/Security/SudoMode/SudoModeServiceInterface.php new file mode 100644 index 000000000..11b1ede43 --- /dev/null +++ b/src/Security/SudoMode/SudoModeServiceInterface.php @@ -0,0 +1,31 @@ +session = new Session([]); + $this->service = new SudoModeService(); + + DBDatetime::set_mock_now('2019-03-01 12:00:00'); + SudoModeService::config()->set('lifetime_minutes', 180); + } + + public function testCheckWithoutActivation() + { + $this->session->clearAll(); + $this->assertFalse($this->service->check($this->session)); + } + + public function testCheckWithLastActivationOutsideLifetimeWindow() + { + // 240 minutes ago + $lastActivated = DBDatetime::now()->getTimestamp() - 240 * 60; + $this->session->set('sudo-mode-last-activated', $lastActivated); + $this->assertFalse($this->service->check($this->session)); + } + + public function testCheckWithLastActivationInsideLifetimeWindow() + { + // 25 minutes ago + $lastActivated = DBDatetime::now()->getTimestamp() - 25 * 60; + $this->session->set('sudo-mode-last-activated', $lastActivated); + $this->assertTrue($this->service->check($this->session)); + } + + public function testActivateAndCheckImmediately() + { + $this->service->activate($this->session); + $this->assertTrue($this->service->check($this->session)); + } + + public function testSudoModeActivatesOnLogin() + { + // Sometimes being logged in carries over from other tests + $this->logOut(); + + /** @var SudoModeServiceInterface $service */ + $service = Injector::inst()->get(SudoModeServiceInterface::class); + $session = Controller::curr()->getRequest()->getSession(); + + // Sudo mode should not be enabled automagically when nobody is logged in + $this->assertFalse($service->check($session)); + + // Ensure sudo mode is activated on login + $this->logInWithPermission(); + $this->assertTrue($service->check($session)); + + // Ensure sudo mode is not active after logging out + $this->logOut(); + $this->assertFalse($service->check($session)); + } +} From af72f4adade243b25d1b43c47d292861a5820ba7 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 30 Jan 2023 15:01:11 +1300 Subject: [PATCH 3/3] ENH Add translations from security-extensions module --- lang/da.yml | 1 + lang/de_DE.yml | 1 + lang/en.yml | 1 + lang/eo.yml | 1 + lang/nl.yml | 1 + lang/sl.yml | 1 + lang/sv.yml | 1 + 7 files changed, 7 insertions(+) diff --git a/lang/da.yml b/lang/da.yml index 11f2530f5..3513aae04 100644 --- a/lang/da.yml +++ b/lang/da.yml @@ -228,6 +228,7 @@ da: PLURALS: one: 'En bruger' other: '{count} brugere' + RequiresPasswordChangeOnNextLogin: 'Kræver ændring af kodeord næste gang der logges ind' SINGULARNAME: Bruger SUBJECTPASSWORDCHANGED: 'Dit kodeord er blevet ændret' SUBJECTPASSWORDRESET: 'Link til at nulstille dit kodeord' diff --git a/lang/de_DE.yml b/lang/de_DE.yml index 63202c75e..023b5ba25 100644 --- a/lang/de_DE.yml +++ b/lang/de_DE.yml @@ -135,6 +135,7 @@ de_DE: PLURALS: one: 'Ein Mitglied' other: '{count} Mitglieder' + RequiresPasswordChangeOnNextLogin: 'Beim nächsten Login muss das Passwort geändert werden.' SINGULARNAME: Mitglied SURNAME: Nachname YOUROLDPASSWORD: 'Ihr altes Passwort' diff --git a/lang/en.yml b/lang/en.yml index c4155571b..1fae6ffdb 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -276,6 +276,7 @@ en: PLURALS: one: 'A Member' other: '{count} Members' + RequiresPasswordChangeOnNextLogin: 'Requires password change on next log in' SINGULARNAME: Member SUBJECTPASSWORDCHANGED: 'Your password has been changed' SUBJECTPASSWORDRESET: 'Your password reset link' diff --git a/lang/eo.yml b/lang/eo.yml index 0776ee3e0..fa4372c7e 100644 --- a/lang/eo.yml +++ b/lang/eo.yml @@ -277,6 +277,7 @@ eo: PLURALS: one: 'Unu membro' other: '{count} membroj' + RequiresPasswordChangeOnNextLogin: 'Bezonas ŝanĝi pasvorton je sekva elsaluto' SINGULARNAME: Membro SUBJECTPASSWORDCHANGED: 'Via pasvorto estas ŝanĝita' SUBJECTPASSWORDRESET: 'Via pasvorto reagordis ligilon' diff --git a/lang/nl.yml b/lang/nl.yml index 9d7104cb5..3bfdefacb 100644 --- a/lang/nl.yml +++ b/lang/nl.yml @@ -275,6 +275,7 @@ nl: PLURALS: one: 'Een lid' other: '{count} leden' + RequiresPasswordChangeOnNextLogin: 'Wachtwoord veranderen is verplicht bij volgende aanmelding' SINGULARNAME: Lid SUBJECTPASSWORDCHANGED: 'Je wachtwoord is aangepast' SUBJECTPASSWORDRESET: 'Je wachtwoord opnieuw instellen' diff --git a/lang/sl.yml b/lang/sl.yml index 77b0d418c..6158c33ca 100644 --- a/lang/sl.yml +++ b/lang/sl.yml @@ -294,6 +294,7 @@ sl: two: '{count} uporabnika' few: '{count} uporabnikov' other: '{count} uporabnikov' + RequiresPasswordChangeOnNextLogin: 'Ob naslednji prijavi zahtevaj spremembo gesla' SINGULARNAME: Uporabnik SUBJECTPASSWORDCHANGED: 'Geslo je bilo spremenjeno' SUBJECTPASSWORDRESET: 'Povezava za resetiranje vašega gesla' diff --git a/lang/sv.yml b/lang/sv.yml index 9da66f8ce..82f93b14d 100644 --- a/lang/sv.yml +++ b/lang/sv.yml @@ -224,6 +224,7 @@ sv: PLURALS: one: 'En medlem' other: '{count} medlemmar' + RequiresPasswordChangeOnNextLogin: 'Kräver ändring av lösenord vid nästa inloggning' SINGULARNAME: Medlem SUBJECTPASSWORDCHANGED: 'Ditt lösenord har ändrats' SUBJECTPASSWORDRESET: 'Din återställningslänk'