From f3541e1c81de7a935a0a127571dbc8204e67e426 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Wed, 31 Jan 2024 20:38:51 +0100 Subject: [PATCH] fix requests for container with "tokens" name in it --- composer.json | 1 + src/Common/Api/Operation.php | 14 +++++++++++- src/Common/Api/OperatorTrait.php | 17 +++++++------- src/Common/Auth/AuthHandler.php | 11 +--------- src/Common/Service/Builder.php | 6 ++--- src/Identity/v2/Api.php | 13 ++++++----- src/Identity/v3/Api.php | 7 +++--- src/Identity/v3/Models/Token.php | 22 +++++++++---------- tests/sample/ObjectStore/v1/ContainerTest.php | 9 ++++++++ tests/unit/Common/Auth/AuthHandlerTest.php | 2 +- tests/unit/Identity/v2/ServiceTest.php | 4 ++-- tests/unit/Identity/v3/ServiceTest.php | 8 +++---- tests/unit/TestCase.php | 6 ++++- 13 files changed, 70 insertions(+), 50 deletions(-) diff --git a/composer.json b/composer.json index d0e78e252..4ef57fb00 100644 --- a/composer.json +++ b/composer.json @@ -34,6 +34,7 @@ "justinrainbow/json-schema": "^5.2" }, "require-dev": { + "ext-json": "*", "ergebnis/composer-normalize": "^2.0", "friendsofphp/php-cs-fixer": "^3", "php-coveralls/php-coveralls": "^2.0", diff --git a/src/Common/Api/Operation.php b/src/Common/Api/Operation.php index d42cb0905..79ea84478 100644 --- a/src/Common/Api/Operation.php +++ b/src/Common/Api/Operation.php @@ -27,6 +27,9 @@ class Operation /** @var []Parameter The parameters of this operation */ private $params; + /** @var bool Whether this operation should skip authentication */ + private $skipAuth; + /** * @param array $definition The data definition (in array form) that will populate this * operation. Usually this is retrieved from an {@see ApiInterface} @@ -42,6 +45,7 @@ public function __construct(array $definition) } $this->params = self::toParamArray($definition['params']); + $this->skipAuth = $definition['skipAuth'] ?? false; } public function getPath(): string @@ -54,10 +58,18 @@ public function getMethod(): string return $this->method; } + /** + * Indicates if operation must be run without authentication. This is useful for getting authentication tokens. + */ + public function getSkipAuth(): bool + { + return $this->skipAuth; + } + /** * Indicates whether this operation supports a parameter. * - * @param $key The name of a parameter + * @param string $key The name of a parameter */ public function hasParam(string $key): bool { diff --git a/src/Common/Api/OperatorTrait.php b/src/Common/Api/OperatorTrait.php index 7eed4e1bb..021111e7d 100644 --- a/src/Common/Api/OperatorTrait.php +++ b/src/Common/Api/OperatorTrait.php @@ -23,7 +23,7 @@ trait OperatorTrait public function __construct(ClientInterface $client, ApiInterface $api) { $this->client = $client; - $this->api = $api; + $this->api = $api; } /** @@ -55,8 +55,8 @@ public function __debugInfo() * {@see Promise} object. In order for this to happen, the called methods need to be in the * following format: `createAsync`, where `create` is the sequential method being wrapped. * - * @param $methodName the name of the method being invoked - * @param $args the arguments to be passed to the sequential method + * @param string $methodName the name of the method being invoked + * @param array $args the arguments to be passed to the sequential method * * @return Promise * @@ -92,22 +92,23 @@ public function getOperation(array $definition): Operation return new Operation($definition); } - /** - * @throws \Exception - */ protected function sendRequest(Operation $operation, array $userValues = [], bool $async = false) { $operation->validate($userValues); $options = (new RequestSerializer())->serializeOptions($operation, $userValues); - $method = $async ? 'requestAsync' : 'request'; + $method = $async ? 'requestAsync' : 'request'; - $uri = Utils::uri_template($operation->getPath(), $userValues); + $uri = Utils::uri_template($operation->getPath(), $userValues); if (array_key_exists('requestOptions', $userValues)) { $options += $userValues['requestOptions']; } + if ($operation->getSkipAuth()) { + $options['openstack.skip_auth'] = true; + } + return $this->client->$method($operation->getMethod(), $uri, $options); } diff --git a/src/Common/Auth/AuthHandler.php b/src/Common/Auth/AuthHandler.php index de396cc0e..28df0d1c8 100644 --- a/src/Common/Auth/AuthHandler.php +++ b/src/Common/Auth/AuthHandler.php @@ -43,7 +43,7 @@ public function __invoke(RequestInterface $request, array $options) { $fn = $this->nextHandler; - if ($this->shouldIgnore($request)) { + if (isset($options['openstack.skip_auth']) && $options['openstack.skip_auth']) { return $fn($request, $options); } @@ -55,13 +55,4 @@ public function __invoke(RequestInterface $request, array $options) return $fn(Utils::modifyRequest($request, $modify), $options); } - - /** - * Internal method which prevents infinite recursion. For certain requests, like the initial - * auth call itself, we do NOT want to send a token. - */ - private function shouldIgnore(RequestInterface $request): bool - { - return false !== strpos((string) $request->getUri(), 'tokens') && 'POST' == $request->getMethod(); - } } diff --git a/src/Common/Service/Builder.php b/src/Common/Service/Builder.php index 55fb92bf7..e9cb25598 100644 --- a/src/Common/Service/Builder.php +++ b/src/Common/Service/Builder.php @@ -83,7 +83,7 @@ public function createService(string $namespace, array $serviceOptions = []): Se return new $serviceClass($options['httpClient'], new $apiClass()); } - private function stockHttpClient(array &$options, string $serviceName) + private function stockHttpClient(array &$options, string $serviceName): void { if (!isset($options['httpClient']) || !($options['httpClient'] instanceof ClientInterface)) { if (false !== stripos($serviceName, 'identity')) { @@ -105,7 +105,7 @@ private function stockHttpClient(array &$options, string $serviceName) /** * @codeCoverageIgnore */ - private function addDebugMiddleware(array $options, HandlerStack &$stack) + private function addDebugMiddleware(array $options, HandlerStack &$stack): void { if (!empty($options['debugLog']) && !empty($options['logger']) @@ -118,7 +118,7 @@ private function addDebugMiddleware(array $options, HandlerStack &$stack) /** * @codeCoverageIgnore */ - private function stockAuthHandler(array &$options) + private function stockAuthHandler(array &$options): void { if (!isset($options['authHandler'])) { $options['authHandler'] = function () use ($options) { diff --git a/src/Identity/v2/Api.php b/src/Identity/v2/Api.php index 5024b3597..f22c970dd 100644 --- a/src/Identity/v2/Api.php +++ b/src/Identity/v2/Api.php @@ -14,20 +14,21 @@ class Api implements ApiInterface public function postToken(): array { return [ - 'method' => 'POST', - 'path' => 'tokens', - 'params' => [ - 'username' => [ + 'method' => 'POST', + 'path' => 'tokens', + 'skipAuth' => true, + 'params' => [ + 'username' => [ 'type' => 'string', 'required' => true, 'path' => 'auth.passwordCredentials', ], - 'password' => [ + 'password' => [ 'type' => 'string', 'required' => true, 'path' => 'auth.passwordCredentials', ], - 'tenantId' => [ + 'tenantId' => [ 'type' => 'string', 'path' => 'auth', ], diff --git a/src/Identity/v3/Api.php b/src/Identity/v3/Api.php index 8f07da58d..418684c91 100644 --- a/src/Identity/v3/Api.php +++ b/src/Identity/v3/Api.php @@ -16,9 +16,10 @@ public function __construct() public function postTokens(): array { return [ - 'method' => 'POST', - 'path' => 'auth/tokens', - 'params' => [ + 'method' => 'POST', + 'path' => 'auth/tokens', + 'skipAuth' => true, + 'params' => [ 'methods' => $this->params->methods(), 'user' => $this->params->user(), 'application_credential' => $this->params->applicationCredential(), diff --git a/src/Identity/v3/Models/Token.php b/src/Identity/v3/Models/Token.php index 7a7c23f2e..d4054d7c2 100644 --- a/src/Identity/v3/Models/Token.php +++ b/src/Identity/v3/Models/Token.php @@ -89,27 +89,27 @@ public function retrieve() } /** - * @param array $data {@see \OpenStack\Identity\v3\Api::postTokens} + * @param array $userOptions {@see \OpenStack\Identity\v3\Api::postTokens} */ - public function create(array $data): Creatable + public function create(array $userOptions): Creatable { - if (isset($data['user'])) { - $data['methods'] = ['password']; - if (!isset($data['user']['id']) && empty($data['user']['domain'])) { + if (isset($userOptions['user'])) { + $userOptions['methods'] = ['password']; + if (!isset($userOptions['user']['id']) && empty($userOptions['user']['domain'])) { throw new InvalidArgumentException('When authenticating with a username, you must also provide either the domain name '.'or domain ID to which the user belongs to. Alternatively, if you provide a user ID instead, '.'you do not need to provide domain information.'); } - } elseif (isset($data['application_credential'])) { - $data['methods'] = ['application_credential']; - if (!isset($data['application_credential']['id']) || !isset($data['application_credential']['secret'])) { + } elseif (isset($userOptions['application_credential'])) { + $userOptions['methods'] = ['application_credential']; + if (!isset($userOptions['application_credential']['id']) || !isset($userOptions['application_credential']['secret'])) { throw new InvalidArgumentException('When authenticating with a application_credential, you must provide application credential ID '.' and application credential secret.'); } - } elseif (isset($data['tokenId'])) { - $data['methods'] = ['token']; + } elseif (isset($userOptions['tokenId'])) { + $userOptions['methods'] = ['token']; } else { throw new InvalidArgumentException('Either a user, tokenId or application_credential must be provided.'); } - $response = $this->execute($this->api->postTokens(), $data); + $response = $this->execute($this->api->postTokens(), $userOptions); $token = $this->populateFromResponse($response); // Cache response as an array to export if needed. diff --git a/tests/sample/ObjectStore/v1/ContainerTest.php b/tests/sample/ObjectStore/v1/ContainerTest.php index a24999db1..af5dee376 100644 --- a/tests/sample/ObjectStore/v1/ContainerTest.php +++ b/tests/sample/ObjectStore/v1/ContainerTest.php @@ -132,4 +132,13 @@ public function testDelete(Container $createdContainer) $this->expectException(BadResponseError::class); $createdContainer->retrieve(); } + + public function testTokensContainer() + { + $container = $this->getService()->createContainer(['name' => $this->randomStr() . '_tokens']); + $this->assertNotNull($container->name); + + // this would send POST request with 'tokens' in the URL + $container->resetMetadata([]); + } } \ No newline at end of file diff --git a/tests/unit/Common/Auth/AuthHandlerTest.php b/tests/unit/Common/Auth/AuthHandlerTest.php index 6a535489c..2ada179ae 100644 --- a/tests/unit/Common/Auth/AuthHandlerTest.php +++ b/tests/unit/Common/Auth/AuthHandlerTest.php @@ -35,7 +35,7 @@ public function test_it_should_bypass_auth_http_requests() // Fake a Keystone request $request = new Request('POST', 'https://my-openstack.org:5000/v2.0/tokens'); - self::assertEquals($request, call_user_func_array($this->handler, [$request, []])); + self::assertEquals($request, call_user_func_array($this->handler, [$request, ['openstack.skip_auth' => true]])); } public function test_it_should_generate_a_new_token_if_the_current_token_is_either_expired_or_not_set() diff --git a/tests/unit/Identity/v2/ServiceTest.php b/tests/unit/Identity/v2/ServiceTest.php index 766cc7b95..101370b3c 100644 --- a/tests/unit/Identity/v2/ServiceTest.php +++ b/tests/unit/Identity/v2/ServiceTest.php @@ -40,7 +40,7 @@ public function test_it_authenticates() 'tenantId' => $options['tenantId'], ]]; - $this->setupMock('POST', 'tokens', $expectedJson, [], 'token-post'); + $this->setupMock('POST', 'tokens', $expectedJson, [], 'token-post', true); list($token, $baseUrl) = $this->service->authenticate($options); @@ -64,7 +64,7 @@ public function test_it_generates_tokens() 'tenantId' => $options['tenantId'], ]]; - $this->setupMock('POST', 'tokens', $expectedJson, [], 'token-post'); + $this->setupMock('POST', 'tokens', $expectedJson, [], 'token-post', true); self::assertInstanceOf(Token::class, $this->service->generateToken($options)); } diff --git a/tests/unit/Identity/v3/ServiceTest.php b/tests/unit/Identity/v3/ServiceTest.php index da39e6719..0067a7e44 100644 --- a/tests/unit/Identity/v3/ServiceTest.php +++ b/tests/unit/Identity/v3/ServiceTest.php @@ -58,7 +58,7 @@ public function test_it_authenticates() ] ]; - $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token'); + $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token', true); list($token, $url) = $this->service->authenticate($userOptions); @@ -231,7 +231,7 @@ public function test_it_throws_exception_if_no_endpoint_found() ] ]; - $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token'); + $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token', true); $this->expectException(\RuntimeException::class); $this->service->authenticate([ @@ -629,7 +629,7 @@ public function test_it_generates_tokens_with_user_creds() ] ]; - $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token'); + $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token', true); $token = $this->service->generateToken($userOptions); self::assertInstanceOf(Models\Token::class, $token); @@ -654,7 +654,7 @@ public function test_it_generates_token_with_token_id() ] ]; - $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token'); + $this->setupMock('POST', 'auth/tokens', ['auth' => $expectedJson], [], 'token', true); $token = $this->service->generateToken($userOptions); self::assertInstanceOf(Models\Token::class, $token); diff --git a/tests/unit/TestCase.php b/tests/unit/TestCase.php index d1752fe13..7ba925bf9 100644 --- a/tests/unit/TestCase.php +++ b/tests/unit/TestCase.php @@ -42,7 +42,7 @@ protected function getFixture($file) return Message::parseResponse(file_get_contents($path)); } - protected function setupMock($method, $path, $body = null, array $headers = [], $response = null) + protected function setupMock($method, $path, $body = null, array $headers = [], $response = null, $skipAuth = false) { $options = ['headers' => $headers]; @@ -54,6 +54,10 @@ protected function setupMock($method, $path, $body = null, array $headers = [], $response = $this->getFixture($response); } + if ($skipAuth) { + $options['openstack.skip_auth'] = true; + } + $this->client ->request($method, $path, $options) ->shouldBeCalled()