Fix Swift container requests with "tokens" in its name (#396)

php-opencloud · Feb 1, 2024 · c32ab23 · c32ab23
1 parent ba44841
commit c32ab23
Show file tree

Hide file tree

Showing 14 changed files with 98 additions and 56 deletions.
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}
@@ -41,7 +44,8 @@ public function __construct(array $definition)
             $this->jsonKey = $definition['jsonKey'];
         }
 
-        $this->params = self::toParamArray($definition['params']);
+        $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
@@ -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,21 @@ 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';
 
-        $uri     = Utils::uri_template($operation->getPath(), $userValues);
+        $uri = Utils::uri_template($operation->getPath(), $userValues);
 
         if (array_key_exists('requestOptions', $userValues)) {
             $options += $userValues['requestOptions'];
         }
 
+        $options['openstack.skip_auth'] = $operation->getSkipAuth();
+
         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,12 @@ public function __invoke(RequestInterface $request, array $options)
     {
         $fn = $this->nextHandler;
 
-        if ($this->shouldIgnore($request)) {
+        if (!isset($options['openstack.skip_auth'])) {
+            // Deprecated. Left for backward compatibility only.
+            if ($this->shouldIgnore($request)) {
+                return $fn($request, $options);
+            }
+        } elseif ($options['openstack.skip_auth']) {
             return $fn($request, $options);
         }
 

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/Compute/v2/TestCase.php b/tests/sample/Compute/v2/TestCase.php
@@ -68,7 +68,8 @@ protected function createServer(): Server
             ]
         );
 
-        $server->waitUntilActive(60);
+        $server->waitUntilActive(120);
+        $this->assertEquals('ACTIVE', $server->status);
 
         return $server;
     }

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/Api/OperatorTraitTest.php b/tests/unit/Common/Api/OperatorTraitTest.php
@@ -38,26 +38,25 @@ public function test_it_returns_operations()
     {
         self::assertInstanceOf(
             Operation::class,
-            $this->operator->getOperation($this->def, [])
+            $this->operator->getOperation($this->def)
         );
     }
 
     public function test_it_sends_a_request_when_operations_are_executed()
     {
-        $this->client->request('GET', 'test', ['headers' => []])->willReturn(new Response());
+        $this->mockRequest('GET', 'test', new Response());
 
-        $this->operator->execute($this->def, []);
-
-        $this->addToAssertionCount(1);
+        $this->operator->execute($this->def);
     }
 
     public function test_it_sends_a_request_when_async_operations_are_executed()
     {
-        $this->client->requestAsync('GET', 'test', ['headers' => []])->willReturn(new Promise());
-
-        $this->operator->executeAsync($this->def, []);
+        $this->client
+            ->requestAsync('GET', 'test', ['headers' => [], 'openstack.skip_auth' => false])
+            ->shouldBeCalled()
+            ->willReturn(new Promise());
 
-        $this->addToAssertionCount(1);
+        $this->operator->executeAsync($this->def);
     }
 
     public function test_it_wraps_sequential_ops_in_promise_when_async_is_appended_to_method_name()
@@ -75,13 +74,13 @@ public function test_it_wraps_sequential_ops_in_promise_when_async_is_appended_t
 
     public function test_it_throws_exception_when_async_is_called_on_a_non_existent_method()
     {
-		$this->expectException(\RuntimeException::class);
+        $this->expectException(\RuntimeException::class);
         $this->operator->fooAsync();
     }
 
     public function test_undefined_methods_result_in_error()
     {
-		$this->expectException(\Exception::class);
+        $this->expectException(\Exception::class);
         $this->operator->foo();
     }
 
@@ -103,16 +102,18 @@ public function test_it_populates_models_from_arrays()
 
     public function test_guzzle_options_are_forwarded()
     {
-        $this->client->request('GET', 'test', ['headers' => [], 'stream' => true])->willReturn(new Response());
+        $this->client
+            ->request('GET', 'test', ['headers' => [], 'openstack.skip_auth' => false, 'stream' => true])
+            ->shouldBeCalled()
+            ->willReturn(new Response());
 
         $this->operator->execute($this->def, [
-            'requestOptions' => ['stream' => true]
+            'requestOptions' => ['stream' => true],
         ]);
-
-        $this->addToAssertionCount(1);
     }
 }
 
+
 class TestResource extends AbstractResource
 {
 }

diff --git a/tests/unit/Common/Auth/AuthHandlerTest.php b/tests/unit/Common/Auth/AuthHandlerTest.php
@@ -35,6 +35,14 @@ 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, ['openstack.skip_auth' => true]]));
+    }
+
+    public function test_it_should_bypass_auth_http_requests_backward_compatibility()
+    {
+        // 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, []]));
     }
 

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->mockRequest('POST', 'tokens', 'token-post', $expectedJson, []);
+        $this->mockRequest('POST', 'tokens', 'token-post', $expectedJson, [], true);
 
         [$token, $baseUrl] = $this->service->authenticate($options);
 
@@ -64,7 +64,7 @@ public function test_it_generates_tokens()
             'tenantId' => $options['tenantId'],
         ]];
 
-        $this->mockRequest('POST', 'tokens', 'token-post', $expectedJson, []);
+        $this->mockRequest('POST', 'tokens', 'token-post', $expectedJson, [], 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->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson]);
+        $this->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson], [], true);
 
         [$token, $url] = $this->service->authenticate($userOptions);
 
@@ -231,7 +231,7 @@ public function test_it_throws_exception_if_no_endpoint_found()
             ]
         ];
 
-        $this->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson]);
+        $this->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson], [], true);
 		$this->expectException(\RuntimeException::class);
 
         $this->service->authenticate([
@@ -626,7 +626,7 @@ public function test_it_generates_tokens_with_user_creds()
             ]
         ];
 
-        $this->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson]);
+        $this->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson], [], true);
 
         $token = $this->service->generateToken($userOptions);
         self::assertInstanceOf(Models\Token::class, $token);
@@ -651,7 +651,7 @@ public function test_it_generates_token_with_token_id()
             ]
         ];
 
-        $this->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson]);
+        $this->mockRequest('POST', 'auth/tokens', 'token', ['auth' => $expectedJson], [], true);
 
         $token = $this->service->generateToken($userOptions);
         self::assertInstanceOf(Models\Token::class, $token);

diff --git a/tests/unit/TestCase.php b/tests/unit/TestCase.php
@@ -43,6 +43,7 @@ protected function getFixture($file)
         return Message::parseResponse(file_get_contents($path));
     }
 
+
     /**
      * Mocks request
      *
@@ -51,12 +52,16 @@ protected function getFixture($file)
      * @param string|\GuzzleHttp\Psr7\Response|\Throwable $response the file name of the response fixture or a Response object
      * @param string|array|null $body request body. If type is array, it will be encoded as JSON.
      * @param array $headers request headers
+     * @param bool $skipAuth true if the api call skips authentication
      *
      * @throws \GuzzleHttp\Exception\GuzzleException
      */
-    protected function mockRequest(string $method, $uri, $response = null, $body = null, array $headers = []): MethodProphecy
+    protected function mockRequest(string $method, $uri, $response = null, $body = null, array $headers = [], $skipAuth = false): MethodProphecy
     {
-        $options = ['headers' => $headers];
+        $options = [
+            'headers'             => $headers,
+            'openstack.skip_auth' => $skipAuth,
+        ];
 
         if (!empty($body)) {
             $options[is_array($body) ? 'json' : 'body'] = $body;