From 48a4dfa74e51a0f2481940e44f6e9ddd61c40bbf Mon Sep 17 00:00:00 2001 From: Garion Herman Date: Tue, 10 Sep 2019 11:23:06 +1200 Subject: [PATCH 1/2] FIX Prevent incompatible versions of Subsites from installing alongside MFA --- composer.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/composer.json b/composer.json index 89f348e7..7858f78c 100644 --- a/composer.json +++ b/composer.json @@ -39,6 +39,9 @@ "silverstripe/reports": "^3.7", "squizlabs/php_codesniffer": "^3.0" }, + "conflict": { + "silverstripe/subsites": "<1.4.2" + }, "suggest": { "silverstripe/totp-authenticator": "Adds a method to authenticate with you phone using a time-based one-time password.", "silverstripe/webauthn-authenticator": "Adds a method to authenticate with security keys or built-in platform authenticators." From c7677217b47273e1d3c2ee54a90e7347e98e971a Mon Sep 17 00:00:00 2001 From: Garion Herman Date: Thu, 12 Sep 2019 10:44:58 +1200 Subject: [PATCH 2/2] FIX Resolve new PSR-12 linting violations --- src/Authenticator/LoginForm.php | 26 ++++++++++++++----- src/Authenticator/MemberAuthenticator.php | 1 + src/BackupCode/Method.php | 4 ++- src/BackupCode/RegisterHandler.php | 4 ++- src/BackupCode/VerifyHandler.php | 4 ++- .../AdminRegistrationController.php | 7 +++-- src/Exception/InvalidMethodException.php | 1 + .../AccountReset/AccountResetHandler.php | 1 - .../AccountReset/MFAResetExtension.php | 4 ++- .../AccountReset/MemberExtension.php | 10 ++++--- .../AccountReset/SecurityAdminExtension.php | 4 ++- .../AccountReset/SecurityExtension.php | 4 ++- src/Extension/ChangePasswordExtension.php | 7 +++-- src/Extension/MemberExtension.php | 4 ++- src/JSONResponse.php | 4 ++- .../Handler/RegisterHandlerInterface.php | 4 ++- src/Method/Handler/VerifyHandlerInterface.php | 4 ++- src/Method/MethodInterface.php | 4 ++- src/Model/MFARegisteredMethod.php | 4 ++- src/Report/EnabledMembers.php | 4 ++- src/Service/BackupCodeGenerator.php | 4 ++- src/Service/BackupCodeGeneratorInterface.php | 4 ++- src/Service/EncryptionAdapterInterface.php | 4 ++- src/Service/EnforcementManager.php | 7 +++-- src/Service/MethodRegistry.php | 4 ++- src/Service/Notification.php | 4 ++- src/Service/RegisteredMethodManager.php | 4 ++- src/State/AvailableMethodDetails.php | 4 ++- src/State/BackupCode.php | 4 ++- src/State/RegisteredMethodDetails.php | 4 ++- src/State/Result.php | 4 ++- src/Store/SessionStore.php | 4 ++- src/Store/StoreInterface.php | 4 ++- tests/Behat/Context/LoginContext.php | 4 ++- tests/php/Service/BackupCodeGeneratorTest.php | 4 ++- tests/php/Stub/BasicMath/Method.php | 1 + .../Stub/BasicMath/MethodRegisterHandler.php | 4 ++- .../Stub/BasicMath/MethodVerifyHandler.php | 4 ++- 38 files changed, 131 insertions(+), 46 deletions(-) diff --git a/src/Authenticator/LoginForm.php b/src/Authenticator/LoginForm.php index 32394322..550a1b79 100644 --- a/src/Authenticator/LoginForm.php +++ b/src/Authenticator/LoginForm.php @@ -1,4 +1,6 @@ -getMember() : null; $loggedInMember = Member::currentUser(); - if (($loggedInMember === null && $sessionMember === null) + if ( + ($loggedInMember === null + && $sessionMember === null) || !$this->getSudoModeService()->check($this->controller->getSession()) ) { return $this->jsonResponse( @@ -242,7 +246,9 @@ public function finishRegistration(): HTTPResponse $sessionMember = $store ? $store->getMember() : null; $loggedInMember = Member::currentUser(); - if (($loggedInMember === null && $sessionMember === null) + if ( + ($loggedInMember === null + && $sessionMember === null) || !$this->getSudoModeService()->check($this->controller->getSession() ?: new Session([])) ) { return $this->jsonResponse( @@ -275,7 +281,9 @@ public function finishRegistration(): HTTPResponse // required to log in though. The "mustLogin" flag is set at the beginning of the MFA process if they have at // least one method registered. They should always do that first. In that case we should assert // "isLoginComplete" - if ((!$mustLogin || $this->isVerificationComplete($store)) + if ( + (!$mustLogin + || $this->isVerificationComplete($store)) && $enforcementManager->hasCompletedRegistration($sessionMember) ) { $this->doPerformLogin($sessionMember); @@ -323,8 +331,11 @@ public function startVerification(): HTTPResponse $request = $this->getRequest(); $store = $this->getStore(); // If we don't have a valid member we shouldn't be here, or if sudo mode is not active yet. - if (!$store || !$store->getMember() || - !$this->getSudoModeService()->check($this->controller->getSession() ?: new Session([]))) { + if ( + !$store + || !$store->getMember() + || !$this->getSudoModeService()->check($this->controller->getSession() ?: new Session([])) + ) { return $this->jsonResponse(['message' => 'Forbidden'], 403); } @@ -428,7 +439,8 @@ public function redirectAfterSuccessfulLogin() // This is potentially redundant logic as the member should only be logged in if they've fully registered. // They're allowed to login if they can skip - so only do assertions if they're not allowed to skip // We'll also check that they've registered the required MFA details - if (!$enforcementManager->canSkipMFA($member) + if ( + !$enforcementManager->canSkipMFA($member) && !$enforcementManager->hasCompletedRegistration($member) ) { $member->logOut(); diff --git a/src/Authenticator/MemberAuthenticator.php b/src/Authenticator/MemberAuthenticator.php index 74f53069..75bbd582 100644 --- a/src/Authenticator/MemberAuthenticator.php +++ b/src/Authenticator/MemberAuthenticator.php @@ -1,4 +1,5 @@ getRequest(); // Ensure CSRF and sudo-mode protection - if (!SecurityToken::inst()->checkRequest($request) + if ( + !SecurityToken::inst()->checkRequest($request) || !$this->getSudoModeService()->check($this->getSession()) ) { return $this->jsonResponse( diff --git a/src/Exception/InvalidMethodException.php b/src/Exception/InvalidMethodException.php index d9cf4051..e4645956 100644 --- a/src/Exception/InvalidMethodException.php +++ b/src/Exception/InvalidMethodException.php @@ -1,4 +1,5 @@ randomToken(); $hash = $this->owner->encryptWithUserSettings($token); - } while (DataObject::get_one(Member::class, [ + } while ( + DataObject::get_one(Member::class, [ '"Member"."AccountResetHash"' => $hash, - ])); + ]) + ); $expiry = DBDatetime::create(); $expiry->setValue( diff --git a/src/Extension/AccountReset/SecurityAdminExtension.php b/src/Extension/AccountReset/SecurityAdminExtension.php index 1edad960..0aeb8e9f 100644 --- a/src/Extension/AccountReset/SecurityAdminExtension.php +++ b/src/Extension/AccountReset/SecurityAdminExtension.php @@ -1,4 +1,6 @@ -RegisteredMFAMethods()->exists() && !$session->get(self::MFA_VERIFIED_ON_CHANGE_PASSWORD) diff --git a/src/Extension/MemberExtension.php b/src/Extension/MemberExtension.php index 95cf365d..9e678348 100644 --- a/src/Extension/MemberExtension.php +++ b/src/Extension/MemberExtension.php @@ -1,4 +1,6 @@ -MainMenu(); foreach ($menu as $candidate) { - if ($candidate->Link + if ( + $candidate->Link && $candidate->Link !== $leftAndMain->Link() && $candidate->MenuItem->controller && singleton($candidate->MenuItem->controller)->canView($member) diff --git a/src/Service/MethodRegistry.php b/src/Service/MethodRegistry.php index 7dcbd892..88c6cb06 100644 --- a/src/Service/MethodRegistry.php +++ b/src/Service/MethodRegistry.php @@ -1,4 +1,6 @@ -