Skip to content

Commit c32ab23

Browse files
k0kak0ka
authored
Fix Swift container requests with "tokens" in its name (#396)
1 parent ba44841 commit c32ab23

File tree

14 files changed

+98
-56
lines changed

14 files changed

+98
-56
lines changed

‎src/Common/Api/Operation.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ class Operation
2727
/** @var []Parameter The parameters of this operation */
2828
private $params;
2929

30+
/** @var bool Whether this operation should skip authentication */
31+
private $skipAuth;
32+
3033
/**
3134
* @param array $definition The data definition (in array form) that will populate this
3235
* operation. Usually this is retrieved from an {@see ApiInterface}
@@ -41,7 +44,8 @@ public function __construct(array $definition)
4144
$this->jsonKey = $definition['jsonKey'];
4245
}
4346

44-
$this->params = self::toParamArray($definition['params']);
47+
$this->params = self::toParamArray($definition['params']);
48+
$this->skipAuth = $definition['skipAuth'] ?? false;
4549
}
4650

4751
public function getPath(): string
@@ -54,10 +58,18 @@ public function getMethod(): string
5458
return $this->method;
5559
}
5660

61+
/**
62+
* Indicates if operation must be run without authentication. This is useful for getting authentication tokens.
63+
*/
64+
public function getSkipAuth(): bool
65+
{
66+
return $this->skipAuth;
67+
}
68+
5769
/**
5870
* Indicates whether this operation supports a parameter.
5971
*
60-
* @param $key The name of a parameter
72+
* @param string $key The name of a parameter
6173
*/
6274
public function hasParam(string $key): bool
6375
{

‎src/Common/Api/OperatorTrait.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ public function __debugInfo()
5555
* {@see Promise} object. In order for this to happen, the called methods need to be in the
5656
* following format: `createAsync`, where `create` is the sequential method being wrapped.
5757
*
58-
* @param $methodName the name of the method being invoked
59-
* @param $args the arguments to be passed to the sequential method
58+
* @param string $methodName the name of the method being invoked
59+
* @param array $args the arguments to be passed to the sequential method
6060
*
6161
* @return Promise
6262
*
@@ -92,22 +92,21 @@ public function getOperation(array $definition): Operation
9292
return new Operation($definition);
9393
}
9494

95-
/**
96-
* @throws \Exception
97-
*/
9895
protected function sendRequest(Operation $operation, array $userValues = [], bool $async = false)
9996
{
10097
$operation->validate($userValues);
10198

10299
$options = (new RequestSerializer())->serializeOptions($operation, $userValues);
103100
$method = $async ? 'requestAsync' : 'request';
104101

105-
$uri = Utils::uri_template($operation->getPath(), $userValues);
102+
$uri = Utils::uri_template($operation->getPath(), $userValues);
106103

107104
if (array_key_exists('requestOptions', $userValues)) {
108105
$options += $userValues['requestOptions'];
109106
}
110107

108+
$options['openstack.skip_auth'] = $operation->getSkipAuth();
109+
111110
return $this->client->$method($operation->getMethod(), $uri, $options);
112111
}
113112

‎src/Common/Auth/AuthHandler.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ public function __invoke(RequestInterface $request, array $options)
4343
{
4444
$fn = $this->nextHandler;
4545

46-
if ($this->shouldIgnore($request)) {
46+
if (!isset($options['openstack.skip_auth'])) {
47+
// Deprecated. Left for backward compatibility only.
48+
if ($this->shouldIgnore($request)) {
49+
return $fn($request, $options);
50+
}
51+
} elseif ($options['openstack.skip_auth']) {
4752
return $fn($request, $options);
4853
}
4954

‎src/Common/Service/Builder.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function createService(string $namespace, array $serviceOptions = []): Se
8383
return new $serviceClass($options['httpClient'], new $apiClass());
8484
}
8585

86-
private function stockHttpClient(array &$options, string $serviceName)
86+
private function stockHttpClient(array &$options, string $serviceName): void
8787
{
8888
if (!isset($options['httpClient']) || !($options['httpClient'] instanceof ClientInterface)) {
8989
if (false !== stripos($serviceName, 'identity')) {
@@ -105,7 +105,7 @@ private function stockHttpClient(array &$options, string $serviceName)
105105
/**
106106
* @codeCoverageIgnore
107107
*/
108-
private function addDebugMiddleware(array $options, HandlerStack &$stack)
108+
private function addDebugMiddleware(array $options, HandlerStack &$stack): void
109109
{
110110
if (!empty($options['debugLog'])
111111
&& !empty($options['logger'])
@@ -118,7 +118,7 @@ private function addDebugMiddleware(array $options, HandlerStack &$stack)
118118
/**
119119
* @codeCoverageIgnore
120120
*/
121-
private function stockAuthHandler(array &$options)
121+
private function stockAuthHandler(array &$options): void
122122
{
123123
if (!isset($options['authHandler'])) {
124124
$options['authHandler'] = function () use ($options) {

‎src/Identity/v2/Api.php

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,21 @@ class Api implements ApiInterface
1414
public function postToken(): array
1515
{
1616
return [
17-
'method' => 'POST',
18-
'path' => 'tokens',
19-
'params' => [
20-
'username' => [
17+
'method' => 'POST',
18+
'path' => 'tokens',
19+
'skipAuth' => true,
20+
'params' => [
21+
'username' => [
2122
'type' => 'string',
2223
'required' => true,
2324
'path' => 'auth.passwordCredentials',
2425
],
25-
'password' => [
26+
'password' => [
2627
'type' => 'string',
2728
'required' => true,
2829
'path' => 'auth.passwordCredentials',
2930
],
30-
'tenantId' => [
31+
'tenantId' => [
3132
'type' => 'string',
3233
'path' => 'auth',
3334
],

‎src/Identity/v3/Api.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ public function __construct()
1616
public function postTokens(): array
1717
{
1818
return [
19-
'method' => 'POST',
20-
'path' => 'auth/tokens',
21-
'params' => [
19+
'method' => 'POST',
20+
'path' => 'auth/tokens',
21+
'skipAuth' => true,
22+
'params' => [
2223
'methods' => $this->params->methods(),
2324
'user' => $this->params->user(),
2425
'application_credential' => $this->params->applicationCredential(),

‎src/Identity/v3/Models/Token.php

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,27 +89,27 @@ public function retrieve()
8989
}
9090

9191
/**
92-
* @param array $data {@see \OpenStack\Identity\v3\Api::postTokens}
92+
* @param array $userOptions {@see \OpenStack\Identity\v3\Api::postTokens}
9393
*/
94-
public function create(array $data): Creatable
94+
public function create(array $userOptions): Creatable
9595
{
96-
if (isset($data['user'])) {
97-
$data['methods'] = ['password'];
98-
if (!isset($data['user']['id']) && empty($data['user']['domain'])) {
96+
if (isset($userOptions['user'])) {
97+
$userOptions['methods'] = ['password'];
98+
if (!isset($userOptions['user']['id']) && empty($userOptions['user']['domain'])) {
9999
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.');
100100
}
101-
} elseif (isset($data['application_credential'])) {
102-
$data['methods'] = ['application_credential'];
103-
if (!isset($data['application_credential']['id']) || !isset($data['application_credential']['secret'])) {
101+
} elseif (isset($userOptions['application_credential'])) {
102+
$userOptions['methods'] = ['application_credential'];
103+
if (!isset($userOptions['application_credential']['id']) || !isset($userOptions['application_credential']['secret'])) {
104104
throw new InvalidArgumentException('When authenticating with a application_credential, you must provide application credential ID '.' and application credential secret.');
105105
}
106-
} elseif (isset($data['tokenId'])) {
107-
$data['methods'] = ['token'];
106+
} elseif (isset($userOptions['tokenId'])) {
107+
$userOptions['methods'] = ['token'];
108108
} else {
109109
throw new InvalidArgumentException('Either a user, tokenId or application_credential must be provided.');
110110
}
111111

112-
$response = $this->execute($this->api->postTokens(), $data);
112+
$response = $this->execute($this->api->postTokens(), $userOptions);
113113
$token = $this->populateFromResponse($response);
114114

115115
// Cache response as an array to export if needed.

‎tests/sample/Compute/v2/TestCase.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ protected function createServer(): Server
6868
]
6969
);
7070

71-
$server->waitUntilActive(60);
71+
$server->waitUntilActive(120);
72+
$this->assertEquals('ACTIVE', $server->status);
7273

7374
return $server;
7475
}

‎tests/sample/ObjectStore/v1/ContainerTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,13 @@ public function testDelete(Container $createdContainer)
132132
$this->expectException(BadResponseError::class);
133133
$createdContainer->retrieve();
134134
}
135+
136+
public function testTokensContainer()
137+
{
138+
$container = $this->getService()->createContainer(['name' => $this->randomStr() . '_tokens']);
139+
$this->assertNotNull($container->name);
140+
141+
// this would send POST request with 'tokens' in the URL
142+
$container->resetMetadata([]);
143+
}
135144
}

‎tests/unit/Common/Api/OperatorTraitTest.php

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,25 @@ public function test_it_returns_operations()
3838
{
3939
self::assertInstanceOf(
4040
Operation::class,
41-
$this->operator->getOperation($this->def, [])
41+
$this->operator->getOperation($this->def)
4242
);
4343
}
4444

4545
public function test_it_sends_a_request_when_operations_are_executed()
4646
{
47-
$this->client->request('GET', 'test', ['headers' => []])->willReturn(new Response());
47+
$this->mockRequest('GET', 'test', new Response());
4848

49-
$this->operator->execute($this->def, []);
50-
51-
$this->addToAssertionCount(1);
49+
$this->operator->execute($this->def);
5250
}
5351

5452
public function test_it_sends_a_request_when_async_operations_are_executed()
5553
{
56-
$this->client->requestAsync('GET', 'test', ['headers' => []])->willReturn(new Promise());
57-
58-
$this->operator->executeAsync($this->def, []);
54+
$this->client
55+
->requestAsync('GET', 'test', ['headers' => [], 'openstack.skip_auth' => false])
56+
->shouldBeCalled()
57+
->willReturn(new Promise());
5958

60-
$this->addToAssertionCount(1);
59+
$this->operator->executeAsync($this->def);
6160
}
6261

6362
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
7574

7675
public function test_it_throws_exception_when_async_is_called_on_a_non_existent_method()
7776
{
78-
$this->expectException(\RuntimeException::class);
77+
$this->expectException(\RuntimeException::class);
7978
$this->operator->fooAsync();
8079
}
8180

8281
public function test_undefined_methods_result_in_error()
8382
{
84-
$this->expectException(\Exception::class);
83+
$this->expectException(\Exception::class);
8584
$this->operator->foo();
8685
}
8786

@@ -103,16 +102,18 @@ public function test_it_populates_models_from_arrays()
103102

104103
public function test_guzzle_options_are_forwarded()
105104
{
106-
$this->client->request('GET', 'test', ['headers' => [], 'stream' => true])->willReturn(new Response());
105+
$this->client
106+
->request('GET', 'test', ['headers' => [], 'openstack.skip_auth' => false, 'stream' => true])
107+
->shouldBeCalled()
108+
->willReturn(new Response());
107109

108110
$this->operator->execute($this->def, [
109-
'requestOptions' => ['stream' => true]
111+
'requestOptions' => ['stream' => true],
110112
]);
111-
112-
$this->addToAssertionCount(1);
113113
}
114114
}
115115

116+
116117
class TestResource extends AbstractResource
117118
{
118119
}

0 commit comments

Comments
 (0)