Skip to content

Commit 092e7e7

Browse files
committed
Change 2FA service to generate the secret on our own and use an external QR service to display the image
1 parent 2db7928 commit 092e7e7

File tree

5 files changed

+44
-186
lines changed

5 files changed

+44
-186
lines changed

app/Http/Controllers/Base/SecurityController.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,10 @@ public function index(Request $request)
9090
*/
9191
public function generateTotp(Request $request)
9292
{
93+
$totpData = $this->twoFactorSetupService->handle($request->user());
94+
9395
return response()->json([
94-
'qrImage' => $this->twoFactorSetupService->handle($request->user()),
96+
'qrImage' => 'https://api.qrserver.com/v1/create-qr-code/?size=200x200&data=' . $totpData,
9597
]);
9698
}
9799

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
11
<?php
2-
/**
3-
* Pterodactyl - Panel
4-
* Copyright (c) 2015 - 2017 Dane Everitt <dane@daneeveritt.com>.
5-
*
6-
* This software is licensed under the terms of the MIT license.
7-
* https://opensource.org/licenses/MIT
8-
*/
92

103
namespace Pterodactyl\Services\Users;
114

5+
use Exception;
6+
use RuntimeException;
127
use Pterodactyl\Models\User;
13-
use PragmaRX\Google2FAQRCode\Google2FA;
148
use Illuminate\Contracts\Encryption\Encrypter;
159
use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
1610
use Illuminate\Contracts\Config\Repository as ConfigRepository;
1711

1812
class TwoFactorSetupService
1913
{
14+
const VALID_BASE32_CHARACTERS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567';
15+
2016
/**
2117
* @var \Illuminate\Contracts\Config\Repository
2218
*/
@@ -27,11 +23,6 @@ class TwoFactorSetupService
2723
*/
2824
private $encrypter;
2925

30-
/**
31-
* @var PragmaRX\Google2FAQRCode\Google2FA
32-
*/
33-
private $google2FA;
34-
3526
/**
3627
* @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface
3728
*/
@@ -42,24 +33,22 @@ class TwoFactorSetupService
4233
*
4334
* @param \Illuminate\Contracts\Config\Repository $config
4435
* @param \Illuminate\Contracts\Encryption\Encrypter $encrypter
45-
* @param PragmaRX\Google2FAQRCode\Google2FA $google2FA
4636
* @param \Pterodactyl\Contracts\Repository\UserRepositoryInterface $repository
4737
*/
4838
public function __construct(
4939
ConfigRepository $config,
5040
Encrypter $encrypter,
51-
Google2FA $google2FA,
5241
UserRepositoryInterface $repository
5342
) {
5443
$this->config = $config;
5544
$this->encrypter = $encrypter;
56-
$this->google2FA = $google2FA;
5745
$this->repository = $repository;
5846
}
5947

6048
/**
6149
* Generate a 2FA token and store it in the database before returning the
62-
* QR code image.
50+
* QR code URL. This URL will need to be attached to a QR generating service in
51+
* order to function.
6352
*
6453
* @param \Pterodactyl\Models\User $user
6554
* @return string
@@ -69,13 +58,26 @@ public function __construct(
6958
*/
7059
public function handle(User $user): string
7160
{
72-
$secret = $this->google2FA->generateSecretKey($this->config->get('pterodactyl.auth.2fa.bytes'));
73-
$image = $this->google2FA->getQRCodeInline($this->config->get('app.name'), $user->email, $secret);
61+
$secret = '';
62+
try {
63+
for ($i = 0; $i < $this->config->get('pterodactyl.auth.2fa.bytes', 16); $i++) {
64+
$secret .= substr(self::VALID_BASE32_CHARACTERS, random_int(0, 31), 1);
65+
}
66+
} catch (Exception $exception) {
67+
throw new RuntimeException($exception->getMessage(), 0, $exception);
68+
}
7469

7570
$this->repository->withoutFreshModel()->update($user->id, [
7671
'totp_secret' => $this->encrypter->encrypt($secret),
7772
]);
7873

79-
return $image;
74+
$company = $this->config->get('app.name');
75+
76+
return sprintf(
77+
'otpauth://totp/%1$s:%2$s?secret=%3$s&issuer=%1$s',
78+
rawurlencode($company),
79+
rawurlencode($user->email),
80+
rawurlencode($secret)
81+
);
8082
}
8183
}

