From 092e7e79fff858ee026608c7dbccab165a67526f Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Fri, 21 Jun 2019 21:55:09 -0700 Subject: [PATCH] Change 2FA service to generate the secret on our own and use an external QR service to display the image --- .../Controllers/Base/SecurityController.php | 4 +- app/Services/Users/TwoFactorSetupService.php | 42 ++--- composer.json | 1 - composer.lock | 151 +----------------- .../Users/TwoFactorSetupServiceTest.php | 32 ++-- 5 files changed, 44 insertions(+), 186 deletions(-) diff --git a/app/Http/Controllers/Base/SecurityController.php b/app/Http/Controllers/Base/SecurityController.php index 3bb85a0b55..2aa9ac1295 100644 --- a/app/Http/Controllers/Base/SecurityController.php +++ b/app/Http/Controllers/Base/SecurityController.php @@ -90,8 +90,10 @@ public function index(Request $request) */ public function generateTotp(Request $request) { + $totpData = $this->twoFactorSetupService->handle($request->user()); + return response()->json([ - 'qrImage' => $this->twoFactorSetupService->handle($request->user()), + 'qrImage' => 'https://api.qrserver.com/v1/create-qr-code/?size=200x200&data=' . $totpData, ]); } diff --git a/app/Services/Users/TwoFactorSetupService.php b/app/Services/Users/TwoFactorSetupService.php index 786cf3d35a..7afb5f2bcd 100644 --- a/app/Services/Users/TwoFactorSetupService.php +++ b/app/Services/Users/TwoFactorSetupService.php @@ -1,22 +1,18 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Services\Users; +use Exception; +use RuntimeException; use Pterodactyl\Models\User; -use PragmaRX\Google2FAQRCode\Google2FA; use Illuminate\Contracts\Encryption\Encrypter; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; use Illuminate\Contracts\Config\Repository as ConfigRepository; class TwoFactorSetupService { + const VALID_BASE32_CHARACTERS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; + /** * @var \Illuminate\Contracts\Config\Repository */ @@ -27,11 +23,6 @@ class TwoFactorSetupService */ private $encrypter; - /** - * @var PragmaRX\Google2FAQRCode\Google2FA - */ - private $google2FA; - /** * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface */ @@ -42,24 +33,22 @@ class TwoFactorSetupService * * @param \Illuminate\Contracts\Config\Repository $config * @param \Illuminate\Contracts\Encryption\Encrypter $encrypter - * @param PragmaRX\Google2FAQRCode\Google2FA $google2FA * @param \Pterodactyl\Contracts\Repository\UserRepositoryInterface $repository */ public function __construct( ConfigRepository $config, Encrypter $encrypter, - Google2FA $google2FA, UserRepositoryInterface $repository ) { $this->config = $config; $this->encrypter = $encrypter; - $this->google2FA = $google2FA; $this->repository = $repository; } /** * Generate a 2FA token and store it in the database before returning the - * QR code image. + * QR code URL. This URL will need to be attached to a QR generating service in + * order to function. * * @param \Pterodactyl\Models\User $user * @return string @@ -69,13 +58,26 @@ public function __construct( */ public function handle(User $user): string { - $secret = $this->google2FA->generateSecretKey($this->config->get('pterodactyl.auth.2fa.bytes')); - $image = $this->google2FA->getQRCodeInline($this->config->get('app.name'), $user->email, $secret); + $secret = ''; + try { + for ($i = 0; $i < $this->config->get('pterodactyl.auth.2fa.bytes', 16); $i++) { + $secret .= substr(self::VALID_BASE32_CHARACTERS, random_int(0, 31), 1); + } + } catch (Exception $exception) { + throw new RuntimeException($exception->getMessage(), 0, $exception); + } $this->repository->withoutFreshModel()->update($user->id, [ 'totp_secret' => $this->encrypter->encrypt($secret), ]); - return $image; + $company = $this->config->get('app.name'); + + return sprintf( + 'otpauth://totp/%1$s:%2$s?secret=%3$s&issuer=%1$s', + rawurlencode($company), + rawurlencode($user->email), + rawurlencode($secret) + ); } } diff --git a/composer.json b/composer.json index 95c96926d1..dcac00e1d4 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,6 @@ "matriphe/iso-639": "^1.2", "nesbot/carbon": "^1.22", "pragmarx/google2fa": "^5.0", - "pragmarx/google2fa-qrcode": "^1.0.3", "predis/predis": "^1.1", "prologue/alerts": "^0.4", "ramsey/uuid": "^3.7", diff --git a/composer.lock b/composer.lock index f1a7e59f25..c5530927e6 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "9c03519785d01a8f710a0f7e65f602e8", + "content-hash": "8a99f4996405b8080a0dcabb2609c39d", "packages": [ { "name": "appstract/laravel-blade-directives", @@ -142,55 +142,6 @@ ], "time": "2018-11-21T19:18:43+00:00" }, - { - "name": "bacon/bacon-qr-code", - "version": "2.0.0", - "source": { - "type": "git", - "url": "https://github.com/Bacon/BaconQrCode.git", - "reference": "eaac909da3ccc32b748a65b127acd8918f58d9b0" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/Bacon/BaconQrCode/zipball/eaac909da3ccc32b748a65b127acd8918f58d9b0", - "reference": "eaac909da3ccc32b748a65b127acd8918f58d9b0", - "shasum": "" - }, - "require": { - "dasprid/enum": "^1.0", - "ext-iconv": "*", - "php": "^7.1" - }, - "require-dev": { - "phly/keep-a-changelog": "^1.4", - "phpunit/phpunit": "^6.4", - "squizlabs/php_codesniffer": "^3.1" - }, - "suggest": { - "ext-imagick": "to generate QR code images" - }, - "type": "library", - "autoload": { - "psr-4": { - "BaconQrCode\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-2-Clause" - ], - "authors": [ - { - "name": "Ben Scholzen 'DASPRiD'", - "email": "mail@dasprids.de", - "homepage": "http://www.dasprids.de", - "role": "Developer" - } - ], - "description": "BaconQrCode is a QR code generator for PHP.", - "homepage": "https://github.com/Bacon/BaconQrCode", - "time": "2018-04-25T17:53:56+00:00" - }, { "name": "cakephp/chronos", "version": "1.2.3", @@ -248,48 +199,6 @@ ], "time": "2018-10-18T22:02:21+00:00" }, - { - "name": "dasprid/enum", - "version": "1.0.0", - "source": { - "type": "git", - "url": "https://github.com/DASPRiD/Enum.git", - "reference": "631ef6e638e9494b0310837fa531bedd908fc22b" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/DASPRiD/Enum/zipball/631ef6e638e9494b0310837fa531bedd908fc22b", - "reference": "631ef6e638e9494b0310837fa531bedd908fc22b", - "shasum": "" - }, - "require-dev": { - "phpunit/phpunit": "^6.4", - "squizlabs/php_codesniffer": "^3.1" - }, - "type": "library", - "autoload": { - "psr-4": { - "DASPRiD\\Enum\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-2-Clause" - ], - "authors": [ - { - "name": "Ben Scholzen 'DASPRiD'", - "email": "mail@dasprids.de", - "homepage": "https://dasprids.de/" - } - ], - "description": "PHP 7.1 enum implementation", - "keywords": [ - "enum", - "map" - ], - "time": "2017-10-25T22:45:27+00:00" - }, { "name": "dnoegel/php-xdg-base-dir", "version": "0.1", @@ -2253,64 +2162,6 @@ ], "time": "2019-03-19T22:44:16+00:00" }, - { - "name": "pragmarx/google2fa-qrcode", - "version": "v1.0.3", - "source": { - "type": "git", - "url": "https://github.com/antonioribeiro/google2fa-qrcode.git", - "reference": "fd5ff0531a48b193a659309cc5fb882c14dbd03f" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/antonioribeiro/google2fa-qrcode/zipball/fd5ff0531a48b193a659309cc5fb882c14dbd03f", - "reference": "fd5ff0531a48b193a659309cc5fb882c14dbd03f", - "shasum": "" - }, - "require": { - "bacon/bacon-qr-code": "~1.0|~2.0", - "php": ">=5.4", - "pragmarx/google2fa": ">=4.0" - }, - "require-dev": { - "khanamiryan/qrcode-detector-decoder": "^1.0", - "phpunit/phpunit": "~4|~5|~6|~7" - }, - "type": "library", - "extra": { - "component": "package", - "branch-alias": { - "dev-master": "1.0-dev" - } - }, - "autoload": { - "psr-4": { - "PragmaRX\\Google2FAQRCode\\": "src/", - "PragmaRX\\Google2FAQRCode\\Tests\\": "tests/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Antonio Carlos Ribeiro", - "email": "acr@antoniocarlosribeiro.com", - "role": "Creator & Designer" - } - ], - "description": "QR Code package for Google2FA", - "keywords": [ - "2fa", - "Authentication", - "Two Factor Authentication", - "google2fa", - "qr code", - "qrcode" - ], - "time": "2019-03-20T16:42:58+00:00" - }, { "name": "predis/predis", "version": "v1.1.1", diff --git a/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php b/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php index 09d37b0d75..2701071a72 100644 --- a/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php +++ b/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php @@ -5,7 +5,6 @@ use Mockery as m; use Tests\TestCase; use Pterodactyl\Models\User; -use PragmaRX\Google2FAQRCode\Google2FA; use Illuminate\Contracts\Config\Repository; use Illuminate\Contracts\Encryption\Encrypter; use Pterodactyl\Services\Users\TwoFactorSetupService; @@ -23,11 +22,6 @@ class TwoFactorSetupServiceTest extends TestCase */ private $encrypter; - /** - * @var PragmaRX\Google2FAQRCode\Google2FA|\Mockery\Mock - */ - private $google2FA; - /** * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface|\Mockery\Mock */ @@ -42,7 +36,6 @@ public function setUp() $this->config = m::mock(Repository::class); $this->encrypter = m::mock(Encrypter::class); - $this->google2FA = m::mock(Google2FA::class); $this->repository = m::mock(UserRepositoryInterface::class); } @@ -53,16 +46,27 @@ public function testSecretAndImageAreReturned() { $model = factory(User::class)->make(); - $this->config->shouldReceive('get')->with('pterodactyl.auth.2fa.bytes')->once()->andReturn(32); - $this->google2FA->shouldReceive('generateSecretKey')->with(32)->once()->andReturn('secretKey'); - $this->config->shouldReceive('get')->with('app.name')->once()->andReturn('CompanyName'); - $this->google2FA->shouldReceive('getQRCodeInline')->with('CompanyName', $model->email, 'secretKey')->once()->andReturn('http://url.com'); - $this->encrypter->shouldReceive('encrypt')->with('secretKey')->once()->andReturn('encryptedSecret'); + $this->config->shouldReceive('get')->with('pterodactyl.auth.2fa.bytes', 16)->andReturn(32); + $this->config->shouldReceive('get')->with('app.name')->andReturn('Company Name'); + $this->encrypter->shouldReceive('encrypt') + ->with(m::on(function ($value) { + return preg_match('/([A-Z234567]{32})/', $value) !== false; + })) + ->once() + ->andReturn('encryptedSecret'); + $this->repository->shouldReceive('withoutFreshModel->update')->with($model->id, ['totp_secret' => 'encryptedSecret'])->once()->andReturnNull(); $response = $this->getService()->handle($model); $this->assertNotEmpty($response); - $this->assertSame('http://url.com', $response); + + $companyName = preg_quote(rawurlencode('Company Name')); + $email = preg_quote(rawurlencode($model->email)); + + $this->assertRegExp( + '/otpauth:\/\/totp\/' . $companyName . ':' . $email . '\?secret=([A-Z234567]{32})&issuer=' . $companyName . '/', + $response + ); } /** @@ -72,6 +76,6 @@ public function testSecretAndImageAreReturned() */ private function getService(): TwoFactorSetupService { - return new TwoFactorSetupService($this->config, $this->encrypter, $this->google2FA, $this->repository); + return new TwoFactorSetupService($this->config, $this->encrypter, $this->repository); } }