Extended state parameter support to be persistent when returned from …

…resource owner

OAuth/RequestDataStorage/SessionStorage.php

@@ -43,14 +43,16 @@ public function __construct(SessionInterface $session)
     public function fetch(ResourceOwnerInterface $resourceOwner, $key, $type = 'token')
     {
         $key = $this->generateKey($resourceOwner, $key, $type);
-        if (null === $token = $this->session->get($key)) {
+        if (null === $data = $this->session->get($key)) {
             throw new \InvalidArgumentException('No data available in storage.');
         }
 
         // request tokens are one time use only
-        $this->session->remove($key);
+        if (in_array($type, ['token', 'csrf_state'])) {
+            $this->session->remove($key);
+        }
 
-        return $token;
+        return $data;
     }
 
     /**
@@ -65,7 +67,11 @@ public function save(ResourceOwnerInterface $resourceOwner, $value, $type = 'tok
 
             $key = $this->generateKey($resourceOwner, $value['oauth_token'], 'token');
         } else {
-            $key = $this->generateKey($resourceOwner, \is_array($value) ? reset($value) : $value, $type);
+            $key = $this->generateKey($resourceOwner, $this->getStorageKey($value), $type);
+        }
+
+        if (is_object($value)) {
+            $value = serialize($value);
         }
 
         $this->session->set($key, $value);
@@ -84,4 +90,22 @@ protected function generateKey(ResourceOwnerInterface $resourceOwner, $key, $typ
     {
         return sprintf('_hwi_oauth.%s.%s.%s.%s', $resourceOwner->getName(), $resourceOwner->getOption('client_id'), $type, $key);
     }
+
+    /**
+     * @param array|string|object $value
+     *
+     * @return string
+     */
+    private function getStorageKey($value): string
+    {
+        if (\is_array($value)) {
+            $storageKey = reset($value);
+        } else if (is_object($value)) {
+            $storageKey = \get_class($value);
+        } else {
+            $storageKey = $value;
+        }
+
+        return (string) $storageKey;
+    }
 }

OAuth/RequestDataStorageInterface.php

@@ -33,15 +33,15 @@ interface RequestDataStorageInterface
      * @param string                 $key
      * @param string                 $type
      *
-     * @return array
+     * @return mixed
      */
     public function fetch(ResourceOwnerInterface $resourceOwner, $key, $type = 'token');
 
     /**
      * Save a request data to the storage.
      *
      * @param ResourceOwnerInterface $resourceOwner
-     * @param array|string           $value
+     * @param array|string|object    $value
      * @param string                 $type
      */
     public function save(ResourceOwnerInterface $resourceOwner, $value, $type = 'token');

OAuth/ResourceOwner/AbstractResourceOwner.php

@@ -71,6 +71,11 @@ abstract class AbstractResourceOwner implements ResourceOwnerInterface
      */
     protected $storage;
 
+    /**
+     * @var bool
+     */
+    private $stateLoaded = false;
+
     /**
      * @param HttpMethodsClient           $httpClient Httplug client
      * @param HttpUtils                   $httpUtils  Http utils
@@ -159,6 +164,19 @@ public function addPaths(array $paths)
      */
     public function getState(): StateInterface
     {
+        if ($this->stateLoaded) {
+            return $this->state;
+        }
+
+        // lazy-loading for stored states
+        $storedData = $this->storage->fetch($this, State::class, 'state');
+        if (null !== $storedData && false !== $storedState = unserialize($storedData)) {
+            foreach ($storedState->getAll() as $key => $value) {
+                $this->addStateParameter($key, $value);
+            }
+        }
+        $this->stateLoaded = true;
+
         return $this->state;
     }
 
@@ -167,7 +185,21 @@ public function getState(): StateInterface
      */
     public function addStateParameter(string $key, string $value): void
     {
-        $this->state->add($key, $value);
+        if (!$this->state->has($key)) {
+            $this->state->add($key, $value);
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function storeState(StateInterface $state = null): void
+    {
+        if (null === $state) {
+            return;
+        }
+
+        $this->storage->save($this, $state, 'state');
     }
 
     /**

OAuth/ResourceOwner/GenericOAuth2ResourceOwner.php

@@ -151,7 +151,8 @@ public function isCsrfTokenValid($csrfToken)
         }
 
         try {
-            return null !== $this->storage->fetch($this, urldecode($csrfToken), 'csrf_state');
+            return null !== $csrfToken
+                && null !== $this->storage->fetch($this, urldecode($csrfToken), 'csrf_state');
         } catch (\InvalidArgumentException $e) {
             throw new AuthenticationException('Given CSRF token is not valid.');
         }

OAuth/ResourceOwnerInterface.php

@@ -122,6 +122,11 @@ public function refreshAccessToken($refreshToken, array $extraParameters = []);
      */
     public function getState(): StateInterface;
 
+    /**
+     * @param StateInterface|null $state
+     */
+    public function storeState(StateInterface $state = null);
+
     /**
      * @param string $key
      * @param string $value

OAuth/State/State.php

@@ -27,11 +27,12 @@ final class State implements StateInterface
     private $values = [];
 
     /**
-     * @param string|array<string,string> The state parameter as a string or assoc array
+     * @param string|array<string,string>|null $parameters The state parameter as a string or assoc array
+     * @param bool $keepCsrf                               Whether to keep the CSRF token in the state or not
      *
      * @throws InvalidArgumentException
      */
-    public function __construct($parameters)
+    public function __construct($parameters, bool $keepCsrf = true)
     {
         if (!\is_array($parameters)) {
             $parameters = $this->parseStringParameter($parameters);
@@ -43,6 +44,9 @@ public function __construct($parameters)
             }
 
             foreach ($parameters as $key => $value) {
+                if (false === $keepCsrf && $key === self::CSRF_TOKEN_KEY) {
+                    continue;
+                }
                 $this->add($key, $value);
             }
         }
@@ -72,14 +76,25 @@ public function get(string $key): ?string
         return $this->values[$key];
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function has(string $key): bool
+    {
+        return array_key_exists($key, $this->values);
+    }
+
     public function setCsrfToken(string $token): void
     {
         $this->values[self::CSRF_TOKEN_KEY] = $token;
     }
 
     public function getAll(): array
     {
-        return $this->values;
+        $values = $this->values;
+        unset($values[self::CSRF_TOKEN_KEY]);
+
+        return $values;
     }
 
     public function getCsrfToken(): ?string
@@ -88,8 +103,6 @@ public function getCsrfToken(): ?string
     }
 
     /**
-     * {@inheritdoc}
-     *
      * Encodes the array of values to a string so it can be stored in a query parameter.
      * Returns the plain value if only the default key or CSRF token has been set.
      */
@@ -107,7 +120,7 @@ public function encode(): ?string
             return urlencode($this->values[self::CSRF_TOKEN_KEY]);
         }
 
