From 46888bc6464eae06f69750155e287ae614a03cf7 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Wed, 13 Apr 2022 17:17:37 +1200 Subject: [PATCH] ENH PHP 8.1 compatibility --- src/BackupCode/RegisterHandler.php | 4 ++-- src/BackupCode/VerifyHandler.php | 10 +++++----- src/Extension/MemberExtension.php | 2 +- src/Report/EnabledMembers.php | 2 +- src/RequestHandler/VerificationHandlerTrait.php | 5 +++-- src/Service/BackupCodeGenerator.php | 10 +++++----- src/Service/EnforcementManager.php | 2 +- src/Service/MethodRegistry.php | 16 ++++++++-------- src/Service/Notification.php | 2 +- src/Service/SchemaGenerator.php | 4 ++-- src/Store/SessionStore.php | 6 +++--- .../Authenticator/ChangePasswordHandlerTest.php | 2 +- tests/php/Authenticator/LoginHandlerTest.php | 8 ++++---- tests/php/BackupCode/RegisterHandlerTest.php | 10 +++++++--- tests/php/BackupCode/VerifyHandlerTest.php | 2 +- tests/php/Service/BackupCodeGeneratorTest.php | 2 +- .../php/Service/RegisteredMethodManagerTest.php | 2 +- .../php/Stub/BasicMath/MethodRegisterHandler.php | 4 ++-- tests/php/Stub/BasicMath/MethodVerifyHandler.php | 4 ++-- 19 files changed, 51 insertions(+), 46 deletions(-) diff --git a/src/BackupCode/RegisterHandler.php b/src/BackupCode/RegisterHandler.php index 50ebfe3a..7d81a55c 100644 --- a/src/BackupCode/RegisterHandler.php +++ b/src/BackupCode/RegisterHandler.php @@ -66,14 +66,14 @@ public function start(StoreInterface $store): array $method, array_map(function (BackupCode $backupCode) { return json_encode($backupCode); - }, $codes) + }, $codes ?? []) ); // Return un-hashed codes for the front-end UI return [ 'codes' => array_map(function (BackupCode $backupCode) { return $backupCode->getCode(); - }, $codes), + }, $codes ?? []), ]; } diff --git a/src/BackupCode/VerifyHandler.php b/src/BackupCode/VerifyHandler.php index 094a492c..2f4e6f06 100644 --- a/src/BackupCode/VerifyHandler.php +++ b/src/BackupCode/VerifyHandler.php @@ -59,7 +59,7 @@ public function start(StoreInterface $store, RegisteredMethod $method): array */ public function verify(HTTPRequest $request, StoreInterface $store, RegisteredMethod $registeredMethod): Result { - $bodyJSON = json_decode($request->getBody(), true); + $bodyJSON = json_decode($request->getBody() ?? '', true); if (!isset($bodyJSON['code'])) { throw new RuntimeException( @@ -69,10 +69,10 @@ public function verify(HTTPRequest $request, StoreInterface $store, RegisteredMe $code = $bodyJSON['code']; - $candidates = json_decode($registeredMethod->Data, true); + $candidates = json_decode($registeredMethod->Data ?? '', true); foreach ($candidates as $index => $candidate) { - $candidateData = json_decode($candidate, true) ?? []; + $candidateData = json_decode($candidate ?? '', true) ?? []; $backupCode = BackupCode::create( $code, $candidateData['hash'] ?? '', @@ -85,7 +85,7 @@ public function verify(HTTPRequest $request, StoreInterface $store, RegisteredMe // Remove the verified code from the valid list of codes - array_splice($candidates, $index, 1); + array_splice($candidates, $index ?? 0, 1); $registeredMethod->Data = json_encode($candidates); $registeredMethod->write(); $this->notification->send( @@ -93,7 +93,7 @@ public function verify(HTTPRequest $request, StoreInterface $store, RegisteredMe 'SilverStripe/MFA/Email/Notification_backupcodeused', [ 'subject' => _t(self::class . '.MFAREMOVED', 'A recovery code was used to access your account'), - 'CodesRemaining' => count($candidates), + 'CodesRemaining' => count($candidates ?? []), ] ); return Result::create(); diff --git a/src/Extension/MemberExtension.php b/src/Extension/MemberExtension.php index e01d3b46..447d72bc 100644 --- a/src/Extension/MemberExtension.php +++ b/src/Extension/MemberExtension.php @@ -85,7 +85,7 @@ public function getRegisteredMethodNames(): string { $arr = array_map(function ($method) { return $method->getMethod()->getName(); - }, $this->owner->RegisteredMFAMethods()->toArray()); + }, $this->owner->RegisteredMFAMethods()->toArray() ?? []); return implode(', ', $arr); } diff --git a/src/Report/EnabledMembers.php b/src/Report/EnabledMembers.php index 1020bd11..efa0c2e0 100644 --- a/src/Report/EnabledMembers.php +++ b/src/Report/EnabledMembers.php @@ -160,7 +160,7 @@ public function formatMethodsColumn($_, Member $record): string return implode(', ', array_map(function (RegisteredMethod $method) { return $method->getMethod()->getName(); - }, $methods->filter('MemberID', $record->ID)->toArray())); + }, $methods->filter('MemberID', $record->ID)->toArray() ?? [])); } /** diff --git a/src/RequestHandler/VerificationHandlerTrait.php b/src/RequestHandler/VerificationHandlerTrait.php index 3b5cc741..7218edff 100644 --- a/src/RequestHandler/VerificationHandlerTrait.php +++ b/src/RequestHandler/VerificationHandlerTrait.php @@ -133,10 +133,11 @@ protected function isVerificationComplete(StoreInterface $store): bool $successfulMethods = $store->getVerifiedMethods(); // Zero is "not complete". There's different config for optional MFA - if (!is_array($successfulMethods) || !count($successfulMethods)) { + if (!is_array($successfulMethods) || !count($successfulMethods ?? [])) { return false; } - return count($successfulMethods) >= Config::inst()->get(EnforcementManager::class, 'required_mfa_methods'); + $required = Config::inst()->get(EnforcementManager::class, 'required_mfa_methods'); + return count($successfulMethods ?? []) >= $required; } } diff --git a/src/Service/BackupCodeGenerator.php b/src/Service/BackupCodeGenerator.php index 8c1e8134..3ffc9e17 100644 --- a/src/Service/BackupCodeGenerator.php +++ b/src/Service/BackupCodeGenerator.php @@ -46,10 +46,10 @@ public function generate(): array $charset = $this->getCharacterSet(); $codes = []; - while (count($codes) < $codeCount) { + while (count($codes ?? []) < $codeCount) { $code = $this->generateCode($charset, $codeLength); - if (!in_array($code, $codes)) { + if (!in_array($code, $codes ?? [])) { $hashData = Security::encrypt_password($code); $codes[] = BackupCode::create($code, $hashData['password'], $hashData['algorithm'], $hashData['salt']); } @@ -77,11 +77,11 @@ public function getCharacterSet(): array protected function generateCode(array $charset, int $codeLength): string { $characters = []; - $numberOfOptions = count($charset); - while (count($characters) < $codeLength) { + $numberOfOptions = count($charset ?? []); + while (count($characters ?? []) < $codeLength) { $key = random_int(0, $numberOfOptions - 1); // zero based array $characters[] = $charset[$key]; } - return implode($characters); + return implode($characters ?? ''); } } diff --git a/src/Service/EnforcementManager.php b/src/Service/EnforcementManager.php index 4221aad9..fb94be29 100644 --- a/src/Service/EnforcementManager.php +++ b/src/Service/EnforcementManager.php @@ -260,7 +260,7 @@ protected function isEnabled(): bool $methodRegistry = MethodRegistry::singleton(); $methods = $methodRegistry->getMethods(); // If there are no methods available excluding backup codes, do not redirect - if (!count($methods) || (count($methods) === 1 && $methodRegistry->getBackupMethod() !== null)) { + if (!count($methods ?? []) || (count($methods ?? []) === 1 && $methodRegistry->getBackupMethod() !== null)) { return false; } diff --git a/src/Service/MethodRegistry.php b/src/Service/MethodRegistry.php index e1e98100..c603ed55 100644 --- a/src/Service/MethodRegistry.php +++ b/src/Service/MethodRegistry.php @@ -60,7 +60,7 @@ public function getMethods(): array } $configuredMethods = (array) $this->config()->get('methods'); - $configuredMethods = array_filter($configuredMethods); + $configuredMethods = array_filter($configuredMethods ?? []); $this->ensureNoDuplicateMethods($configuredMethods); $allMethods = []; @@ -89,7 +89,7 @@ public function getMethods(): array */ public function hasMethods(): bool { - return count($this->getMethods()) > 0; + return count($this->getMethods() ?? []) > 0; } /** @@ -101,7 +101,7 @@ public function hasMethods(): bool public function isBackupMethod(MethodInterface $method): bool { $configuredBackupMethod = $this->config()->get('default_backup_method'); - return is_string($configuredBackupMethod) && is_a($method, $configuredBackupMethod); + return is_string($configuredBackupMethod) && is_a($method, $configuredBackupMethod ?? ''); } /** @@ -145,14 +145,14 @@ public function getMethodByURLSegment(string $segment): ?MethodInterface */ private function ensureNoDuplicateMethods(array $configuredMethods): void { - $uniqueMethods = array_unique($configuredMethods); + $uniqueMethods = array_unique($configuredMethods ?? []); if ($uniqueMethods === $configuredMethods) { return; } // Get the method class names that were added more than once and format them into a string so we can // tell the developer which classes were incorrectly configured - $duplicates = array_unique(array_diff_key($configuredMethods, $uniqueMethods)); + $duplicates = array_unique(array_diff_key($configuredMethods ?? [], $uniqueMethods)); $methodNames = implode('; ', $duplicates); throw new UnexpectedValueException( 'Cannot register MFA methods more than once. Check your config: ' . $methodNames @@ -169,15 +169,15 @@ private function ensureNoDuplicateURLSegments(array $allMethods): void { $allURLSegments = array_map(function (MethodInterface $method) { return $method->getURLSegment(); - }, $allMethods); - $uniqueURLSegments = array_unique($allURLSegments); + }, $allMethods ?? []); + $uniqueURLSegments = array_unique($allURLSegments ?? []); if ($allURLSegments === $uniqueURLSegments) { return; } // Get the method URL segments that were added more than once and format them into a string so we can // tell the developer which classes were incorrectly configured - $duplicates = array_unique(array_diff_key($allURLSegments, $uniqueURLSegments)); + $duplicates = array_unique(array_diff_key($allURLSegments ?? [], $uniqueURLSegments)); $urlSegments = implode('; ', $duplicates); throw new UnexpectedValueException( 'Cannot register multiple MFA methods with the same URL segment: ' . $urlSegments diff --git a/src/Service/Notification.php b/src/Service/Notification.php index b9e07f24..84da3fb3 100644 --- a/src/Service/Notification.php +++ b/src/Service/Notification.php @@ -77,7 +77,7 @@ public function send(Member $member, string $template, array $data = []): bool foreach (['to', 'from', 'subject'] as $header) { if (isset($data[$header])) { - $method = 'set' . ucwords($header); + $method = 'set' . ucwords($header ?? ''); $email->$method($data[$header]); } } diff --git a/src/Service/SchemaGenerator.php b/src/Service/SchemaGenerator.php index 2c77283e..19c810e5 100644 --- a/src/Service/SchemaGenerator.php +++ b/src/Service/SchemaGenerator.php @@ -36,7 +36,7 @@ public function getSchema(Member $member) // Skip registration details if the user has already registered this method $exclude = array_map(function (RegisteredMethodDetailsInterface $methodDetails) { return $methodDetails->jsonSerialize()['urlSegment']; - }, $registeredMethods); + }, $registeredMethods ?? []); // Also skip the backup method $backupMethod = MethodRegistry::singleton()->getBackupMethod(); @@ -80,7 +80,7 @@ public function getAvailableMethods(array $exclude = []) // Compile details for methods that aren't already registered to the user foreach ($allMethods as $method) { // Omit specified exclusions or methods that are configured as back-up methods - if (in_array($method->getURLSegment(), $exclude)) { + if (in_array($method->getURLSegment(), $exclude ?? [])) { continue; } $availableMethods[] = Injector::inst()->create(AvailableMethodDetailsInterface::class, $method); diff --git a/src/Store/SessionStore.php b/src/Store/SessionStore.php index 5b5095b5..6d8e1d2a 100644 --- a/src/Store/SessionStore.php +++ b/src/Store/SessionStore.php @@ -116,7 +116,7 @@ public function getMethod(): ?string */ public function setMethod(?string $method): StoreInterface { - if (in_array($method, $this->getVerifiedMethods())) { + if (in_array($method, $this->getVerifiedMethods() ?? [])) { throw new InvalidMethodException('You cannot verify with a method you have already verified'); } @@ -144,7 +144,7 @@ public function addState(array $state): StoreInterface public function addVerifiedMethod(string $method): StoreInterface { - if (!in_array($method, $this->verifiedMethods)) { + if (!in_array($method, $this->verifiedMethods ?? [])) { $this->verifiedMethods[] = $method; } @@ -257,7 +257,7 @@ public function serialize(): string */ public function unserialize($serialized): void { - $data = json_decode($serialized, true); + $data = json_decode($serialized ?? '', true); $this->__unserialize($data); } } diff --git a/tests/php/Authenticator/ChangePasswordHandlerTest.php b/tests/php/Authenticator/ChangePasswordHandlerTest.php index 02d5c325..adc4ae3a 100644 --- a/tests/php/Authenticator/ChangePasswordHandlerTest.php +++ b/tests/php/Authenticator/ChangePasswordHandlerTest.php @@ -107,7 +107,7 @@ public function testGetSchema() $response = $this->get('Security/changepassword/mfa/schema'); $this->assertSame('application/json', $response->getHeader('Content-Type')); - $schema = json_decode($response->getBody(), true); + $schema = json_decode($response->getBody() ?? '', true); $this->assertArrayHasKey('endpoints', $schema); $this->assertArrayHasKey('verify', $schema['endpoints']); diff --git a/tests/php/Authenticator/LoginHandlerTest.php b/tests/php/Authenticator/LoginHandlerTest.php index 211a5c7b..99cda50f 100644 --- a/tests/php/Authenticator/LoginHandlerTest.php +++ b/tests/php/Authenticator/LoginHandlerTest.php @@ -129,7 +129,7 @@ public function testMFASchemaEndpointReturnsMethodDetails() $result = $this->get(Controller::join_links(Security::login_url(), 'default/mfa/schema')); - $response = json_decode($result->getBody(), true); + $response = json_decode($result->getBody() ?? '', true); $this->assertArrayHasKey('registeredMethods', $response); $this->assertArrayHasKey('availableMethods', $response); @@ -164,7 +164,7 @@ public function testMFASchemaEndpointShowsRegisteredMethodsIfSetUp() $result = $this->get(Controller::join_links(Security::login_url(), 'default/mfa/schema')); - $response = json_decode($result->getBody(), true); + $response = json_decode($result->getBody() ?? '', true); $this->assertArrayHasKey('registeredMethods', $response); $this->assertArrayHasKey('availableMethods', $response); @@ -194,7 +194,7 @@ public function testMFASchemaEndpointProvidesDefaultMethodIfSet() $result = $this->get(Controller::join_links(Security::login_url(), 'default/mfa/schema')); - $response = json_decode($result->getBody(), true); + $response = json_decode($result->getBody() ?? '', true); $this->assertArrayHasKey('registeredMethods', $response); $this->assertArrayHasKey('availableMethods', $response); @@ -331,7 +331,7 @@ public function testStartVerificationIncludesACSRFToken() $request = new HTTPRequest('GET', '/'); $request->setSession(new Session([])); $request->setRouteParams(['Method' => 'basic-math']); - $response = json_decode($handler->startVerification($request)->getBody()); + $response = json_decode($handler->startVerification($request)->getBody() ?? ''); $this->assertNotNull($response->SecurityID); $this->assertTrue(SecurityToken::inst()->check($response->SecurityID)); diff --git a/tests/php/BackupCode/RegisterHandlerTest.php b/tests/php/BackupCode/RegisterHandlerTest.php index 2ddb84a7..ee859fae 100644 --- a/tests/php/BackupCode/RegisterHandlerTest.php +++ b/tests/php/BackupCode/RegisterHandlerTest.php @@ -50,7 +50,7 @@ public function testStartReturnsPlainTextCodes() $props = $handler->start($store); $this->assertArrayHasKey('codes', $props, 'Codes are returned from the start method'); - $this->assertGreaterThan(0, count($props['codes']), 'At least one code is provided'); + $this->assertGreaterThan(0, count($props['codes'] ?? []), 'At least one code is provided'); } public function testStartStoresHashesOfBackupCodesOnMember() @@ -81,9 +81,13 @@ public function testStartStoresHashesOfBackupCodesOnMember() $this->assertJson($registeredMethod->Data, 'Backup codes are stored as valid JSON'); // Parse the stored codes on the member to compare with what was given to the UI - $storedCodes = json_decode($registeredMethod->Data, true); + $storedCodes = json_decode($registeredMethod->Data ?? '', true); - $this->assertCount(count($codes), $storedCodes, 'Stored code count matches that of those returned to the UI'); + $this->assertCount( + count($codes ?? []), + $storedCodes, + 'Stored code count matches that of those returned to the UI' + ); foreach ($codes as $code) { $this->assertNotContains($code, $storedCodes, 'Codes returned to UI are plaintext (different from stored)'); diff --git a/tests/php/BackupCode/VerifyHandlerTest.php b/tests/php/BackupCode/VerifyHandlerTest.php index 59d411c3..f0555185 100644 --- a/tests/php/BackupCode/VerifyHandlerTest.php +++ b/tests/php/BackupCode/VerifyHandlerTest.php @@ -46,7 +46,7 @@ public function testVerifyInvalidatesCodesThatHaveBeenUsed() $this->assertTrue($handler->verify($request, $store, $method)->isSuccessful()); $method = DataObject::get_by_id(RegisteredMethod::class, $method->ID); - $codes = json_decode($method->Data, true); + $codes = json_decode($method->Data ?? '', true); $this->assertCount(3, $codes, 'Only 3 codes remain against the method'); diff --git a/tests/php/Service/BackupCodeGeneratorTest.php b/tests/php/Service/BackupCodeGeneratorTest.php index 1cbc8d92..1af2c5ce 100644 --- a/tests/php/Service/BackupCodeGeneratorTest.php +++ b/tests/php/Service/BackupCodeGeneratorTest.php @@ -28,7 +28,7 @@ public function testGenerate() foreach ($result as $backupCode) { $this->assertSame( 6, - strlen($backupCode->getCode()), + strlen($backupCode->getCode() ?? ''), 'Generated codes are of configured length' ); } diff --git a/tests/php/Service/RegisteredMethodManagerTest.php b/tests/php/Service/RegisteredMethodManagerTest.php index a77ef722..880f0f30 100644 --- a/tests/php/Service/RegisteredMethodManagerTest.php +++ b/tests/php/Service/RegisteredMethodManagerTest.php @@ -54,7 +54,7 @@ public function testRegisterForMemberWritesToExistingRegisteredMethod() RegisteredMethodManager::singleton()->registerForMember($member, $method, ['foo' => 'bar']); $this->assertCount(1, $member->RegisteredMFAMethods()); - $this->assertSame(['foo' => 'bar'], json_decode($member->RegisteredMFAMethods()->first()->Data, true)); + $this->assertSame(['foo' => 'bar'], json_decode($member->RegisteredMFAMethods()->first()->Data ?? '', true)); } public function testRegisterForMemberCreatesNewMethod() diff --git a/tests/php/Stub/BasicMath/MethodRegisterHandler.php b/tests/php/Stub/BasicMath/MethodRegisterHandler.php index eb42883c..a3d038cc 100644 --- a/tests/php/Stub/BasicMath/MethodRegisterHandler.php +++ b/tests/php/Stub/BasicMath/MethodRegisterHandler.php @@ -36,9 +36,9 @@ public function start(StoreInterface $store): array */ public function register(HTTPRequest $request, StoreInterface $store): Result { - $parameters = json_decode($request->getBody(), true); + $parameters = json_decode($request->getBody() ?? '', true); - if (!array_key_exists('number', $parameters)) { + if (!array_key_exists('number', $parameters ?? [])) { return Result::create(false, 'The required user input was not provided to register this method'); } diff --git a/tests/php/Stub/BasicMath/MethodVerifyHandler.php b/tests/php/Stub/BasicMath/MethodVerifyHandler.php index 9d6571fd..188c327e 100644 --- a/tests/php/Stub/BasicMath/MethodVerifyHandler.php +++ b/tests/php/Stub/BasicMath/MethodVerifyHandler.php @@ -39,7 +39,7 @@ public function start(StoreInterface $store, RegisteredMethod $registeredMethod) } $store->setState([ - 'answer' => array_sum($numbers), + 'answer' => array_sum($numbers ?? []), ]); return [ @@ -57,7 +57,7 @@ public function start(StoreInterface $store, RegisteredMethod $registeredMethod) */ public function verify(HTTPRequest $request, StoreInterface $store, RegisteredMethod $registeredMethod): Result { - $body = json_decode($request->getBody(), true); + $body = json_decode($request->getBody() ?? '', true); if (!$body['answer']) { return Result::create(false, 'Answer was missing');