Skip to content

Commit

Permalink
[SDK-3646] Reliability and performance improvements to CookieStore (#649
Browse files Browse the repository at this point in the history 
)

* Improve CookieStore reliability under some circumstances

* Adjust tests to reflect CookieStore changes

* Ignore code coverage on untestable circumstances

* Remove leftover debug code

* Temporarily lock dependency due to break upstream

* Code quality improvements

* Additional improvements

* Expand tests
  • Loading branch information
@evansims
evansims authored Sep 24, 2022
1 parent 1e34266 commit 04b1f5d
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 110 deletions.
2 changes: 1 addition & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"pestphp/pest": "^1.21",
"php-http/mock-client": "^1.5",
"phpstan/phpstan": "^1.7",
"phpstan/phpstan-strict-rules": "^1.3",
"phpstan/phpstan-strict-rules": "1.4.3",
"phpunit/phpunit": "^9.5",
"rector/rector": "^0.13.6",
"squizlabs/php_codesniffer": "^3.7",
Expand Down
192 changes: 121 additions & 71 deletions src/Store/CookieStore.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
*/
final class CookieStore implements StoreInterface
{
public const USE_CRYPTO = true;
public const KEY_HASHING_ALGO = 'sha256';
public const KEY_CHUNKING_THRESHOLD = 3072;
public const KEY_CHUNKING_THRESHOLD = 2048;
public const KEY_SEPARATOR = '_';
public const VAL_CRYPTO_ALGO = 'aes-128-gcm';

Expand Down Expand Up @@ -48,6 +47,16 @@ final class CookieStore implements StoreInterface
*/
private bool $deferring = false;

/**
* Determine if changes have been made since the last setState.
*/
private bool $dirty = false;

/**
* Determine if changes have been made since the last setState.
*/
private bool $encrypt = true;

/**
* CookieStore constructor.
*
Expand All @@ -68,13 +77,7 @@ public function __construct(

$this->configuration = $configuration;
$this->namespace = (string) $namespace;

// @phpstan-ignore-next-line
if (self::USE_CRYPTO) {
$this->namespace = hash(self::KEY_HASHING_ALGO, $this->namespace);
}

$this->threshold = self::KEY_CHUNKING_THRESHOLD - strlen($this->namespace) + 2;
$this->threshold = self::KEY_CHUNKING_THRESHOLD - strlen($this->namespace);

$this->getState();
}
Expand All @@ -95,6 +98,25 @@ public function getThreshold(): int
return $this->threshold;
}

/**
* Returns the current encryption state
*/
public function getEncrypted(): bool
{
return $this->encrypt;
}

/**
* Toggle the encryption state
*
* @param bool $encrypt Enable or disable cookie encryption.
*/
public function setEncrypted(bool $encrypt = true): self
{
$this->encrypt = $encrypt;
return $this;
}

/**
* Defer saving state changes to destination to improve performance during blocks of changes.
*
Expand All @@ -103,14 +125,13 @@ public function getThreshold(): int
public function defer(
bool $deferring
): void {
$this->deferring = $deferring;

// If we were deferring state saving and we've been asked to cancel that deference
if ($this->deferring && ! $deferring) {
if (! $deferring) {
// Immediately push the state to the host device.
$this->setState();
}

// Update our deference state.
$this->deferring = $deferring;
}

/**
Expand All @@ -125,6 +146,10 @@ public function getState(
): array {
// Overwrite our internal state with one passed (presumably during unit tests.)
if ($state !== null) {
if ($this->store !== $state) {
$this->dirty = true;
}

return $this->store = $state;
}

Expand All @@ -151,7 +176,7 @@ public function getState(
}

// If no cookies were found, set an empty state and continue.
if (mb_strlen($data) === 0) {
if ($data === '') {
return $this->store = [];
}

Expand All @@ -171,9 +196,16 @@ public function getState(

/**
* Push our storage state to the source for persistence.
*
* @psalm-suppress UnusedFunctionCall
*/
public function setState(): self
{
public function setState(
bool $force = false
): self {
if (!$this->dirty && !$force) {
return $this;
}

$setOptions = $this->getCookieOptions();
$deleteOptions = $this->getCookieOptions(-1000);
$existing = [];
Expand Down Expand Up @@ -212,7 +244,7 @@ public function setState(): self
// @codeCoverageIgnoreStart
if (! defined('AUTH0_TESTS_DIR')) {
/** @var array{expires?: int, path?: string, domain?: string, secure?: bool, httponly?: bool, samesite?: 'Lax'|'lax'|'None'|'none'|'Strict'|'strict', url_encode?: int} $setOptions */
setcookie($cookieName, $chunk, $setOptions);
setrawcookie($cookieName, $chunk, $setOptions);
}
// @codeCoverageIgnoreEnd

Expand All @@ -233,14 +265,15 @@ public function setState(): self
// @codeCoverageIgnoreStart
if (! defined('AUTH0_TESTS_DIR')) {
/** @var array{expires?: int, path?: string, domain?: string, secure?: bool, httponly?: bool, samesite?: 'Lax'|'lax'|'None'|'none'|'Strict'|'strict', url_encode?: int} $deleteOptions */
setcookie($cookieName, '', $deleteOptions);
setrawcookie($cookieName, '', $deleteOptions);
}
// @codeCoverageIgnoreEnd

// Clear PHP's internal COOKIE global of the orphaned cookie.
unset($_COOKIE[$cookieName]);
}

$this->dirty = false;
return $this;
}

Expand All @@ -260,7 +293,10 @@ public function set(
[$key, \Auth0\SDK\Exception\ArgumentException::missing('key')],
])->isString();