-        $encoded = urlencode(self::encodeArray($this->values));
+        $encoded = urlencode($this->encodeValues());
 
         return '' !== $encoded ? $encoded : null;
     }
@@ -117,7 +130,7 @@ public function encode(): ?string
      *
      * @return array<string,string>|null
      */
-    private function parseStringParameter(string $queryParameter = null): ?array
+    private function parseStringParameter(?string $queryParameter = null): ?array
     {
         $urlDecoded = urldecode($queryParameter);
         $values = json_decode(base64_decode($urlDecoded), true);
@@ -130,13 +143,11 @@ private function parseStringParameter(string $queryParameter = null): ?array
     }
 
     /**
-     * @param array $array The array to encode
-     *
      * @return string The encoded array
      */
-    private static function encodeArray(array $array): string
+    private function encodeValues(): string
     {
-        return base64_encode(json_encode($array));
+        return base64_encode(json_encode($this->values));
     }
 
     /**
@@ -160,4 +171,14 @@ private function isAssociatedArray(?array $array): bool
 
         return array_keys($array) !== range(0, \count($array) - 1);
     }
+
+    public function serialize(): ?string
+    {
+        return serialize($this->values);
+    }
+
+    public function unserialize($serialized): void
+    {
+        $this->values = unserialize($serialized);
+    }
 }

OAuth/StateInterface.php

@@ -14,7 +14,7 @@
 use HWI\Bundle\OAuthBundle\OAuth\Exception\StateRetrievalException;
 use Symfony\Component\Config\Definition\Exception\DuplicateKeyException;
 
-interface StateInterface
+interface StateInterface extends \Serializable
 {
     /**
      * @param string $key   The key to store a value to
@@ -33,6 +33,13 @@ public function add(string $key, string $value);
      */
     public function get(string $key): ?string;
 
+    /**
+     * @param string $key
+     *
+     * @return bool
+     */
+    public function has(string $key): bool;
+
     /**
      * @return array<string, string>
      */

Security/Http/ResourceOwnerMap.php

@@ -11,6 +11,8 @@
 
 namespace HWI\Bundle\OAuthBundle\Security\Http;
 
+use HWI\Bundle\OAuthBundle\OAuth\ResourceOwnerInterface;
+use HWI\Bundle\OAuthBundle\OAuth\State\State;
 use Symfony\Component\DependencyInjection\ContainerAwareInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
@@ -83,7 +85,11 @@ public function getResourceOwnerByName($name)
             return null;
         }
 
-        return $this->container->get('hwi_oauth.resource_owner.'.$name);
+        /** @var ResourceOwnerInterface $resourceOwner */
+        $resourceOwner = $this->container->get('hwi_oauth.resource_owner.'.$name);
+
+
+        return $resourceOwner;
     }
 
     /**
@@ -93,7 +99,14 @@ public function getResourceOwnerByRequest(Request $request)
     {
         foreach ($this->resourceOwners as $name => $checkPath) {
             if ($this->httpUtils->checkRequestPath($request, $checkPath)) {
-                return [$this->getResourceOwnerByName($name), $checkPath];
+                $resourceOwner = $this->getResourceOwnerByName($name);
+
+                // save the round-tripped state to the resource owner
+                if (null !== $resourceOwner) {
+                    $resourceOwner->storeState(new State($request->get('state'), false));
+                }
+
+                return [$resourceOwner, $checkPath];
             }
         }
 

Security/OAuthUtils.php

@@ -297,10 +297,10 @@ protected function getResourceOwnerCheckPath($name)
     }
 
     /**
-     * @param string|null            $queryParameter The query parameter to parse and add to the State
+     * @param string|array<string, string>|null            $queryParameter The query parameter to parse and add to the State
      * @param ResourceOwnerInterface $resourceOwner  The resource owner holding the state to be added to
      */
-    private function addQueryParameterToState(?string $queryParameter, ResourceOwnerInterface $resourceOwner): void
+    private function addQueryParameterToState($queryParameter, ResourceOwnerInterface $resourceOwner): void
     {
         if (null === $queryParameter) {
             return;

Tests/Fixtures/MyCustomProvider.php

@@ -66,4 +66,8 @@ public function getState(): StateInterface
     public function addStateParameter(string $key, string $value): void
     {
     }
+
+    public function storeState(StateInterface $state = null)
+    {
+    }
 }