Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix container requests with "tokens" in its name #396

Merged
merged 8 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/Common/Api/Operation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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
Expand All @@ -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
{
Expand Down
11 changes: 5 additions & 6 deletions src/Common/Api/OperatorTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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);
}

Expand Down
7 changes: 6 additions & 1 deletion src/Common/Auth/AuthHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
6 changes: 3 additions & 3 deletions src/Common/Service/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Expand All @@ -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'])
Expand All @@ -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) {
Expand Down
13 changes: 7 additions & 6 deletions src/Identity/v2/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],
Expand Down
7 changes: 4 additions & 3 deletions src/Identity/v3/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
22 changes: 11 additions & 11 deletions src/Identity/v3/Models/Token.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion tests/sample/Compute/v2/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ protected function createServer(): Server
]
);

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

return $server;
}
Expand Down
9 changes: 9 additions & 0 deletions tests/sample/ObjectStore/v1/ContainerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
}
}
31 changes: 16 additions & 15 deletions tests/unit/Common/Api/OperatorTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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();
}

Expand All @@ -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
{
}
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/Common/Auth/AuthHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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, []]));
}

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/Identity/v2/ServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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));
}
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/Identity/v3/ServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ protected function getFixture($file)
return Message::parseResponse(file_get_contents($path));
}


/**
* Mocks request
*
Expand All @@ -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;
Expand Down
Loading