diff --git a/composer.json b/composer.json index b8d964d..95ed182 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ "guzzlehttp/psr7": "^2" }, "require-dev": { - "phpunit/phpunit": "^9.6", + "phpunit/phpunit": "^11.3", "squizlabs/php_codesniffer": "^3", "silverstripe/documentation-lint": "^1", "silverstripe/standards": "^1", diff --git a/src/VerifyHandler.php b/src/VerifyHandler.php index b420e06..74054a3 100644 --- a/src/VerifyHandler.php +++ b/src/VerifyHandler.php @@ -22,6 +22,7 @@ use Webauthn\PublicKeyCredentialSource; use Webauthn\TokenBinding\TokenBindingNotSupportedHandler; use Cose\Algorithm\Signature\ECDSA\ES256; +use Error; class VerifyHandler implements VerifyHandlerInterface { @@ -125,6 +126,10 @@ public function verify(HTTPRequest $request, StoreInterface $store, RegisteredMe private function makeAuthenticatorDataBase64UrlSafe(array $data): array { try { + if (!array_key_exists('credentials', $data)) { + // this wil be immediately caught in the catch block below() + throw new Exception(); + } $decodedCredentials = base64_decode($data['credentials']); $jsonCredientials = json_decode($decodedCredentials, true); $jsonCredientials['response']['authenticatorData'] = str_replace( diff --git a/tests/RegisterHandlerTest.php b/tests/RegisterHandlerTest.php index 9cdb0bb..950096a 100644 --- a/tests/RegisterHandlerTest.php +++ b/tests/RegisterHandlerTest.php @@ -26,6 +26,8 @@ use Webauthn\PublicKeyCredentialLoader; use Webauthn\PublicKeyCredentialSource; use Webauthn\TrustPath\EmptyTrustPath; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\TestCase; class RegisterHandlerTest extends SapphireTest { @@ -88,8 +90,8 @@ protected function tearDown(): void /** * @param string $baseUrl * @param string $expected - * @dataProvider hostProvider */ + #[DataProvider('hostProvider')] public function testRelyingPartyEntityDomainIncludesSilverStripeDomain(string $baseUrl, string $expected) { $_SERVER['HTTP_HOST'] = $baseUrl; @@ -112,7 +114,7 @@ public function testRelyingPartyEntityDomainIncludesSilverStripeDomain(string $b /** * @return array */ - public function hostProvider(): array + public static function hostProvider(): array { return [ 'domain only' => ['http://example.com', 'example.com'], @@ -167,8 +169,8 @@ public function testRegisterReturnsErrorWhenRequiredInformationIsMissing() * @param int $expectedCredentialCount * @param callable $responseValidatorMockCallback * @throws Exception - * @dataProvider registerProvider */ + #[DataProvider('registerProvider')] public function testRegister( $mockResponse, $expectedResult, @@ -178,7 +180,7 @@ public function testRegister( ) { /** @var RegisterHandler&MockObject $handlerMock */ $handlerMock = $this->getMockBuilder(RegisterHandler::class) - ->setMethods(['getPublicKeyCredentialLoader', 'getAuthenticatorAttestationResponseValidator']) + ->onlyMethods(['getPublicKeyCredentialLoader', 'getAuthenticatorAttestationResponseValidator']) ->getMock(); $publicKeyCredentialSourceMock = $this->createMock(PublicKeyCredentialSource::class); @@ -233,7 +235,7 @@ public function testRegister( * * @return array[] */ - public function registerProvider() + public static function registerProvider() { // phpcs:disable $testSource = PublicKeyCredentialSource::createFromArray([ @@ -253,27 +255,27 @@ public function registerProvider() 'counter' => 123456789, ]); // phpcs:enable - - $authDataMock = $this->createMock(AuthenticatorData::class); - $authDataMock->expects($this->exactly(4))->method('hasAttestedCredentialData') + $testCase = new RegisterHandlerTest('MyTestCase'); + $authDataMock = $testCase->createMock(AuthenticatorData::class); + $authDataMock->expects($testCase->exactly(4))->method('hasAttestedCredentialData') // The first call is the "response indicates incomplete data" test case, second is "valid response", // third is "invalid response" ->willReturnOnConsecutiveCalls(false, true, true, true); - $authDataMock->expects($this->any())->method('getAttestedCredentialData')->willReturn( + $authDataMock->expects($testCase->any())->method('getAttestedCredentialData')->willReturn( $testSource->getAttestedCredentialData() ); - $authDataMock->expects($this->any())->method('getSignCount')->willReturn(1); + $authDataMock->expects($testCase->any())->method('getSignCount')->willReturn(1); - $attestationMock = $this->createMock(AttestationObject::class); - $attestationMock->expects($this->any())->method('getAuthData')->willReturn($authDataMock); + $attestationMock = $testCase->createMock(AttestationObject::class); + $attestationMock->expects($testCase->any())->method('getAuthData')->willReturn($authDataMock); - $responseMock = $this->createMock(AuthenticatorAttestationResponse::class); - $responseMock->expects($this->any())->method('getAttestationObject')->willReturn($attestationMock); + $responseMock = $testCase->createMock(AuthenticatorAttestationResponse::class); + $responseMock->expects($testCase->any())->method('getAttestationObject')->willReturn($attestationMock); return [ 'wrong response return type' => [ // Deliberately the wrong child implementation of \Webauthn\AuthenticatorResponse - $this->createMock(AuthenticatorAssertionResponse::class), + $testCase->createMock(AuthenticatorAssertionResponse::class), new Result(false, 'Unexpected response type found'), 0, ], @@ -286,10 +288,10 @@ public function registerProvider() $responseMock, new Result(true), 1, - function (MockObject $responseValidatorMock) { + function (MockObject $responseValidatorMock) use ($testCase) { // Specifically setting expectations for the result of the response validator's "check" call $responseValidatorMock - ->expects($this->once()) + ->expects($testCase->once()) ->method('check') ->willReturnCallback(function (): bool { return true; @@ -300,10 +302,10 @@ function (MockObject $responseValidatorMock) { $responseMock, new Result(true), 1, - function (MockObject $responseValidatorMock) { + function (MockObject $responseValidatorMock) use ($testCase) { // Specifically setting expectations for the result of the response validator's "check" call $responseValidatorMock - ->expects($this->once()) + ->expects($testCase->once()) ->method('check') ->willReturnCallback(function (): bool { return true; @@ -336,9 +338,9 @@ function (SessionStore $store) use ($testSource) { $responseMock, new Result(false, 'I am a test'), 0, - function (MockObject $responseValidatorMock) { + function (MockObject $responseValidatorMock) use ($testCase) { // Specifically setting expectations for the result of the response validator's "check" call - $responseValidatorMock->expects($this->once())->method('check') + $responseValidatorMock->expects($testCase->once())->method('check') ->willThrowException(new Exception('I am a test')); }, ], diff --git a/tests/VerifyHandlerTest.php b/tests/VerifyHandlerTest.php index 994276f..b65387a 100644 --- a/tests/VerifyHandlerTest.php +++ b/tests/VerifyHandlerTest.php @@ -22,6 +22,7 @@ use Webauthn\PublicKeyCredentialSource; use Webauthn\TrustPath\EmptyTrustPath; use SilverStripe\WebAuthn\ResponseDataException; +use PHPUnit\Framework\Attributes\DataProvider; class VerifyHandlerTest extends SapphireTest { @@ -123,8 +124,8 @@ public function testVerifyReturnsErrorWhenRequiredInformationIsMissing() * @param AuthenticatorResponse $mockResponse * @param Result $expectedResult * @param callable $responseValidatorMockCallback - * @dataProvider verifyProvider */ + #[DataProvider('verifyProvider')] public function testVerify( $mockResponse, $expectedResult, @@ -132,7 +133,7 @@ public function testVerify( ) { /** @var VerifyHandler&MockObject $handlerMock */ $handlerMock = $this->getMockBuilder(VerifyHandler::class) - ->setMethods(['getPublicKeyCredentialLoader', 'getAuthenticatorAssertionResponseValidator']) + ->onlyMethods(['getPublicKeyCredentialLoader', 'getAuthenticatorAssertionResponseValidator']) ->getMock(); $publicKeyCredentialSourceMock = $this->createMock(PublicKeyCredentialSource::class); @@ -178,21 +179,22 @@ public function testVerify( * * @return array[] */ - public function verifyProvider() + public static function verifyProvider() { + $testCase = new VerifyHandlerTest('MyTestCase'); return [ 'wrong response return type' => [ // Deliberately the wrong child implementation of \Webauthn\AuthenticatorResponse - $this->createMock(AuthenticatorAttestationResponse::class), + $testCase->createMock(AuthenticatorAttestationResponse::class), new Result(false, 'Unexpected response type found'), ], 'valid response' => [ - $this->createMock(AuthenticatorAssertionResponse::class), + $testCase->createMock(AuthenticatorAssertionResponse::class), new Result(true), - function (MockObject $responseValidatorMock) { + function (MockObject $responseValidatorMock) use ($testCase) { // Specifically setting expectations for the result of the response validator's "check" call $responseValidatorMock - ->expects($this->once()) + ->expects($testCase->once()) ->method('check') ->willReturnCallback(function (): bool { return true; @@ -200,11 +202,11 @@ function (MockObject $responseValidatorMock) { }, ], 'invalid response' => [ - $this->createMock(AuthenticatorAssertionResponse::class), + $testCase->createMock(AuthenticatorAssertionResponse::class), new Result(false, 'I am a test'), - function (MockObject $responseValidatorMock) { + function (MockObject $responseValidatorMock) use ($testCase) { // Specifically setting expectations for the result of the response validator's "check" call - $responseValidatorMock->expects($this->once())->method('check') + $responseValidatorMock->expects($testCase->once())->method('check') ->willThrowException(new Exception('I am a test')); }, ], @@ -212,8 +214,8 @@ function (MockObject $responseValidatorMock) { } /** - * @dataProvider provideMakeAuthenticatorDataBase64UrlSafe */ + #[DataProvider('provideMakeAuthenticatorDataBase64UrlSafe')] public function testMakeAuthenticatorDataBase64UrlSafe(array $data, string $expected, bool $exception) { $reflector = new \ReflectionClass(VerifyHandler::class); @@ -232,7 +234,7 @@ public function testMakeAuthenticatorDataBase64UrlSafe(array $data, string $expe } } - public function provideMakeAuthenticatorDataBase64UrlSafe(): array + public static function provideMakeAuthenticatorDataBase64UrlSafe(): array { $makeData = function ($authenticatorData) { $a = []; @@ -258,12 +260,12 @@ public function provideMakeAuthenticatorDataBase64UrlSafe(): array 'exception' => false, ], 'change slash' => [ - 'credentials' => $makeData('abc-def/ghi'), + 'data' => $makeData('abc-def/ghi'), 'expected' => 'abc-def_ghi', 'exception' => false, ], 'change plus and slash' => [ - 'credentials' => $makeData('abc+def/ghi'), + 'data' => $makeData('abc+def/ghi'), 'expected' => 'abc-def_ghi', 'exception' => false, ],