Skip to content

Commit

Permalink
Merge pull request #545 from creative-commoners/pulls/5.2/remove-self
Browse files Browse the repository at this point in the history
ENH Use class name instead of self
  • Loading branch information
GuySartorelli authored Jun 17, 2024
2 parents 0870162 + 84cdae1 commit 4fe8a1c
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 27 deletions.
4 changes: 2 additions & 2 deletions src/Authenticator/ChangePasswordHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/Authenticator/LoginHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions src/BackupCode/VerifyHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ?? []),
]
);
Expand Down
2 changes: 1 addition & 1 deletion src/Controller/AdminRegistrationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/Extension/MemberExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/FormField/RegisteredMFAMethodListField.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Service/Notification.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/Service/RegisteredMethodManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
64 changes: 50 additions & 14 deletions tests/php/Authenticator/RegisterHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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());
}

Expand Down Expand Up @@ -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());
}
Expand All @@ -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',
Expand All @@ -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()));
}

Expand All @@ -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);
}
Expand All @@ -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());
}
Expand All @@ -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());
}
Expand Down Expand Up @@ -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());
}
Expand All @@ -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
Expand All @@ -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(),
Expand All @@ -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());
}
Expand Down

0 comments on commit 4fe8a1c

Please sign in to comment.