Skip to content

Commit

Permalink
Merge pull request #39 from City-of-Helsinki/UHF-8377
Browse files Browse the repository at this point in the history
UHF-8377: Use Vault to store credentials
  • Loading branch information
tuutti authored Jun 16, 2023
2 parents b65a55c + 8d8bb1d commit 8776973
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 29 deletions.
8 changes: 7 additions & 1 deletion helfi_navigation.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ services:
- '@http_client'
- '@helfi_api_base.environment_resolver'
- '@logger.channel.helfi_navigation'
- '@config.factory'
- '@helfi_navigation.api_authorization'
helfi_navigation.menu_manager:
class: Drupal\helfi_navigation\MainMenuManager
arguments:
Expand All @@ -39,3 +39,9 @@ services:
- '@language_manager'
- '@cache_tags.invalidator'
- '@helfi_navigation.api_manager'

helfi_navigation.api_authorization:
class: Drupal\helfi_navigation\ApiAuthorization
arguments:
- '@config.factory'
- '@helfi_api_base.vault_manager'
49 changes: 49 additions & 0 deletions src/ApiAuthorization.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

declare(strict_types = 1);

namespace Drupal\helfi_navigation;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\helfi_api_base\Vault\VaultManager;

/**
* A BC layer to handle API authorization.
*/
final class ApiAuthorization {

public const VAULT_MANAGER_KEY = 'helfi_navigation';

/**
* Constructs a new instance.
*
* @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
* The configuration factory service.
* @param \Drupal\helfi_api_base\Vault\VaultManager $vaultManager
* The vault manager service.
*/
public function __construct(
private readonly ConfigFactoryInterface $configFactory,
private readonly VaultManager $vaultManager,
) {
}

/**
* Gets the authorization token.
*
* @return string|null
* The authorization token.
*/
public function getAuthorization() : ?string {
if ($authorization = $this->vaultManager->get(self::VAULT_MANAGER_KEY)) {
return $authorization->data();
}

// Provide a BC layer to fetch API keys from previously used
// configuration.
// @todo remove this once all projects have migrated to Vault.
return $this->configFactory->get('helfi_navigation.api')
?->get('key');
}

}
21 changes: 4 additions & 17 deletions src/ApiManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Drupal\Component\Datetime\TimeInterface;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Config\ConfigException;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\File\Exception\FileNotExistsException;
use Drupal\helfi_api_base\Cache\CacheKeyTrait;
use Drupal\helfi_api_base\Environment\EnvironmentResolverInterface;
Expand All @@ -30,13 +29,6 @@ class ApiManager {

use CacheKeyTrait;

/**
* The authorization token.
*
* @var null|string
*/
private ?string $authorization = NULL;

/**
* The previous exception.
*
Expand Down Expand Up @@ -64,16 +56,16 @@ class ApiManager {
* EnvironmentResolver helper class.
* @param \Psr\Log\LoggerInterface $logger
* Logger channel.
* @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
* The config factory.
* @param \Drupal\helfi_navigation\ApiAuthorization $apiAuthorization
* The API authorization service.
*/
public function __construct(
private readonly TimeInterface $time,
private readonly CacheBackendInterface $cache,
private readonly ClientInterface $httpClient,
private readonly EnvironmentResolverInterface $environmentResolver,
private readonly LoggerInterface $logger,
private readonly ConfigFactoryInterface $configFactory
private readonly ApiAuthorization $apiAuthorization
) {
}

Expand Down Expand Up @@ -272,12 +264,7 @@ public function hasAuthorization(): bool {
* The authorization token.
*/
public function getAuthorization() : ?string {
if (!$this->authorization) {
$this->authorization = $this->configFactory
->get('helfi_navigation.api')
->get('key');
}
return $this->authorization;
return $this->apiAuthorization->getAuthorization();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/ExternalMenuTreeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

use Drupal\Core\Template\Attribute;
use Drupal\Core\Url;
use Drupal\helfi_api_base\Link\InternalDomainResolver;
use Drupal\helfi_api_base\Link\UrlHelper;
use Drupal\helfi_navigation\Plugin\Menu\ExternalMenuLink;
use Drupal\helfi_api_base\Link\InternalDomainResolver;
use Symfony\Component\HttpFoundation\RequestStack;

/**
Expand Down
6 changes: 3 additions & 3 deletions src/Plugin/Block/ExternalMenuBlockBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@

use Drupal\Core\Cache\Cache;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\helfi_navigation\ExternalMenuBlockInterface;
use Drupal\helfi_navigation\ExternalMenuTreeBuilder;
use Drupal\helfi_api_base\Language\DefaultLanguageResolver;
use Drupal\helfi_navigation\ApiManager;
use Drupal\helfi_navigation\ApiResponse;
use Drupal\helfi_api_base\Language\DefaultLanguageResolver;
use Drupal\helfi_navigation\ExternalMenuBlockInterface;
use Drupal\helfi_navigation\ExternalMenuTreeBuilder;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Plugin/Block/MobileMenuFallbackBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
use Drupal\Core\Path\PathMatcherInterface;
use Drupal\Core\Template\Attribute;
use Drupal\Core\Url;
use Drupal\helfi_navigation\ApiManager;
use Drupal\helfi_api_base\Language\DefaultLanguageResolver;
use Drupal\helfi_navigation\ApiManager;
use Drupal\menu_link_content\MenuLinkContentInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

Expand Down
60 changes: 60 additions & 0 deletions tests/src/Unit/ApiAuthorizationTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

declare(strict_types = 1);

namespace Drupal\Tests\helfi_navigation\Unit;

use Drupal\helfi_api_base\Vault\AuthorizationToken;
use Drupal\helfi_api_base\Vault\VaultManager;
use Drupal\helfi_navigation\ApiAuthorization;
use Drupal\Tests\UnitTestCase;
use Prophecy\PhpUnit\ProphecyTrait;

/**
* @coversDefaultClass \Drupal\helfi_navigation\ApiAuthorization
* @group helfi_navigation
*/
class ApiAuthorizationTest extends UnitTestCase {

use ProphecyTrait;

/**
* @covers ::__construct
* @covers ::getAuthorization
*/
public function testVaultAuthorization() : void {
$vaultManager = new VaultManager([
new AuthorizationToken(ApiAuthorization::VAULT_MANAGER_KEY, '123'),
]);
$sut = new ApiAuthorization(
$this->getConfigFactoryStub([]),
$vaultManager,
);
$this->assertEquals('123', $sut->getAuthorization());
}

/**
* @covers ::__construct
* @covers ::getAuthorization
*/
public function testEmptyAuthorization() : void {
$sut = new ApiAuthorization(
$this->getConfigFactoryStub([]),
new VaultManager([]),
);
$this->assertNull($sut->getAuthorization());
}

/**
* @covers ::__construct
* @covers ::getAuthorization
*/
public function testFallbackConfigAuthorization() : void {
$sut = new ApiAuthorization(
$this->getConfigFactoryStub(['helfi_navigation.api' => ['key' => '123']]),
new VaultManager([]),
);
$this->assertEquals('123', $sut->getAuthorization());
}

}
36 changes: 30 additions & 6 deletions tests/src/Unit/ApiManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@
use Drupal\helfi_api_base\Environment\EnvironmentResolver;
use Drupal\helfi_api_base\Environment\EnvironmentResolverInterface;
use Drupal\helfi_api_base\Environment\Project;
use Drupal\helfi_api_base\Vault\AuthorizationToken;
use Drupal\helfi_api_base\Vault\VaultManager;
use Drupal\helfi_navigation\ApiAuthorization;
use Drupal\helfi_navigation\ApiManager;
use Drupal\helfi_navigation\CacheValue;
use Drupal\helfi_navigation\ApiResponse;
use Drupal\helfi_navigation\CacheValue;
use Drupal\Tests\helfi_api_base\Traits\ApiTestTrait;
use Drupal\Tests\UnitTestCase;
use GuzzleHttp\ClientInterface;
Expand Down Expand Up @@ -123,11 +126,12 @@ private function getSut(
$client,
$environmentResolver,
$logger,
$this->getConfigFactoryStub([
'helfi_navigation.api' => [
'key' => $apiKey,
],
]),
new ApiAuthorization(
$this->getConfigFactoryStub([]),
new VaultManager([
new AuthorizationToken(ApiAuthorization::VAULT_MANAGER_KEY, '123'),
])
),
);
}

Expand All @@ -142,6 +146,8 @@ private function getSut(
* @covers ::hasAuthorization
* @covers ::getAuthorization
* @covers \Drupal\helfi_navigation\ApiResponse::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testUpdateMainMenu() : void {
$requests = [];
Expand Down Expand Up @@ -173,6 +179,8 @@ public function testUpdateMainMenu() : void {
* @covers \Drupal\helfi_navigation\CacheValue::hasExpired
* @covers \Drupal\helfi_navigation\CacheValue::__construct
* @covers \Drupal\helfi_navigation\ApiResponse::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testGetExternalMenu() : void {
$requests = [];
Expand Down Expand Up @@ -204,6 +212,8 @@ public function testGetExternalMenu() : void {
* @covers \Drupal\helfi_navigation\CacheValue::hasExpired
* @covers \Drupal\helfi_navigation\CacheValue::__construct
* @covers \Drupal\helfi_navigation\ApiResponse::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testGetMainMenu() : void {
$requests = [];
Expand Down Expand Up @@ -234,6 +244,8 @@ public function testGetMainMenu() : void {
* @covers \Drupal\helfi_navigation\CacheValue::hasExpired
* @covers \Drupal\helfi_navigation\CacheValue::__construct
* @covers \Drupal\helfi_navigation\ApiResponse::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testStaleCacheOnRequestFailure() : void {
$requests = [];
Expand Down Expand Up @@ -270,6 +282,8 @@ public function testStaleCacheOnRequestFailure() : void {
* @covers \Drupal\helfi_navigation\CacheValue::hasExpired
* @covers \Drupal\helfi_navigation\CacheValue::__construct
* @covers \Drupal\helfi_navigation\ApiResponse::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testStaleCacheUpdate() : void {
$time = time();
Expand Down Expand Up @@ -310,6 +324,8 @@ public function testStaleCacheUpdate() : void {
* @covers ::cache
* @covers ::getDefaultRequestOptions
* @covers ::getUrl
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testRequestLoggingException() : void {
$this->expectException(GuzzleException::class);
Expand All @@ -334,6 +350,8 @@ public function testRequestLoggingException() : void {
* @covers ::cache
* @covers ::getDefaultRequestOptions
* @covers ::getUrl
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testMockFallbackException() : void {
$this->expectException(FileNotExistsException::class);
Expand Down Expand Up @@ -362,6 +380,8 @@ public function testMockFallbackException() : void {
* @covers ::getUrl
* @covers \Drupal\helfi_navigation\CacheValue::__construct
* @covers \Drupal\helfi_navigation\ApiResponse::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testMockFallback() : void {
// Use logger to verify that mock file is actually used.
Expand Down Expand Up @@ -391,6 +411,8 @@ public function testMockFallback() : void {
* @covers ::cache
* @covers ::getDefaultRequestOptions
* @covers ::getUrl
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testFastRequestFailure() : void {
// Override environment name so we don't fallback to mock responses.
Expand Down Expand Up @@ -432,6 +454,8 @@ public function testFastRequestFailure() : void {
* @covers \Drupal\helfi_navigation\CacheValue::hasExpired
* @covers \Drupal\helfi_navigation\CacheValue::__construct
* @covers \Drupal\helfi_navigation\ApiResponse::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::__construct
* @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization
*/
public function testCacheBypass() : void {
$requests = [];
Expand Down

0 comments on commit 8776973

Please sign in to comment.