From 56cd76fb1fefb045665d5ccf0f192654e38cfb99 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 19 Dec 2023 11:27:43 +0100 Subject: [PATCH 1/5] fix: If apiKey is set people don't set custom headers Since our README explicitly states that using withProxy($apiKey) is in lieu of using custom headers for the Authorization header, our SDK should make sure to check if the proxyApiKey is set, and if so, use it to set the Authorization header also for client registration and client metrics. --- src/Client/DefaultRegistrationService.php | 4 ++++ src/Metrics/DefaultMetricsSender.php | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Client/DefaultRegistrationService.php b/src/Client/DefaultRegistrationService.php index da5e2213..33b4c7b4 100755 --- a/src/Client/DefaultRegistrationService.php +++ b/src/Client/DefaultRegistrationService.php @@ -57,6 +57,10 @@ public function register(iterable $strategyHandlers): bool 'started' => (new DateTimeImmutable())->format('c'), 'interval' => $this->configuration->getMetricsInterval(), ], JSON_THROW_ON_ERROR))); + + if ($this->configuration->getProxyKey() !== null) { + $request = $request->withHeader('Authorization', $this->configuration->getProxyKey()); + } foreach ($this->configuration->getHeaders() as $name => $value) { $request = $request->withHeader($name, $value); } diff --git a/src/Metrics/DefaultMetricsSender.php b/src/Metrics/DefaultMetricsSender.php index df51a221..0beac4ac 100755 --- a/src/Metrics/DefaultMetricsSender.php +++ b/src/Metrics/DefaultMetricsSender.php @@ -31,12 +31,15 @@ public function sendMetrics(MetricsBucket $bucket): void 'instanceId' => $this->configuration->getInstanceId(), 'bucket' => $bucket->jsonSerialize(), ], JSON_THROW_ON_ERROR))); + if ($this->configuration->getProxyKey() !== null) { + $request = $request->withHeader('Authorization', $this->configuration->getProxyKey()); + } foreach ($this->configuration->getHeaders() as $name => $value) { $request = $request->withHeader($name, $value); } try { - $this->httpClient->sendRequest($request); + $response = $this->httpClient->sendRequest($request); } catch (ClientExceptionInterface) { // ignore the error } From 7021e41aaf91a4027e6f19b76da9cda99641bb2b Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 19 Dec 2023 11:43:48 +0100 Subject: [PATCH 2/5] chore: Added tests proving that withProxy does indeed send authorization headers --- .../Client/DefaultRegistrationServiceTest.php | 19 +++++++++++++++++++ tests/Metrics/DefaultMetricsSenderTest.php | 11 +++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/Client/DefaultRegistrationServiceTest.php b/tests/Client/DefaultRegistrationServiceTest.php index 4fba1aa6..b75b9ac6 100755 --- a/tests/Client/DefaultRegistrationServiceTest.php +++ b/tests/Client/DefaultRegistrationServiceTest.php @@ -110,4 +110,23 @@ public function testRegistrationException() $this->pushResponse(new RuntimeException("This exception shouldn't be propagated"), 1, 404); $instance->register([new DefaultStrategyHandler()]); } + + public function testAuthorizationKeyFromProxyKey() { + $configuration = (new UnleashConfiguration('', '', '')) + ->setProxyKey('apiKey') + ->setCache($this->getCache()) + ->setStaleCache($this->getFreshCacheInstance()) + ->setStaleTtl(30) + ->setTtl(0); + + $instance = new DefaultRegistrationService( + $this->httpClient, + new HttpFactory(), + $configuration + ); + $this->pushResponse([], 2, 202); + self::assertTrue($instance->register([])); + $request = $this->requestHistory[0]['request']; + self::assertEquals('apiKey', $request->getHeaderLine('Authorization')); + } } diff --git a/tests/Metrics/DefaultMetricsSenderTest.php b/tests/Metrics/DefaultMetricsSenderTest.php index baa4daa4..f576923e 100755 --- a/tests/Metrics/DefaultMetricsSenderTest.php +++ b/tests/Metrics/DefaultMetricsSenderTest.php @@ -68,4 +68,15 @@ public function testSendMetricsFailure() $bucket = new MetricsBucket(new DateTimeImmutable(), new DateTimeImmutable()); $this->instance->sendMetrics($bucket); } + + public function testSendMetricsWithProxyKey() + { + $this->configuration->setProxyKey('apiKey'); + $bucket = new MetricsBucket(new DateTimeImmutable(), new DateTimeImmutable()); + $this->pushResponse([]); + $this->instance->sendMetrics($bucket); + /** @var RequestInterface $request */ + $request = $this->requestHistory[0]['request']; + self::assertEquals('apiKey', $request->getHeaderLine('Authorization')); + } } From b54d009f2bdce927bcc5717b6cc04a791d7419f5 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 19 Dec 2023 13:03:37 +0100 Subject: [PATCH 3/5] take 2. Now adds proxy key to headers list --- src/Client/DefaultRegistrationService.php | 3 --- src/Metrics/DefaultMetricsSender.php | 5 +---- src/UnleashBuilder.php | 1 + .../Client/DefaultRegistrationServiceTest.php | 19 ------------------ tests/Metrics/DefaultMetricsSenderTest.php | 11 ---------- tests/UnleashBuilderTest.php | 20 +++++++++++++++++++ 6 files changed, 22 insertions(+), 37 deletions(-) diff --git a/src/Client/DefaultRegistrationService.php b/src/Client/DefaultRegistrationService.php index 33b4c7b4..81d19344 100755 --- a/src/Client/DefaultRegistrationService.php +++ b/src/Client/DefaultRegistrationService.php @@ -58,9 +58,6 @@ public function register(iterable $strategyHandlers): bool 'interval' => $this->configuration->getMetricsInterval(), ], JSON_THROW_ON_ERROR))); - if ($this->configuration->getProxyKey() !== null) { - $request = $request->withHeader('Authorization', $this->configuration->getProxyKey()); - } foreach ($this->configuration->getHeaders() as $name => $value) { $request = $request->withHeader($name, $value); } diff --git a/src/Metrics/DefaultMetricsSender.php b/src/Metrics/DefaultMetricsSender.php index 0beac4ac..df51a221 100755 --- a/src/Metrics/DefaultMetricsSender.php +++ b/src/Metrics/DefaultMetricsSender.php @@ -31,15 +31,12 @@ public function sendMetrics(MetricsBucket $bucket): void 'instanceId' => $this->configuration->getInstanceId(), 'bucket' => $bucket->jsonSerialize(), ], JSON_THROW_ON_ERROR))); - if ($this->configuration->getProxyKey() !== null) { - $request = $request->withHeader('Authorization', $this->configuration->getProxyKey()); - } foreach ($this->configuration->getHeaders() as $name => $value) { $request = $request->withHeader($name, $value); } try { - $response = $this->httpClient->sendRequest($request); + $this->httpClient->sendRequest($request); } catch (ClientExceptionInterface) { // ignore the error } diff --git a/src/UnleashBuilder.php b/src/UnleashBuilder.php index d2b02f87..6f0cac3c 100755 --- a/src/UnleashBuilder.php +++ b/src/UnleashBuilder.php @@ -529,6 +529,7 @@ public function build(): Unleash if ($this->proxyKey !== null) { $configuration->setProxyKey($this->proxyKey); + $configuration->setHeaders(array_merge($this->headers, ['Authorization' => $this->proxyKey])); $proxyRepository = new DefaultUnleashProxyRepository( $configuration, $httpClient, diff --git a/tests/Client/DefaultRegistrationServiceTest.php b/tests/Client/DefaultRegistrationServiceTest.php index b75b9ac6..4fba1aa6 100755 --- a/tests/Client/DefaultRegistrationServiceTest.php +++ b/tests/Client/DefaultRegistrationServiceTest.php @@ -110,23 +110,4 @@ public function testRegistrationException() $this->pushResponse(new RuntimeException("This exception shouldn't be propagated"), 1, 404); $instance->register([new DefaultStrategyHandler()]); } - - public function testAuthorizationKeyFromProxyKey() { - $configuration = (new UnleashConfiguration('', '', '')) - ->setProxyKey('apiKey') - ->setCache($this->getCache()) - ->setStaleCache($this->getFreshCacheInstance()) - ->setStaleTtl(30) - ->setTtl(0); - - $instance = new DefaultRegistrationService( - $this->httpClient, - new HttpFactory(), - $configuration - ); - $this->pushResponse([], 2, 202); - self::assertTrue($instance->register([])); - $request = $this->requestHistory[0]['request']; - self::assertEquals('apiKey', $request->getHeaderLine('Authorization')); - } } diff --git a/tests/Metrics/DefaultMetricsSenderTest.php b/tests/Metrics/DefaultMetricsSenderTest.php index f576923e..baa4daa4 100755 --- a/tests/Metrics/DefaultMetricsSenderTest.php +++ b/tests/Metrics/DefaultMetricsSenderTest.php @@ -68,15 +68,4 @@ public function testSendMetricsFailure() $bucket = new MetricsBucket(new DateTimeImmutable(), new DateTimeImmutable()); $this->instance->sendMetrics($bucket); } - - public function testSendMetricsWithProxyKey() - { - $this->configuration->setProxyKey('apiKey'); - $bucket = new MetricsBucket(new DateTimeImmutable(), new DateTimeImmutable()); - $this->pushResponse([]); - $this->instance->sendMetrics($bucket); - /** @var RequestInterface $request */ - $request = $this->requestHistory[0]['request']; - self::assertEquals('apiKey', $request->getHeaderLine('Authorization')); - } } diff --git a/tests/UnleashBuilderTest.php b/tests/UnleashBuilderTest.php index fa5956a0..2b9482cd 100755 --- a/tests/UnleashBuilderTest.php +++ b/tests/UnleashBuilderTest.php @@ -883,6 +883,26 @@ public function testBuilderWithProxyKeyYieldsProxyUnleash() self::assertInstanceOf(DefaultProxyUnleash::class, $base->build()); } + public function testbuilderWithProxyKeyYieldsAuthorizationHeader() { + $base = $this->instance->withProxy('proxy-key')->withAppUrl('https://localhost')->withInstanceId('test')->withAppName('test'); + $unleash = $base->build(); + $reflection = new ReflectionObject($unleash); + $repositoryProperty = $reflection->getProperty('repository'); + $repositoryProperty->setAccessible(true); + $repository = $repositoryProperty->getValue($unleash); + + $reflection = new ReflectionObject($repository); + $configurationProperty = $reflection->getProperty('configuration'); + $configurationProperty->setAccessible(true); + $configuration = $configurationProperty->getValue($repository); + + $reflection = new ReflectionObject($configuration); + $headersPropertyBuilt = $reflection->getProperty('headers'); + $headersPropertyBuilt->setAccessible(true); + $headersBuilt = $headersPropertyBuilt->getValue($configuration); + self::assertArrayHasKey('Authorization', $headersBuilt); + } + private function getConfiguration(DefaultUnleash $unleash): UnleashConfiguration { $configurationProperty = (new ReflectionObject($unleash))->getProperty('configuration'); From ddceba2d985f9959554ed2456b8921a092a769c6 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 19 Dec 2023 13:05:11 +0100 Subject: [PATCH 4/5] chore: removed added space --- src/Client/DefaultRegistrationService.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Client/DefaultRegistrationService.php b/src/Client/DefaultRegistrationService.php index 81d19344..da5e2213 100755 --- a/src/Client/DefaultRegistrationService.php +++ b/src/Client/DefaultRegistrationService.php @@ -57,7 +57,6 @@ public function register(iterable $strategyHandlers): bool 'started' => (new DateTimeImmutable())->format('c'), 'interval' => $this->configuration->getMetricsInterval(), ], JSON_THROW_ON_ERROR))); - foreach ($this->configuration->getHeaders() as $name => $value) { $request = $request->withHeader($name, $value); } From 532e5c91cb814d51800397eb768d6882155acc26 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 19 Dec 2023 13:06:14 +0100 Subject: [PATCH 5/5] chore: php fixer --- tests/UnleashBuilderTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/UnleashBuilderTest.php b/tests/UnleashBuilderTest.php index 2b9482cd..2668c456 100755 --- a/tests/UnleashBuilderTest.php +++ b/tests/UnleashBuilderTest.php @@ -883,7 +883,8 @@ public function testBuilderWithProxyKeyYieldsProxyUnleash() self::assertInstanceOf(DefaultProxyUnleash::class, $base->build()); } - public function testbuilderWithProxyKeyYieldsAuthorizationHeader() { + public function testbuilderWithProxyKeyYieldsAuthorizationHeader() + { $base = $this->instance->withProxy('proxy-key')->withAppUrl('https://localhost')->withInstanceId('test')->withAppName('test'); $unleash = $base->build(); $reflection = new ReflectionObject($unleash);