From 70e4bd7147a43f0c22b470153fb9d2ff00017103 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 26 May 2016 14:38:45 +0200 Subject: [PATCH 1/5] Using a PSR6 cache instead of file cache --- .../custom_authentication_provider.rst | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/cookbook/security/custom_authentication_provider.rst b/cookbook/security/custom_authentication_provider.rst index 220d8cdc5ac..7bd0c3c1380 100644 --- a/cookbook/security/custom_authentication_provider.rst +++ b/cookbook/security/custom_authentication_provider.rst @@ -208,6 +208,7 @@ the ``PasswordDigest`` header value matches with the user's password. // src/AppBundle/Security/Authentication/Provider/WsseProvider.php namespace AppBundle\Security\Authentication\Provider; + use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; @@ -218,12 +219,12 @@ the ``PasswordDigest`` header value matches with the user's password. class WsseProvider implements AuthenticationProviderInterface { private $userProvider; - private $cacheDir; + private $cachePool; - public function __construct(UserProviderInterface $userProvider, $cacheDir) + public function __construct(UserProviderInterface $userProvider, CacheItemPoolInterface $cachePool) { $this->userProvider = $userProvider; - $this->cacheDir = $cacheDir; + $this->$cachePool = $cachePool; } public function authenticate(TokenInterface $token) @@ -258,19 +259,15 @@ the ``PasswordDigest`` header value matches with the user's password. return false; } + $cacheItem = $this->cacheService->getItem(md5($nonce)); // Validate that the nonce is *not* used in the last 5 minutes // if it has, this could be a replay attack - if ( - file_exists($this->cacheDir.'/'.md5($nonce)) - && file_get_contents($this->cacheDir.'/'.md5($nonce)) + 300 > time() - ) { + if ($cacheItem->isHit()) { throw new NonceExpiredException('Previously used nonce detected'); } // If cache directory does not exist we create it - if (!is_dir($this->cacheDir)) { - mkdir($this->cacheDir, 0777, true); - } - file_put_contents($this->cacheDir.'/'.md5($nonce), time()); + $cacheItem->set(null)->expiresAfter($this->lifetime - (time() - $created)); + $this->cacheService->save($cacheItem); // Validate Secret $expected = base64_encode(sha1(base64_decode($nonce).$created.$secret, true)); @@ -411,7 +408,7 @@ to service ids that do not exist yet: ``wsse.security.authentication.provider`` class: AppBundle\Security\Authentication\Provider\WsseProvider arguments: - '' # User Provider - - '%kernel.cache_dir%/security/nonces' + - '@cache' public: false wsse.security.authentication.listener: @@ -433,7 +430,7 @@ to service ids that do not exist yet: ``wsse.security.authentication.provider`` public="false" > - %kernel.cache_dir%/security/nonces + setPublic(false); From e4fe164eabf6a23ead97a409516c1798ced0487c Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 26 May 2016 14:54:43 +0200 Subject: [PATCH 2/5] Typos --- cookbook/security/custom_authentication_provider.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cookbook/security/custom_authentication_provider.rst b/cookbook/security/custom_authentication_provider.rst index 7bd0c3c1380..b985476891a 100644 --- a/cookbook/security/custom_authentication_provider.rst +++ b/cookbook/security/custom_authentication_provider.rst @@ -224,7 +224,7 @@ the ``PasswordDigest`` header value matches with the user's password. public function __construct(UserProviderInterface $userProvider, CacheItemPoolInterface $cachePool) { $this->userProvider = $userProvider; - $this->$cachePool = $cachePool; + $this->cachePool = $cachePool; } public function authenticate(TokenInterface $token) @@ -259,7 +259,7 @@ the ``PasswordDigest`` header value matches with the user's password. return false; } - $cacheItem = $this->cacheService->getItem(md5($nonce)); + $cacheItem = $this->cachePool->getItem(md5($nonce)); // Validate that the nonce is *not* used in the last 5 minutes // if it has, this could be a replay attack if ($cacheItem->isHit()) { @@ -267,7 +267,7 @@ the ``PasswordDigest`` header value matches with the user's password. } // If cache directory does not exist we create it $cacheItem->set(null)->expiresAfter($this->lifetime - (time() - $created)); - $this->cacheService->save($cacheItem); + $this->cachePool->save($cacheItem); // Validate Secret $expected = base64_encode(sha1(base64_decode($nonce).$created.$secret, true)); From 4d8b0f63fa05f75884dd7cda862013f2f2ef688b Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 26 May 2016 14:55:53 +0200 Subject: [PATCH 3/5] Code style --- cookbook/security/custom_authentication_provider.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cookbook/security/custom_authentication_provider.rst b/cookbook/security/custom_authentication_provider.rst index b985476891a..01281ca31c6 100644 --- a/cookbook/security/custom_authentication_provider.rst +++ b/cookbook/security/custom_authentication_provider.rst @@ -127,7 +127,7 @@ set an authenticated token in the token storage if successful. public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager) { - $this->tokenStorage = $tokenStorage; + $this->tokenStorage = $tokenStorage; $this->authenticationManager = $authenticationManager; } @@ -224,7 +224,7 @@ the ``PasswordDigest`` header value matches with the user's password. public function __construct(UserProviderInterface $userProvider, CacheItemPoolInterface $cachePool) { $this->userProvider = $userProvider; - $this->cachePool = $cachePool; + $this->cachePool = $cachePool; } public function authenticate(TokenInterface $token) From d42dcc4facce7431b24cc9fb6013348c6073f112 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 26 May 2016 15:03:29 +0200 Subject: [PATCH 4/5] Updated comments --- cookbook/security/custom_authentication_provider.rst | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cookbook/security/custom_authentication_provider.rst b/cookbook/security/custom_authentication_provider.rst index 01281ca31c6..6213f3686bb 100644 --- a/cookbook/security/custom_authentication_provider.rst +++ b/cookbook/security/custom_authentication_provider.rst @@ -259,14 +259,17 @@ the ``PasswordDigest`` header value matches with the user's password. return false; } + // Try to fetch the cache item from pool $cacheItem = $this->cachePool->getItem(md5($nonce)); - // Validate that the nonce is *not* used in the last 5 minutes - // if it has, this could be a replay attack + + // Validate that the nonce is *not* in cache + // if it is, this could be a replay attack if ($cacheItem->isHit()) { throw new NonceExpiredException('Previously used nonce detected'); } - // If cache directory does not exist we create it - $cacheItem->set(null)->expiresAfter($this->lifetime - (time() - $created)); + + // Store the item in cache for 5 minutes + $cacheItem->set(null)->expiresAfter(300); $this->cachePool->save($cacheItem); // Validate Secret From 06741aa6cae4fc8df212fef322c15847220035a1 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Tue, 31 May 2016 09:52:33 +0200 Subject: [PATCH 5/5] Changes according to the feedback given --- cookbook/security/custom_authentication_provider.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cookbook/security/custom_authentication_provider.rst b/cookbook/security/custom_authentication_provider.rst index 6213f3686bb..0410519eae5 100644 --- a/cookbook/security/custom_authentication_provider.rst +++ b/cookbook/security/custom_authentication_provider.rst @@ -127,7 +127,7 @@ set an authenticated token in the token storage if successful. public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager) { - $this->tokenStorage = $tokenStorage; + $this->tokenStorage = $tokenStorage; $this->authenticationManager = $authenticationManager; } @@ -224,7 +224,7 @@ the ``PasswordDigest`` header value matches with the user's password. public function __construct(UserProviderInterface $userProvider, CacheItemPoolInterface $cachePool) { $this->userProvider = $userProvider; - $this->cachePool = $cachePool; + $this->cachePool = $cachePool; } public function authenticate(TokenInterface $token) @@ -411,7 +411,7 @@ to service ids that do not exist yet: ``wsse.security.authentication.provider`` class: AppBundle\Security\Authentication\Provider\WsseProvider arguments: - '' # User Provider - - '@cache' + - '@cache.app' public: false wsse.security.authentication.listener: @@ -433,7 +433,7 @@ to service ids that do not exist yet: ``wsse.security.authentication.provider`` public="false" > - + setPublic(false);