From ac721e57b9f990045866bc523b522af7bce69067 Mon Sep 17 00:00:00 2001 From: Nicolas CARPi Date: Wed, 7 Dec 2022 23:10:13 +0100 Subject: [PATCH] add php-cs-fixer --- .gitignore | 1 + .php-cs-fixer.dist.php | 60 +++++++++ composer.json | 3 +- lib/Providers/Qr/BaconQrCodeProvider.php | 18 +-- lib/Providers/Qr/BaseHTTPQRCodeProvider.php | 2 +- lib/Providers/Qr/EndroidQrCodeProvider.php | 5 +- .../Qr/EndroidQrCodeWithLogoProvider.php | 3 +- lib/Providers/Qr/HandlesDataUri.php | 4 +- lib/Providers/Qr/QRException.php | 4 +- lib/Providers/Qr/QRServerProvider.php | 10 +- lib/Providers/Rng/CSRNGProvider.php | 2 +- lib/Providers/Rng/HashRNGProvider.php | 2 +- lib/Providers/Rng/IRNGProvider.php | 2 +- lib/Providers/Rng/OpenSSLRNGProvider.php | 2 +- lib/Providers/Rng/RNGException.php | 6 +- lib/Providers/Time/HttpTimeProvider.php | 14 +-- lib/Providers/Time/ITimeProvider.php | 2 +- .../Time/LocalMachineTimeProvider.php | 2 +- lib/Providers/Time/NTPTimeProvider.php | 18 +-- lib/Providers/Time/TimeException.php | 6 +- lib/TwoFactorAuth.php | 116 +++++++++--------- lib/TwoFactorAuthException.php | 4 +- tests/MightNotMakeAssertions.php | 2 +- tests/Providers/Qr/IQRCodeProviderTest.php | 6 +- tests/Providers/Rng/CSRNGProviderTest.php | 4 +- tests/Providers/Rng/HashRNGProviderTest.php | 2 +- tests/Providers/Rng/IRNGProviderTest.php | 4 +- tests/Providers/Rng/NeedsRngLengths.php | 2 +- .../Providers/Rng/OpenSSLRNGProviderTest.php | 2 +- tests/Providers/Rng/TestRNGProvider.php | 4 +- tests/Providers/Time/ITimeProviderTest.php | 8 +- tests/Providers/Time/TestTimeProvider.php | 4 +- tests/TwoFactorAuthTest.php | 9 +- testsDependency/BaconQRCodeTest.php | 19 ++- testsDependency/EndroidQRCodeTest.php | 5 +- 35 files changed, 215 insertions(+), 142 deletions(-) create mode 100644 .php-cs-fixer.dist.php diff --git a/.gitignore b/.gitignore index 5c1c961..25b5131 100644 --- a/.gitignore +++ b/.gitignore @@ -190,3 +190,4 @@ composer.lock .vs/ .phpunit.result.cache +.php-cs-fixer.cache diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php new file mode 100644 index 0000000..3294d07 --- /dev/null +++ b/.php-cs-fixer.dist.php @@ -0,0 +1,60 @@ +name('/\.php|\.php.dist$/') + ->exclude('build') + ->exclude('demo') + ->exclude('docs') + ->in(['lib', 'tests', 'testsDependency']) +; + +$config = new PhpCsFixer\Config(); + +return $config->setRules(array( + '@PSR2' => true, + '@PHP71Migration' => true, + 'array_syntax' => ['syntax' => 'long'], + 'class_attributes_separation' => true, + 'declare_strict_types' => true, + 'dir_constant' => true, + 'is_null' => true, + 'no_homoglyph_names' => true, + 'no_null_property_initialization' => true, + 'no_php4_constructor' => true, + 'no_unused_imports' => true, + 'no_useless_else' => true, + 'non_printable_character' => true, + 'ordered_imports' => true, + 'ordered_class_elements' => true, + 'php_unit_construct' => true, + 'pow_to_exponentiation' => true, + 'psr_autoloading' => true, + 'random_api_migration' => true, + 'return_assignment' => true, + 'self_accessor' => true, + 'semicolon_after_instruction' => true, + 'short_scalar_cast' => true, + 'simplified_null_return' => true, + 'single_blank_line_before_namespace' => true, + 'single_class_element_per_statement' => true, + 'single_line_comment_style' => true, + 'single_quote' => true, + 'space_after_semicolon' => true, + 'standardize_not_equals' => true, + 'strict_param' => true, + 'ternary_operator_spaces' => true, + 'trailing_comma_in_multiline' => true, + 'trim_array_spaces' => true, + 'unary_operator_spaces' => true, + 'global_namespace_import' => [ + 'import_classes' => true, + 'import_functions' => true, + 'import_constants' => true, + ], +)) + ->setFinder($finder) + ->setRiskyAllowed(true) +; diff --git a/composer.json b/composer.json index 5b6f872..4f1de86 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,8 @@ }, "require-dev": { "phpunit/phpunit": "@stable", - "php-parallel-lint/php-parallel-lint": "^1.2" + "php-parallel-lint/php-parallel-lint": "^1.2", + "friendsofphp/php-cs-fixer": "^3.13" }, "suggest": { "bacon/bacon-qr-code": "Needed for BaconQrCodeProvider provider", diff --git a/lib/Providers/Qr/BaconQrCodeProvider.php b/lib/Providers/Qr/BaconQrCodeProvider.php index cf88687..529d887 100644 --- a/lib/Providers/Qr/BaconQrCodeProvider.php +++ b/lib/Providers/Qr/BaconQrCodeProvider.php @@ -2,17 +2,17 @@ namespace RobThree\Auth\Providers\Qr; -use BaconQrCode\Writer; -use BaconQrCode\Renderer\ImageRenderer; -use BaconQrCode\Renderer\RendererStyle\RendererStyle; -use BaconQrCode\Renderer\RendererStyle\Fill; use BaconQrCode\Renderer\Color\Rgb; -use BaconQrCode\Renderer\RendererStyle\EyeFill; - use BaconQrCode\Renderer\Image\EpsImageBackEnd; use BaconQrCode\Renderer\Image\ImageBackEndInterface; use BaconQrCode\Renderer\Image\ImagickImageBackEnd; use BaconQrCode\Renderer\Image\SvgImageBackEnd; +use BaconQrCode\Renderer\ImageRenderer; + +use BaconQrCode\Renderer\RendererStyle\EyeFill; +use BaconQrCode\Renderer\RendererStyle\Fill; +use BaconQrCode\Renderer\RendererStyle\RendererStyle; +use BaconQrCode\Writer; use RuntimeException; class BaconQrCodeProvider implements IQRCodeProvider @@ -22,7 +22,7 @@ class BaconQrCodeProvider implements IQRCodeProvider */ public function __construct(private int $borderWidth = 4, private string $backgroundColour = '#ffffff', private string $foregroundColour = '#000000', private string $format = 'png') { - if (! class_exists(ImagickImageBackEnd::class)) { + if (!class_exists(ImagickImageBackEnd::class)) { throw new RuntimeException('Make sure you are using version 2 of Bacon QR Code'); } @@ -94,7 +94,7 @@ class BaconQrCodeProvider implements IQRCodeProvider new EyeFill(null, null), new EyeFill(null, null), new EyeFill(null, null) - ) + ), )); } @@ -119,7 +119,7 @@ class BaconQrCodeProvider implements IQRCodeProvider $input = trim($input, '#'); if (strlen($input) != 3 && strlen($input) != 6) { - throw new \RuntimeException('Colour should be a 3 or 6 character value after the #'); + throw new RuntimeException('Colour should be a 3 or 6 character value after the #'); } // split the array into three chunks diff --git a/lib/Providers/Qr/BaseHTTPQRCodeProvider.php b/lib/Providers/Qr/BaseHTTPQRCodeProvider.php index 9c2daf4..e18f131 100644 --- a/lib/Providers/Qr/BaseHTTPQRCodeProvider.php +++ b/lib/Providers/Qr/BaseHTTPQRCodeProvider.php @@ -17,7 +17,7 @@ abstract class BaseHTTPQRCodeProvider implements IQRCodeProvider CURLOPT_DNS_CACHE_TIMEOUT => 10, CURLOPT_TIMEOUT => 10, CURLOPT_SSL_VERIFYPEER => $this->verifyssl, - CURLOPT_USERAGENT => 'TwoFactorAuth' + CURLOPT_USERAGENT => 'TwoFactorAuth', )); $data = curl_exec($curlhandle); diff --git a/lib/Providers/Qr/EndroidQrCodeProvider.php b/lib/Providers/Qr/EndroidQrCodeProvider.php index 68573c7..671cb02 100755 --- a/lib/Providers/Qr/EndroidQrCodeProvider.php +++ b/lib/Providers/Qr/EndroidQrCodeProvider.php @@ -14,8 +14,11 @@ use Endroid\QrCode\Writer\PngWriter; class EndroidQrCodeProvider implements IQRCodeProvider { public $bgcolor; + public $color; + public $margin; + public $errorcorrectionlevel; protected $endroid4 = false; @@ -65,7 +68,7 @@ class EndroidQrCodeProvider implements IQRCodeProvider $g = hexdec($split[1]); $b = hexdec($split[2]); - return $this->endroid4 ? new Color($r, $g, $b, 0) : ['r' => $r, 'g' => $g, 'b' => $b, 'a' => 0]; + return $this->endroid4 ? new Color($r, $g, $b, 0) : array('r' => $r, 'g' => $g, 'b' => $b, 'a' => 0); } private function handleErrorCorrectionLevel(string $level): string diff --git a/lib/Providers/Qr/EndroidQrCodeWithLogoProvider.php b/lib/Providers/Qr/EndroidQrCodeWithLogoProvider.php index c174449..a0ba41e 100755 --- a/lib/Providers/Qr/EndroidQrCodeWithLogoProvider.php +++ b/lib/Providers/Qr/EndroidQrCodeWithLogoProvider.php @@ -9,6 +9,7 @@ use Endroid\QrCode\Writer\PngWriter; class EndroidQrCodeWithLogoProvider extends EndroidQrCodeProvider { protected $logoPath; + protected $logoSize; /** @@ -49,7 +50,7 @@ class EndroidQrCodeWithLogoProvider extends EndroidQrCodeProvider if (!$this->endroid4 && $this->logoPath) { $qrCode->setLogoPath($this->logoPath); if ($this->logoSize) { - $qrCode->setLogoSize($this->logoSize[0], isset($this->logoSize[1]) ? $this->logoSize[1] : null); + $qrCode->setLogoSize($this->logoSize[0], $this->logoSize[1] ?? null); } } diff --git a/lib/Providers/Qr/HandlesDataUri.php b/lib/Providers/Qr/HandlesDataUri.php index aa47874..20fc98d 100644 --- a/lib/Providers/Qr/HandlesDataUri.php +++ b/lib/Providers/Qr/HandlesDataUri.php @@ -2,8 +2,8 @@ namespace RobThree\Auth\Providers\Qr; -use function preg_match; use function base64_decode; +use function preg_match; trait HandlesDataUri { @@ -13,7 +13,7 @@ trait HandlesDataUri return array( 'mimetype' => $m['mimetype'], 'encoding' => $m['encoding'], - 'data' => base64_decode($m['data']) + 'data' => base64_decode($m['data'], true), ); } diff --git a/lib/Providers/Qr/QRException.php b/lib/Providers/Qr/QRException.php index 3fc61a7..a0970c0 100644 --- a/lib/Providers/Qr/QRException.php +++ b/lib/Providers/Qr/QRException.php @@ -4,4 +4,6 @@ namespace RobThree\Auth\Providers\Qr; use RobThree\Auth\TwoFactorAuthException; -class QRException extends TwoFactorAuthException {} +class QRException extends TwoFactorAuthException +{ +} diff --git a/lib/Providers/Qr/QRServerProvider.php b/lib/Providers/Qr/QRServerProvider.php index 6d76ab2..f29e142 100644 --- a/lib/Providers/Qr/QRServerProvider.php +++ b/lib/Providers/Qr/QRServerProvider.php @@ -34,11 +34,6 @@ class QRServerProvider extends BaseHTTPQRCodeProvider return $this->getContent($this->getUrl($qrtext, $size)); } - private function decodeColor(string $value): string - { - return vsprintf('%d-%d-%d', sscanf($value, "%02x%02x%02x")); - } - public function getUrl(string $qrtext, int $size): string { return 'https://api.qrserver.com/v1/create-qr-code/' @@ -51,4 +46,9 @@ class QRServerProvider extends BaseHTTPQRCodeProvider . '&format=' . strtolower($this->format) . '&data=' . rawurlencode($qrtext); } + + private function decodeColor(string $value): string + { + return vsprintf('%d-%d-%d', sscanf($value, '%02x%02x%02x')); + } } diff --git a/lib/Providers/Rng/CSRNGProvider.php b/lib/Providers/Rng/CSRNGProvider.php index b80d0ee..755c4c2 100644 --- a/lib/Providers/Rng/CSRNGProvider.php +++ b/lib/Providers/Rng/CSRNGProvider.php @@ -1,4 +1,4 @@ - array( 'Connection: close', 'User-agent: TwoFactorAuth HttpTimeProvider (https://github.com/RobThree/TwoFactorAuth)', - 'Cache-Control: no-cache' - ) - ) + 'Cache-Control: no-cache', + ), + ), ); } $this->options = $options; @@ -62,10 +63,9 @@ class HttpTimeProvider implements ITimeProvider return DateTime::createFromFormat($this->expectedtimeformat, trim(substr($h, 5)))->getTimestamp(); } } - throw new \Exception('Invalid or no "Date:" header found'); - } catch (\Exception $ex) { + throw new Exception('Invalid or no "Date:" header found'); + } catch (Exception $ex) { throw new TimeException(sprintf('Unable to retrieve time from %s (%s)', $this->url, $ex->getMessage())); } - } } diff --git a/lib/Providers/Time/ITimeProvider.php b/lib/Providers/Time/ITimeProvider.php index 4799f17..7917346 100644 --- a/lib/Providers/Time/ITimeProvider.php +++ b/lib/Providers/Time/ITimeProvider.php @@ -1,4 +1,4 @@ - $this->timeout, 'usec' => 0]); + socket_set_option($sock, SOL_SOCKET, SO_RCVTIMEO, array('sec' => $this->timeout, 'usec' => 0)); socket_connect($sock, $this->host, $this->port); - /* Send request */ + // Send request $msg = "\010" . str_repeat("\0", 47); socket_send($sock, $msg, strlen($msg), 0); - /* Receive response and close socket */ + // Receive response and close socket if (socket_recv($sock, $recv, 48, MSG_WAITALL) === false) { - throw new \Exception(socket_strerror(socket_last_error($sock))); + throw new Exception(socket_strerror(socket_last_error($sock))); } socket_close($sock); - /* Interpret response */ + // Interpret response $data = unpack('N12', $recv); $timestamp = (int) sprintf('%u', $data[9]); - /* NTP is number of seconds since 0000 UT on 1 January 1900 Unix time is seconds since 0000 UT on 1 January 1970 */ + // NTP is number of seconds since 0000 UT on 1 January 1900 Unix time is seconds since 0000 UT on 1 January 1970 return $timestamp - 2208988800; } catch (Exception $ex) { throw new TimeException(sprintf('Unable to retrieve time from %s (%s)', $this->host, $ex->getMessage())); diff --git a/lib/Providers/Time/TimeException.php b/lib/Providers/Time/TimeException.php index c5a06e2..9c82846 100644 --- a/lib/Providers/Time/TimeException.php +++ b/lib/Providers/Time/TimeException.php @@ -1,7 +1,9 @@ -digits)), $this->digits, '0', STR_PAD_LEFT); + return str_pad((string) ($value % 10** $this->digits), $this->digits, '0', STR_PAD_LEFT); } /** @@ -100,26 +100,6 @@ class TwoFactorAuth return $timeslice > 0; } - /** - * Timing-attack safe comparison of 2 codes (see http://blog.ircmaxell.com/2014/11/its-all-about-time.html) - */ - private function codeEquals(string $safe, string $user): bool - { - if (function_exists('hash_equals')) { - return hash_equals($safe, $user); - } - // In general, it's not possible to prevent length leaks. So it's OK to leak the length. The important part is that - // we don't leak information about the difference of the two strings. - if (strlen($safe) === strlen($user)) { - $result = 0; - for ($i = 0; $i < strlen($safe); $i++) { - $result |= (ord($safe[$i]) ^ ord($user[$i])); - } - return $result === 0; - } - return false; - } - /** * Get data-uri of QRCode */ @@ -144,7 +124,7 @@ class TwoFactorAuth if ($timeproviders === null) { $timeproviders = array( new NTPTimeProvider(), - new HttpTimeProvider() + new HttpTimeProvider(), ); } @@ -164,16 +144,6 @@ class TwoFactorAuth } } - private function getTime(?int $time = null): int - { - return ($time === null) ? $this->getTimeProvider()->getTime() : $time; - } - - private function getTimeSlice(?int $time = null, int $offset = 0): int - { - return (int) floor($time / $this->period) + ($offset * $this->period); - } - /** * Builds a string to be encoded in a QR code */ @@ -187,32 +157,6 @@ class TwoFactorAuth . '&digits=' . intval($this->digits); } - private function base32Decode(string $value): string - { - if (strlen($value) == 0) { - return ''; - } - - if (preg_match('/[^' . preg_quote(self::$_base32dict) . ']/', $value) !== 0) { - throw new TwoFactorAuthException('Invalid base32 string'); - } - - $buffer = ''; - foreach (str_split($value) as $char) { - if ($char !== '=') { - $buffer .= str_pad(decbin(self::$_base32lookup[$char]), 5, '0', STR_PAD_LEFT); - } - } - $length = strlen($buffer); - $blocks = trim(chunk_split(substr($buffer, 0, $length - ($length % 8)), 8, ' ')); - - $output = ''; - foreach (explode(' ', $blocks) as $block) { - $output .= chr(bindec(str_pad($block, 8, '0', STR_PAD_RIGHT))); - } - return $output; - } - /** * @throws TwoFactorAuthException */ @@ -256,4 +200,60 @@ class TwoFactorAuth } return $this->timeprovider; } + + /** + * Timing-attack safe comparison of 2 codes (see http://blog.ircmaxell.com/2014/11/its-all-about-time.html) + */ + private function codeEquals(string $safe, string $user): bool + { + if (function_exists('hash_equals')) { + return hash_equals($safe, $user); + } + // In general, it's not possible to prevent length leaks. So it's OK to leak the length. The important part is that + // we don't leak information about the difference of the two strings. + if (strlen($safe) === strlen($user)) { + $result = 0; + for ($i = 0; $i < strlen($safe); $i++) { + $result |= (ord($safe[$i]) ^ ord($user[$i])); + } + return $result === 0; + } + return false; + } + + private function getTime(?int $time = null): int + { + return ($time === null) ? $this->getTimeProvider()->getTime() : $time; + } + + private function getTimeSlice(?int $time = null, int $offset = 0): int + { + return (int) floor($time / $this->period) + ($offset * $this->period); + } + + private function base32Decode(string $value): string + { + if (strlen($value) == 0) { + return ''; + } + + if (preg_match('/[^' . preg_quote(self::$_base32dict) . ']/', $value) !== 0) { + throw new TwoFactorAuthException('Invalid base32 string'); + } + + $buffer = ''; + foreach (str_split($value) as $char) { + if ($char !== '=') { + $buffer .= str_pad(decbin(self::$_base32lookup[$char]), 5, '0', STR_PAD_LEFT); + } + } + $length = strlen($buffer); + $blocks = trim(chunk_split(substr($buffer, 0, $length - ($length % 8)), 8, ' ')); + + $output = ''; + foreach (explode(' ', $blocks) as $block) { + $output .= chr(bindec(str_pad($block, 8, '0', STR_PAD_RIGHT))); + } + return $output; + } } diff --git a/lib/TwoFactorAuthException.php b/lib/TwoFactorAuthException.php index d240856..e5740f9 100644 --- a/lib/TwoFactorAuthException.php +++ b/lib/TwoFactorAuthException.php @@ -4,4 +4,6 @@ namespace RobThree\Auth; use Exception; -class TwoFactorAuthException extends Exception {} +class TwoFactorAuthException extends Exception +{ +} diff --git a/tests/MightNotMakeAssertions.php b/tests/MightNotMakeAssertions.php index a7fbded..6301cee 100644 --- a/tests/MightNotMakeAssertions.php +++ b/tests/MightNotMakeAssertions.php @@ -1,4 +1,4 @@ -time = $time; } diff --git a/tests/TwoFactorAuthTest.php b/tests/TwoFactorAuthTest.php index 41b8474..acc9c15 100644 --- a/tests/TwoFactorAuthTest.php +++ b/tests/TwoFactorAuthTest.php @@ -1,11 +1,12 @@ -setAccessible(true); // Test vectors from: https://tools.ietf.org/html/rfc4648#page-12 @@ -144,7 +145,7 @@ class TwoFactorAuthTest extends TestCase // "In some circumstances, the use of padding ("=") in base-encoded data is not required or used." $tfa = new TwoFactorAuth('Test'); - $method = new \ReflectionMethod(TwoFactorAuth::class, 'base32Decode'); + $method = new ReflectionMethod(TwoFactorAuth::class, 'base32Decode'); $method->setAccessible(true); // Test vectors from: https://tools.ietf.org/html/rfc4648#page-12 diff --git a/testsDependency/BaconQRCodeTest.php b/testsDependency/BaconQRCodeTest.php index e46f13d..1ceeb53 100644 --- a/testsDependency/BaconQRCodeTest.php +++ b/testsDependency/BaconQRCodeTest.php @@ -1,12 +1,13 @@ -expectException(\RuntimeException::class); + if (!class_exists(ImagickImageBackEnd::class)) { + $this->expectException(RuntimeException::class); $qr = new BaconQrCodeProvider(1, '#000', '#FFF', 'svg'); } else { @@ -31,31 +32,29 @@ class BaconQRCodeTest extends TestCase public function testBadTextColour() { - $this->expectException(\RuntimeException::class); + $this->expectException(RuntimeException::class); new BaconQrCodeProvider(1, 'not-a-colour', '#FFF'); } public function testBadBackgroundColour() { - $this->expectException(\RuntimeException::class); + $this->expectException(RuntimeException::class); new BaconQrCodeProvider(1, '#000', 'not-a-colour'); } public function testBadTextColourHexRef() { - $this->expectException(\RuntimeException::class); + $this->expectException(RuntimeException::class); new BaconQrCodeProvider(1, '#AAAA', '#FFF'); } public function testBadBackgroundColourHexRef() { - $this->expectException(\RuntimeException::class); + $this->expectException(RuntimeException::class); new BaconQrCodeProvider(1, '#000', '#AAAA'); } - - } diff --git a/testsDependency/EndroidQRCodeTest.php b/testsDependency/EndroidQRCodeTest.php index 254138e..0534b45 100644 --- a/testsDependency/EndroidQRCodeTest.php +++ b/testsDependency/EndroidQRCodeTest.php @@ -1,11 +1,11 @@ -assertEquals('image/png', $data['mimetype']); $this->assertEquals('base64', $data['encoding']); $this->assertNotEmpty($data['data']); - } }