$this->store[(string) $key] = $value;
if (! isset($this->store[(string) $key]) || $this->store[(string) $key] !== $value) {
$this->store[(string) $key] = $value;
$this->dirty = true;
}

if (! $this->deferring) {
$this->setState();
Expand Down Expand Up @@ -303,7 +339,10 @@ public function delete(
[$key, \Auth0\SDK\Exception\ArgumentException::missing('key')],
])->isString();

unset($this->store[(string) $key]);
if (isset($this->store[(string) $key])) {
unset($this->store[(string) $key]);
$this->dirty = true;
}

if (! $this->deferring) {
$this->setState();
Expand All @@ -315,7 +354,10 @@ public function delete(
*/
public function purge(): void
{
$this->store = [];
if ($this->store !== []) {
$this->store = [];
$this->dirty = true;
}

if (! $this->deferring) {
$this->setState();
Expand Down Expand Up @@ -351,10 +393,9 @@ public function getCookieOptions(
$options['samesite'] = 'Lax';
}

$domain = $this->configuration->getCookieDomain() ?? $_SERVER['HTTP_HOST'] ?? null;
$domain = $this->configuration->getCookieDomain() ?? null;

if ($domain !== null) {
/** @var string $domain */
if ($domain !== null && $domain !== $_SERVER['HTTP_HOST']) {
$options['domain'] = $domain;
}

Expand All @@ -364,47 +405,70 @@ public function getCookieOptions(
/**
* Encrypt data for safe storage format for a cookie.
*
* @param array<mixed> $data Data to encrypt.
* @param array<mixed> $data Data to encrypt.
* @param array<mixed> $options Additional configuration options.
*
* @psalm-suppress TypeDoesNotContainType
*/
private function encrypt(
array $data
public function encrypt(
array $data,
array $options = []
): string {
// @codeCoverageIgnoreStart
// @phpstan-ignore-next-line
if (! self::USE_CRYPTO) {
return base64_encode(json_encode(serialize($data), JSON_THROW_ON_ERROR));
if (! $this->encrypt) {
$data = $options['encoded1'] ?? json_encode($data);

if (! is_string($data)) {
return '';
}

return rawurlencode($data);
}
// @codeCoverageIgnoreEnd

$secret = $this->configuration->getCookieSecret();
$ivLen = openssl_cipher_iv_length(self::VAL_CRYPTO_ALGO);
$ivLen = $options['ivLen'] ?? openssl_cipher_iv_length(self::VAL_CRYPTO_ALGO);
$tag = null;

if ($secret === null) {
throw \Auth0\SDK\Exception\ConfigurationException::requiresCookieSecret();
}

// @codeCoverageIgnoreStart
if ($ivLen === false) {
if (! is_int($ivLen)) {
return '';
}

$iv = $options['iv'] ?? openssl_random_pseudo_bytes($ivLen);

if (! is_string($iv)) {
return '';
}

$data = $options['encoded1'] ?? json_encode($data);

if (! is_string($data)) {
return '';
}
// @codeCoverageIgnoreEnd

$iv = openssl_random_pseudo_bytes($ivLen);
// Encrypt the PHP array.
$encrypted = $options['encrypted'] ?? openssl_encrypt($data, self::VAL_CRYPTO_ALGO, $secret, 0, $iv, $tag);
$iv = $options['iv'] ?? $iv;
$tag = $options['tag'] ?? $tag;

// @codeCoverageIgnoreStart
// @phpstan-ignore-next-line
if ($iv === false) {
if (! is_string($encrypted)) {
return '';
}
// @codeCoverageIgnoreEnd

// Encrypt the serialized PHP array.
$encrypted = openssl_encrypt(serialize($data), self::VAL_CRYPTO_ALGO, $secret, 0, $iv, $tag);
if (! is_string($tag)) {
return '';
}

// Return a JSON encoded object containing the crypto tag and iv, and the encrypted data.
return json_encode(serialize(['tag' => base64_encode($tag), 'iv' => base64_encode($iv), 'data' => $encrypted]), JSON_THROW_ON_ERROR);
$encoded = $options['encoded2'] ?? json_encode(['tag' => base64_encode($tag), 'iv' => base64_encode($iv), 'data' => $encrypted]);

if (is_string($encoded)) {
return rawurlencode($encoded);
}

return '';
}

/**
Expand All @@ -416,30 +480,19 @@ private function encrypt(
*
* @psalm-suppress TypeDoesNotContainType
*/
private function decrypt(
public function decrypt(
string $data
) {
// @codeCoverageIgnoreStart
// @phpstan-ignore-next-line
if (! self::USE_CRYPTO) {
$returns = [];
$decoded = base64_decode($data, true);

if (is_string($decoded)) {
$decoded = json_decode($decoded, true, 512, JSON_THROW_ON_ERROR);
}

if (is_string($decoded)) {
$decoded = unserialize($decoded);
}
if (! $this->encrypt) {
$decoded = rawurldecode($data);
$decoded = json_decode($decoded, true);

if (is_array($decoded)) {
$returns = $decoded;
return $decoded;
}

return $returns;
return [];
}
// @codeCoverageIgnoreEnd

[$data] = Toolkit::filter([$data])->string()->trim();

Expand All @@ -453,14 +506,11 @@ private function decrypt(
throw \Auth0\SDK\Exception\ConfigurationException::requiresCookieSecret();
}

$data = json_decode((string) $data, true);
$decoded = rawurldecode((string) $data);
$stripped = stripslashes($decoded);
$data = json_decode($stripped, true, 512);

if (! is_string($data)) {
return null;
}

/** @var array{iv?: int|string|null, tag?: int|string|null, data: string} */
$data = unserialize($data);
/** @var array{iv?: int|string|null, tag?: int|string|null, data: string} $data */

if (! isset($data['iv']) || ! isset($data['tag']) || ! is_string($data['iv']) || ! is_string($data['tag'])) {
return null;
Expand All @@ -469,17 +519,17 @@ private function decrypt(
$iv = base64_decode($data['iv'], true);
$tag = base64_decode($data['tag'], true);

if ($iv === false || $tag === false) {
if (! is_string($iv) || ! is_string($tag)) {
return null;
}

$data = openssl_decrypt($data['data'], self::VAL_CRYPTO_ALGO, $secret, 0, $iv, $tag);

if ($data === false) {
if (! is_string($data)) {
return null;
}

$data = unserialize($data);
$data = json_decode($data, true);

/** @var array<mixed> $data */
return $data;
Expand Down
Loading

0 comments on commit 04b1f5d

Please sign in to comment.