From 84cdae1f7c29682c1e24c003272b0868d5eab0c0 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Wed, 5 Jun 2024 17:05:35 +1200 Subject: [PATCH] ENH Use class name instead of self --- src/Authenticator/ChangePasswordHandler.php | 4 +- src/Authenticator/LoginHandler.php | 2 +- src/BackupCode/VerifyHandler.php | 7 +- .../AdminRegistrationController.php | 2 +- src/Extension/MemberExtension.php | 4 +- .../RegisteredMFAMethodListField.php | 2 +- src/Service/Notification.php | 2 +- src/Service/RegisteredMethodManager.php | 6 +- .../php/Authenticator/RegisterHandlerTest.php | 64 +++++++++++++++---- 9 files changed, 66 insertions(+), 27 deletions(-) diff --git a/src/Authenticator/ChangePasswordHandler.php b/src/Authenticator/ChangePasswordHandler.php index 41942021..5a662c17 100644 --- a/src/Authenticator/ChangePasswordHandler.php +++ b/src/Authenticator/ChangePasswordHandler.php @@ -192,7 +192,7 @@ public function verifyMFACheck(HTTPRequest $request): HTTPResponse ], 202); } - $request->getSession()->set(self::MFA_VERIFIED_ON_CHANGE_PASSWORD, true); + $request->getSession()->set(ChangePasswordHandler::MFA_VERIFIED_ON_CHANGE_PASSWORD, true); $store->clear($request); return $this->jsonResponse([ @@ -211,7 +211,7 @@ public function changepassword() $hash && $member && $member->RegisteredMFAMethods()->exists() - && !$session->get(self::MFA_VERIFIED_ON_CHANGE_PASSWORD) + && !$session->get(ChangePasswordHandler::MFA_VERIFIED_ON_CHANGE_PASSWORD) ) { Injector::inst()->create(StoreInterface::class, $member)->save($this->getRequest()); return $this->mfa(); diff --git a/src/Authenticator/LoginHandler.php b/src/Authenticator/LoginHandler.php index 7f764d7c..b05486ec 100644 --- a/src/Authenticator/LoginHandler.php +++ b/src/Authenticator/LoginHandler.php @@ -521,7 +521,7 @@ public function getMember() * @param LoggerInterface $logger * @return $this */ - public function setLogger(LoggerInterface $logger): self + public function setLogger(LoggerInterface $logger): LoginHandler { $this->logger = $logger; return $this; diff --git a/src/BackupCode/VerifyHandler.php b/src/BackupCode/VerifyHandler.php index 6110a614..c338ff6d 100644 --- a/src/BackupCode/VerifyHandler.php +++ b/src/BackupCode/VerifyHandler.php @@ -25,7 +25,7 @@ class VerifyHandler implements VerifyHandlerInterface */ protected $notification; - public function setNotificationService(Notification $notification): self + public function setNotificationService(Notification $notification): VerifyHandler { $this->notification = $notification; return $this; @@ -92,7 +92,10 @@ public function verify(HTTPRequest $request, StoreInterface $store, RegisteredMe $registeredMethod->Member(), 'SilverStripe/MFA/Email/Notification_backupcodeused', [ - 'subject' => _t(self::class . '.MFAREMOVED', 'A recovery code was used to access your account'), + 'subject' => _t( + VerifyHandler::class . '.MFAREMOVED', + 'A recovery code was used to access your account' + ), 'CodesRemaining' => count($candidates ?? []), ] ); diff --git a/src/Controller/AdminRegistrationController.php b/src/Controller/AdminRegistrationController.php index 2a75cc53..e28d71e0 100644 --- a/src/Controller/AdminRegistrationController.php +++ b/src/Controller/AdminRegistrationController.php @@ -272,7 +272,7 @@ protected function jsonResponse(array $response, int $code = 200): HTTPResponse * @param LoggerInterface|null $logger * @return $this */ - public function setLogger(?LoggerInterface $logger): self + public function setLogger(?LoggerInterface $logger): AdminRegistrationController { $this->logger = $logger; return $this; diff --git a/src/Extension/MemberExtension.php b/src/Extension/MemberExtension.php index bc072337..20c278b7 100644 --- a/src/Extension/MemberExtension.php +++ b/src/Extension/MemberExtension.php @@ -121,7 +121,7 @@ public function updateCMSFields(FieldList $fields): FieldList */ public function currentUserCanViewMFAConfig(): bool { - return (Permission::check(self::MFA_ADMINISTER_REGISTERED_METHODS) + return (Permission::check(MemberExtension::MFA_ADMINISTER_REGISTERED_METHODS) || $this->currentUserCanEditMFAConfig()); } @@ -160,7 +160,7 @@ public function providePermissions(): array ); return [ - self::MFA_ADMINISTER_REGISTERED_METHODS => [ + MemberExtension::MFA_ADMINISTER_REGISTERED_METHODS => [ 'name' => $label, 'category' => $category, 'help' => $description, diff --git a/src/FormField/RegisteredMFAMethodListField.php b/src/FormField/RegisteredMFAMethodListField.php index a2cacf2d..65dc4d34 100644 --- a/src/FormField/RegisteredMFAMethodListField.php +++ b/src/FormField/RegisteredMFAMethodListField.php @@ -30,7 +30,7 @@ public function __construct(string $name, ?string $title, int $value) public function Field($properties = array()) { - return $this->renderWith(self::class); + return $this->renderWith(RegisteredMFAMethodListField::class); } /** diff --git a/src/Service/Notification.php b/src/Service/Notification.php index 84da3fb3..80d5a040 100644 --- a/src/Service/Notification.php +++ b/src/Service/Notification.php @@ -43,7 +43,7 @@ class Notification */ protected $logger; - public function setLogger(LoggerInterface $logger): self + public function setLogger(LoggerInterface $logger): Notification { $this->logger = $logger; return $this; diff --git a/src/Service/RegisteredMethodManager.php b/src/Service/RegisteredMethodManager.php index 71dadf97..db18eb03 100644 --- a/src/Service/RegisteredMethodManager.php +++ b/src/Service/RegisteredMethodManager.php @@ -31,7 +31,7 @@ class RegisteredMethodManager */ protected $notification; - public function setNotificationService(Notification $notification): self + public function setNotificationService(Notification $notification): RegisteredMethodManager { $this->notification = $notification; return $this; @@ -94,7 +94,7 @@ public function registerForMember(Member $member, MethodInterface $method, $data 'SilverStripe/MFA/Email/Notification_register', [ 'subject' => _t( - self::class . '.MFAADDED', + RegisteredMethodManager::class . '.MFAADDED', 'A multi-factor authentication method was added to your account' ), 'MethodName' => $method->getName(), @@ -186,7 +186,7 @@ public function deleteFromMember(Member $member, MethodInterface $method): bool 'SilverStripe/MFA/Email/Notification_removed', [ 'subject' => _t( - self::class . '.MFAREMOVED', + RegisteredMethodManager::class . '.MFAREMOVED', 'A multi-factor authentication method was removed from your account' ), 'MethodName' => $method->getName(), diff --git a/tests/php/Authenticator/RegisterHandlerTest.php b/tests/php/Authenticator/RegisterHandlerTest.php index abcff8f8..42beab13 100644 --- a/tests/php/Authenticator/RegisterHandlerTest.php +++ b/tests/php/Authenticator/RegisterHandlerTest.php @@ -44,7 +44,7 @@ protected function setUp(): void */ public function testRegisterRouteIsPrivateWithGETMethod() { - $response = $this->get(self::URL); + $response = $this->get(RegisterHandlerTest::URL); $this->assertEquals(403, $response->getStatusCode()); } @@ -54,7 +54,7 @@ public function testRegisterRouteIsPrivateWithGETMethod() public function testRegisterRouteIsPrivateWithPOSTMethod() { // See https://github.com/silverstripe/silverstripe-framework/pull/8987 for why we have to provide $data. - $response = $this->post(self::URL, ['dummy' => 'data']); + $response = $this->post(RegisterHandlerTest::URL, ['dummy' => 'data']); $this->assertEquals(403, $response->getStatusCode()); } @@ -83,7 +83,7 @@ public function testStartRegistrationFailsWhenRegisteredMethodExists() $this->scaffoldPartialLogin($staleMember); - $response = $this->get(self::URL); + $response = $this->get(RegisterHandlerTest::URL); $this->assertEquals(400, $response->getStatusCode()); $this->assertStringContainsString('This member already has an MFA method', $response->getBody()); } @@ -95,7 +95,7 @@ public function testStartRegistrationFailsWhenMethodIsAlreadyRegistered() { $this->logInAs('stale-member'); - $response = $this->get(self::URL); + $response = $this->get(RegisterHandlerTest::URL); $this->assertEquals(400, $response->getStatusCode()); $this->assertStringContainsString( 'That method has already been registered against this Member', @@ -113,7 +113,7 @@ public function testStartRegistrationSucceeds() $this->scaffoldPartialLogin($freshMember); - $response = $this->get(self::URL); + $response = $this->get(RegisterHandlerTest::URL); $this->assertEquals(200, $response->getStatusCode(), sprintf('Body: %s', $response->getBody())); } @@ -126,7 +126,7 @@ public function testStartRegistrationProvidesACSRFToken() $this->scaffoldPartialLogin($freshMember); - $response = $this->get(self::URL); + $response = $this->get(RegisterHandlerTest::URL); $this->assertEquals(200, $response->getStatusCode(), sprintf('Body: %s', $response->getBody())); $this->assertSame(SecurityToken::inst()->getValue(), json_decode($response->getBody())->SecurityID); } @@ -141,7 +141,13 @@ public function testFinishRegistrationFailsWhenCalledDirectly() $this->scaffoldPartialLogin($freshMember); - $response = $this->post(self::URL, ['dummy' => 'data'], null, $this->session(), json_encode(['number' => 7])); + $response = $this->post( + RegisterHandlerTest::URL, + ['dummy' => 'data'], + null, + $this->session(), + json_encode(['number' => 7]) + ); $this->assertEquals(400, $response->getStatusCode()); $this->assertStringContainsString('No registration in progress', $response->getBody()); } @@ -154,9 +160,15 @@ public function testFinishRegistrationFailsWhenMethodIsMismatched() /** @var Member $freshMember */ $freshMember = $this->objFromFixture(Member::class, 'fresh-member'); - $this->scaffoldPartialLogin($freshMember, self::class); // Purposefully set to the wrong class + $this->scaffoldPartialLogin($freshMember, RegisterHandlerTest::class); // Purposefully set to the wrong class - $response = $this->post(self::URL, ['dummy' => 'data'], null, $this->session(), json_encode(['number' => 7])); + $response = $this->post( + RegisterHandlerTest::URL, + ['dummy' => 'data'], + null, + $this->session(), + json_encode(['number' => 7]) + ); $this->assertEquals(400, $response->getStatusCode()); $this->assertStringContainsString('Method does not match registration in progress', $response->getBody()); } @@ -192,7 +204,13 @@ public function testFinishRegistrationFailsWhenMethodCannotBeRegistered() $this->scaffoldPartialLogin($freshMember, 'mock-method'); - $response = $this->post(self::URL, ['dummy' => 'data'], null, $this->session(), json_encode(['number' => 7])); + $response = $this->post( + RegisterHandlerTest::URL, + ['dummy' => 'data'], + null, + $this->session(), + json_encode(['number' => 7]) + ); $this->assertEquals(400, $response->getStatusCode()); $this->assertStringContainsString('No. Bad user', $response->getBody()); } @@ -207,7 +225,13 @@ public function testFinishRegistrationSucceeds() $this->scaffoldPartialLogin($freshMember, 'basic-math'); - $response = $this->post(self::URL, ['dummy' => 'data'], null, $this->session(), json_encode(['number' => 7])); + $response = $this->post( + RegisterHandlerTest::URL, + ['dummy' => 'data'], + null, + $this->session(), + json_encode(['number' => 7]) + ); $this->assertEquals(201, $response->getStatusCode()); // Make sure the registration made it into the database @@ -225,14 +249,20 @@ public function testFinishRegistrationValidatesCSRF() $this->scaffoldPartialLogin($freshMember); - $response = $this->post(self::URL, ['dummy' => 'data'], null, $this->session(), json_encode(['number' => 7])); + $response = $this->post( + RegisterHandlerTest::URL, + ['dummy' => 'data'], + null, + $this->session(), + json_encode(['number' => 7]) + ); $this->assertEquals(403, $response->getStatusCode()); $this->assertStringContainsString('Your request timed out', $response->getBody()); $this->scaffoldPartialLogin($freshMember, 'basic-math'); $response = $this->post( - self::URL, + RegisterHandlerTest::URL, [SecurityToken::inst()->getName() => SecurityToken::inst()->getValue()], null, $this->session(), @@ -251,7 +281,13 @@ public function testEnforcesSudoMode() $freshMember = $this->objFromFixture(Member::class, 'fresh-member'); $this->scaffoldPartialLogin($freshMember, 'basic-math'); - $response = $this->post(self::URL, ['dummy' => 'data'], null, $this->session(), json_encode(['number' => 7])); + $response = $this->post( + RegisterHandlerTest::URL, + ['dummy' => 'data'], + null, + $this->session(), + json_encode(['number' => 7]) + ); $this->assertSame(403, $response->getStatusCode()); $this->assertStringContainsString('You must be logged in or logging in', (string) $response->getBody()); }