fix: enforce back-channel logout on subsequent requests by rehydrating session state

[kishore7snehil]

Changes

Auth0::getState() rehydrated the user, id token, access token, scope, expiration, and refresh token from session storage, but not the backchannel key. setBackchannel() persists that key to session storage on login, yet getState() never read it back, so on any subsequent request getBackchannel() returned null and the enforcement check in getCredentials() short-circuited. A back-channel logout that had been queued was therefore not applied to later requests, and the session stayed authenticated.

🐛 Bug Fix:

• Auth0::getState() now rehydrates the backchannel key from session storage alongside the other persisted session values, so a queued back-channel logout is enforced on subsequent requests
• SdkState registers backchannel in its property defaults and validators, so the rehydrated value is accepted when the state is reconstructed

References

N/A

Testing

• Added getCredentials() enforces a queued backchannel logout on a subsequent request to the Auth0 unit tests, which logs in, queues a back-channel logout, then asserts a fresh SDK instance (rehydrating from session storage) reports no credentials

• This change adds unit test coverage

• This change has been tested on the latest version of the platform/language

Contributor Checklist

• I have read the Auth0 general contribution guidelines
• I have read the Auth0 code of conduct

[nandan-bhat]

This is the right fix, and I like that it is deliberately not wrapped in a persist flag the way the blocks above it are. Revocation should not be something you can accidentally switch off through a persistence setting.

But that only gets us half way, because getCredentials() still gates the enforcement block on the id token at line 371:

if (null !== $idToken) {
    $cache = $this->configuration()->getBackchannelLogoutCache();
    $backchannel = $state->getBackchannel();
    // ...
}

idToken is only rehydrated when getPersistIdToken() is true, a few lines above this one. persistIdToken defaults to true, but it is a documented option and apps do turn it off to keep the session cookie small.

So with persistIdToken => false: user still gets rehydrated, backchannel now gets rehydrated thanks to this line, but idToken is null, so the block at 371 is skipped and getCredentials() hands back a live session. That is the same outcome the ticket is about, just reached a different way.

Since this line now makes the key available unconditionally, could we gate on the key itself?

if (null !== $backchannel) {

I think that is actually the more correct check. The comment at 370 says the block is there to tell a real session apart from a bearer token being authorized, and backchannel is only ever set inside exchange(), never for bearer tokens. So it separates the two cases just as well as the id token does, and it stops the enforcement path depending on an unrelated persistence setting. It would also let you drop the duplicate getBackchannel() call at 375.

[nandan-bhat]

Worth calling out that this entry is doing real work and is not just bookkeeping.

ConfigurableMixin::applyConfiguration() does this at line 41:

if (! array_key_exists($configKey, $defaults)) {
    continue;
}

So before this PR, backchannel had a declared property and a working setBackchannel(), but was absent from the defaults list, which means the array key would have been dropped on the floor without a word. The new line in getState() would have looked correct and done nothing.

No change needed. It might be worth a short comment though, because this looks like a list you could tidy up without consequence, and removing this entry would quietly reopen the vulnerability with every test still green.

[nandan-bhat]

Three things about this setup line.

The ticket says all storage backends are affected, but this only exercises SessionStore. The default is CookieStore, so the configuration most people actually run is the one with no coverage. The bug was in getState(), which is backend agnostic, so a CookieStore case should pass without any extra production changes and would cover the default path.

new SdkConfiguration($this->configuration) builds a second configuration object just to hand to the store, separate from the one the Auth0 instance ends up creating from $config. It works, since SessionStore only uses the config for cookie params during start(), but it reads like it might be intentional when I think it is just awkward. Worth a comment or a restructure.

Last one: 'auth0_session' is a fixed namespace and nothing resets $_SESSION. beforeEach only clears $_GET and $_COOKIE, and there are other tests in this file using SessionStore at 374 and 821. pest:ci runs with --order-by=random and --fail-on-risky, so this is the kind of leak that turns into a confusing failure on some unrelated PR later. A uniqid() namespace, or $_SESSION = []; in beforeEach, would sort it.

[nandan-bhat]

This assertion is right and it does fail without the fix, which I checked.

Two suggestions to make it harder to pass by accident. Right now toBeNull() is also what you would get if the session simply failed to rehydrate, for example a storage mix up or a broken exchange, so a future regression that breaks rehydration entirely would still show green here. Asserting that credentials exist first, then queueing the logout, then asserting null, is what the older test at 1140 does and it pins the cause down:

$fresh = new Auth0($config);
expect($fresh->getCredentials())->not->toBeNull();  // rehydration works
// then queue the logout and assert null on another fresh instance

Second, could we add a 'persistIdToken' => false variant? That is the configuration I flagged on src/Auth0.php, where the key is rehydrated but the enforcement block is skipped anyway. It fails today, so it would only go in alongside the guard change, but it is the case most likely to quietly regress.