From 098b8eac4c5cbd14c7fa3574f7b96fd729b41701 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 17 May 2017 08:52:44 +0200 Subject: [PATCH 1/8] * Introducing namespace * code styling * using class constant in tests --- PHPGangsta/GoogleAuthenticator.php | 68 +++++++-------- tests/GoogleAuthenticatorTest.php | 128 +++++++++++++++++++---------- 2 files changed, 117 insertions(+), 79 deletions(-) diff --git a/PHPGangsta/GoogleAuthenticator.php b/PHPGangsta/GoogleAuthenticator.php index 02c645f..1322c88 100644 --- a/PHPGangsta/GoogleAuthenticator.php +++ b/PHPGangsta/GoogleAuthenticator.php @@ -1,16 +1,17 @@ _getBase32LookupTable(); // Valid secret lengths are 80 to 640 bits @@ -30,7 +30,7 @@ public function createSecret($secretLength = 16) throw new Exception('Bad secret length'); } $secret = ''; - $rnd = false; + $rnd = false; if (function_exists('random_bytes')) { $rnd = random_bytes($secretLength); } elseif (function_exists('mcrypt_create_iv')) { @@ -60,8 +60,7 @@ public function createSecret($secretLength = 16) * * @return string */ - public function getCode($secret, $timeSlice = null) - { + public function getCode($secret, $timeSlice = null) { if ($timeSlice === null) { $timeSlice = floor(time() / 30); } @@ -69,7 +68,7 @@ public function getCode($secret, $timeSlice = null) $secretkey = $this->_base32Decode($secret); // Pack time into binary string - $time = chr(0).chr(0).chr(0).chr(0).pack('N*', $timeSlice); + $time = chr(0) . chr(0) . chr(0) . chr(0) . pack('N*', $timeSlice); // Hash it with users secret key $hm = hash_hmac('SHA1', $time, $secretkey, true); // Use last nipple of result as index/offset @@ -98,18 +97,17 @@ public function getCode($secret, $timeSlice = null) * * @return string */ - public function getQRCodeGoogleUrl($name, $secret, $title = null, $params = array()) - { - $width = !empty($params['width']) && (int) $params['width'] > 0 ? (int) $params['width'] : 200; - $height = !empty($params['height']) && (int) $params['height'] > 0 ? (int) $params['height'] : 200; - $level = !empty($params['level']) && array_search($params['level'], array('L', 'M', 'Q', 'H')) !== false ? $params['level'] : 'M'; + public function getQRCodeGoogleUrl($name, $secret, $title = null, $params = []) { + $width = !empty($params['width']) && (int)$params['width'] > 0 ? (int)$params['width'] : 200; + $height = !empty($params['height']) && (int)$params['height'] > 0 ? (int)$params['height'] : 200; + $level = !empty($params['level']) && array_search($params['level'], ['L', 'M', 'Q', 'H']) !== false ? $params['level'] : 'M'; - $urlencoded = urlencode('otpauth://totp/'.$name.'?secret='.$secret.''); + $urlencoded = urlencode('otpauth://totp/' . $name . '?secret=' . $secret); if (isset($title)) { - $urlencoded .= urlencode('&issuer='.urlencode($title)); + $urlencoded .= urlencode('&issuer=' . urlencode($title)); } - return 'https://chart.googleapis.com/chart?chs='.$width.'x'.$height.'&chld='.$level.'|0&cht=qr&chl='.$urlencoded.''; + return 'https://chart.googleapis.com/chart?chs=' . $width . 'x' . $height . '&chld=' . $level . '|0&cht=qr&chl=' . $urlencoded . ''; } /** @@ -122,8 +120,7 @@ public function getQRCodeGoogleUrl($name, $secret, $title = null, $params = arra * * @return bool */ - public function verifyCode($secret, $code, $discrepancy = 1, $currentTimeSlice = null) - { + public function verifyCode($secret, $code, $discrepancy = 1, $currentTimeSlice = null) { if ($currentTimeSlice === null) { $currentTimeSlice = floor(time() / 30); } @@ -147,10 +144,9 @@ public function verifyCode($secret, $code, $discrepancy = 1, $currentTimeSlice = * * @param int $length * - * @return PHPGangsta_GoogleAuthenticator + * @return GoogleAuthenticator */ - public function setCodeLength($length) - { + public function setCodeLength($length) { $this->_codeLength = $length; return $this; @@ -163,28 +159,28 @@ public function setCodeLength($length) * * @return bool|string */ - protected function _base32Decode($secret) - { + protected function _base32Decode($secret) { if (empty($secret)) { return ''; } - $base32chars = $this->_getBase32LookupTable(); + $base32chars = $this->_getBase32LookupTable(); $base32charsFlipped = array_flip($base32chars); $paddingCharCount = substr_count($secret, $base32chars[32]); - $allowedValues = array(6, 4, 3, 1, 0); + $allowedValues = [6, 4, 3, 1, 0]; if (!in_array($paddingCharCount, $allowedValues)) { return false; } for ($i = 0; $i < 4; ++$i) { - if ($paddingCharCount == $allowedValues[$i] && - substr($secret, -($allowedValues[$i])) != str_repeat($base32chars[32], $allowedValues[$i])) { + if ($paddingCharCount == $allowedValues[$i] + && substr($secret, -($allowedValues[$i])) != str_repeat($base32chars[32], $allowedValues[$i]) + ) { return false; } } - $secret = str_replace('=', '', $secret); - $secret = str_split($secret); + $secret = str_replace('=', '', $secret); + $secret = str_split($secret); $binaryString = ''; for ($i = 0; $i < count($secret); $i = $i + 8) { $x = ''; @@ -208,15 +204,14 @@ protected function _base32Decode($secret) * * @return array */ - protected function _getBase32LookupTable() - { - return array( + protected function _getBase32LookupTable() { + return [ 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', // 7 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', // 15 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', // 23 'Y', 'Z', '2', '3', '4', '5', '6', '7', // 31 '=', // padding char - ); + ]; } /** @@ -228,8 +223,7 @@ protected function _getBase32LookupTable() * * @return bool True if the two strings are identical */ - private function timingSafeEquals($safeString, $userString) - { + private function timingSafeEquals($safeString, $userString) { if (function_exists('hash_equals')) { return hash_equals($safeString, $userString); } diff --git a/tests/GoogleAuthenticatorTest.php b/tests/GoogleAuthenticatorTest.php index 56fc2c1..7950e9b 100644 --- a/tests/GoogleAuthenticatorTest.php +++ b/tests/GoogleAuthenticatorTest.php @@ -1,44 +1,68 @@ googleAuthenticator = new PHPGangsta_GoogleAuthenticator(); + /** + * setUp + * + * @return void + */ + protected function setUp() { + $this->googleAuthenticator = new GoogleAuthenticator(); } - public function codeProvider() - { + /** + * codeProvider + * + * @return array + */ + public function codeProvider() { // Secret, time, code - return array( - array('SECRET', '0', '200470'), - array('SECRET', '1385909245', '780018'), - array('SECRET', '1378934578', '705013'), - ); + return [ + ['SECRET', '0', '200470'], + ['SECRET', '1385909245', '780018'], + ['SECRET', '1378934578', '705013'], + ]; } - public function testItCanBeInstantiated() - { - $ga = new PHPGangsta_GoogleAuthenticator(); + /** + * testItCanBeInstantiated + * + * @return void + */ + public function testItCanBeInstantiated() { + $ga = $this->googleAuthenticator; - $this->assertInstanceOf('PHPGangsta_GoogleAuthenticator', $ga); + $this->assertInstanceOf(GoogleAuthenticator::class, $ga); } - public function testCreateSecretDefaultsToSixteenCharacters() - { - $ga = $this->googleAuthenticator; + /** + * testCreateSecretDefaultsToSixteenCharacters + * + * @return void + */ + public function testCreateSecretDefaultsToSixteenCharacters() { + $ga = $this->googleAuthenticator; $secret = $ga->createSecret(); $this->assertEquals(strlen($secret), 16); } - public function testCreateSecretLengthCanBeSpecified() - { + /** + * testCreateSecretLengthCanBeSpecified + * + * @return void + */ + public function testCreateSecretLengthCanBeSpecified() { $ga = $this->googleAuthenticator; for ($secretLength = 16; $secretLength < 100; ++$secretLength) { @@ -49,20 +73,27 @@ public function testCreateSecretLengthCanBeSpecified() } /** + * testGetCodeReturnsCorrectValues + * * @dataProvider codeProvider + * + * @return void */ - public function testGetCodeReturnsCorrectValues($secret, $timeSlice, $code) - { + public function testGetCodeReturnsCorrectValues($secret, $timeSlice, $code) { $generatedCode = $this->googleAuthenticator->getCode($secret, $timeSlice); $this->assertEquals($code, $generatedCode); } - public function testGetQRCodeGoogleUrlReturnsCorrectUrl() - { - $secret = 'SECRET'; - $name = 'Test'; - $url = $this->googleAuthenticator->getQRCodeGoogleUrl($name, $secret); + /** + * testGetQRCodeGoogleUrlReturnsCorrectUrl + * + * @return void + */ + public function testGetQRCodeGoogleUrlReturnsCorrectUrl() { + $secret = 'SECRET'; + $name = 'Test'; + $url = $this->googleAuthenticator->getQRCodeGoogleUrl($name, $secret); $urlParts = parse_url($url); parse_str($urlParts['query'], $queryStringArray); @@ -71,41 +102,54 @@ public function testGetQRCodeGoogleUrlReturnsCorrectUrl() $this->assertEquals($urlParts['host'], 'chart.googleapis.com'); $this->assertEquals($urlParts['path'], '/chart'); - $expectedChl = 'otpauth://totp/'.$name.'?secret='.$secret; + $expectedChl = 'otpauth://totp/' . $name . '?secret=' . $secret; $this->assertEquals($queryStringArray['chl'], $expectedChl); } - public function testVerifyCode() - { + /** + * testVerifyCode + * + * @return void + */ + public function testVerifyCode() { $secret = 'SECRET'; - $code = $this->googleAuthenticator->getCode($secret); + $code = $this->googleAuthenticator->getCode($secret); $result = $this->googleAuthenticator->verifyCode($secret, $code); $this->assertEquals(true, $result); - $code = 'INVALIDCODE'; + $code = 'INVALIDCODE'; $result = $this->googleAuthenticator->verifyCode($secret, $code); $this->assertEquals(false, $result); } - public function testVerifyCodeWithLeadingZero() - { + /** + * testVerifyCodeWithLeadingZero + * + * @return void + */ + public function testVerifyCodeWithLeadingZero() { $secret = 'SECRET'; - $code = $this->googleAuthenticator->getCode($secret); + $code = $this->googleAuthenticator->getCode($secret); $result = $this->googleAuthenticator->verifyCode($secret, $code); $this->assertEquals(true, $result); - $code = '0'.$code; + $code = '0' . $code; $result = $this->googleAuthenticator->verifyCode($secret, $code); $this->assertEquals(false, $result); } - public function testSetCodeLength() - { + /** + * testSetCodeLength + * + * @return void + */ + public function testSetCodeLength() { $result = $this->googleAuthenticator->setCodeLength(6); - $this->assertInstanceOf('PHPGangsta_GoogleAuthenticator', $result); + $this->assertInstanceOf(GoogleAuthenticator::class, $result); } + } From 0c330682db06625189074fd301210140bc5fb3d6 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 17 May 2017 11:17:06 +0200 Subject: [PATCH 2/8] * extracted getOtpauthUrl method. usefull when dealing with mobile devices or want to use other QR code generators --- PHPGangsta/GoogleAuthenticator.php | 32 ++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/PHPGangsta/GoogleAuthenticator.php b/PHPGangsta/GoogleAuthenticator.php index 1322c88..3369de3 100644 --- a/PHPGangsta/GoogleAuthenticator.php +++ b/PHPGangsta/GoogleAuthenticator.php @@ -1,6 +1,7 @@ _getBase32LookupTable(); @@ -102,12 +104,12 @@ public function getQRCodeGoogleUrl($name, $secret, $title = null, $params = []) $height = !empty($params['height']) && (int)$params['height'] > 0 ? (int)$params['height'] : 200; $level = !empty($params['level']) && array_search($params['level'], ['L', 'M', 'Q', 'H']) !== false ? $params['level'] : 'M'; - $urlencoded = urlencode('otpauth://totp/' . $name . '?secret=' . $secret); - if (isset($title)) { - $urlencoded .= urlencode('&issuer=' . urlencode($title)); - } + $otpauthUrl = $this->getOtpauthUrl($name, $secret, $title); - return 'https://chart.googleapis.com/chart?chs=' . $width . 'x' . $height . '&chld=' . $level . '|0&cht=qr&chl=' . $urlencoded . ''; + return 'https://chart.googleapis.com/chart?chs=' . + $width . 'x' . $height . + '&chld=' . $level . + '|0&cht=qr&chl=' . urlencode($otpauthUrl); } /** @@ -173,7 +175,7 @@ protected function _base32Decode($secret) { return false; } for ($i = 0; $i < 4; ++$i) { - if ($paddingCharCount == $allowedValues[$i] + if ($paddingCharCount == $allowedValues[$i] && substr($secret, -($allowedValues[$i])) != str_repeat($base32chars[32], $allowedValues[$i]) ) { return false; @@ -243,4 +245,22 @@ private function timingSafeEquals($safeString, $userString) { // They are only identical strings if $result is exactly 0... return $result === 0; } + + /** + * getOtpauthUrl + * + * @param string $name + * @param string $secret + * @param string $title + * + * @return string + */ + public function getOtpauthUrl($name, $secret, $title) { + $otpauthUrl = 'otpauth://totp/' . urlencode($name) . '?secret=' . urlencode($secret); + if (isset($title)) { + $otpauthUrl .= '&issuer=' . urlencode($title); + } + + return $otpauthUrl; + } } From af99dda66d878d7cc165649610b23d15bf142961 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 12 Jul 2017 08:45:37 +0000 Subject: [PATCH 3/8] Fixes unit tests for PHP7 --- composer.json | 3 +++ tests/GoogleAuthenticatorTest.php | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 9de7cc1..1c92fe2 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,9 @@ }, "require": { "php": ">=5.3" + }, + "require-dev": { + "phpunit/phpunit": "*" }, "autoload": { "classmap": ["PHPGangsta/GoogleAuthenticator.php"] diff --git a/tests/GoogleAuthenticatorTest.php b/tests/GoogleAuthenticatorTest.php index 7950e9b..400020d 100644 --- a/tests/GoogleAuthenticatorTest.php +++ b/tests/GoogleAuthenticatorTest.php @@ -1,13 +1,13 @@ Date: Wed, 12 Jul 2017 14:03:20 +0000 Subject: [PATCH 4/8] fixes tests for PHP < 5.5 --- tests/GoogleAuthenticatorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/GoogleAuthenticatorTest.php b/tests/GoogleAuthenticatorTest.php index 400020d..4a685ce 100644 --- a/tests/GoogleAuthenticatorTest.php +++ b/tests/GoogleAuthenticatorTest.php @@ -42,7 +42,7 @@ public function codeProvider() { public function testItCanBeInstantiated() { $ga = $this->googleAuthenticator; - $this->assertInstanceOf(GoogleAuthenticator::class, $ga); + $this->assertInstanceOf("GoogleAuthenticator", $ga); } /** From 472be1ded7df5112708918961e3b7a360f1e878e Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 12 Jul 2017 16:20:31 +0200 Subject: [PATCH 5/8] * fixes for php < 5.5 --- tests/GoogleAuthenticatorTest.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/GoogleAuthenticatorTest.php b/tests/GoogleAuthenticatorTest.php index 4a685ce..da1d56a 100644 --- a/tests/GoogleAuthenticatorTest.php +++ b/tests/GoogleAuthenticatorTest.php @@ -2,6 +2,7 @@ use PHPGangsta\GoogleAuthenticator; use PHPUnit\Framework\TestCase; + require_once __DIR__ . '/../vendor/autoload.php'; /** @@ -27,11 +28,11 @@ protected function setUp() { */ public function codeProvider() { // Secret, time, code - return [ - ['SECRET', '0', '200470'], - ['SECRET', '1385909245', '780018'], - ['SECRET', '1378934578', '705013'], - ]; + return array( + array('SECRET', '0', '200470'), + array('SECRET', '1385909245', '780018'), + array('SECRET', '1378934578', '705013'), + ); } /** @@ -42,7 +43,7 @@ public function codeProvider() { public function testItCanBeInstantiated() { $ga = $this->googleAuthenticator; - $this->assertInstanceOf("GoogleAuthenticator", $ga); + $this->assertInstanceOf("PHPGangsta\GoogleAuthenticator", $ga); } /** @@ -77,6 +78,10 @@ public function testCreateSecretLengthCanBeSpecified() { * * @dataProvider codeProvider * + * @param $secret + * @param $timeSlice + * @param $code + * * @return void */ public function testGetCodeReturnsCorrectValues($secret, $timeSlice, $code) { @@ -149,7 +154,7 @@ public function testVerifyCodeWithLeadingZero() { public function testSetCodeLength() { $result = $this->googleAuthenticator->setCodeLength(6); - $this->assertInstanceOf(GoogleAuthenticator::class, $result); + $this->assertInstanceOf("PHPGangsta\GoogleAuthenticator", $result); } } From d0ba0c537b11b63b8c1a689e893926736090c9ed Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 12 Jul 2017 16:43:15 +0200 Subject: [PATCH 6/8] * fixes arrays for php < 5.5 --- PHPGangsta/GoogleAuthenticator.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/PHPGangsta/GoogleAuthenticator.php b/PHPGangsta/GoogleAuthenticator.php index 3369de3..4f88313 100644 --- a/PHPGangsta/GoogleAuthenticator.php +++ b/PHPGangsta/GoogleAuthenticator.php @@ -1,6 +1,7 @@ 0 ? (int)$params['width'] : 200; $height = !empty($params['height']) && (int)$params['height'] > 0 ? (int)$params['height'] : 200; - $level = !empty($params['level']) && array_search($params['level'], ['L', 'M', 'Q', 'H']) !== false ? $params['level'] : 'M'; + $level = !empty($params['level']) && array_search($params['level'], array('L', 'M', 'Q', 'H')) !== false ? $params['level'] : 'M'; $otpauthUrl = $this->getOtpauthUrl($name, $secret, $title); @@ -170,7 +171,7 @@ protected function _base32Decode($secret) { $base32charsFlipped = array_flip($base32chars); $paddingCharCount = substr_count($secret, $base32chars[32]); - $allowedValues = [6, 4, 3, 1, 0]; + $allowedValues = array(6, 4, 3, 1, 0); if (!in_array($paddingCharCount, $allowedValues)) { return false; } @@ -207,13 +208,13 @@ protected function _base32Decode($secret) { * @return array */ protected function _getBase32LookupTable() { - return [ + return array( 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', // 7 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', // 15 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', // 23 'Y', 'Z', '2', '3', '4', '5', '6', '7', // 31 '=', // padding char - ]; + ); } /** From 6a009f1da4773a9ccd8fcbfcfac5ead892dcd14d Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 12 Jul 2017 17:05:47 +0200 Subject: [PATCH 7/8] * no phpunit needed in composer. --- composer.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/composer.json b/composer.json index 1c92fe2..9de7cc1 100644 --- a/composer.json +++ b/composer.json @@ -19,9 +19,6 @@ }, "require": { "php": ">=5.3" - }, - "require-dev": { - "phpunit/phpunit": "*" }, "autoload": { "classmap": ["PHPGangsta/GoogleAuthenticator.php"] From fbb8f3f0f54dee58db790dd8e3ea801048c324b5 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 12 Jul 2017 17:16:08 +0200 Subject: [PATCH 8/8] * use phpunit from composer --- .travis.yml | 2 +- composer.json | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index d6fa336..f1d5777 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,4 +12,4 @@ install: - export PATH="$HOME/.composer/vendor/bin:$PATH" - composer install -script: phpunit --coverage-text --configuration tests/phpunit.xml +script: vendor/bin/phpunit --coverage-text --configuration tests/phpunit.xml diff --git a/composer.json b/composer.json index 9de7cc1..6428bf8 100644 --- a/composer.json +++ b/composer.json @@ -20,9 +20,12 @@ "require": { "php": ">=5.3" }, - "autoload": { - "classmap": ["PHPGangsta/GoogleAuthenticator.php"] - } + "require-dev": { + "phpunit/phpunit": "*" + }, + "autoload": { + "classmap": ["PHPGangsta/GoogleAuthenticator.php"] + } }