Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Add consistent CSRF token verification to API endpoints; address secu…
…rity concern with non-CSRF protected endpoints
  • Loading branch information
DaneEveritt committed Nov 17, 2021
1 parent cc31a0a commit bf9cbe2
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 14 deletions.
2 changes: 2 additions & 0 deletions app/Http/Kernel.php
Expand Up @@ -75,6 +75,7 @@ class Kernel extends HttpKernel
ApiSubstituteBindings::class,
'api..key:' . ApiKey::TYPE_APPLICATION,
AuthenticateApplicationUser::class,
VerifyCsrfToken::class,
AuthenticateIPAccess::class,
],
'client-api' => [
Expand All @@ -85,6 +86,7 @@ class Kernel extends HttpKernel
SubstituteClientApiBindings::class,
'api..key:' . ApiKey::TYPE_ACCOUNT,
AuthenticateIPAccess::class,
VerifyCsrfToken::class,
// This is perhaps a little backwards with the Client API, but logically you'd be unable
// to create/get an API key without first enabling 2FA on the account, so I suppose in the
// end it makes sense.
Expand Down
3 changes: 2 additions & 1 deletion app/Http/Middleware/Api/AuthenticateKey.php
Expand Up @@ -8,6 +8,7 @@
use Pterodactyl\Models\User;
use Pterodactyl\Models\ApiKey;
use Illuminate\Auth\AuthManager;
use Illuminate\Support\Facades\Session;
use Illuminate\Contracts\Encryption\Encrypter;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
Expand Down Expand Up @@ -55,7 +56,7 @@ public function __construct(ApiKeyRepositoryInterface $repository, AuthManager $
public function handle(Request $request, Closure $next, int $keyType)
{
if (is_null($request->bearerToken()) && is_null($request->user())) {
throw new HttpException(401, null, null, ['WWW-Authenticate' => 'Bearer']);
throw new HttpException(401, 'A bearer token or valid user session cookie must be provided to access this endpoint.', null, ['WWW-Authenticate' => 'Bearer']);
}

// This is a request coming through using cookies, we have an authenticated user
Expand Down
41 changes: 34 additions & 7 deletions app/Http/Middleware/VerifyCsrfToken.php
Expand Up @@ -2,18 +2,45 @@

namespace Pterodactyl\Http\Middleware;

use Closure;
use Pterodactyl\Models\ApiKey;
use Illuminate\Foundation\Http\Middleware\VerifyCsrfToken as BaseVerifier;

class VerifyCsrfToken extends BaseVerifier
{
/**
* The URIs that should be excluded from CSRF verification.
* The URIs that should be excluded from CSRF verification. These are
* never hit by the front-end, and require specific token validation
* to work.
*
* @var array
* @var string[]
*/
protected $except = [
'remote/*',
'daemon/*',
'api/*',
];
protected $except = ['remote/*', 'daemon/*'];

/**
* Manually apply CSRF protection to routes depending on the authentication
* mechanism being used. If the API request is using an API key that exists
* in the database we can safely ignore CSRF protections, since that would be
* a manually initiated request by a user or server.
*
* All other requests should go through the standard CSRF protections that
* Laravel affords us. This code will be removed in v2 since we have switched
* to using Sanctum for the API endpoints, which handles that for us automatically.
*
* @param \Illuminate\Http\Request $request
* @param \Closure $next
* @return mixed
*
* @throws \Illuminate\Session\TokenMismatchException
*/
public function handle($request, Closure $next)
{
$key = $request->attributes->get('api_key');

if ($key instanceof ApiKey && $key->exists) {
return $next($request);
}

return parent::handle($request, $next);
}
}
13 changes: 12 additions & 1 deletion resources/scripts/api/http.ts
Expand Up @@ -7,10 +7,21 @@ const http: AxiosInstance = axios.create({
'X-Requested-With': 'XMLHttpRequest',
Accept: 'application/json',
'Content-Type': 'application/json',
'X-CSRF-Token': (window as any).X_CSRF_TOKEN as string || '',
},
});

http.interceptors.request.use(req => {
const cookies = document.cookie.split(';').reduce((obj, val) => {
const [ key, value ] = val.trim().split('=').map(decodeURIComponent);

return { ...obj, [key]: value };
}, {} as Record<string, string>);

req.headers['X-XSRF-TOKEN'] = cookies['XSRF-TOKEN'] || 'nil';

return req;
});

http.interceptors.request.use(req => {
if (!req.url?.endsWith('/resources') && (req.url?.indexOf('_debugbar') || -1) < 0) {
store.getActions().progress.startContinuous();
Expand Down
6 changes: 5 additions & 1 deletion resources/views/admin/nodes/view/configuration.blade.php
Expand Up @@ -70,7 +70,11 @@
@parent
<script>
$('#configTokenBtn').on('click', function (event) {
$.getJSON('{{ route('admin.nodes.view.configuration.token', $node->id) }}').done(function (data) {
$.ajax({
method: 'POST',
url: '{{ route('admin.nodes.view.configuration.token', $node->id) }}',
headers: { 'X-CSRF-TOKEN': '{{ csrf_token() }}' },
}).done(function (data) {
swal({
type: 'success',
title: 'Token created.',
Expand Down
4 changes: 2 additions & 2 deletions resources/views/admin/settings/mail.blade.php
Expand Up @@ -145,9 +145,9 @@ function testSettings() {
showLoaderOnConfirm: true
}, function () {
$.ajax({
method: 'GET',
method: 'POST',
url: '/admin/settings/mail/test',
headers: { 'X-CSRF-Token': $('input[name="_token"]').val() }
headers: { 'X-CSRF-TOKEN': $('input[name="_token"]').val() }
}).fail(function (jqXHR) {
showErrorDialog(jqXHR, 'test');
}).done(function () {
Expand Down
4 changes: 2 additions & 2 deletions routes/admin.php
Expand Up @@ -66,8 +66,8 @@
Route::group(['prefix' => 'settings'], function () {
Route::get('/', 'Settings\IndexController@index')->name('admin.settings');
Route::get('/mail', 'Settings\MailController@index')->name('admin.settings.mail');
Route::get('/mail/test', 'Settings\MailController@test')->name('admin.settings.mail.test');
Route::get('/advanced', 'Settings\AdvancedController@index')->name('admin.settings.advanced');
Route::post('/mail/test', 'Settings\MailController@test')->name('admin.settings.mail.test');

Route::patch('/', 'Settings\IndexController@update');
Route::patch('/mail', 'Settings\MailController@update');
Expand Down Expand Up @@ -153,12 +153,12 @@
Route::get('/view/{node}/allocation', 'Nodes\NodeViewController@allocations')->name('admin.nodes.view.allocation');
Route::get('/view/{node}/servers', 'Nodes\NodeViewController@servers')->name('admin.nodes.view.servers');
Route::get('/view/{node}/system-information', 'Nodes\SystemInformationController');
Route::get('/view/{node}/settings/token', 'NodeAutoDeployController')->name('admin.nodes.view.configuration.token');

Route::post('/new', 'NodesController@store');
Route::post('/view/{node}/allocation', 'NodesController@createAllocation');
Route::post('/view/{node}/allocation/remove', 'NodesController@allocationRemoveBlock')->name('admin.nodes.view.allocation.removeBlock');
Route::post('/view/{node}/allocation/alias', 'NodesController@allocationSetAlias')->name('admin.nodes.view.allocation.setAlias');
Route::post('/view/{node}/settings/token', 'NodeAutoDeployController')->name('admin.nodes.view.configuration.token');

Route::patch('/view/{node}/settings', 'NodesController@updateSettings');

Expand Down

0 comments on commit bf9cbe2

Please sign in to comment.