Merge pull request #437 from creative-commoners/pulls/4/sapphire-test…

…-nine

API phpunit 9 support

composer.json

@@ -25,16 +25,16 @@
         }
     ],
     "require": {
-        "php": ">=7.1.0",
-        "silverstripe/framework": "^4.1",
+        "php": "^7.3 || ^8.0",
+        "silverstripe/framework": "^4.10",
         "silverstripe/admin": "^1.1",
         "silverstripe/siteconfig": "^4.1",
         "defuse/php-encryption": "^2.2",
         "silverstripe/login-forms": "^4.0",
         "silverstripe/security-extensions": "^4.0"
     },
     "require-dev": {
-        "sminnee/phpunit": "^5.7",
+        "phpunit/phpunit": "^9.5",
         "squizlabs/php_codesniffer": "^3.0"
     },
     "conflict": {

phpunit.xml.dist

@@ -1,7 +1,9 @@
 <phpunit bootstrap="vendor/silverstripe/framework/tests/bootstrap.php" colors="true">
-    <testsuite name="Default">
-        <directory>tests</directory>
-    </testsuite>
+    <testsuites>
+        <testsuite name="Default">
+            <directory>tests</directory>
+        </testsuite>
+    </testsuites>
 
     <filter>
         <whitelist addUncoveredFilesFromWhitelist="true">

tests/Behat/Context/LoginContext.php

@@ -4,6 +4,7 @@
 
 namespace SilverStripe\MFA\Tests\Behat\Context;
 
+use PHPUnit\Framework\Assert;
 use SilverStripe\CMS\Tests\Behaviour\LoginContext as CMSLoginContext;
 use SilverStripe\BehatExtension\Context\LoginContext as BehatLoginContext;
 use SilverStripe\MFA\Extension\SiteConfigExtension;
@@ -32,7 +33,7 @@ public function multiFactorAuthenticationIsOptional()
     {
         /** @var SiteConfig&SiteConfigExtension $siteConfig */
         $siteConfig = SiteConfig::current_site_config();
-        assertNotNull($siteConfig, 'Current SiteConfig record could not be found!');
+        Assert::assertNotNull($siteConfig, 'Current SiteConfig record could not be found!');
 
         $siteConfig->MFARequired = false;
         $siteConfig->write();

tests/php/Authenticator/ChangePasswordHandlerTest.php

@@ -2,7 +2,8 @@
 
 namespace SilverStripe\MFA\Tests\Authenticator;
 
-use PHPUnit_Framework_MockObject_MockObject;
+use LogicException;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
@@ -26,7 +27,7 @@ class ChangePasswordHandlerTest extends FunctionalTest
 {
     protected static $fixture_file = 'ChangePasswordHandlerTest.yml';
 
-    protected function setUp()
+    protected function setUp(): void
     {
         parent::setUp();
 
@@ -60,7 +61,7 @@ public function testMFADoesNotLoadWhenAUserIsLoggedIn()
     {
         $this->logInAs('simon');
         $response = $this->get('Security/changepassword');
-        $this->assertContains('OldPassword', $response->getBody());
+        $this->assertStringContainsString('OldPassword', $response->getBody());
     }
 
     public function testMFADoesNotLoadWhenAUserDoesNotHaveRegisteredMethods()
@@ -71,8 +72,12 @@ public function testMFADoesNotLoadWhenAUserDoesNotHaveRegisteredMethods()
         $token = $member->generateAutologinTokenAndStoreHash();
         $response = $this->get("Security/changepassword?m={$memberId}&t={$token}");
 
-        $this->assertContains('NewPassword1', $response->getBody(), 'There should be a new password field');
-        $this->assertContains('NewPassword2', $response->getBody(), 'There should be a confirm new password field');
+        $this->assertStringContainsString('NewPassword1', $response->getBody(), 'There should be a new password field');
+        $this->assertStringContainsString(
+            'NewPassword2',
+            $response->getBody(),
+            'There should be a confirm new password field'
+        );
     }
 
     public function testMFALoadsWhenAUserHasConfiguredMethods()
@@ -83,8 +88,12 @@ public function testMFALoadsWhenAUserHasConfiguredMethods()
         $token = $member->generateAutologinTokenAndStoreHash();
         $response = $this->get("Security/changepassword?m={$memberId}&t={$token}");
 
-        $this->assertNotContains('type="password"', $response->getBody(), 'Password form should be circumvented');
-        $this->assertContains('id="mfa-app"', $response->getBody(), 'MFA screen should be displayed');
+        $this->assertStringNotContainsString(
+            'type="password"',
+            $response->getBody(),
+            'Password form should be circumvented'
+        );
+        $this->assertStringContainsString('id="mfa-app"', $response->getBody(), 'MFA screen should be displayed');
     }
 
     public function testGetSchema()
@@ -114,13 +123,11 @@ public function testMfaRedirectsBackWithoutMember()
         $this->assertEquals(302, $response->getStatusCode());
     }
 
-    /**
-     * @expectedException \LogicException
-     * @expectedExceptionMessage Store not found, please create one first.
-     */
     public function testStartMfaCheckThrowsExceptionWithoutStore()
     {
-        /** @var ChangePasswordHandler&PHPUnit_Framework_MockObject_MockObject $handler */
+        $this->expectException(\LogicException::class);
+        $this->expectExceptionMessage('Store not found, please create one first.');
+        /** @var ChangePasswordHandler&MockObject $handler */
         $handler = $this->getMockBuilder(ChangePasswordHandler::class)
             ->disableOriginalConstructor()
             ->setMethods(['getStore'])
@@ -133,7 +140,7 @@ public function testStartMfaCheckThrowsExceptionWithoutStore()
 
     public function testStartMfaReturnsForbiddenWithoutMember()
     {
-        /** @var ChangePasswordHandler&PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var ChangePasswordHandler&MockObject $handler */
         $handler = $this->getMockBuilder(ChangePasswordHandler::class)
             ->disableOriginalConstructor()
             ->setMethods(['getStore'])
@@ -150,7 +157,7 @@ public function testStartMfaReturnsForbiddenWithoutMember()
 
     public function testMfaStartsVerificationResponse()
     {
-        /** @var ChangePasswordHandler&PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var ChangePasswordHandler&MockObject $handler */
         $handler = $this->getMockBuilder(ChangePasswordHandler::class)
             ->disableOriginalConstructor()
             ->setMethods(['getStore', 'createStartVerificationResponse'])
@@ -175,13 +182,13 @@ public function testMfaStartsVerificationResponse()
 
     public function testVerifyMfaCheckReturnsForbiddenOnVerificationFailure()
     {
-        /** @var ChangePasswordHandler&PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var ChangePasswordHandler&MockObject $handler */
         $handler = $this->getMockBuilder(ChangePasswordHandler::class)
             ->disableOriginalConstructor()
             ->setMethods(['getStore', 'completeVerificationRequest'])
             ->getMock();
 
-        /** @var LoggerInterface&PHPUnit_Framework_MockObject_MockObject $logger */
+        /** @var LoggerInterface&MockObject $logger */
         $logger = $this->createMock(LoggerInterface::class);
         $logger->expects($this->once())->method('debug');
         $handler->setLogger($logger);
@@ -199,7 +206,7 @@ public function testVerifyMfaCheckReturnsForbiddenOnVerificationFailure()
 
     public function testVerifyMfaCheckReturnsUnsuccessfulValidationResult()
     {
-        /** @var ChangePasswordHandler&PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var ChangePasswordHandler&MockObject $handler */
         $handler = $this->getMockBuilder(ChangePasswordHandler::class)
             ->disableOriginalConstructor()
             ->setMethods(['getStore', 'completeVerificationRequest'])
@@ -214,12 +221,12 @@ public function testVerifyMfaCheckReturnsUnsuccessfulValidationResult()
 
         $response = $handler->verifyMFACheck(new HTTPRequest('GET', ''));
         $this->assertEquals(401, $response->getStatusCode());
-        $this->assertContains('It is a test', $response->getBody());
+        $this->assertStringContainsString('It is a test', $response->getBody());
     }
 
     public function testVerifyMfaReturnsWhenVerificationIsNotComplete()
     {
-        /** @var ChangePasswordHandler&PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var ChangePasswordHandler&MockObject $handler */
         $handler = $this->getMockBuilder(ChangePasswordHandler::class)
             ->disableOriginalConstructor()
             ->setMethods(['getStore', 'completeVerificationRequest', 'isVerificationComplete'])
@@ -233,12 +240,12 @@ public function testVerifyMfaReturnsWhenVerificationIsNotComplete()
 
         $response = $handler->verifyMFACheck(new HTTPRequest('GET', ''));
         $this->assertEquals(202, $response->getStatusCode());
-        $this->assertContains('Additional authentication required', $response->getBody());
+        $this->assertStringContainsString('Additional authentication required', $response->getBody());
     }
 
     public function testVerifyMfaResultSuccessful()
     {
-        /** @var ChangePasswordHandler&PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var ChangePasswordHandler&MockObject $handler */
         $handler = $this->getMockBuilder(ChangePasswordHandler::class)
             ->disableOriginalConstructor()
             ->setMethods(['getStore', 'completeVerificationRequest', 'isVerificationComplete'])
@@ -255,6 +262,6 @@ public function testVerifyMfaResultSuccessful()
         $response = $handler->verifyMFACheck($request);
 
         $this->assertEquals(200, $response->getStatusCode());
-        $this->assertContains('Multi-factor authenticated', $response->getBody());
+        $this->assertStringContainsString('Multi-factor authenticated', $response->getBody());
     }
 }

tests/php/Authenticator/LoginHandlerTest.php

@@ -2,7 +2,7 @@
 
 namespace SilverStripe\MFA\Tests\Authenticator;
 
-use PHPUnit_Framework_MockObject_MockObject;
+use PHPUnit\Framework\MockObject\MockObject;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
@@ -14,6 +14,7 @@
 use SilverStripe\MFA\Authenticator\LoginHandler;
 use SilverStripe\MFA\Authenticator\MemberAuthenticator;
 use SilverStripe\MFA\Extension\MemberExtension;
+use SilverStripe\MFA\Exception\MemberNotFoundException;
 use SilverStripe\MFA\Method\Handler\VerifyHandlerInterface;
 use SilverStripe\MFA\Method\MethodInterface;
 use SilverStripe\MFA\Model\RegisteredMethod;
@@ -35,7 +36,7 @@ class LoginHandlerTest extends FunctionalTest
 {
     protected static $fixture_file = 'LoginHandlerTest.yml';
 
-    protected function setUp()
+    protected function setUp(): void
     {
         parent::setUp();
         Config::modify()->set(MethodRegistry::class, 'methods', [Method::class]);
@@ -52,7 +53,7 @@ protected function setUp()
             ]
         ]);
 
-        /** @var SudoModeServiceInterface&PHPUnit_Framework_MockObject_MockObject $sudoModeService */
+        /** @var SudoModeServiceInterface&MockObject $sudoModeService */
         $sudoModeService = $this->createMock(SudoModeServiceInterface::class);
         $sudoModeService->expects($this->any())->method('check')->willReturn(true);
         Injector::inst()->registerService($sudoModeService, SudoModeServiceInterface::class);
@@ -76,7 +77,7 @@ public function testMFAStepIsAdded()
 
     public function testMFARedirectsBackWhenSudoModeIsInactive()
     {
-        /** @var SudoModeServiceInterface&PHPUnit_Framework_MockObject_MockObject $sudoModeService */
+        /** @var SudoModeServiceInterface&MockObject $sudoModeService */
         $sudoModeService = $this->createMock(SudoModeServiceInterface::class);
         $sudoModeService->expects($this->once())->method('check')->willReturn(false);
         Injector::inst()->registerService($sudoModeService, SudoModeServiceInterface::class);
@@ -150,7 +151,7 @@ public function testMFASchemaEndpointReturnsMethodDetails()
         $this->assertSame($method->getName(), $firstMethod['name']);
         $this->assertSame($registerHandler->getDescription(), $firstMethod['description']);
         $this->assertSame($registerHandler->getSupportLink(), $firstMethod['supportLink']);
-        $this->assertContains('client/dist/images', $firstMethod['thumbnail']);
+        $this->assertStringContainsString('client/dist/images', $firstMethod['thumbnail']);
         $this->assertSame('BasicMathRegister', $firstMethod['component']);
     }
 
@@ -181,7 +182,7 @@ public function testMFASchemaEndpointShowsRegisteredMethodsIfSetUp()
         $this->assertSame($method->getName(), $result['name']);
         $this->assertSame('BasicMathLogin', $result['component']);
         $this->assertSame('https://google.com', $result['supportLink']);
-        $this->assertContains('totp.svg', $result['thumbnail']);
+        $this->assertStringContainsString('totp.svg', $result['thumbnail']);
     }
 
     public function testMFASchemaEndpointProvidesDefaultMethodIfSet()
@@ -221,7 +222,7 @@ public function testCannotSkipMFA($mfaRequired, $member = 'robbie')
         }
 
         $response = $this->get(Controller::join_links(Security::login_url(), 'default/mfa/skip'));
-        $this->assertContains('You cannot skip MFA registration', $response->getBody());
+        $this->assertStringContainsString('You cannot skip MFA registration', $response->getBody());
     }
 
     /**
@@ -308,11 +309,9 @@ public function testBackURLIsPreservedWhenSkipping($memberFixture)
         $this->assertStringEndsWith('admin/pages', $response->getHeader('location'));
     }
 
-    /**
-     * @expectedException \SilverStripe\MFA\Exception\MemberNotFoundException
-     */
     public function testGetMemberThrowsExceptionWithoutMember()
     {
+        $this->expectException(MemberNotFoundException::class);
         $this->logOut();
         $handler = new LoginHandler('foo', $this->createMock(MemberAuthenticator::class));
         $handler->setRequest(new HTTPRequest('GET', '/'));
@@ -393,7 +392,7 @@ public function testVerifyAssertsValidCSRFToken()
         $response = $handler->finishVerification($request);
 
         $this->assertSame(403, $response->getStatusCode());
-        $this->assertContains('Your request timed out', $response->getBody());
+        $this->assertStringContainsString('Your request timed out', $response->getBody());
 
         $request = new HTTPRequest('GET', '/', [
             SecurityToken::inst()->getName() => SecurityToken::inst()->getValue()
@@ -437,7 +436,7 @@ public function testStartVerificationReturnsForbiddenWithoutSudoMode()
         $member = $this->objFromFixture(Member::class, 'robbie');
         $this->scaffoldPartialLogin($member);
 
-        /** @var SudoModeServiceInterface&PHPUnit_Framework_MockObject_MockObject $sudoModeService */
+        /** @var SudoModeServiceInterface&MockObject $sudoModeService */
         $sudoModeService = $this->createMock(SudoModeServiceInterface::class);
         $sudoModeService->method('check')->willReturn(false);
         Injector::inst()->registerService($sudoModeService, SudoModeServiceInterface::class);
@@ -472,7 +471,7 @@ public function testFinishVerificationHandlesMembersLockedOut()
 
         $response = $handler->finishVerification($request);
         $this->assertEquals(403, $response->getStatusCode());
-        $this->assertContains('Your account is temporarily locked', (string) $response->getBody());
+        $this->assertStringContainsString('Your account is temporarily locked', (string) $response->getBody());
     }
 
     public function testFinishVerificationChecksSudoModeIsActive()
@@ -488,14 +487,17 @@ public function testFinishVerificationChecksSudoModeIsActive()
         $store->setMethod('basic-math');
         $handler->setStore($store);
 
-        /** @var SudoModeServiceInterface&PHPUnit_Framework_MockObject_MockObject $sudoModeService */
+        /** @var SudoModeServiceInterface&MockObject $sudoModeService */
         $sudoModeService = $this->createMock(SudoModeServiceInterface::class);
         $sudoModeService->method('check')->willReturn(false);
         Injector::inst()->registerService($sudoModeService, SudoModeServiceInterface::class);
 
         $response = $handler->finishVerification($request);
         $this->assertEquals(403, $response->getStatusCode());
-        $this->assertContains('You need to re-verify your account before continuing', (string) $response->getBody());
+        $this->assertStringContainsString(
+            'You need to re-verify your account before continuing',
+            (string) $response->getBody()
+        );
     }
 
     public function testFinishVerificationPassesExceptionMessagesThroughFromMethodsWithValidationFailures()
@@ -505,7 +507,7 @@ public function testFinishVerificationPassesExceptionMessagesThroughFromMethodsW
         $member->config()->set('lock_out_after_incorrect_logins', 5);
         $failedLogins = $member->FailedLoginCount;
 
-        /** @var LoginHandler|PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var LoginHandler|MockObject $handler */
         $handler = $this->getMockBuilder(LoginHandler::class)
             ->setMethods(['completeVerificationRequest'])
             ->disableOriginalConstructor()
@@ -524,7 +526,7 @@ public function testFinishVerificationPassesExceptionMessagesThroughFromMethodsW
         $response = $handler->finishVerification($request);
 
         $this->assertEquals(401, $response->getStatusCode());
-        $this->assertContains('It failed because it\'s mocked', (string) $response->getBody());
+        $this->assertStringContainsString('It failed because it\'s mocked', (string) $response->getBody());
         $this->assertSame($failedLogins + 1, $member->FailedLoginCount, 'Failed login is registered');
     }
 
@@ -540,7 +542,7 @@ public function testFinishVerificationWillRedirectToTheBackURLSetAsLoginIsStarte
 
         $this->doLogin($member, 'Password123', 'admin/pages');
 
-        /** @var LoginHandler|PHPUnit_Framework_MockObject_MockObject $handler */
+        /** @var LoginHandler|MockObject $handler */
         $handler = $this->getMockBuilder(LoginHandler::class)
             ->setMethods(['completeVerificationRequest'])
             ->disableOriginalConstructor()