From 71006763fe1277e1dbb85e1405f76f43474caa99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ture=20Gj=C3=B8rup?= Date: Fri, 26 Jun 2020 13:52:31 +0200 Subject: [PATCH 1/2] DDBTEAM-538: Changed config option for allowed clients to allow .env values --- src/DependencyInjection/Configuration.php | 2 +- src/Security/TokenAuthenticator.php | 6 ++-- tests/TokenAuthenticatorTest.php | 34 +++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 95c706b..4059289 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -26,7 +26,7 @@ public function getConfigTreeBuilder() ->scalarNode('openplatform_id')->defaultValue('my_id')->end() ->scalarNode('openplatform_secret')->defaultValue('my_secret')->end() ->scalarNode('openplatform_introspection_url')->defaultValue('https://login.bib.dk/oauth/introspection')->end() - ->arrayNode('openplatform_allowed_clients')->scalarPrototype()->end()->end() + ->scalarNode('openplatform_allowed_clients')->defaultValue('')->end() ->scalarNode('http_client')->defaultValue('Symfony\Contracts\HttpClient\HttpClientInterface')->end() ->scalarNode('auth_token_cache')->defaultNull()->end() ->scalarNode('auth_logger')->defaultNull()->end() diff --git a/src/Security/TokenAuthenticator.php b/src/Security/TokenAuthenticator.php index 143dca5..162b49e 100644 --- a/src/Security/TokenAuthenticator.php +++ b/src/Security/TokenAuthenticator.php @@ -44,7 +44,7 @@ class TokenAuthenticator extends AbstractGuardAuthenticator * Open Platform secret * @param string $openplatformIntrospectionUrl * Open Platform introspection URL - * @param array $openplatformAllowedClients + * @param string $openplatformAllowedClients * An allow list of client id's. Supply an empty array to allow all. * @param HttpClientInterface $httpClient * Http client for calls to Open Platform @@ -53,12 +53,12 @@ class TokenAuthenticator extends AbstractGuardAuthenticator * @param LoggerInterface|null $logger * Logger for error logging */ - public function __construct(string $openplatformId, string $openplatformSecret, string $openplatformIntrospectionUrl, array $openplatformAllowedClients, HttpClientInterface $httpClient, AdapterInterface $tokenCache = null, LoggerInterface $logger = null) + public function __construct(string $openplatformId, string $openplatformSecret, string $openplatformIntrospectionUrl, string $openplatformAllowedClients, HttpClientInterface $httpClient, AdapterInterface $tokenCache = null, LoggerInterface $logger = null) { $this->clientId = $openplatformId; $this->clientSecret = $openplatformSecret; $this->endPoint = $openplatformIntrospectionUrl; - $this->allowedClients = $openplatformAllowedClients; + $this->allowedClients = empty($openplatformAllowedClients) ? [] : explode(',', $openplatformAllowedClients); $this->client = $httpClient; $this->cache = $tokenCache; diff --git a/tests/TokenAuthenticatorTest.php b/tests/TokenAuthenticatorTest.php index 5f284aa..0a38437 100644 --- a/tests/TokenAuthenticatorTest.php +++ b/tests/TokenAuthenticatorTest.php @@ -50,7 +50,7 @@ public function setUp(): void */ public function testTokenAuthenticatorFunctions(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $request = new Request(); $this->assertFalse($this->tokenAuthenticator->supports($request), 'Token authenticator should not support requests without authorization'); @@ -64,7 +64,7 @@ public function testTokenAuthenticatorFunctions(): void */ public function testCachedTokensAreReturnedFromCache(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->cache->expects($this->once())->method('getItem')->with('12345678'); @@ -85,7 +85,7 @@ public function testCachedTokensAreReturnedFromCache(): void */ public function testTokenCallToOpenPlatform(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -104,7 +104,7 @@ public function testTokenCallToOpenPlatform(): void */ public function testAccessDeniedIfRequestNot200(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -121,7 +121,7 @@ public function testAccessDeniedIfRequestNot200(): void */ public function testAccessDeniedIfRequestException(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -139,7 +139,7 @@ public function testAccessDeniedIfRequestException(): void */ public function testNonActiveUserDenied(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -158,7 +158,7 @@ public function testNonActiveUserDenied(): void */ public function testNonAnonymousTokenTypeDenied(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -177,7 +177,7 @@ public function testNonAnonymousTokenTypeDenied(): void */ public function testExpiredTokenIsDenied(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -196,7 +196,7 @@ public function testExpiredTokenIsDenied(): void */ public function testErrorTokenIsDenied(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -221,7 +221,7 @@ public function testErrorTokenIsDenied(): void */ public function testInvalidJsonTokenIsDenied(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -266,7 +266,7 @@ public function testInvalidJsonTokenIsDenied(): void */ public function testActiveUSerAllowed(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator([]); + $this->tokenAuthenticator = $this->getTokenAuthenticator(''); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -287,7 +287,7 @@ public function testActiveUSerAllowed(): void */ public function testCachedTokensClientIsAllowed(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator(['allowed-client-id']); + $this->tokenAuthenticator = $this->getTokenAuthenticator('allowed-client-id'); $this->cache->method('getItem')->willReturn($this->item); $this->cache->expects($this->once())->method('getItem')->with('12345678'); @@ -309,7 +309,7 @@ public function testCachedTokensClientIsAllowed(): void */ public function testCachedTokensClientIsNotAllowed(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator(['allowed-client-id']); + $this->tokenAuthenticator = $this->getTokenAuthenticator('allowed-client-id'); $this->cache->method('getItem')->willReturn($this->item); $this->cache->expects($this->once())->method('getItem')->with('12345678'); @@ -332,7 +332,7 @@ public function testCachedTokensClientIsNotAllowed(): void */ public function testAgencyShouldBeAllowed(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator(['allowed-client-id']); + $this->tokenAuthenticator = $this->getTokenAuthenticator('allowed-client-id'); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -351,7 +351,7 @@ public function testAgencyShouldBeAllowed(): void */ public function testAgencyShouldNotBeAllowed(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator(['allowed-client-id']); + $this->tokenAuthenticator = $this->getTokenAuthenticator('allowed-client-id'); $this->cache->method('getItem')->willReturn($this->item); $this->item->method('isHit')->willReturn(false); @@ -366,13 +366,13 @@ public function testAgencyShouldNotBeAllowed(): void /** * Helper function to setup TokenAuthenticator with/without allowed clients. * - * @param array $allowedClients + * @param string $allowedClients * An allow list of client id's. Supply an empty array to allow all. * * @return TokenAuthenticator * A configured TokenAuthenticator */ - private function getTokenAuthenticator(array $allowedClients) + private function getTokenAuthenticator(string $allowedClients) { return new TokenAuthenticator('id', 'secret', 'https://auth.test', $allowedClients, $this->httpClient, $this->cache, $this->logger); } From df50eabfc12037d2f18ab65fc0d8821f74a9d2a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ture=20Gj=C3=B8rup?= Date: Fri, 26 Jun 2020 14:48:23 +0200 Subject: [PATCH 2/2] DDBTEAM-538: Added test for multiple allowed client id's --- src/Security/TokenAuthenticator.php | 3 ++- tests/TokenAuthenticatorTest.php | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Security/TokenAuthenticator.php b/src/Security/TokenAuthenticator.php index 162b49e..d2d5ed1 100644 --- a/src/Security/TokenAuthenticator.php +++ b/src/Security/TokenAuthenticator.php @@ -58,7 +58,8 @@ public function __construct(string $openplatformId, string $openplatformSecret, $this->clientId = $openplatformId; $this->clientSecret = $openplatformSecret; $this->endPoint = $openplatformIntrospectionUrl; - $this->allowedClients = empty($openplatformAllowedClients) ? [] : explode(',', $openplatformAllowedClients); + + $this->allowedClients = empty($openplatformAllowedClients) ? [] : array_map('trim', explode(',', $openplatformAllowedClients)); $this->client = $httpClient; $this->cache = $tokenCache; diff --git a/tests/TokenAuthenticatorTest.php b/tests/TokenAuthenticatorTest.php index 0a38437..fdf0597 100644 --- a/tests/TokenAuthenticatorTest.php +++ b/tests/TokenAuthenticatorTest.php @@ -287,14 +287,14 @@ public function testActiveUSerAllowed(): void */ public function testCachedTokensClientIsAllowed(): void { - $this->tokenAuthenticator = $this->getTokenAuthenticator('allowed-client-id'); + $this->tokenAuthenticator = $this->getTokenAuthenticator('allowed-client-id-1, allowed-client-id-2, allowed-client-id-3'); $this->cache->method('getItem')->willReturn($this->item); $this->cache->expects($this->once())->method('getItem')->with('12345678'); $this->item->method('isHit')->willReturn(true); $user = new User(); - $user->setClientId('allowed-client-id'); + $user->setClientId('allowed-client-id-2'); $expires = new \DateTime('now + 2 days', new \DateTimeZone('UTC')); $user->setExpires($expires); $this->item->method('get')->willReturn($user);