mirrors/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2024-11-28 00:38:46 +03:00
Code Issues Projects Releases Packages Wiki Activity

Fixed single step shortening endpoint

Browse source
This commit is contained in:
Alejandro Celaya 2021-01-21 19:26:19 +01:00
parent b5b3a50bb2
commit da9896a28b
8 changed files with 121 additions and 80 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
module/Rest/config/auth.config.php
View file

@ -11,9 +11,12 @@ return [
'auth' => [
'routes_whitelist' => [
Action\HealthAction::class,
Action\ShortUrl\SingleStepCreateShortUrlAction::class,
ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME,
],
'routes_with_query_api_key' => [
Action\ShortUrl\SingleStepCreateShortUrlAction::class,
],
],
'dependencies' => [
@ -23,7 +26,11 @@ return [
],
ConfigAbstractFactory::class => [
Middleware\AuthenticationMiddleware::class => [Service\ApiKeyService::class, 'config.auth.routes_whitelist'],
Middleware\AuthenticationMiddleware::class => [
Service\ApiKeyService::class,
'config.auth.routes_whitelist',
'config.auth.routes_with_query_api_key',
],
],
];

1
module/Rest/config/dependencies.config.php
View file

@ -57,7 +57,6 @@ return [
Action\ShortUrl\CreateShortUrlAction::class => [Service\UrlShortener::class, 'config.url_shortener.domain'],
Action\ShortUrl\SingleStepCreateShortUrlAction::class => [
Service\UrlShortener::class,
ApiKeyService::class,
'config.url_shortener.domain',
],
Action\ShortUrl\EditShortUrlAction::class => [Service\ShortUrlService::class],

27
module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php
View file

@ -8,49 +8,28 @@ use Psr\Http\Message\ServerRequestInterface as Request;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction
{
protected const ROUTE_PATH = '/short-urls/shorten';
protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET];
private ApiKeyServiceInterface $apiKeyService;
public function __construct(
UrlShortenerInterface $urlShortener,
ApiKeyServiceInterface $apiKeyService,
array $domainConfig
) {
parent::__construct($urlShortener, $domainConfig);
$this->apiKeyService = $apiKeyService;
}
/**
* @throws ValidationException
*/
protected function buildShortUrlData(Request $request): CreateShortUrlData
{
$query = $request->getQueryParams();
$longUrl = $query['longUrl'] ?? null;
$apiKeyResult = $this->apiKeyService->check($query['apiKey'] ?? '');
if (! $apiKeyResult->isValid()) {
throw ValidationException::fromArray([
'apiKey' => 'No API key was provided or it is not valid',
]);
}
if ($longUrl === null) {
throw ValidationException::fromArray([
'longUrl' => 'A URL was not provided',
]);
}
$apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
return new CreateShortUrlData($longUrl, [], ShortUrlMeta::fromRawData([
ShortUrlMetaInputFilter::API_KEY => $apiKeyResult->apiKey(),
ShortUrlMetaInputFilter::API_KEY => $apiKey,
// This will usually be null, unless this API key enforces one specific domain
ShortUrlMetaInputFilter::DOMAIN => $request->getAttribute(ShortUrlMetaInputFilter::DOMAIN),
]));

28
module/Rest/src/Exception/MissingAuthenticationException.php
View file

@ -18,18 +18,36 @@ class MissingAuthenticationException extends RuntimeException implements Problem
private const TITLE = 'Invalid authorization';
private const TYPE = 'INVALID_AUTHORIZATION';
public static function fromExpectedTypes(array $expectedTypes): self
public static function forHeaders(array $expectedHeaders): self
{
$e = new self(sprintf(
$e = self::withMessage(sprintf(
'Expected one of the following authentication headers, ["%s"], but none were provided',
implode('", "', $expectedTypes),
implode('", "', $expectedHeaders),
));
$e->additional = [
'expectedTypes' => $expectedHeaders, // Deprecated
'expectedHeaders' => $expectedHeaders,
];
$e->detail = $e->getMessage();
return $e;
}
public static function forQueryParam(string $param): self
{
$e = self::withMessage(sprintf('Expected authentication to be provided in "%s" query param', $param));
$e->additional = ['param' => $param];
return $e;
}
private static function withMessage(string $message): self
{
$e = new self($message);
$e->detail = $message;
$e->title = self::TITLE;
$e->type = self::TYPE;
$e->status = StatusCodeInterface::STATUS_UNAUTHORIZED;
$e->additional = ['expectedTypes' => $expectedTypes];
return $e;
}

32
module/Rest/src/Middleware/AuthenticationMiddleware.php
View file

@ -8,6 +8,7 @@ use Fig\Http\Message\RequestMethodInterface;
use Fig\Http\Message\StatusCodeInterface;
use Mezzio\Router\RouteResult;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
@ -24,11 +25,16 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
private ApiKeyServiceInterface $apiKeyService;
private array $routesWhitelist;
private array $routesWithQueryApiKey;
public function __construct(ApiKeyServiceInterface $apiKeyService, array $routesWhitelist)
{
public function __construct(
ApiKeyServiceInterface $apiKeyService,
array $routesWhitelist,
array $routesWithQueryApiKey
) {
$this->apiKeyService = $apiKeyService;
$this->routesWhitelist = $routesWhitelist;
$this->routesWithQueryApiKey = $routesWithQueryApiKey;
}
public function process(Request $request, RequestHandlerInterface $handler): Response
@ -44,11 +50,7 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
return $handler->handle($request);
}
$apiKey = $request->getHeaderLine(self::API_KEY_HEADER);
if (empty($apiKey)) {
throw MissingAuthenticationException::fromExpectedTypes([self::API_KEY_HEADER]);
}
$apiKey = $this->getApiKeyFromRequest($request, $routeResult);
$result = $this->apiKeyService->check($apiKey);
if (! $result->isValid()) {
throw VerifyAuthenticationException::forInvalidApiKey();
@ -61,4 +63,20 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
{
return $request->getAttribute(ApiKey::class);
}
private function getApiKeyFromRequest(ServerRequestInterface $request, RouteResult $routeResult): string
{
$routeName = $routeResult->getMatchedRouteName();
$query = $request->getQueryParams();
$isRouteWithApiKeyInQuery = contains($this->routesWithQueryApiKey, $routeName);
$apiKey = $isRouteWithApiKeyInQuery ? ($query['apiKey'] ?? '') : $request->getHeaderLine(self::API_KEY_HEADER);
if (empty($apiKey)) {
throw $isRouteWithApiKeyInQuery
? MissingAuthenticationException::forQueryParam('apiKey')
: MissingAuthenticationException::forHeaders([self::API_KEY_HEADER]);
}
return $apiKey;
}
}

26
module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php
View file

@ -16,8 +16,6 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use Shlinkio\Shlink\Rest\Service\ApiKeyCheckResult;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
class SingleStepCreateShortUrlActionTest extends TestCase
{
@ -30,11 +28,9 @@ class SingleStepCreateShortUrlActionTest extends TestCase
public function setUp(): void
{
$this->urlShortener = $this->prophesize(UrlShortenerInterface::class);
$this->apiKeyService = $this->prophesize(ApiKeyServiceInterface::class);
$this->action = new SingleStepCreateShortUrlAction(
$this->urlShortener->reveal(),
$this->apiKeyService->reveal(),
[
'schema' => 'http',
'hostname' => 'foo.com',
@ -42,26 +38,12 @@ class SingleStepCreateShortUrlActionTest extends TestCase
);
}
/** @test */
public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(): void
{
$request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
$findApiKey = $this->apiKeyService->check('abc123')->willReturn(new ApiKeyCheckResult());
$this->expectException(ValidationException::class);
$findApiKey->shouldBeCalledOnce();
$this->action->handle($request);
}
/** @test */
public function errorResponseIsReturnedIfNoUrlIsProvided(): void
{
$request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
$findApiKey = $this->apiKeyService->check('abc123')->willReturn(new ApiKeyCheckResult(new ApiKey()));
$request = new ServerRequest();
$this->expectException(ValidationException::class);
$findApiKey->shouldBeCalledOnce();
$this->action->handle($request);
}
@ -70,13 +52,10 @@ class SingleStepCreateShortUrlActionTest extends TestCase
public function properDataIsPassedWhenGeneratingShortCode(): void
{
$apiKey = new ApiKey();
$key = $apiKey->toString();
$request = (new ServerRequest())->withQueryParams([
'apiKey' => $key,
'longUrl' => 'http://foobar.com',
]);
$findApiKey = $this->apiKeyService->check($key)->willReturn(new ApiKeyCheckResult($apiKey));
])->withAttribute(ApiKey::class, $apiKey);
$generateShortCode = $this->urlShortener->shorten(
Argument::that(function (string $argument): bool {
Assert::assertEquals('http://foobar.com', $argument);
@ -89,7 +68,6 @@ class SingleStepCreateShortUrlActionTest extends TestCase
$resp = $this->action->handle($request);
self::assertEquals(200, $resp->getStatusCode());
$findApiKey->shouldHaveBeenCalled();
$generateShortCode->shouldHaveBeenCalled();
}
}

45
module/Rest/test/Exception/MissingAuthenticationExceptionTest.php
View file

@ -16,21 +16,22 @@ class MissingAuthenticationExceptionTest extends TestCase
* @test
* @dataProvider provideExpectedTypes
*/
public function exceptionIsProperlyCreatedFromExpectedTypes(array $expectedTypes): void
public function exceptionIsProperlyCreatedFromExpectedHeaders(array $expectedHeaders): void
{
$expectedMessage = sprintf(
'Expected one of the following authentication headers, ["%s"], but none were provided',
implode('", "', $expectedTypes),
implode('", "', $expectedHeaders),
);
$e = MissingAuthenticationException::fromExpectedTypes($expectedTypes);
$e = MissingAuthenticationException::forHeaders($expectedHeaders);
$this->assertCommonExceptionShape($e);
self::assertEquals($expectedMessage, $e->getMessage());
self::assertEquals($expectedMessage, $e->getDetail());
self::assertEquals('Invalid authorization', $e->getTitle());
self::assertEquals('INVALID_AUTHORIZATION', $e->getType());
self::assertEquals(401, $e->getStatus());
self::assertEquals(['expectedTypes' => $expectedTypes], $e->getAdditionalData());
self::assertEquals([
'expectedTypes' => $expectedHeaders,
'expectedHeaders' => $expectedHeaders,
], $e->getAdditionalData());
}
public function provideExpectedTypes(): iterable
@ -40,4 +41,34 @@ class MissingAuthenticationExceptionTest extends TestCase
yield [[]];
yield [['foo', 'bar', 'baz']];
}
/**
* @test
* @dataProvider provideExpectedParam
*/
public function exceptionIsProperlyCreatedFromExpectedQueryParam(string $param): void
{
$expectedMessage = sprintf('Expected authentication to be provided in "%s" query param', $param);
$e = MissingAuthenticationException::forQueryParam($param);
$this->assertCommonExceptionShape($e);
self::assertEquals($expectedMessage, $e->getMessage());
self::assertEquals($expectedMessage, $e->getDetail());
self::assertEquals(['param' => $param], $e->getAdditionalData());
}
public function provideExpectedParam(): iterable
{
yield ['foo'];
yield ['bar'];
yield ['something'];
}
private function assertCommonExceptionShape(MissingAuthenticationException $e): void
{
self::assertEquals('Invalid authorization', $e->getTitle());
self::assertEquals('INVALID_AUTHORIZATION', $e->getType());
self::assertEquals(401, $e->getStatus());
}
}

31
module/Rest/test/Middleware/AuthenticationMiddlewareTest.php
View file

@ -38,7 +38,11 @@ class AuthenticationMiddlewareTest extends TestCase
public function setUp(): void
{
$this->apiKeyService = $this->prophesize(ApiKeyServiceInterface::class);
$this->middleware = new AuthenticationMiddleware($this->apiKeyService->reveal(), [HealthAction::class]);
$this->middleware = new AuthenticationMiddleware(
$this->apiKeyService->reveal(),
[HealthAction::class],
['with_query_api_key'],
);
$this->handler = $this->prophesize(RequestHandlerInterface::class);
}
@ -82,27 +86,34 @@ class AuthenticationMiddlewareTest extends TestCase
* @test
* @dataProvider provideRequestsWithoutApiKey
*/
public function throwsExceptionWhenNoApiKeyIsProvided(ServerRequestInterface $request): void
{
public function throwsExceptionWhenNoApiKeyIsProvided(
ServerRequestInterface $request,
string $expectedMessage
): void {
$this->apiKeyService->check(Argument::any())->shouldNotBeCalled();
$this->handler->handle($request)->shouldNotBeCalled();
$this->expectException(MissingAuthenticationException::class);
$this->expectExceptionMessage(
'Expected one of the following authentication headers, ["X-Api-Key"], but none were provided',
);
$this->expectExceptionMessage($expectedMessage);
$this->middleware->process($request, $this->handler->reveal());
}
public function provideRequestsWithoutApiKey(): iterable
{
$baseRequest = ServerRequestFactory::fromGlobals()->withAttribute(
$baseRequest = fn (string $routeName) => ServerRequestFactory::fromGlobals()->withAttribute(
RouteResult::class,
RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []),
RouteResult::fromRoute(new Route($routeName, $this->getDummyMiddleware()), []),
);
$apiKeyMessage = 'Expected one of the following authentication headers, ["X-Api-Key"], but none were provided';
$queryMessage = 'Expected authentication to be provided in "apiKey" query param';
yield 'no api key' => [$baseRequest];
yield 'empty api key' => [$baseRequest->withHeader('X-Api-Key', '')];
yield 'no api key in header' => [$baseRequest('bar'), $apiKeyMessage];
yield 'empty api key in header' => [$baseRequest('bar')->withHeader('X-Api-Key', ''), $apiKeyMessage];
yield 'no api key in query' => [$baseRequest('with_query_api_key'), $queryMessage];
yield 'empty api key in query' => [
$baseRequest('with_query_api_key')->withQueryParams(['apiKey' => '']),
$queryMessage,
];
}
/** @test */