fix requests for container with "tokens" name in it

php-opencloud · Jan 31, 2024 · f3541e1 · f3541e1
1 parent 9f817df
commit f3541e1
Show file tree

Hide file tree

Showing 13 changed files with 70 additions and 50 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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([]);
+    }
 }
diff --git 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
@@ -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
@@ -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
@@ -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()