diff --git a/CHANGELOG.md b/CHANGELOG.md index 804d01dd5..6b4508658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project follows [Semantic Versioning](http://semver.org) guidelines. ### Changed * Moved Docker image setting to be on the startup management page for a server rather than the details page. This value changes based on the Nest and Egg that are selected. +* Two-Factor authentication tokens are now 32 bytes in length, and are stored encrypted at rest in the database. ## v0.7.0-beta.1 (Derelict Dermodactylus) ### Added diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 12f3df533..9fab7b53e 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -202,7 +202,7 @@ class LoginController extends Controller return $this->sendFailedLoginResponse($request); } - if (! $G2FA->verifyKey($user->totp_secret, $request->input('2fa_token'), 2)) { + if (! $G2FA->verifyKey(Crypt::decrypt($user->totp_secret), $request->input('2fa_token'), 2)) { event(new \Illuminate\Auth\Events\Failed($user, $credentials)); return $this->sendFailedLoginResponse($request); diff --git a/app/Http/Controllers/Base/SecurityController.php b/app/Http/Controllers/Base/SecurityController.php index d22c0ddb9..62f07738c 100644 --- a/app/Http/Controllers/Base/SecurityController.php +++ b/app/Http/Controllers/Base/SecurityController.php @@ -27,7 +27,6 @@ namespace Pterodactyl\Http\Controllers\Base; use Illuminate\Http\Request; use Prologue\Alerts\AlertsMessageBag; -use Illuminate\Contracts\Session\Session; use Pterodactyl\Http\Controllers\Controller; use Pterodactyl\Services\Users\TwoFactorSetupService; use Pterodactyl\Services\Users\ToggleTwoFactorService; @@ -52,11 +51,6 @@ class SecurityController extends Controller */ protected $repository; - /** - * @var \Illuminate\Contracts\Session\Session - */ - protected $session; - /** * @var \Pterodactyl\Services\Users\ToggleTwoFactorService */ @@ -72,7 +66,6 @@ class SecurityController extends Controller * * @param \Prologue\Alerts\AlertsMessageBag $alert * @param \Illuminate\Contracts\Config\Repository $config - * @param \Illuminate\Contracts\Session\Session $session * @param \Pterodactyl\Contracts\Repository\SessionRepositoryInterface $repository * @param \Pterodactyl\Services\Users\ToggleTwoFactorService $toggleTwoFactorService * @param \Pterodactyl\Services\Users\TwoFactorSetupService $twoFactorSetupService @@ -80,7 +73,6 @@ class SecurityController extends Controller public function __construct( AlertsMessageBag $alert, ConfigRepository $config, - Session $session, SessionRepositoryInterface $repository, ToggleTwoFactorService $toggleTwoFactorService, TwoFactorSetupService $twoFactorSetupService @@ -88,7 +80,6 @@ class SecurityController extends Controller $this->alert = $alert; $this->config = $config; $this->repository = $repository; - $this->session = $session; $this->toggleTwoFactorService = $toggleTwoFactorService; $this->twoFactorSetupService = $twoFactorSetupService; } @@ -122,7 +113,9 @@ class SecurityController extends Controller */ public function generateTotp(Request $request) { - return response()->json($this->twoFactorSetupService->handle($request->user())); + return response()->json([ + 'qrImage' => $this->twoFactorSetupService->handle($request->user()), + ]); } /** diff --git a/app/Models/User.php b/app/Models/User.php index 7b09165aa..39e4a0a03 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -63,6 +63,7 @@ class User extends Model implements 'language', 'use_totp', 'totp_secret', + 'totp_authenticated_at', 'gravatar', 'root_admin', ]; @@ -78,6 +79,11 @@ class User extends Model implements 'gravatar' => 'boolean', ]; + /** + * @var array + */ + protected $dates = [self::CREATED_AT, self::UPDATED_AT, 'totp_authenticated_at']; + /** * The attributes excluded from the model's JSON form. * diff --git a/app/Services/Users/ToggleTwoFactorService.php b/app/Services/Users/ToggleTwoFactorService.php index 56ec6953a..e03a76389 100644 --- a/app/Services/Users/ToggleTwoFactorService.php +++ b/app/Services/Users/ToggleTwoFactorService.php @@ -1,66 +1,82 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Services\Users; +use Carbon\Carbon; use Pterodactyl\Models\User; -use PragmaRX\Google2FA\Contracts\Google2FA; +use PragmaRX\Google2FA\Google2FA; +use Illuminate\Contracts\Config\Repository; +use Illuminate\Contracts\Encryption\Encrypter; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; use Pterodactyl\Exceptions\Service\User\TwoFactorAuthenticationTokenInvalid; class ToggleTwoFactorService { /** - * @var \PragmaRX\Google2FA\Contracts\Google2FA + * @var \Illuminate\Contracts\Config\Repository */ - protected $google2FA; + private $config; + + /** + * @var \Illuminate\Contracts\Encryption\Encrypter + */ + private $encrypter; + + /** + * @var \PragmaRX\Google2FA\Google2FA + */ + private $google2FA; /** * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface */ - protected $repository; + private $repository; /** * ToggleTwoFactorService constructor. * - * @param \PragmaRX\Google2FA\Contracts\Google2FA $google2FA + * @param \Illuminate\Contracts\Encryption\Encrypter $encrypter + * @param \PragmaRX\Google2FA\Google2FA $google2FA + * @param \Illuminate\Contracts\Config\Repository $config * @param \Pterodactyl\Contracts\Repository\UserRepositoryInterface $repository */ public function __construct( + Encrypter $encrypter, Google2FA $google2FA, + Repository $config, UserRepositoryInterface $repository ) { + $this->config = $config; + $this->encrypter = $encrypter; $this->google2FA = $google2FA; $this->repository = $repository; } /** - * @param int|\Pterodactyl\Models\User $user - * @param string $token - * @param null|bool $toggleState + * Toggle 2FA on an account only if the token provided is valid. + * + * @param \Pterodactyl\Models\User $user + * @param string $token + * @param bool|null $toggleState * @return bool * * @throws \Pterodactyl\Exceptions\Model\DataValidationException * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException * @throws \Pterodactyl\Exceptions\Service\User\TwoFactorAuthenticationTokenInvalid */ - public function handle($user, $token, $toggleState = null) + public function handle(User $user, string $token, bool $toggleState = null): bool { - if (! $user instanceof User) { - $user = $this->repository->find($user); - } + $window = $this->config->get('pterodactyl.auth.2fa.window'); + $secret = $this->encrypter->decrypt($user->totp_secret); - if (! $this->google2FA->verifyKey($user->totp_secret, $token, 2)) { + $isValidToken = $this->google2FA->verifyKey($secret, $token, $window); + + if (! $isValidToken) { throw new TwoFactorAuthenticationTokenInvalid; } $this->repository->withoutFresh()->update($user->id, [ + 'totp_authenticated_at' => Carbon::now(), 'use_totp' => (is_null($toggleState) ? ! $user->use_totp : $toggleState), ]); diff --git a/app/Services/Users/TwoFactorSetupService.php b/app/Services/Users/TwoFactorSetupService.php index 608a3643a..a8554ccfc 100644 --- a/app/Services/Users/TwoFactorSetupService.php +++ b/app/Services/Users/TwoFactorSetupService.php @@ -10,7 +10,8 @@ namespace Pterodactyl\Services\Users; use Pterodactyl\Models\User; -use PragmaRX\Google2FA\Contracts\Google2FA; +use PragmaRX\Google2FA\Google2FA; +use Illuminate\Contracts\Encryption\Encrypter; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; use Illuminate\Contracts\Config\Repository as ConfigRepository; @@ -19,58 +20,62 @@ class TwoFactorSetupService /** * @var \Illuminate\Contracts\Config\Repository */ - protected $config; + private $config; /** - * @var \PragmaRX\Google2FA\Contracts\Google2FA + * @var \Illuminate\Contracts\Encryption\Encrypter */ - protected $google2FA; + private $encrypter; + + /** + * @var \PragmaRX\Google2FA\Google2FA + */ + private $google2FA; /** * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface */ - protected $repository; + private $repository; /** * TwoFactorSetupService constructor. * * @param \Illuminate\Contracts\Config\Repository $config - * @param \PragmaRX\Google2FA\Contracts\Google2FA $google2FA + * @param \Illuminate\Contracts\Encryption\Encrypter $encrypter + * @param \PragmaRX\Google2FA\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. + * Generate a 2FA token and store it in the database before returning the + * QR code image. * - * @param int|\Pterodactyl\Models\User $user - * @return array + * @param \Pterodactyl\Models\User $user + * @return string * * @throws \Pterodactyl\Exceptions\Model\DataValidationException * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException */ - public function handle($user) + public function handle(User $user): string { - if (! $user instanceof User) { - $user = $this->repository->find($user); - } - - $secret = $this->google2FA->generateSecretKey(); + $secret = $this->google2FA->generateSecretKey($this->config->get('pterodactyl.auth.2fa.bytes')); $image = $this->google2FA->getQRCodeGoogleUrl($this->config->get('app.name'), $user->email, $secret); - $this->repository->withoutFresh()->update($user->id, ['totp_secret' => $secret]); + $this->repository->withoutFresh()->update($user->id, [ + 'totp_secret' => $this->encrypter->encrypt($secret), + ]); - return [ - 'qrImage' => $image, - 'secret' => $secret, - ]; + return $image; } } diff --git a/composer.json b/composer.json index 58903551f..fabe01f0f 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "mtdowling/cron-expression": "^1.2", "nesbot/carbon": "^1.22", "nicolaslopezj/searchable": "^1.9", - "pragmarx/google2fa": "^1.0", + "pragmarx/google2fa": "^2.0", "predis/predis": "^1.1", "prologue/alerts": "^0.4", "ramsey/uuid": "^3.7", @@ -46,7 +46,7 @@ "require-dev": { "barryvdh/laravel-debugbar": "^2.4", "barryvdh/laravel-ide-helper": "^2.4", - "friendsofphp/php-cs-fixer": "^2.4", + "friendsofphp/php-cs-fixer": "^2.8.0", "fzaninotto/faker": "^1.6", "mockery/mockery": "^0.9", "php-mock/php-mock-phpunit": "^1.1", diff --git a/composer.lock b/composer.lock index 2979fad37..9895b8330 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "content-hash": "3758867d4fb2d20e4b4e45b7c410f79b", + "content-hash": "a393763d136e25a93fd5b636229496cf", "packages": [ { "name": "appstract/laravel-blade-directives", @@ -61,16 +61,16 @@ }, { "name": "aws/aws-sdk-php", - "version": "3.36.37", + "version": "3.38.1", "source": { "type": "git", "url": "https://github.com/aws/aws-sdk-php.git", - "reference": "a6d7fd9f32c63d018a6603a36174b4cb971fccd9" + "reference": "9f704274f4748d2039a16d45b3388ed8dde74e89" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/aws/aws-sdk-php/zipball/a6d7fd9f32c63d018a6603a36174b4cb971fccd9", - "reference": "a6d7fd9f32c63d018a6603a36174b4cb971fccd9", + "url": "https://api.github.com/repos/aws/aws-sdk-php/zipball/9f704274f4748d2039a16d45b3388ed8dde74e89", + "reference": "9f704274f4748d2039a16d45b3388ed8dde74e89", "shasum": "" }, "require": { @@ -137,61 +137,7 @@ "s3", "sdk" ], - "time": "2017-11-03T16:39:35+00:00" - }, - { - "name": "christian-riesen/base32", - "version": "1.3.1", - "source": { - "type": "git", - "url": "https://github.com/ChristianRiesen/base32.git", - "reference": "0a31e50c0fa9b1692d077c86ac188eecdcbaf7fa" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/ChristianRiesen/base32/zipball/0a31e50c0fa9b1692d077c86ac188eecdcbaf7fa", - "reference": "0a31e50c0fa9b1692d077c86ac188eecdcbaf7fa", - "shasum": "" - }, - "require": { - "php": ">=5.3.0" - }, - "require-dev": { - "phpunit/phpunit": "4.*", - "satooshi/php-coveralls": "0.*" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "1.1.x-dev" - } - }, - "autoload": { - "psr-4": { - "Base32\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Christian Riesen", - "email": "chris.riesen@gmail.com", - "homepage": "http://christianriesen.com", - "role": "Developer" - } - ], - "description": "Base32 encoder/decoder according to RFC 4648", - "homepage": "https://github.com/ChristianRiesen/base32", - "keywords": [ - "base32", - "decode", - "encode", - "rfc4648" - ], - "time": "2016-05-05T11:49:03+00:00" + "time": "2017-11-09T19:15:59+00:00" }, { "name": "daneeveritt/login-notifications", @@ -2055,6 +2001,68 @@ ], "time": "2017-11-04T11:48:34+00:00" }, + { + "name": "paragonie/constant_time_encoding", + "version": "v2.2.0", + "source": { + "type": "git", + "url": "https://github.com/paragonie/constant_time_encoding.git", + "reference": "9e7d88e6e4015c2f06a3fa22f06e1d5faa77e6c4" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/paragonie/constant_time_encoding/zipball/9e7d88e6e4015c2f06a3fa22f06e1d5faa77e6c4", + "reference": "9e7d88e6e4015c2f06a3fa22f06e1d5faa77e6c4", + "shasum": "" + }, + "require": { + "php": "^7" + }, + "require-dev": { + "phpunit/phpunit": "^6", + "vimeo/psalm": "^0.3|^1" + }, + "type": "library", + "autoload": { + "psr-4": { + "ParagonIE\\ConstantTime\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Paragon Initiative Enterprises", + "email": "security@paragonie.com", + "homepage": "https://paragonie.com", + "role": "Maintainer" + }, + { + "name": "Steve 'Sc00bz' Thomas", + "email": "steve@tobtu.com", + "homepage": "https://www.tobtu.com", + "role": "Original Developer" + } + ], + "description": "Constant-time Implementations of RFC 4648 Encoding (Base-64, Base-32, Base-16)", + "keywords": [ + "base16", + "base32", + "base32_decode", + "base32_encode", + "base64", + "base64_decode", + "base64_encode", + "bin2hex", + "encoding", + "hex", + "hex2bin", + "rfc4648" + ], + "time": "2017-09-22T14:55:37+00:00" + }, { "name": "paragonie/random_compat", "version": "v2.0.11", @@ -2105,26 +2113,28 @@ }, { "name": "pragmarx/google2fa", - "version": "v1.0.1", + "version": "v2.0.6", "source": { "type": "git", "url": "https://github.com/antonioribeiro/google2fa.git", - "reference": "b346dc138339b745c5831405d00cff7c1351aa0d" + "reference": "bc2d654305e4d09254125f8cd390a7fbc4742d46" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/antonioribeiro/google2fa/zipball/b346dc138339b745c5831405d00cff7c1351aa0d", - "reference": "b346dc138339b745c5831405d00cff7c1351aa0d", + "url": "https://api.github.com/repos/antonioribeiro/google2fa/zipball/bc2d654305e4d09254125f8cd390a7fbc4742d46", + "reference": "bc2d654305e4d09254125f8cd390a7fbc4742d46", "shasum": "" }, "require": { - "christian-riesen/base32": "~1.3", + "paragonie/constant_time_encoding": "~1.0|~2.0", "paragonie/random_compat": "~1.4|~2.0", "php": ">=5.4", "symfony/polyfill-php56": "~1.2" }, "require-dev": { - "phpspec/phpspec": "~2.1" + "bacon/bacon-qr-code": "~1.0", + "phpspec/phpspec": "~2.1", + "phpunit/phpunit": "~4" }, "suggest": { "bacon/bacon-qr-code": "Required to generate inline QR Codes." @@ -2132,11 +2142,8 @@ "type": "library", "extra": { "component": "package", - "frameworks": [ - "Laravel" - ], "branch-alias": { - "dev-master": "1.0-dev" + "dev-master": "2.0-dev" } }, "autoload": { @@ -2157,12 +2164,13 @@ ], "description": "A One Time Password Authentication package, compatible with Google Authenticator.", "keywords": [ + "2fa", "Authentication", "Two Factor Authentication", "google2fa", "laravel" ], - "time": "2016-07-18T20:25:04+00:00" + "time": "2017-09-12T06:55:05+00:00" }, { "name": "predis/predis", @@ -3796,16 +3804,16 @@ }, { "name": "watson/validating", - "version": "3.1.1", + "version": "3.1.2", "source": { "type": "git", "url": "https://github.com/dwightwatson/validating.git", - "reference": "ade13078bf2e820e244603446114a28eda51b08c" + "reference": "22edd06d45893f5d4f79c9e901bd7fbce174a79f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/dwightwatson/validating/zipball/ade13078bf2e820e244603446114a28eda51b08c", - "reference": "ade13078bf2e820e244603446114a28eda51b08c", + "url": "https://api.github.com/repos/dwightwatson/validating/zipball/22edd06d45893f5d4f79c9e901bd7fbce174a79f", + "reference": "22edd06d45893f5d4f79c9e901bd7fbce174a79f", "shasum": "" }, "require": { @@ -3842,7 +3850,7 @@ "laravel", "validation" ], - "time": "2017-10-08T22:42:01+00:00" + "time": "2017-11-06T21:35:49+00:00" }, { "name": "webmozart/assert", @@ -4291,16 +4299,16 @@ }, { "name": "friendsofphp/php-cs-fixer", - "version": "v2.8.0", + "version": "v2.8.1", "source": { "type": "git", "url": "https://github.com/FriendsOfPHP/PHP-CS-Fixer.git", - "reference": "89e7b083f27241e03dd776cb8d6781c77e341db6" + "reference": "04f71e56e03ba2627e345e8c949c80dcef0e683e" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/FriendsOfPHP/PHP-CS-Fixer/zipball/89e7b083f27241e03dd776cb8d6781c77e341db6", - "reference": "89e7b083f27241e03dd776cb8d6781c77e341db6", + "url": "https://api.github.com/repos/FriendsOfPHP/PHP-CS-Fixer/zipball/04f71e56e03ba2627e345e8c949c80dcef0e683e", + "reference": "04f71e56e03ba2627e345e8c949c80dcef0e683e", "shasum": "" }, "require": { @@ -4367,7 +4375,7 @@ } ], "description": "A tool to automatically fix PHP code style", - "time": "2017-11-03T02:21:46+00:00" + "time": "2017-11-09T13:31:39+00:00" }, { "name": "fzaninotto/faker", @@ -4421,23 +4429,23 @@ }, { "name": "gecko-packages/gecko-php-unit", - "version": "v2.2", + "version": "v3.0", "source": { "type": "git", "url": "https://github.com/GeckoPackages/GeckoPHPUnit.git", - "reference": "ab525fac9a9ffea219687f261b02008b18ebf2d1" + "reference": "6a866551dffc2154c1b091bae3a7877d39c25ca3" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/GeckoPackages/GeckoPHPUnit/zipball/ab525fac9a9ffea219687f261b02008b18ebf2d1", - "reference": "ab525fac9a9ffea219687f261b02008b18ebf2d1", + "url": "https://api.github.com/repos/GeckoPackages/GeckoPHPUnit/zipball/6a866551dffc2154c1b091bae3a7877d39c25ca3", + "reference": "6a866551dffc2154c1b091bae3a7877d39c25ca3", "shasum": "" }, "require": { - "php": "^5.3.6 || ^7.0" + "php": "^7.0" }, "require-dev": { - "phpunit/phpunit": "^4.8.35 || ^5.4.3" + "phpunit/phpunit": "^6.0" }, "suggest": { "ext-dom": "When testing with xml.", @@ -4445,6 +4453,11 @@ "phpunit/phpunit": "This is an extension for it so make sure you have it some way." }, "type": "library", + "extra": { + "branch-alias": { + "dev-master": "3.0-dev" + } + }, "autoload": { "psr-4": { "GeckoPackages\\PHPUnit\\": "src/PHPUnit" @@ -4461,7 +4474,7 @@ "filesystem", "phpunit" ], - "time": "2017-08-23T07:39:54+00:00" + "time": "2017-08-23T07:46:41+00:00" }, { "name": "hamcrest/hamcrest-php", diff --git a/config/app.php b/config/app.php index c193e42e8..2f9da6704 100644 --- a/config/app.php +++ b/config/app.php @@ -171,7 +171,6 @@ return [ /* * Additional Dependencies */ - PragmaRX\Google2FA\Vendor\Laravel\ServiceProvider::class, igaster\laravelTheme\themeServiceProvider::class, Prologue\Alerts\AlertsServiceProvider::class, Krucas\Settings\Providers\SettingsServiceProvider::class, @@ -213,7 +212,6 @@ return [ 'File' => Illuminate\Support\Facades\File::class, 'Fractal' => Spatie\Fractal\FractalFacade::class, 'Gate' => Illuminate\Support\Facades\Gate::class, - 'Google2FA' => PragmaRX\Google2FA\Vendor\Laravel\Facade::class, 'Hash' => Illuminate\Support\Facades\Hash::class, 'Input' => Illuminate\Support\Facades\Input::class, 'Inspiring' => Illuminate\Foundation\Inspiring::class, diff --git a/config/pterodactyl.php b/config/pterodactyl.php index bd157df23..ad371bce9 100644 --- a/config/pterodactyl.php +++ b/config/pterodactyl.php @@ -23,6 +23,11 @@ return [ */ 'auth' => [ 'notifications' => env('LOGIN_NOTIFICATIONS', false), + '2fa' => [ + 'bytes' => 32, + 'window' => env('APP_2FA_WINDOW', 4), + 'verify_newer' => true, + ], ], /* diff --git a/database/migrations/2017_11_11_161922_Add2FaLastAuthorizationTimeColumn.php b/database/migrations/2017_11_11_161922_Add2FaLastAuthorizationTimeColumn.php new file mode 100644 index 000000000..53cb6526b --- /dev/null +++ b/database/migrations/2017_11_11_161922_Add2FaLastAuthorizationTimeColumn.php @@ -0,0 +1,60 @@ +text('totp_secret')->nullable()->change(); + $table->timestampTz('totp_authenticated_at')->after('totp_secret')->nullable(); + }); + + DB::transaction(function () { + DB::table('users')->get()->each(function ($user) { + if (is_null($user->totp_secret)) { + return; + } + + DB::table('users')->where('id', $user->id)->update([ + 'totp_secret' => Crypt::encrypt($user->totp_secret), + 'updated_at' => Carbon::now()->toIso8601String(), + ]); + }); + }); + } + + /** + * Reverse the migrations. + */ + public function down() + { + DB::transaction(function () { + DB::table('users')->get()->each(function ($user) { + if (is_null($user->totp_secret)) { + return; + } + + DB::table('users')->where('id', $user->id)->update([ + 'totp_secret' => Crypt::decrypt($user->totp_secret), + 'updated_at' => Carbon::now()->toIso8601String(), + ]); + }); + }); + + DB::statement('ALTER TABLE users MODIFY totp_secret CHAR(16) DEFAULT NULL'); + + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('totp_authenticated_at'); + }); + } +} diff --git a/public/themes/pterodactyl/js/frontend/2fa-modal.js b/public/themes/pterodactyl/js/frontend/2fa-modal.js index 022ece2ff..d542b377c 100644 --- a/public/themes/pterodactyl/js/frontend/2fa-modal.js +++ b/public/themes/pterodactyl/js/frontend/2fa-modal.js @@ -42,7 +42,6 @@ var TwoFactorModal = (function () { $('#qr_image_insert').attr('src', image.src).slideDown(); }); }); - $('#2fa_secret_insert').html(data.secret); $('#open2fa').modal('show'); }).fail(function (jqXHR) { alert('An error occured while attempting to load the 2FA setup modal. Please try again.'); diff --git a/resources/themes/pterodactyl/base/security.blade.php b/resources/themes/pterodactyl/base/security.blade.php index a3a6cc51c..7c4693dd4 100644 --- a/resources/themes/pterodactyl/base/security.blade.php +++ b/resources/themes/pterodactyl/base/security.blade.php @@ -106,8 +106,8 @@
-
-
Loading QR Code...
+
+ Loading QR Code...
@lang('base.security.2fa_checkpoint_help')
diff --git a/tests/Unit/Http/Controllers/Base/SecurityControllerTest.php b/tests/Unit/Http/Controllers/Base/SecurityControllerTest.php index 727f2ab54..3c821729e 100644 --- a/tests/Unit/Http/Controllers/Base/SecurityControllerTest.php +++ b/tests/Unit/Http/Controllers/Base/SecurityControllerTest.php @@ -1,69 +1,41 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Tests\Unit\Http\Controllers\Base; use Mockery as m; -use Tests\TestCase; -use Illuminate\Http\Request; -use Pterodactyl\Models\User; use Prologue\Alerts\AlertsMessageBag; -use Illuminate\Contracts\Session\Session; use Illuminate\Contracts\Config\Repository; -use Tests\Assertions\ControllerAssertionsTrait; +use Tests\Unit\Http\Controllers\ControllerTestCase; use Pterodactyl\Services\Users\TwoFactorSetupService; use Pterodactyl\Services\Users\ToggleTwoFactorService; use Pterodactyl\Http\Controllers\Base\SecurityController; use Pterodactyl\Contracts\Repository\SessionRepositoryInterface; use Pterodactyl\Exceptions\Service\User\TwoFactorAuthenticationTokenInvalid; -class SecurityControllerTest extends TestCase +class SecurityControllerTest extends ControllerTestCase { - use ControllerAssertionsTrait; - /** - * @var \Prologue\Alerts\AlertsMessageBag + * @var \Prologue\Alerts\AlertsMessageBag|\Mockery\Mock */ protected $alert; /** - * @var \Illuminate\Contracts\Config\Repository + * @var \Illuminate\Contracts\Config\Repository|\Mockery\Mock */ protected $config; /** - * @var \Pterodactyl\Http\Controllers\Base\SecurityController - */ - protected $controller; - - /** - * @var \Pterodactyl\Contracts\Repository\SessionRepositoryInterface + * @var \Pterodactyl\Contracts\Repository\SessionRepositoryInterface|\Mockery\Mock */ protected $repository; /** - * @var \Illuminate\Http\Request - */ - protected $request; - - /** - * @var \Illuminate\Contracts\Session\Session - */ - protected $session; - - /** - * @var \Pterodactyl\Services\Users\ToggleTwoFactorService + * @var \Pterodactyl\Services\Users\ToggleTwoFactorService|\Mockery\Mock */ protected $toggleTwoFactorService; /** - * @var \Pterodactyl\Services\Users\TwoFactorSetupService + * @var \Pterodactyl\Services\Users\TwoFactorSetupService|\Mockery\Mock */ protected $twoFactorSetupService; @@ -77,19 +49,8 @@ class SecurityControllerTest extends TestCase $this->alert = m::mock(AlertsMessageBag::class); $this->config = m::mock(Repository::class); $this->repository = m::mock(SessionRepositoryInterface::class); - $this->request = m::mock(Request::class); - $this->session = m::mock(Session::class); $this->toggleTwoFactorService = m::mock(ToggleTwoFactorService::class); $this->twoFactorSetupService = m::mock(TwoFactorSetupService::class); - - $this->controller = new SecurityController( - $this->alert, - $this->config, - $this->session, - $this->repository, - $this->toggleTwoFactorService, - $this->twoFactorSetupService - ); } /** @@ -97,13 +58,12 @@ class SecurityControllerTest extends TestCase */ public function testIndexControllerWithDatabaseDriver() { - $model = factory(User::class)->make(); + $model = $this->setRequestUser(); $this->config->shouldReceive('get')->with('session.driver')->once()->andReturn('database'); - $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model); $this->repository->shouldReceive('getUserSessions')->with($model->id)->once()->andReturn(['sessions']); - $response = $this->controller->index($this->request); + $response = $this->getController()->index($this->request); $this->assertIsViewResponse($response); $this->assertViewNameEquals('base.security', $response); $this->assertViewHasKey('sessions', $response); @@ -117,7 +77,7 @@ class SecurityControllerTest extends TestCase { $this->config->shouldReceive('get')->with('session.driver')->once()->andReturn('redis'); - $response = $this->controller->index($this->request); + $response = $this->getController()->index($this->request); $this->assertIsViewResponse($response); $this->assertViewNameEquals('base.security', $response); $this->assertViewHasKey('sessions', $response); @@ -129,14 +89,13 @@ class SecurityControllerTest extends TestCase */ public function testGenerateTotpController() { - $model = factory(User::class)->make(); + $model = $this->setRequestUser(); - $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model); - $this->twoFactorSetupService->shouldReceive('handle')->with($model)->once()->andReturn(['string']); + $this->twoFactorSetupService->shouldReceive('handle')->with($model)->once()->andReturn('qrCodeImage'); - $response = $this->controller->generateTotp($this->request); + $response = $this->getController()->generateTotp($this->request); $this->assertIsJsonResponse($response); - $this->assertResponseJsonEquals(['string'], $response); + $this->assertResponseJsonEquals(['qrImage' => 'qrCodeImage'], $response); } /** @@ -144,13 +103,12 @@ class SecurityControllerTest extends TestCase */ public function testDisableTotpControllerSuccess() { - $model = factory(User::class)->make(); + $model = $this->setRequestUser(); - $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model); $this->request->shouldReceive('input')->with('token')->once()->andReturn('testToken'); $this->toggleTwoFactorService->shouldReceive('handle')->with($model, 'testToken', false)->once()->andReturnNull(); - $response = $this->controller->disableTotp($this->request); + $response = $this->getController()->disableTotp($this->request); $this->assertIsRedirectResponse($response); $this->assertRedirectRouteEquals('account.security', $response); } @@ -160,16 +118,14 @@ class SecurityControllerTest extends TestCase */ public function testDisableTotpControllerWhenExceptionIsThrown() { - $model = factory(User::class)->make(); + $model = $this->setRequestUser(); - $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model); $this->request->shouldReceive('input')->with('token')->once()->andReturn('testToken'); - $this->toggleTwoFactorService->shouldReceive('handle')->with($model, 'testToken', false)->once() - ->andThrow(new TwoFactorAuthenticationTokenInvalid); - $this->alert->shouldReceive('danger')->with(trans('base.security.2fa_disable_error'))->once()->andReturnSelf() - ->shouldReceive('flash')->withNoArgs()->once()->andReturnNull(); + $this->toggleTwoFactorService->shouldReceive('handle')->with($model, 'testToken', false)->once()->andThrow(new TwoFactorAuthenticationTokenInvalid); + $this->alert->shouldReceive('danger')->with(trans('base.security.2fa_disable_error'))->once()->andReturnSelf(); + $this->alert->shouldReceive('flash')->withNoArgs()->once()->andReturnNull(); - $response = $this->controller->disableTotp($this->request); + $response = $this->getController()->disableTotp($this->request); $this->assertIsRedirectResponse($response); $this->assertRedirectRouteEquals('account.security', $response); } @@ -179,13 +135,28 @@ class SecurityControllerTest extends TestCase */ public function testRevokeController() { - $model = factory(User::class)->make(); + $model = $this->setRequestUser(); - $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model); $this->repository->shouldReceive('deleteUserSession')->with($model->id, 123)->once()->andReturnNull(); - $response = $this->controller->revoke($this->request, 123); + $response = $this->getController()->revoke($this->request, 123); $this->assertIsRedirectResponse($response); $this->assertRedirectRouteEquals('account.security', $response); } + + /** + * Return an instance of the controller for testing with mocked dependencies. + * + * @return \Pterodactyl\Http\Controllers\Base\SecurityController + */ + private function getController(): SecurityController + { + return new SecurityController( + $this->alert, + $this->config, + $this->repository, + $this->toggleTwoFactorService, + $this->twoFactorSetupService + ); + } } diff --git a/tests/Unit/Jobs/Schedule/RunTaskJobTest.php b/tests/Unit/Jobs/Schedule/RunTaskJobTest.php index 176eb4d85..c72ab33b5 100644 --- a/tests/Unit/Jobs/Schedule/RunTaskJobTest.php +++ b/tests/Unit/Jobs/Schedule/RunTaskJobTest.php @@ -64,7 +64,7 @@ class RunTaskJobTest extends TestCase { parent::setUp(); Bus::fake(); - Carbon::setTestNow(); + Carbon::setTestNow(Carbon::now()); $this->commandRepository = m::mock(CommandRepositoryInterface::class); $this->config = m::mock(Repository::class); diff --git a/tests/Unit/Services/DaemonKeys/DaemonKeyProviderServiceTest.php b/tests/Unit/Services/DaemonKeys/DaemonKeyProviderServiceTest.php index 7c240b083..87d5f506b 100644 --- a/tests/Unit/Services/DaemonKeys/DaemonKeyProviderServiceTest.php +++ b/tests/Unit/Services/DaemonKeys/DaemonKeyProviderServiceTest.php @@ -44,7 +44,7 @@ class DaemonKeyProviderServiceTest extends TestCase public function setUp() { parent::setUp(); - Carbon::setTestNow(); + Carbon::setTestNow(Carbon::now()); $this->keyCreationService = m::mock(DaemonKeyCreationService::class); $this->keyUpdateService = m::mock(DaemonKeyUpdateService::class); diff --git a/tests/Unit/Services/Users/ToggleTwoFactorServiceTest.php b/tests/Unit/Services/Users/ToggleTwoFactorServiceTest.php index ae45ec8f1..c8d1cc852 100644 --- a/tests/Unit/Services/Users/ToggleTwoFactorServiceTest.php +++ b/tests/Unit/Services/Users/ToggleTwoFactorServiceTest.php @@ -1,37 +1,42 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Tests\Unit\Services\Users; use Mockery as m; +use Carbon\Carbon; use Tests\TestCase; use Pterodactyl\Models\User; -use PragmaRX\Google2FA\Contracts\Google2FA; +use PragmaRX\Google2FA\Google2FA; +use Illuminate\Contracts\Config\Repository; +use Illuminate\Contracts\Encryption\Encrypter; use Pterodactyl\Services\Users\ToggleTwoFactorService; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; class ToggleTwoFactorServiceTest extends TestCase { - /** - * @var \PragmaRX\Google2FA\Contracts\Google2FA - */ - protected $google2FA; + const TEST_WINDOW_INT = 4; + const USER_TOTP_SECRET = 'encryptedValue'; + const DECRYPTED_USER_SECRET = 'decryptedValue'; /** - * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface + * @var \Illuminate\Contracts\Config\Repository|\Mockery\Mock */ - protected $repository; + private $config; /** - * @var \Pterodactyl\Services\Users\ToggleTwoFactorService + * @var \Illuminate\Contracts\Encryption\Encrypter|\Mockery\Mock */ - protected $service; + private $encrypter; + + /** + * @var \PragmaRX\Google2FA\Google2FA|\Mockery\Mock + */ + private $google2FA; + + /** + * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface|\Mockery\Mock + */ + private $repository; /** * Setup tests. @@ -39,11 +44,15 @@ class ToggleTwoFactorServiceTest extends TestCase public function setUp() { parent::setUp(); + Carbon::setTestNow(Carbon::now()); + $this->config = m::mock(Repository::class); + $this->encrypter = m::mock(Encrypter::class); $this->google2FA = m::mock(Google2FA::class); $this->repository = m::mock(UserRepositoryInterface::class); - $this->service = new ToggleTwoFactorService($this->google2FA, $this->repository); + $this->config->shouldReceive('get')->with('pterodactyl.auth.2fa.window')->once()->andReturn(self::TEST_WINDOW_INT); + $this->encrypter->shouldReceive('decrypt')->with(self::USER_TOTP_SECRET)->once()->andReturn(self::DECRYPTED_USER_SECRET); } /** @@ -51,13 +60,15 @@ class ToggleTwoFactorServiceTest extends TestCase */ public function testTwoFactorIsEnabledForUser() { - $model = factory(User::class)->make(['totp_secret' => 'secret', 'use_totp' => false]); + $model = factory(User::class)->make(['totp_secret' => self::USER_TOTP_SECRET, 'use_totp' => false]); - $this->google2FA->shouldReceive('verifyKey')->with($model->totp_secret, 'test-token', 2)->once()->andReturn(true); - $this->repository->shouldReceive('withoutFresh')->withNoArgs()->once()->andReturnSelf() - ->shouldReceive('update')->with($model->id, ['use_totp' => true])->once()->andReturnNull(); + $this->google2FA->shouldReceive('verifyKey')->with(self::DECRYPTED_USER_SECRET, 'test-token', self::TEST_WINDOW_INT)->once()->andReturn(true); + $this->repository->shouldReceive('withoutFresh->update')->with($model->id, [ + 'totp_authenticated_at' => Carbon::now(), + 'use_totp' => true, + ])->once()->andReturnNull(); - $this->assertTrue($this->service->handle($model, 'test-token')); + $this->assertTrue($this->getService()->handle($model, 'test-token')); } /** @@ -65,13 +76,15 @@ class ToggleTwoFactorServiceTest extends TestCase */ public function testTwoFactorIsDisabled() { - $model = factory(User::class)->make(['totp_secret' => 'secret', 'use_totp' => true]); + $model = factory(User::class)->make(['totp_secret' => self::USER_TOTP_SECRET, 'use_totp' => true]); - $this->google2FA->shouldReceive('verifyKey')->with($model->totp_secret, 'test-token', 2)->once()->andReturn(true); - $this->repository->shouldReceive('withoutFresh')->withNoArgs()->once()->andReturnSelf() - ->shouldReceive('update')->with($model->id, ['use_totp' => false])->once()->andReturnNull(); + $this->google2FA->shouldReceive('verifyKey')->with(self::DECRYPTED_USER_SECRET, 'test-token', self::TEST_WINDOW_INT)->once()->andReturn(true); + $this->repository->shouldReceive('withoutFresh->update')->with($model->id, [ + 'totp_authenticated_at' => Carbon::now(), + 'use_totp' => false, + ])->once()->andReturnNull(); - $this->assertTrue($this->service->handle($model, 'test-token')); + $this->assertTrue($this->getService()->handle($model, 'test-token')); } /** @@ -79,13 +92,15 @@ class ToggleTwoFactorServiceTest extends TestCase */ public function testTwoFactorRemainsDisabledForUser() { - $model = factory(User::class)->make(['totp_secret' => 'secret', 'use_totp' => false]); + $model = factory(User::class)->make(['totp_secret' => self::USER_TOTP_SECRET, 'use_totp' => false]); - $this->google2FA->shouldReceive('verifyKey')->with($model->totp_secret, 'test-token', 2)->once()->andReturn(true); - $this->repository->shouldReceive('withoutFresh')->withNoArgs()->once()->andReturnSelf() - ->shouldReceive('update')->with($model->id, ['use_totp' => false])->once()->andReturnNull(); + $this->google2FA->shouldReceive('verifyKey')->with(self::DECRYPTED_USER_SECRET, 'test-token', self::TEST_WINDOW_INT)->once()->andReturn(true); + $this->repository->shouldReceive('withoutFresh->update')->with($model->id, [ + 'totp_authenticated_at' => Carbon::now(), + 'use_totp' => false, + ])->once()->andReturnNull(); - $this->assertTrue($this->service->handle($model, 'test-token', false)); + $this->assertTrue($this->getService()->handle($model, 'test-token', false)); } /** @@ -95,23 +110,19 @@ class ToggleTwoFactorServiceTest extends TestCase */ public function testExceptionIsThrownIfTokenIsInvalid() { - $model = factory(User::class)->make(); + $model = factory(User::class)->make(['totp_secret' => self::USER_TOTP_SECRET]); $this->google2FA->shouldReceive('verifyKey')->once()->andReturn(false); - $this->service->handle($model, 'test-token'); + $this->getService()->handle($model, 'test-token'); } /** - * Test that an integer can be passed in place of a user model. + * Return an instance of the service with mocked dependencies. + * + * @return \Pterodactyl\Services\Users\ToggleTwoFactorService */ - public function testIntegerCanBePassedInPlaceOfUserModel() + private function getService(): ToggleTwoFactorService { - $model = factory(User::class)->make(['totp_secret' => 'secret', 'use_totp' => false]); - - $this->repository->shouldReceive('find')->with($model->id)->once()->andReturn($model); - $this->google2FA->shouldReceive('verifyKey')->once()->andReturn(true); - $this->repository->shouldReceive('withoutFresh->update')->once()->andReturnNull(); - - $this->assertTrue($this->service->handle($model->id, 'test-token')); + return new ToggleTwoFactorService($this->encrypter, $this->google2FA, $this->config, $this->repository); } } diff --git a/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php b/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php index e58d99f2c..d6f5f8b90 100644 --- a/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php +++ b/tests/Unit/Services/Users/TwoFactorSetupServiceTest.php @@ -1,43 +1,37 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Tests\Unit\Services\Users; use Mockery as m; use Tests\TestCase; use Pterodactyl\Models\User; +use PragmaRX\Google2FA\Google2FA; use Illuminate\Contracts\Config\Repository; -use PragmaRX\Google2FA\Contracts\Google2FA; +use Illuminate\Contracts\Encryption\Encrypter; use Pterodactyl\Services\Users\TwoFactorSetupService; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; class TwoFactorSetupServiceTest extends TestCase { /** - * @var \Illuminate\Contracts\Config\Repository + * @var \Illuminate\Contracts\Config\Repository|\Mockery\Mock */ - protected $config; + private $config; /** - * @var \PragmaRX\Google2FA\Contracts\Google2FA + * @var \Illuminate\Contracts\Encryption\Encrypter|\Mockery\Mock */ - protected $google2FA; + private $encrypter; /** - * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface + * @var \PragmaRX\Google2FA\Google2FA|\Mockery\Mock */ - protected $repository; + private $google2FA; /** - * @var \Pterodactyl\Services\Users\TwoFactorSetupService + * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface|\Mockery\Mock */ - protected $service; + private $repository; /** * Setup tests. @@ -47,10 +41,9 @@ class TwoFactorSetupServiceTest extends TestCase parent::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); - - $this->service = new TwoFactorSetupService($this->config, $this->google2FA, $this->repository); } /** @@ -60,34 +53,25 @@ class TwoFactorSetupServiceTest extends TestCase { $model = factory(User::class)->make(); - $this->google2FA->shouldReceive('generateSecretKey')->withNoArgs()->once()->andReturn('secretKey'); + $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('getQRCodeGoogleUrl')->with('CompanyName', $model->email, 'secretKey') - ->once()->andReturn('http://url.com'); - $this->repository->shouldReceive('withoutFresh')->withNoArgs()->once()->andReturnSelf() - ->shouldReceive('update')->with($model->id, ['totp_secret' => 'secretKey'])->once()->andReturnNull(); + $this->google2FA->shouldReceive('getQRCodeGoogleUrl')->with('CompanyName', $model->email, 'secretKey')->once()->andReturn('http://url.com'); + $this->encrypter->shouldReceive('encrypt')->with('secretKey')->once()->andReturn('encryptedSecret'); + $this->repository->shouldReceive('withoutFresh->update')->with($model->id, ['totp_secret' => 'encryptedSecret'])->once()->andReturnNull(); - $response = $this->service->handle($model); + $response = $this->getService()->handle($model); $this->assertNotEmpty($response); - $this->assertArrayHasKey('qrImage', $response); - $this->assertArrayHasKey('secret', $response); - $this->assertEquals('http://url.com', $response['qrImage']); - $this->assertEquals('secretKey', $response['secret']); + $this->assertSame('http://url.com', $response); } /** - * Test that an integer can be passed in place of the user model. + * Return an instance of the service to test with mocked dependencies. + * + * @return \Pterodactyl\Services\Users\TwoFactorSetupService */ - public function testIntegerCanBePassedInPlaceOfUserModel() + private function getService(): TwoFactorSetupService { - $model = factory(User::class)->make(); - - $this->repository->shouldReceive('find')->with($model->id)->once()->andReturn($model); - $this->google2FA->shouldReceive('generateSecretKey')->withNoArgs()->once()->andReturnNull(); - $this->config->shouldReceive('get')->with('app.name')->once()->andReturnNull(); - $this->google2FA->shouldReceive('getQRCodeGoogleUrl')->once()->andReturnNull(); - $this->repository->shouldReceive('withoutFresh->update')->once()->andReturnNull(); - - $this->assertTrue(is_array($this->service->handle($model->id))); + return new TwoFactorSetupService($this->config, $this->encrypter, $this->google2FA, $this->repository); } }