Skip to content

Commit 000db11

Browse files
fix: clear all session entries in SessionStore::purge() regardless of key order (#851)
### Changes `SessionStore::purge()` walked `$_SESSION` with `while ($sessionKey = key($session))` and advanced with `next()`. That loop condition is also falsy when a session key is `0` or `""`, so if such a key appeared before the SDK's own prefixed entries, iteration stopped early and left the remaining `auth0_*` entries in the session. Because `Auth0::logout()` clears state through `purge()`, the leftover entries could persist after logout when the surrounding application had written a falsy-keyed value to `$_SESSION`. **🐛 Bug Fix:** - `SessionStore::purge()` now iterates a snapshot of the session keys via `array_keys()` instead of the `key()`/`next()` cursor, so a falsy key can no longer terminate the loop early and every prefixed entry is removed - The prefix check uses `str_starts_with()` guarded by `is_string()`. On v9 the previous loop already cast the key with `mb_substr((string) $sessionKey, ...)`, so the `is_string()` guard here is hardening. The uncast `mb_substr()` path only exists on `8.x`, where dropping it is a genuine `TypeError` fix, so the `8.x` backport is where that matters - `purge()` stays read-only and no longer touches `$_SESSION` when it is unset, matching the prior behavior ### References N/A ### Testing - Added `purge() clears values when a falsy key precedes them in the session`, which seeds an empty-string and an integer `0` key before the prefixed entry and asserts the prefixed entry is removed while the non-Auth0 entries survive - Added `purge() clears values when a falsy integer key precedes them in the session`, covering the `php_serialize` leading-integer-key case with no empty-string key present - Reset `$_SESSION` in `beforeEach` so the seeded falsy keys do not leak into other tests under randomized ordering - [x] This change adds unit test coverage - [x] This change has been tested on the latest version of the platform/language ### Contributor Checklist - [x] I have read the [Auth0 general contribution guidelines](https://github.com/auth0/open-source-template/blob/master/GENERAL-CONTRIBUTING.md) - [x] I have read the [Auth0 code of conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md)
1 parent 74d502c commit 000db11

2 files changed

Lines changed: 50 additions & 8 deletions

File tree

src/Store/SessionStore.php

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Auth0\SDK\Utility\Toolkit;
1010

1111
use function defined;
12+
use function is_string;
1213

1314
/**
1415
* This class provides a layer to persist data using PHP Sessions.
@@ -96,16 +97,16 @@ public function purge(): void
9697
{
9798
$this->start();
9899

99-
$session = $_SESSION ?? [];
100-
$prefix = $this->sessionPrefix . '_';
100+
if (! isset($_SESSION)) {
101+
return;
102+
}
101103

102-
if ([] !== $session) {
103-
while ($sessionKey = key($session)) {
104-
if (mb_substr((string) $sessionKey, 0, mb_strlen($prefix)) === $prefix) {
105-
unset($_SESSION[$sessionKey]);
106-
}
104+
$prefix = $this->sessionPrefix . '_';
107105

108-
next($session);
106+
// Snapshot keys first so a falsy key (0 or "") cannot terminate iteration early.
107+
foreach (array_keys($_SESSION) as $sessionKey) {
108+
if (is_string($sessionKey) && str_starts_with($sessionKey, $prefix)) {
109+
unset($_SESSION[$sessionKey]);
109110
}
110111
}
111112
}

tests/Unit/Store/SessionStoreTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
session_destroy();
1414
}
1515

16+
// session_destroy() leaves $_SESSION populated, so reset it to isolate tests.
17+
$_SESSION = [];
18+
1619
$this->configuration = new SdkConfiguration([
1720
'domain' => MockDomain::valid(),
1821
'clientId' => uniqid(),
@@ -75,6 +78,44 @@
7578
fn() => uniqid(),
7679
]]);
7780

81+
test('purge() clears values when a falsy key precedes them in the session', function(string $key, string $value): void {
82+
// A falsy key positioned before the prefixed keys must not stop the purge early.
83+
$_SESSION = ['' => 'falsy-string'] + $_SESSION;
84+
$_SESSION[0] = 'falsy-int';
85+
$_SESSION[$this->namespace . '_' . $key] = $value;
86+
expect(isset($_SESSION[$this->namespace . '_' . $key]))->toBeTrue();
87+
88+
$this->store->purge();
89+
90+
expect($this->store->get($key))->toBeNull();
91+
expect(isset($_SESSION[$this->namespace . '_' . $key]))->toBeFalse();
92+
93+
// Non-Auth0 entries must be left untouched.
94+
expect($_SESSION[''])->toEqual('falsy-string');
95+
expect($_SESSION[0])->toEqual('falsy-int');
96+
})->with(['mocked data' => [
97+
fn() => uniqid(),
98+
fn() => uniqid(),
99+
]]);
100+
101+
test('purge() clears values when a falsy integer key precedes them in the session', function(string $key, string $value): void {
102+
// The php_serialize handler leaves a leading integer key. It must not stop the purge early.
103+
$_SESSION = [0 => 'falsy-int'];
104+
$_SESSION[$this->namespace . '_' . $key] = $value;
105+
expect(isset($_SESSION[$this->namespace . '_' . $key]))->toBeTrue();
106+
107+
$this->store->purge();
108+
109+
expect($this->store->get($key))->toBeNull();
110+
expect(isset($_SESSION[$this->namespace . '_' . $key]))->toBeFalse();
111+
112+
// The non-Auth0 integer key must survive.
113+
expect($_SESSION[0])->toEqual('falsy-int');
114+
})->with(['mocked data' => [
115+
fn() => uniqid(),
116+
fn() => uniqid(),
117+
]]);
118+
78119
test('regenerate() preserves stored values', function(string $key, string $value): void {
79120
$this->store->set($key, $value);
80121
expect($this->store->get($key))->toEqual($value);

0 commit comments

Comments
 (0)