Skip to content

Commit

Permalink
Merge pull request #888 from acelaya-forks/feature/simplify-auth-checks
Browse files Browse the repository at this point in the history
Deleted everything related with authentication plugins, as shlink onl…
  • Loading branch information
acelaya committed Nov 7, 2020
2 parents 098751d + d6395a3 commit 1bc9e06
Show file tree
Hide file tree
Showing 17 changed files with 99 additions and 452 deletions.
23 changes: 1 addition & 22 deletions module/Rest/config/auth.config.php
Expand Up @@ -14,37 +14,16 @@
Action\ShortUrl\SingleStepCreateShortUrlAction::class,
ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME,
],

'plugins' => [
'factories' => [
Authentication\Plugin\ApiKeyHeaderPlugin::class => ConfigAbstractFactory::class,
],
'aliases' => [
Authentication\Plugin\ApiKeyHeaderPlugin::HEADER_NAME =>
Authentication\Plugin\ApiKeyHeaderPlugin::class,
],
],
],

'dependencies' => [
'factories' => [
Authentication\AuthenticationPluginManager::class =>
Authentication\AuthenticationPluginManagerFactory::class,
Authentication\RequestToHttpAuthPlugin::class => ConfigAbstractFactory::class,

Middleware\AuthenticationMiddleware::class => ConfigAbstractFactory::class,
],
],

ConfigAbstractFactory::class => [
Authentication\Plugin\ApiKeyHeaderPlugin::class => [Service\ApiKeyService::class],

Authentication\RequestToHttpAuthPlugin::class => [Authentication\AuthenticationPluginManager::class],

Middleware\AuthenticationMiddleware::class => [
Authentication\RequestToHttpAuthPlugin::class,
'config.auth.routes_whitelist',
],
Middleware\AuthenticationMiddleware::class => [Service\ApiKeyService::class, 'config.auth.routes_whitelist'],
],

];
4 changes: 2 additions & 2 deletions module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php
Expand Up @@ -9,7 +9,7 @@
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
use Shlinkio\Shlink\Rest\Authentication\Plugin\ApiKeyHeaderPlugin;
use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;

class CreateShortUrlAction extends AbstractCreateShortUrlAction
{
Expand All @@ -28,7 +28,7 @@ protected function buildShortUrlData(Request $request): CreateShortUrlData
]);
}

$payload[ShortUrlMetaInputFilter::API_KEY] = $request->getHeaderLine(ApiKeyHeaderPlugin::HEADER_NAME);
$payload[ShortUrlMetaInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request);
$meta = ShortUrlMeta::fromRawData($payload);

return new CreateShortUrlData($payload['longUrl'], (array) ($payload['tags'] ?? []), $meta);
Expand Down
12 changes: 0 additions & 12 deletions module/Rest/src/Authentication/AuthenticationPluginManager.php

This file was deleted.

This file was deleted.

This file was deleted.

38 changes: 0 additions & 38 deletions module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php

This file was deleted.

This file was deleted.

55 changes: 0 additions & 55 deletions module/Rest/src/Authentication/RequestToHttpAuthPlugin.php

This file was deleted.

This file was deleted.

30 changes: 22 additions & 8 deletions module/Rest/src/Middleware/AuthenticationMiddleware.php
Expand Up @@ -11,19 +11,23 @@
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface;
use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException;
use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;

use function Functional\contains;

class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface, RequestMethodInterface
{
public const API_KEY_HEADER = 'X-Api-Key';

private ApiKeyServiceInterface $apiKeyService;
private array $routesWhitelist;
private RequestToHttpAuthPluginInterface $requestToAuthPlugin;

public function __construct(RequestToHttpAuthPluginInterface $requestToAuthPlugin, array $routesWhitelist)
public function __construct(ApiKeyServiceInterface $apiKeyService, array $routesWhitelist)
{
$this->apiKeyService = $apiKeyService;
$this->routesWhitelist = $routesWhitelist;
$this->requestToAuthPlugin = $requestToAuthPlugin;
}

public function process(Request $request, RequestHandlerInterface $handler): Response
Expand All @@ -39,10 +43,20 @@ public function process(Request $request, RequestHandlerInterface $handler): Res
return $handler->handle($request);
}

$plugin = $this->requestToAuthPlugin->fromRequest($request);
$plugin->verify($request);
$response = $handler->handle($request);
$apiKey = self::apiKeyFromRequest($request);
if (empty($apiKey)) {
throw MissingAuthenticationException::fromExpectedTypes([self::API_KEY_HEADER]);
}

if (! $this->apiKeyService->check($apiKey)) {
throw VerifyAuthenticationException::forInvalidApiKey();
}

return $plugin->update($request, $response);
return $handler->handle($request);
}

public static function apiKeyFromRequest(Request $request): string
{
return $request->getHeaderLine(self::API_KEY_HEADER);
}
}
5 changes: 1 addition & 4 deletions module/Rest/src/Middleware/CrossDomainMiddleware.php
Expand Up @@ -11,7 +11,6 @@
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Rest\Authentication;

use function array_merge;
use function implode;
Expand All @@ -27,9 +26,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface

// Add Allow-Origin header
$response = $response->withHeader('Access-Control-Allow-Origin', $request->getHeader('Origin'))
->withHeader('Access-Control-Expose-Headers', implode(', ', [
Authentication\Plugin\ApiKeyHeaderPlugin::HEADER_NAME,
]));
->withHeader('Access-Control-Expose-Headers', AuthenticationMiddleware::API_KEY_HEADER);
if ($request->getMethod() !== self::METHOD_OPTIONS) {
return $response;
}
Expand Down
12 changes: 2 additions & 10 deletions module/Rest/test-api/Middleware/AuthenticationTest.php
Expand Up @@ -4,22 +4,14 @@

namespace ShlinkioApiTest\Shlink\Rest\Middleware;

use Shlinkio\Shlink\Rest\Authentication\Plugin;
use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin;
use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;

use function implode;
use function sprintf;

class AuthenticationTest extends ApiTestCase
{
/** @test */
public function authorizationErrorIsReturnedIfNoApiKeyIsSent(): void
{
$expectedDetail = sprintf(
'Expected one of the following authentication headers, ["%s"], but none were provided',
implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS),
);
$expectedDetail = 'Expected one of the following authentication headers, ["X-Api-Key"], but none were provided';

$resp = $this->callApi(self::METHOD_GET, '/short-urls');
$payload = $this->getJsonResponsePayload($resp);
Expand All @@ -41,7 +33,7 @@ public function apiKeyErrorIsReturnedWhenProvidedApiKeyIsInvalid(string $apiKey)

$resp = $this->callApi(self::METHOD_GET, '/short-urls', [
'headers' => [
Plugin\ApiKeyHeaderPlugin::HEADER_NAME => $apiKey,
'X-Api-Key' => $apiKey,
],
]);
$payload = $this->getJsonResponsePayload($resp);
Expand Down

This file was deleted.

0 comments on commit 1bc9e06

Please sign in to comment.