mirror of
https://github.com/FOSSBilling/FOSSBilling.git
synced 2026-10-23 03:37:01 +09:00
Harden guest session-state API calls against cross-site requests (#4546)
* Harden guest session-state API calls against cross-site requests Extend the guest authentication guard to the session-bound cart mutations (cart add/remove item, promo apply/remove, reset, currency change), requiring the pre-login session nonce and a same-origin context like client login, signup, and staff login already do. Cart changes accept GET with the nonce in the query string, matching _checkCSRFToken()'s token sources, so shipped fb_api_link usage keeps working. Also pin an explicit SameSite=Lax on the session cookie when security.mode is not strict: an unset attribute leaves cookie behavior to each browser's default, and some browsers send cookies on cross-site requests when it is missing. The installer session already uses Lax. * Default queued cookies to SameSite Lax CookieQueue now defaults to Lax so locale and timezone preference cookies carry an explicit policy instead of leaving the behavior to each browser's unset-attribute default. The CSRF cookie keeps its explicit Strict, and callers can still pass null to omit the attribute. * Sign guest E2E requests with the pre-login nonce The Live Tests failures on the cart guard came from the test helper keeping its own copy of the guarded-endpoint list (login and signup only), so guest cart calls went out unsigned and were rejected with 403. Attach the nonce to every guest request instead - the dispatcher strips the parameter where it is not used - and let guest GET requests carry it in the query string, which _checkCSRFToken() already accepts. * Set the cURL URL after appending guest GET parameters CURLOPT_URL copies the value at set time, so mutating $url after curl_setopt_array() left the handle pointing at the query-less URL and guest GET requests went out without the nonce or parameters. Re-set CURLOPT_URL once the query string is appended.
This commit is contained in:
1 parent
f17a82610a
commit
025ebc0850
6 files changed
+108
-8
No files matched your search
+10
-2
@@ -57,8 +57,11 @@ class ApiClient
|
||||
|
||||
$url = rtrim($baseUrl, '/') . '/api/' . ltrim($endpoint, '/');
|
||||
|
||||
if (in_array(strtolower(trim($endpoint, '/')), ['guest/client/login', 'guest/client/create'], true)
|
||||
&& !isset($payload['CSRFToken'])) {
|
||||
// Guarded guest endpoints (login, signup, staff login, and the
|
||||
// session-bound cart) require the pre-login nonce; the dispatcher strips
|
||||
// the parameter for calls that ignore it, so attach it to every guest
|
||||
// request rather than keep a second copy of the guard's endpoint list.
|
||||
if (str_starts_with(trim($endpoint, '/'), 'guest/') && !isset($payload['CSRFToken'])) {
|
||||
$payload['CSRFToken'] = self::getPreLoginToken($baseUrl);
|
||||
}
|
||||
|
||||
@@ -76,6 +79,11 @@ class ApiClient
|
||||
if (strcasecmp($method, 'POST') === 0) {
|
||||
curl_setopt($ch, CURLOPT_POST, true);
|
||||
curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($payload));
|
||||
} elseif (str_starts_with(trim($endpoint, '/'), 'guest/') && $payload !== []) {
|
||||
// GET requests carrying a nonce (e.g. guarded cart mutations) need
|
||||
// it in the query string; the payload is otherwise unused on GET.
|
||||
$url .= (str_contains($url, '?') ? '&' : '?') . http_build_query($payload);
|
||||
curl_setopt($ch, CURLOPT_URL, $url);
|
||||
}
|
||||
|
||||
$output = curl_exec($ch);
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use FOSSBilling\Http\CookieQueue;
|
||||
use Symfony\Component\HttpFoundation\Response;
|
||||
|
||||
function queuedCookies(Response $response): array
|
||||
{
|
||||
$queued = [];
|
||||
foreach ($response->headers->getCookies() as $cookie) {
|
||||
$queued[$cookie->getName()] = $cookie;
|
||||
}
|
||||
|
||||
return $queued;
|
||||
}
|
||||
|
||||
test('queued cookies default to SameSite Lax', function (): void {
|
||||
$cookies = new CookieQueue();
|
||||
$cookies->queue('fb_locale', 'en_US', strtotime('+1 month'), '/');
|
||||
|
||||
$response = new Response();
|
||||
$cookies->applyToResponse($response);
|
||||
|
||||
expect(queuedCookies($response)['fb_locale']->getSameSite())->toBe('lax');
|
||||
});
|
||||
|
||||
test('queued cookies can pin another policy or omit the attribute', function (): void {
|
||||
$cookies = new CookieQueue();
|
||||
$cookies->queue('fb_strict', 'value', 0, '/', null, false, false, 'Strict');
|
||||
$cookies->queue('fb_plain', 'value', 0, '/', null, false, false, null);
|
||||
|
||||
$response = new Response();
|
||||
$cookies->applyToResponse($response);
|
||||
|
||||
$queued = queuedCookies($response);
|
||||
|
||||
expect($queued['fb_strict']->getSameSite())->toBe('strict')
|
||||
->and($queued['fb_plain']->getSameSite())->toBeNull();
|
||||
});
|
||||
Reference in new issue
Block a user