composer.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
"matriphe/iso-639": "^1.2",
3131
"nesbot/carbon": "^1.22",
3232
"pragmarx/google2fa": "^5.0",
33-
"pragmarx/google2fa-qrcode": "^1.0.3",
3433
"predis/predis": "^1.1",
3534
"prologue/alerts": "^0.4",
3635
"ramsey/uuid": "^3.7",

composer.lock

Lines changed: 1 addition & 150 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/Unit/Services/Users/TwoFactorSetupServiceTest.php

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
use Mockery as m;
66
use Tests\TestCase;
77
use Pterodactyl\Models\User;
8-
use PragmaRX\Google2FAQRCode\Google2FA;
98
use Illuminate\Contracts\Config\Repository;
109
use Illuminate\Contracts\Encryption\Encrypter;
1110
use Pterodactyl\Services\Users\TwoFactorSetupService;
@@ -23,11 +22,6 @@ class TwoFactorSetupServiceTest extends TestCase
2322
*/
2423
private $encrypter;
2524

26-
/**
27-
* @var PragmaRX\Google2FAQRCode\Google2FA|\Mockery\Mock
28-
*/
29-
private $google2FA;
30-
3125
/**
3226
* @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface|\Mockery\Mock
3327
*/
@@ -42,7 +36,6 @@ public function setUp()
4236

4337
$this->config = m::mock(Repository::class);
4438
$this->encrypter = m::mock(Encrypter::class);
45-
$this->google2FA = m::mock(Google2FA::class);
4639
$this->repository = m::mock(UserRepositoryInterface::class);
4740
}
4841

@@ -53,16 +46,27 @@ public function testSecretAndImageAreReturned()
5346
{
5447
$model = factory(User::class)->make();
5548

56-
$this->config->shouldReceive('get')->with('pterodactyl.auth.2fa.bytes')->once()->andReturn(32);
57-
$this->google2FA->shouldReceive('generateSecretKey')->with(32)->once()->andReturn('secretKey');
58-
$this->config->shouldReceive('get')->with('app.name')->once()->andReturn('CompanyName');
59-
$this->google2FA->shouldReceive('getQRCodeInline')->with('CompanyName', $model->email, 'secretKey')->once()->andReturn('http://url.com');
60-
$this->encrypter->shouldReceive('encrypt')->with('secretKey')->once()->andReturn('encryptedSecret');
49+
$this->config->shouldReceive('get')->with('pterodactyl.auth.2fa.bytes', 16)->andReturn(32);
50+
$this->config->shouldReceive('get')->with('app.name')->andReturn('Company Name');
51+
$this->encrypter->shouldReceive('encrypt')
52+
->with(m::on(function ($value) {
53+
return preg_match('/([A-Z234567]{32})/', $value) !== false;
54+
}))
55+
->once()
56+
->andReturn('encryptedSecret');
57+
6158
$this->repository->shouldReceive('withoutFreshModel->update')->with($model->id, ['totp_secret' => 'encryptedSecret'])->once()->andReturnNull();
6259

6360
$response = $this->getService()->handle($model);
6461
$this->assertNotEmpty($response);
65-
$this->assertSame('http://url.com', $response);
62+
63+
$companyName = preg_quote(rawurlencode('Company Name'));
64+
$email = preg_quote(rawurlencode($model->email));
65+
66+
$this->assertRegExp(
67+
'/otpauth:\/\/totp\/' . $companyName . ':' . $email . '\?secret=([A-Z234567]{32})&issuer=' . $companyName . '/',
68+
$response
69+
);
6670
}
6771

6872
/**
@@ -72,6 +76,6 @@ public function testSecretAndImageAreReturned()
7276
*/
7377
private function getService(): TwoFactorSetupService
7478
{
75-
return new TwoFactorSetupService($this->config, $this->encrypter, $this->google2FA, $this->repository);
79+
return new TwoFactorSetupService($this->config, $this->encrypter, $this->repository);
7680
}
7781
}

0 commit comments

Comments
 (0)