Skip to content

Commit 60eff40

Browse files
committed
Fix session management on client API requests; closes pterodactyl#3727
Versions of Pterodactyl prior to 1.6.3 used a different throttle pathway for requests. That pathway found the current request user before continuing on to other in-app middleware, thus the user was available downstream. Changes introduced in 1.6.3 changed the throttler logic, therefore removing this step. As a result, the client API could not always get the currently authenticated user when cookies were used (aka, requests from the Panel UI, and not API directly). This change corrects the logic to get the session setup correctly before falling through to authenticating as a user using the API key. If a cookie is present and a user is found as a result that session will be used. If an API key is provided it is ignored when a cookie is also present. In order to keep the API stateless any session created for an API request stemming from an API key will have the associated session deleted at the end of the request, and the 'Set-Cookies' header will be stripped from the response.
1 parent d0663dc commit 60eff40

File tree

5 files changed

+53
-47
lines changed

5 files changed

+53
-47
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ This file is a running track of new features and fixes to each version of the pa
33

44
This project follows [Semantic Versioning](http://semver.org) guidelines.
55

6+
## v1.6.4
7+
### Fixed
8+
* Fixes a session management bug that would cause a user who signs out of one browser to be unintentionally logged out of other browser sessions when using the client API.
9+
610
## v1.6.3
711
### Fixed
812
* **[Security]** Changes logout endpoint to be a POST request with CSRF-token validation to prevent a malicious actor from triggering a user logout.

app/Http/Kernel.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
use Illuminate\Foundation\Http\Kernel as HttpKernel;
1919
use Pterodactyl\Http\Middleware\Api\AuthenticateKey;
2020
use Illuminate\Routing\Middleware\SubstituteBindings;
21-
use Pterodactyl\Http\Middleware\Api\SetSessionDriver;
2221
use Illuminate\Session\Middleware\AuthenticateSession;
2322
use Illuminate\View\Middleware\ShareErrorsFromSession;
2423
use Pterodactyl\Http\Middleware\MaintenanceMiddleware;
@@ -27,6 +26,7 @@
2726
use Pterodactyl\Http\Middleware\Api\AuthenticateIPAccess;
2827
use Pterodactyl\Http\Middleware\Api\ApiSubstituteBindings;
2928
use Illuminate\Foundation\Http\Middleware\ValidatePostSize;
29+
use Pterodactyl\Http\Middleware\Api\HandleStatelessRequest;
3030
use Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse;
3131
use Pterodactyl\Http\Middleware\Api\Daemon\DaemonAuthenticate;
3232
use Pterodactyl\Http\Middleware\RequireTwoFactorAuthentication;
@@ -68,18 +68,18 @@ class Kernel extends HttpKernel
6868
RequireTwoFactorAuthentication::class,
6969
],
7070
'api' => [
71+
HandleStatelessRequest::class,
7172
IsValidJson::class,
7273
ApiSubstituteBindings::class,
73-
SetSessionDriver::class,
7474
'api..key:' . ApiKey::TYPE_APPLICATION,
7575
AuthenticateApplicationUser::class,
7676
AuthenticateIPAccess::class,
7777
],
7878
'client-api' => [
79+
HandleStatelessRequest::class,
80+
IsValidJson::class,
7981
StartSession::class,
80-
SetSessionDriver::class,
8182
AuthenticateSession::class,
82-
IsValidJson::class,
8383
SubstituteClientApiBindings::class,
8484
'api..key:' . ApiKey::TYPE_ACCOUNT,
8585
AuthenticateIPAccess::class,

app/Http/Middleware/Api/AuthenticateKey.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ public function __construct(ApiKeyRepositoryInterface $repository, AuthManager $
4242
}
4343

4444
/**
45-
* Handle an API request by verifying that the provided API key
46-
* is in a valid format and exists in the database.
45+
* Handle an API request by verifying that the provided API key is in a valid
46+
* format and exists in the database. If there is currently a user in the session
47+
* do not even bother to look at the token (they provided a cookie for this to
48+
* be the case).
4749
*
4850
* @return mixed
4951
*
@@ -56,17 +58,17 @@ public function handle(Request $request, Closure $next, int $keyType)
5658
throw new HttpException(401, null, null, ['WWW-Authenticate' => 'Bearer']);
5759
}
5860

59-
$raw = $request->bearerToken();
60-
61-
// This is a request coming through using cookies, we have an authenticated user not using
62-
// an API key. Make some fake API key models and continue on through the process.
63-
if (empty($raw) && $request->user() instanceof User) {
61+
// This is a request coming through using cookies, we have an authenticated user
62+
// not using an API key. Make some fake API key models and continue on through
63+
// the process.
64+
if ($request->user() instanceof User) {
6465
$model = (new ApiKey())->forceFill([
6566
'user_id' => $request->user()->id,
6667
'key_type' => ApiKey::TYPE_ACCOUNT,
6768
]);
6869
} else {
69-
$model = $this->authenticateApiKey($raw, $keyType);
70+
$model = $this->authenticateApiKey($request->bearerToken(), $keyType);
71+
7072
$this->auth->guard()->loginUsingId($model->user_id);
7173
}
7274

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Pterodactyl\Http\Middleware\Api;
4+
5+
use Closure;
6+
use Illuminate\Http\Request;
7+
8+
class HandleStatelessRequest
9+
{
10+
/**
11+
* Ensure that the 'Set-Cookie' header is removed from the response if
12+
* a bearer token is present and there is an api_key in the request attributes.
13+
*
14+
* This will also delete the session from the database automatically so that
15+
* it is effectively treated as a stateless request. Any additional requests
16+
* attempting to use that session will find no data.
17+
*
18+
* @return \Illuminate\Http\Response
19+
*/
20+
public function handle(Request $request, Closure $next)
21+
{
22+
/** @var \Illuminate\Http\Response $response */
23+
$response = $next($request);
24+
25+
if (!is_null($request->bearerToken()) && $request->isJson()) {
26+
$request->session()->getHandler()->destroy(
27+
$request->session()->getId()
28+
);
29+
30+
$response->headers->remove('Set-Cookie');
31+
}
32+
33+
return $response;
34+
}
35+
}

app/Http/Middleware/Api/SetSessionDriver.php

Lines changed: 0 additions & 35 deletions
This file was deleted.

0 commit comments

Comments
 (0)