Skip to content

Commit

Permalink
[SDK-3722] Fix: Stateless strategies should not invoke stateful sessi…
Browse files Browse the repository at this point in the history 
…on classes (#662)

* [SDK-3722] Fix: Stateless strategies should not invoke stateful session checks

* Fix: getState($reset: true) should still be run on clear(), for in-memory state cleanup.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@evansims @github-actions
evansims and github-actions[bot] committed Oct 21, 2022
1 parent 81b8ddd commit e727a9b
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 45 deletions.
121 changes: 77 additions & 44 deletions src/Auth0.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Auth0\SDK\Contract\API\ManagementInterface;
use Auth0\SDK\Contract\Auth0Interface;
use Auth0\SDK\Contract\TokenInterface;
use Auth0\SDK\Exception\ConfigurationException;
use Auth0\SDK\Utility\HttpResponse;
use Auth0\SDK\Utility\PKCE;
use Auth0\SDK\Utility\Toolkit;
Expand Down Expand Up @@ -93,11 +94,16 @@ public function login(
?string $redirectUrl = null,
?array $params = null
): string {
if (! $this->configuration()->usingStatefulness()) {
throw ConfigurationException::requiresStatefulness('Auth0->login()');
}

$this->deferStateSaving();

$store = $this->getTransientStore();
$params = $params ?? [];
$state = $params['state'] ?? $this->getTransientStore()->issue('state');
$params['nonce'] = $params['nonce'] ?? $this->getTransientStore()->issue('nonce');
$state = $params['state'] ?? $store?->issue('state') ?? uniqid();
$params['nonce'] = $params['nonce'] ?? $store?->issue('nonce') ?? uniqid();
$params['max_age'] = $params['max_age'] ?? $this->configuration()->getTokenMaxAge();

unset($params['state']);
Expand All @@ -106,11 +112,11 @@ public function login(
$codeVerifier = PKCE::generateCodeVerifier(128);
$params['code_challenge'] = PKCE::generateCodeChallenge($codeVerifier);
$params['code_challenge_method'] = 'S256';
$this->getTransientStore()->store('code_verifier', $codeVerifier);
$store?->store('code_verifier', $codeVerifier);
}

if ($params['max_age'] !== null) {
$this->getTransientStore()->store('max_age', (string) $params['max_age']);
$store?->store('max_age', (string) $params['max_age']);
}

$this->deferStateSaving(false);
Expand All @@ -122,6 +128,10 @@ public function signup(
?string $redirectUrl = null,
?array $params = null
): string {
if (! $this->configuration()->usingStatefulness()) {
throw ConfigurationException::requiresStatefulness('Auth0->signup()');
}

$params = Toolkit::merge([
'screen_hint' => 'signup',
], $params);
Expand All @@ -135,6 +145,10 @@ public function handleInvitation(
?string $redirectUrl = null,
?array $params = null
): ?string {
if (! $this->configuration()->usingStatefulness()) {
throw ConfigurationException::requiresStatefulness('Auth0->handleInvitation()');
}

$invite = $this->getInvitationParameters();

if ($invite !== null) {
Expand All @@ -155,6 +169,10 @@ public function logout(
?string $returnUri = null,
?array $params = null
): string {
if (! $this->configuration()->usingStatefulness()) {
throw ConfigurationException::requiresStatefulness('Auth0->logout()');
}

$this->clear();

return $this->authentication()->getLogoutLink($returnUri, $params);
Expand All @@ -163,20 +181,22 @@ public function logout(
public function clear(
bool $transient = true
): self {
$this->deferStateSaving();
if ($this->configuration()->usingStatefulness()) {
$this->deferStateSaving();

// Delete all data in the session storage medium.
if ($this->configuration()->hasSessionStorage()) {
$this->configuration->getSessionStorage()->purge();
}
// Delete all data in the session storage medium.
if ($this->configuration()->hasSessionStorage()) {
$this->configuration->getSessionStorage()->purge();
}

// Delete all data in the transient storage medium.
if ($this->configuration()->hasTransientStorage() && $transient) {
$this->configuration->getTransientStorage()->purge();
}
// Delete all data in the transient storage medium.
if ($this->configuration()->hasTransientStorage() && $transient) {
$this->configuration->getTransientStorage()->purge();
}

// If state saving had been deferred, disable it and force a update to persistent storage.
$this->deferStateSaving(false);
// If state saving had been deferred, disable it and force a update to persistent storage.
$this->deferStateSaving(false);
}

// Reset the internal state.
$this->getState(true);
Expand All @@ -194,9 +214,10 @@ public function decode(
?int $tokenNow = null,
?int $tokenType = null
): TokenInterface {
$store = $this->getTransientStore();
$tokenType = $tokenType ?? Token::TYPE_ID_TOKEN;
$tokenNonce = $tokenNonce ?? $this->getTransientStore()->getOnce('nonce') ?? null;
$tokenMaxAge = $tokenMaxAge ?? $this->getTransientStore()->getOnce('max_age') ?? null;
$tokenNonce = $tokenNonce ?? $store?->getOnce('nonce') ?? null;
$tokenMaxAge = $tokenMaxAge ?? $store?->getOnce('max_age') ?? null;
$tokenIssuer = null;

$token = new Token($this->configuration, $token, $tokenType);
Expand All @@ -218,8 +239,10 @@ public function decode(
);

// Ensure transient-stored values are cleared, even if overriding values were passed to the method.
$this->getTransientStore()->delete('max_age');
$this->getTransientStore()->delete('nonce');
if ($this->configuration()->usingStatefulness()) {
$store?->delete('max_age');
$store?->delete('nonce');
}

return $token;
}
Expand All @@ -229,11 +252,24 @@ public function exchange(
?string $code = null,
?string $state = null
): bool {
if (! $this->configuration()->usingStatefulness()) {
throw ConfigurationException::requiresStatefulness('Auth0->exchange()');
}

[$redirectUri, $code, $state] = Toolkit::filter([$redirectUri, $code, $state])->string()->trim();

$code = $code ?? $this->getRequestParameter('code');
$state = $state ?? $this->getRequestParameter('state');
$codeVerifier = null;

$store = $this->getTransientStore();
$codeVerifier = $store?->getOnce('code_verifier');
$nonce = $store?->isset('nonce');
$stateVerified = null;

if (null !== $state) {
$stateVerified = $store?->verify('state', $state) ?? false;
}

$user = null;

$this->clear(false);
Expand All @@ -244,18 +280,14 @@ public function exchange(
throw \Auth0\SDK\Exception\StateException::missingCode();
}

if ($state === null || ! $this->getTransientStore()->verify('state', $state)) {
if ($state === null || ! $stateVerified) {
$this->clear();
throw \Auth0\SDK\Exception\StateException::invalidState();
}

if ($this->configuration()->getUsePkce()) {
$codeVerifier = $this->getTransientStore()->getOnce('code_verifier');

if ($codeVerifier === null) {
$this->clear();
throw \Auth0\SDK\Exception\StateException::missingCodeVerifier();
}
if ($this->configuration()->getUsePkce() && $codeVerifier === null) {
$this->clear();
throw \Auth0\SDK\Exception\StateException::missingCodeVerifier();
}

$response = $this->authentication()->codeExchange($code, $redirectUri, $codeVerifier);
Expand Down Expand Up @@ -285,7 +317,7 @@ public function exchange(
}

if (isset($response['id_token'])) {
if (! $this->getTransientStore()->isset('nonce')) {
if (null === $nonce || ! $nonce) {
$this->clear();
throw \Auth0\SDK\Exception\StateException::missingNonce();
}
Expand Down Expand Up @@ -479,7 +511,7 @@ public function setIdToken(
): self {
$this->getState()->setIdToken($idToken);

if ($this->configuration()->hasSessionStorage() && $this->configuration()->getPersistIdToken()) {
if ($this->configuration()->usingStatefulness() && $this->configuration()->hasSessionStorage() && $this->configuration()->getPersistIdToken()) {
$this->configuration()->getSessionStorage()->set('idToken', $idToken);
}

Expand All @@ -491,7 +523,7 @@ public function setUser(
): self {
$this->getState()->setUser($user);

if ($this->configuration()->hasSessionStorage() && $this->configuration()->getPersistUser()) {
if ($this->configuration()->usingStatefulness() && $this->configuration()->hasSessionStorage() && $this->configuration()->getPersistUser()) {
$this->configuration()->getSessionStorage()->set('user', $user);
}

Expand All @@ -503,7 +535,7 @@ public function setAccessToken(
): self {
$this->getState()->setAccessToken($accessToken);

if ($this->configuration()->hasSessionStorage() && $this->configuration()->getPersistAccessToken()) {
if ($this->configuration()->usingStatefulness() && $this->configuration()->hasSessionStorage() && $this->configuration()->getPersistAccessToken()) {
$this->configuration()->getSessionStorage()->set('accessToken', $accessToken);
}

Expand All @@ -515,7 +547,7 @@ public function setRefreshToken(
): self {
$this->getState()->setRefreshToken($refreshToken);

if ($this->configuration()->hasSessionStorage() && $this->configuration()->getPersistRefreshToken()) {
if ($this->configuration()->usingStatefulness() && $this->configuration()->hasSessionStorage() && $this->configuration()->getPersistRefreshToken()) {
$this->configuration()->getSessionStorage()->set('refreshToken', $refreshToken);
}

Expand All @@ -527,7 +559,7 @@ public function setAccessTokenScope(
): self {
$this->getState()->setAccessTokenScope($accessTokenScope);

if ($this->configuration()->hasSessionStorage() && $this->configuration()->getPersistAccessToken()) {
if ($this->configuration()->usingStatefulness() && $this->configuration()->hasSessionStorage() && $this->configuration()->getPersistAccessToken()) {
$this->configuration()->getSessionStorage()->set('accessTokenScope', $accessTokenScope);
}

Expand All @@ -538,8 +570,7 @@ public function setAccessTokenExpiration(
int $accessTokenExpiration
): self {
$this->getState()->setAccessTokenExpiration($accessTokenExpiration);

if ($this->configuration()->hasSessionStorage() && $this->configuration()->getPersistAccessToken()) {
if ($this->configuration()->usingStatefulness() && $this->configuration()->hasSessionStorage() && $this->configuration()->getPersistAccessToken()) {
$this->configuration()->getSessionStorage()->set('accessTokenExpiration', $accessTokenExpiration);
}

Expand Down Expand Up @@ -601,9 +632,9 @@ public function getInvitationParameters(): ?array
/**
* Create a transient storage handler using the configured transientStorage medium.
*/
private function getTransientStore(): TransientStoreHandler
private function getTransientStore(bool $reset = false): ?TransientStoreHandler
{
if ($this->transient === null) {
if (($this->transient === null || $reset) && ($this->configuration()->usingStatefulness() && null !== $this->configuration()->getTransientStorage())) {
$this->transient = new TransientStoreHandler($this->configuration()->getTransientStorage());
}

Expand All @@ -618,7 +649,7 @@ private function getState(bool $reset = false): SdkState
if ($this->state === null || $reset) {
$state = [];

if ($this->configuration()->hasSessionStorage()) {
if ($this->configuration()->usingStatefulness() && $this->configuration()->hasSessionStorage()) {
if ($this->configuration()->getPersistUser()) {
$state['user'] = $this->configuration()->getSessionStorage()->get('user');
}
Expand Down Expand Up @@ -660,12 +691,14 @@ private function getState(bool $reset = false): SdkState
private function deferStateSaving(
bool $deferring = true
): self {
if ($this->configuration()->hasSessionStorage()) {
$this->configuration()->getSessionStorage()->defer($deferring);
}
if ($this->configuration()->usingStatefulness()) {
if ($this->configuration()->hasSessionStorage()) {
$this->configuration()->getSessionStorage()->defer($deferring);
}

if ($this->configuration()->hasTransientStorage()) {
$this->configuration()->getTransientStorage()->defer($deferring);
if ($this->configuration()->hasTransientStorage()) {
$this->configuration()->getTransientStorage()->defer($deferring);
}
}

return $this;
Expand Down
10 changes: 9 additions & 1 deletion src/Configuration/SdkConfiguration.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public function __construct(
$this->setupStateCookies();
$this->setupStateFactories();

if (in_array($this->getStrategy(), self::STRATEGIES_USING_SESSIONS, true)) {
if ($this->usingStatefulness()) {
$this->setupStateStorage();
}

Expand Down Expand Up @@ -1137,6 +1137,14 @@ public function eventDispatcher(): EventDispatcher
return $this->eventDispatcher;
}

/**
* Returns true when the configured `strategy` is 'stateful', meaning it requires an available and configured session.
*/
public function usingStatefulness(): bool
{
return in_array($this->getStrategy(), self::STRATEGIES_USING_SESSIONS, true);
}

/**
* Setup SDK cookie state.
*/
Expand Down
8 changes: 8 additions & 0 deletions src/Exception/ConfigurationException.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ final class ConfigurationException extends \Exception implements Auth0Exception
{
public const MSG_CONFIGURATION_REQUIRED = 'The Auth0 SDK requires an SdkConfiguration be provided at initialization';
public const MSG_STRATEGY_REQUIRED = 'The Auth0 SDK requires a `strategy` to be configured';
public const MSG_SESSION_REQUIRED = 'Method %s requires a stateful `strategy` with sessions configured.';

public const MSG_VALUE_REQUIRED = '`%s` is not configured';

Expand Down Expand Up @@ -48,6 +49,13 @@ public static function requiresStrategy(
return new self(self::MSG_STRATEGY_REQUIRED, 0, $previous);
}

public static function requiresStatefulness(
string $methodName,
?\Throwable $previous = null
): self {
return new self(sprintf(self::MSG_SESSION_REQUIRED, $methodName), 0, $previous);
}

public static function requiresDomain(
?\Throwable $previous = null
): self {
Expand Down

0 comments on commit e727a9b

Please sign in to comment.