From 7b1b00901a6f66cfc800bbf3be0acdcf85c8e6bd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 5 Aug 2016 07:20:40 +0200 Subject: [PATCH 01/39] Created phpstorm meta fle to get ContainerInterop typehint based on service name --- .phpstorm.meta.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .phpstorm.meta.php diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php new file mode 100644 index 00000000..1a5e12de --- /dev/null +++ b/.phpstorm.meta.php @@ -0,0 +1,19 @@ + [ + '' == '@', + ], +]; From 270dbc60285dacf4202c9549e5b5f8ee4c750bcd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 12:40:31 +0200 Subject: [PATCH 02/39] Created new entity_manager configuration, dropping old database first level config key --- config/autoload/database.global.php | 15 -------------- config/autoload/entity-manager.global.php | 20 +++++++++++++++++++ .../src/Factory/EntityManagerFactory.php | 10 ++++++---- .../test/Factory/EntityManagerFactoryTest.php | 6 ++++-- module/Core/config/entity-manager.config.php | 12 +++++++++++ 5 files changed, 42 insertions(+), 21 deletions(-) delete mode 100644 config/autoload/database.global.php create mode 100644 config/autoload/entity-manager.global.php create mode 100644 module/Core/config/entity-manager.config.php diff --git a/config/autoload/database.global.php b/config/autoload/database.global.php deleted file mode 100644 index 4d05ea36..00000000 --- a/config/autoload/database.global.php +++ /dev/null @@ -1,15 +0,0 @@ - [ - 'driver' => 'pdo_mysql', - 'user' => env('DB_USER'), - 'password' => env('DB_PASSWORD'), - 'dbname' => env('DB_NAME', 'shlink'), - 'charset' => 'utf8', - 'driverOptions' => [ - PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8' - ], - ], - -]; diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php new file mode 100644 index 00000000..87f99215 --- /dev/null +++ b/config/autoload/entity-manager.global.php @@ -0,0 +1,20 @@ + [ + 'orm' => [ + 'proxies_dir' => 'data/proxies', + ], + 'connection' => [ + 'driver' => 'pdo_mysql', + 'user' => env('DB_USER'), + 'password' => env('DB_PASSWORD'), + 'dbname' => env('DB_NAME', 'shlink'), + 'charset' => 'utf8', + 'driverOptions' => [ + PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8', + ], + ], + ], + +]; diff --git a/module/Common/src/Factory/EntityManagerFactory.php b/module/Common/src/Factory/EntityManagerFactory.php index e42abb40..533aa322 100644 --- a/module/Common/src/Factory/EntityManagerFactory.php +++ b/module/Common/src/Factory/EntityManagerFactory.php @@ -30,12 +30,14 @@ class EntityManagerFactory implements FactoryInterface $globalConfig = $container->get('config'); $isDevMode = isset($globalConfig['debug']) ? ((bool) $globalConfig['debug']) : false; $cache = $container->has(Cache::class) ? $container->get(Cache::class) : new ArrayCache(); - $dbConfig = isset($globalConfig['database']) ? $globalConfig['database'] : []; + $emConfig = isset($globalConfig['entity_manager']) ? $globalConfig['entity_manager'] : []; + $connecitonConfig = isset($emConfig['connection']) ? $emConfig['connection'] : []; + $ormConfig = isset($emConfig['orm']) ? $emConfig['orm'] : []; - return EntityManager::create($dbConfig, Setup::createAnnotationMetadataConfiguration( - ['module/Core/src/Entity'], + return EntityManager::create($connecitonConfig, Setup::createAnnotationMetadataConfiguration( + isset($ormConfig['entities_paths']) ? $ormConfig['entities_paths'] : [], $isDevMode, - 'data/proxies', + isset($ormConfig['proxies_dir']) ? $ormConfig['proxies_dir'] : null, $cache, false )); diff --git a/module/Common/test/Factory/EntityManagerFactoryTest.php b/module/Common/test/Factory/EntityManagerFactoryTest.php index 53c839ed..2bad3c38 100644 --- a/module/Common/test/Factory/EntityManagerFactoryTest.php +++ b/module/Common/test/Factory/EntityManagerFactoryTest.php @@ -26,8 +26,10 @@ class EntityManagerFactoryTest extends TestCase $sm = new ServiceManager(['services' => [ 'config' => [ 'debug' => true, - 'database' => [ - 'driver' => 'pdo_sqlite', + 'entity_manager' => [ + 'connection' => [ + 'driver' => 'pdo_sqlite', + ], ], ], ]]); diff --git a/module/Core/config/entity-manager.config.php b/module/Core/config/entity-manager.config.php new file mode 100644 index 00000000..e9359519 --- /dev/null +++ b/module/Core/config/entity-manager.config.php @@ -0,0 +1,12 @@ + [ + 'orm' => [ + 'entities_paths' => [ + __DIR__ . '/../src/Entity', + ], + ], + ], + +]; From 2767a14101567c305d5f3ef9eefacb84616b0362 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 12:50:44 +0200 Subject: [PATCH 03/39] Created ApiKey entity --- config/autoload/local.php.dist | 3 +- module/Rest/config/entity-manager.config.php | 12 ++ module/Rest/src/Entity/ApiKey.php | 117 +++++++++++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 module/Rest/config/entity-manager.config.php create mode 100644 module/Rest/src/Entity/ApiKey.php diff --git a/config/autoload/local.php.dist b/config/autoload/local.php.dist index 75c8b9c2..cd1996b9 100644 --- a/config/autoload/local.php.dist +++ b/config/autoload/local.php.dist @@ -1,7 +1,8 @@ true, + 'debug' => true, 'config_cache_enabled' => false, + ]; diff --git a/module/Rest/config/entity-manager.config.php b/module/Rest/config/entity-manager.config.php new file mode 100644 index 00000000..e9359519 --- /dev/null +++ b/module/Rest/config/entity-manager.config.php @@ -0,0 +1,12 @@ + [ + 'orm' => [ + 'entities_paths' => [ + __DIR__ . '/../src/Entity', + ], + ], + ], + +]; diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php new file mode 100644 index 00000000..7dc1614f --- /dev/null +++ b/module/Rest/src/Entity/ApiKey.php @@ -0,0 +1,117 @@ +enabled = true; + $this->key = $this->generateV4Uuid(); + } + + /** + * @return string + */ + public function getKey() + { + return $this->key; + } + + /** + * @param string $key + * @return $this + */ + public function setKey($key) + { + $this->key = $key; + return $this; + } + + /** + * @return \DateTime + */ + public function getExpirationDate() + { + return $this->expirationDate; + } + + /** + * @param \DateTime $expirationDate + * @return $this + */ + public function setExpirationDate($expirationDate) + { + $this->expirationDate = $expirationDate; + return $this; + } + + /** + * @return bool + */ + public function isExpired() + { + if (! isset($this->expirationDate)) { + return false; + } + + return $this->expirationDate >= new \DateTime(); + } + + /** + * @return boolean + */ + public function isEnabled() + { + return $this->enabled; + } + + /** + * @param boolean $enabled + * @return $this + */ + public function setEnabled($enabled) + { + $this->enabled = $enabled; + return $this; + } + + /** + * Tells if this api key is enabled and not expired + * + * @return bool + */ + public function isValid() + { + return $this->isEnabled() && ! $this->isExpired(); + } +} From 7b746f76b063f83d27e2afc8b2d24d57e43f8e4b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 13:18:27 +0200 Subject: [PATCH 04/39] Created APiKeyService and tests --- module/Rest/src/Action/AuthenticateAction.php | 6 +- module/Rest/src/Entity/ApiKey.php | 12 +- module/Rest/src/Service/ApiKeyService.php | 85 +++++++++++ .../src/Service/ApiKeyServiceInterface.php | 31 ++++ .../Rest/test/Service/ApiKeyServiceTest.php | 142 ++++++++++++++++++ 5 files changed, 273 insertions(+), 3 deletions(-) create mode 100644 module/Rest/src/Service/ApiKeyService.php create mode 100644 module/Rest/src/Service/ApiKeyServiceInterface.php create mode 100644 module/Rest/test/Service/ApiKeyServiceTest.php diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 7d564e4f..37abbc56 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -44,10 +44,12 @@ class AuthenticateAction extends AbstractRestAction public function dispatch(Request $request, Response $response, callable $out = null) { $authData = $request->getParsedBody(); - if (! isset($authData['username'], $authData['password'])) { + if (! isset($authData['apiKey'], $authData['username'], $authData['password'])) { return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => $this->translator->translate('You have to provide both "username" and "password"'), + 'message' => $this->translator->translate( + 'You have to provide a valid API key under the "apiKey" param name.' + ), ], 400); } diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index 7dc1614f..e0600e88 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -84,7 +84,7 @@ class ApiKey extends AbstractEntity return false; } - return $this->expirationDate >= new \DateTime(); + return $this->expirationDate < new \DateTime(); } /** @@ -105,6 +105,16 @@ class ApiKey extends AbstractEntity return $this; } + /** + * Disables this API key + * + * @return $this + */ + public function disable() + { + return $this->setEnabled(false); + } + /** * Tells if this api key is enabled and not expired * diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php new file mode 100644 index 00000000..3aa13b62 --- /dev/null +++ b/module/Rest/src/Service/ApiKeyService.php @@ -0,0 +1,85 @@ +em = $em; + } + + /** + * Creates a new ApiKey with provided expiration date + * + * @param \DateTime $expirationDate + * @return ApiKey + */ + public function create(\DateTime $expirationDate = null) + { + $key = new ApiKey(); + if (isset($expirationDate)) { + $key->setExpirationDate($expirationDate); + } + + $this->em->persist($key); + $this->em->flush(); + + return $key; + } + + /** + * Checks if provided key is a valid api key + * + * @param string $key + * @return bool + */ + public function check($key) + { + /** @var ApiKey $apiKey */ + $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ + 'key' => $key, + ]); + if (! isset($apiKey)) { + return false; + } + + return $apiKey->isValid(); + } + + /** + * Disables provided api key + * + * @param string $key + * @return ApiKey + */ + public function disable($key) + { + /** @var ApiKey $apiKey */ + $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ + 'key' => $key, + ]); + if (! isset($apiKey)) { + throw new InvalidArgumentException(sprintf('API key "%s" does not exist and can\'t be disabled', $key)); + } + + $apiKey->disable(); + $this->em->flush(); + return $apiKey; + } +} diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php new file mode 100644 index 00000000..0c1f526b --- /dev/null +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -0,0 +1,31 @@ +em = $this->prophesize(EntityManager::class); + $this->service = new ApiKeyService($this->em->reveal()); + } + + /** + * @test + */ + public function keyIsProperlyCreated() + { + $this->em->flush()->shouldBeCalledTimes(1); + $this->em->persist(Argument::type(ApiKey::class))->shouldBeCalledTimes(1); + + $key = $this->service->create(); + $this->assertNull($key->getExpirationDate()); + } + + /** + * @test + */ + public function keyIsProperlyCreatedWithExpirationDate() + { + $this->em->flush()->shouldBeCalledTimes(1); + $this->em->persist(Argument::type(ApiKey::class))->shouldBeCalledTimes(1); + + $date = new \DateTime('2030-01-01'); + $key = $this->service->create($date); + $this->assertSame($date, $key->getExpirationDate()); + } + + /** + * @test + */ + public function checkReturnsFalseWhenKeyIsInvalid() + { + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['key' => '12345'])->willReturn(null) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->assertFalse($this->service->check('12345')); + } + + /** + * @test + */ + public function checkReturnsFalseWhenKeyIsDisabled() + { + $key = new ApiKey(); + $key->disable(); + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['key' => '12345'])->willReturn($key) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->assertFalse($this->service->check('12345')); + } + + /** + * @test + */ + public function checkReturnsFalseWhenKeyIsExpired() + { + $key = new ApiKey(); + $key->setExpirationDate((new \DateTime())->sub(new \DateInterval('P1D'))); + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['key' => '12345'])->willReturn($key) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->assertFalse($this->service->check('12345')); + } + + /** + * @test + */ + public function checkReturnsTrueWhenConditionsAreFavorable() + { + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['key' => '12345'])->willReturn(new ApiKey()) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->assertTrue($this->service->check('12345')); + } + + /** + * @test + * @expectedException \Shlinkio\Shlink\Common\Exception\InvalidArgumentException + */ + public function disableThrowsExceptionWhenNoTokenIsFound() + { + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['key' => '12345'])->willReturn(null) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->service->disable('12345'); + } + + /** + * @test + */ + public function disableReturnsDisabledKeyWhenFOund() + { + $key = new ApiKey(); + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['key' => '12345'])->willReturn($key) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->em->flush()->shouldBeCalledTimes(1); + + $this->assertTrue($key->isEnabled()); + $returnedKey = $this->service->disable('12345'); + $this->assertFalse($key->isEnabled()); + $this->assertSame($key, $returnedKey); + } +} From 99d7e6dd7d269f2508c138f0dba5dfcf15b63ba7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 13:24:06 +0200 Subject: [PATCH 05/39] Fixed AuthenticateAction not working with only one group of params --- module/Rest/src/Action/AuthenticateAction.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 37abbc56..7ac531df 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -44,7 +44,7 @@ class AuthenticateAction extends AbstractRestAction public function dispatch(Request $request, Response $response, callable $out = null) { $authData = $request->getParsedBody(); - if (! isset($authData['apiKey'], $authData['username'], $authData['password'])) { + if (! isset($authData['apiKey']) && ! isset($authData['username'], $authData['password'])) { return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, 'message' => $this->translator->translate( From 74777c2234f1459d201a9d84aabf257e0b77204f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 18:07:48 +0200 Subject: [PATCH 06/39] Created command to generate a new api key --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 1 + .../CLI/src/Command/Api/DisableKeyCommand.php | 6 ++ .../src/Command/Api/GenerateKeyCommand.php | 56 +++++++++++++++++++ .../Command/Api/GenerateKeyCommandTest.php | 55 ++++++++++++++++++ module/Rest/config/dependencies.config.php | 1 + module/Rest/src/Entity/ApiKey.php | 10 ++++ 7 files changed, 130 insertions(+) create mode 100644 module/CLI/src/Command/Api/DisableKeyCommand.php create mode 100644 module/CLI/src/Command/Api/GenerateKeyCommand.php create mode 100644 module/CLI/test/Command/Api/GenerateKeyCommandTest.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index a9b13a72..3d9ff486 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -11,6 +11,7 @@ return [ Command\GetVisitsCommand::class, Command\ProcessVisitsCommand::class, Command\Config\GenerateCharsetCommand::class, + Command\Api\GenerateKeyCommand::class, ] ], diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index d99f68d9..ca671731 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -17,6 +17,7 @@ return [ Command\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\Config\GenerateCharsetCommand::class => AnnotatedFactory::class, + Command\Api\GenerateKeyCommand::class => AnnotatedFactory::class, ], ], diff --git a/module/CLI/src/Command/Api/DisableKeyCommand.php b/module/CLI/src/Command/Api/DisableKeyCommand.php new file mode 100644 index 00000000..3e52908e --- /dev/null +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -0,0 +1,6 @@ +apiKeyService = $apiKeyService; + $this->translator = $translator; + parent::__construct(null); + } + + public function configure() + { + $this->setName('api-key:generate') + ->setDescription($this->translator->translate('Generates a new valid API key.')) + ->addOption( + 'expirationDate', + 'e', + InputOption::VALUE_OPTIONAL, + $this->translator->translate('The date in which the API key should expire. Use any valid PHP format.') + ); + } + + public function execute(InputInterface $input, OutputInterface $output) + { + $expirationDate = $input->getOption('expirationDate'); + $apiKey = $this->apiKeyService->create(isset($expirationDate) ? new \DateTime($expirationDate) : null); + $output->writeln($this->translator->translate('Generated API key') . sprintf(': %s', $apiKey)); + } +} diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php new file mode 100644 index 00000000..b0d44a56 --- /dev/null +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -0,0 +1,55 @@ +apiKeyService = $this->prophesize(ApiKeyService::class); + $command = new GenerateKeyCommand($this->apiKeyService->reveal(), Translator::factory([])); + $app = new Application(); + $app->add($command); + $this->commandTester = new CommandTester($command); + } + + /** + * @test + */ + public function noExpirationDateIsDefinedIfNotProvided() + { + $this->apiKeyService->create(null)->shouldBeCalledTimes(1); + $this->commandTester->execute([ + 'command' => 'api-key:generate', + ]); + } + + /** + * @test + */ + public function expirationDateIsDefinedIfWhenProvided() + { + $this->apiKeyService->create(Argument::type(\DateTime::class))->shouldBeCalledTimes(1); + $this->commandTester->execute([ + 'command' => 'api-key:generate', + '--expirationDate' => '2016-01-01', + ]); + } +} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index e04c8ba0..8f6dbbfb 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -10,6 +10,7 @@ return [ 'dependencies' => [ 'factories' => [ Service\RestTokenService::class => AnnotatedFactory::class, + Service\ApiKeyService::class => AnnotatedFactory::class, Action\AuthenticateAction::class => AnnotatedFactory::class, Action\CreateShortcodeAction::class => AnnotatedFactory::class, diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index e0600e88..0f458c11 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -124,4 +124,14 @@ class ApiKey extends AbstractEntity { return $this->isEnabled() && ! $this->isExpired(); } + + /** + * The string repesentation of an API key is the key itself + * + * @return string + */ + public function __toString() + { + return $this->getKey(); + } } From dd1bc49b7927c0e5587d82a4f71918d81557f089 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 18:08:09 +0200 Subject: [PATCH 07/39] Added method to ApiKeyService to list api keys --- module/Rest/src/Service/ApiKeyService.php | 12 +++++++++ .../src/Service/ApiKeyServiceInterface.php | 8 ++++++ .../Rest/test/Service/ApiKeyServiceTest.php | 26 +++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index 3aa13b62..00fddfad 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -82,4 +82,16 @@ class ApiKeyService implements ApiKeyServiceInterface $this->em->flush(); return $apiKey; } + + /** + * Lists all existing appi keys + * + * @param bool $enabledOnly Tells if only enabled keys should be returned + * @return ApiKey[] + */ + public function listKeys($enabledOnly = false) + { + $conditions = $enabledOnly ? ['enabled' => true] : []; + return $this->em->getRepository(ApiKey::class)->findBy($conditions); + } } diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index 0c1f526b..84c856ce 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -28,4 +28,12 @@ interface ApiKeyServiceInterface * @return ApiKey */ public function disable($key); + + /** + * Lists all existing appi keys + * + * @param bool $enabledOnly Tells if only enabled keys should be returned + * @return ApiKey[] + */ + public function listKeys($enabledOnly = false); } diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 7375c83d..7ab46432 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -139,4 +139,30 @@ class ApiKeyServiceTest extends TestCase $this->assertFalse($key->isEnabled()); $this->assertSame($key, $returnedKey); } + + /** + * @test + */ + public function listFindsAllApiKeys() + { + $repo = $this->prophesize(EntityRepository::class); + $repo->findBy([])->willReturn([]) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->service->listKeys(); + } + + /** + * @test + */ + public function listEnabledFindsOnlyEnabledApiKeys() + { + $repo = $this->prophesize(EntityRepository::class); + $repo->findBy(['enabled' => true])->willReturn([]) + ->shouldBeCalledTimes(1); + $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); + + $this->service->listKeys(true); + } } From c5382b2a7f352d566762688d78860e7b08a293dc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 18:26:07 +0200 Subject: [PATCH 08/39] Created DisableKeyCommand --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 1 + .../CLI/src/Command/Api/DisableKeyCommand.php | 58 ++++++++++++++++- .../Command/Api/DisableKeyCommandTest.php | 62 +++++++++++++++++++ 4 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 module/CLI/test/Command/Api/DisableKeyCommandTest.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 3d9ff486..5739228c 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -12,6 +12,7 @@ return [ Command\ProcessVisitsCommand::class, Command\Config\GenerateCharsetCommand::class, Command\Api\GenerateKeyCommand::class, + Command\Api\DisableKeyCommand::class, ] ], diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index ca671731..03e3bd2c 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -18,6 +18,7 @@ return [ Command\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\Config\GenerateCharsetCommand::class => AnnotatedFactory::class, Command\Api\GenerateKeyCommand::class => AnnotatedFactory::class, + Command\Api\DisableKeyCommand::class => AnnotatedFactory::class, ], ], diff --git a/module/CLI/src/Command/Api/DisableKeyCommand.php b/module/CLI/src/Command/Api/DisableKeyCommand.php index 3e52908e..738b8b43 100644 --- a/module/CLI/src/Command/Api/DisableKeyCommand.php +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -1,6 +1,62 @@ apiKeyService = $apiKeyService; + $this->translator = $translator; + parent::__construct(null); + } + + public function configure() + { + $this->setName('api-key:disable') + ->setDescription($this->translator->translate('Disables an API key.')) + ->addArgument('apiKey', InputArgument::REQUIRED, $this->translator->translate('The API key to disable')); + } + + public function execute(InputInterface $input, OutputInterface $output) + { + $apiKey = $input->getArgument('apiKey'); + + try { + $this->apiKeyService->disable($apiKey); + $output->writeln(sprintf( + $this->translator->translate('API key %s properly disabled'), + '' . $apiKey . '' + )); + } catch (\InvalidArgumentException $e) { + $output->writeln(sprintf( + '' . $this->translator->translate('API key "%s" does not exist.') . '', + $apiKey + )); + } + } } diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php new file mode 100644 index 00000000..68d5f8c2 --- /dev/null +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -0,0 +1,62 @@ +apiKeyService = $this->prophesize(ApiKeyService::class); + $command = new DisableKeyCommand($this->apiKeyService->reveal(), Translator::factory([])); + $app = new Application(); + $app->add($command); + $this->commandTester = new CommandTester($command); + } + + /** + * @test + */ + public function providedApiKeyIsDisabled() + { + $apiKey = 'abcd1234'; + $this->apiKeyService->disable($apiKey)->shouldBeCalledTimes(1); + $this->commandTester->execute([ + 'command' => 'api-key:disable', + 'apiKey' => $apiKey, + ]); + } + + /** + * @test + */ + public function errorIsReturnedIfServiceThrowsException() + { + $apiKey = 'abcd1234'; + $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class) + ->shouldBeCalledTimes(1); + + $this->commandTester->execute([ + 'command' => 'api-key:disable', + 'apiKey' => $apiKey, + ]); + $output = $this->commandTester->getDisplay(); + $this->assertEquals('API key "abcd1234" does not exist.' . PHP_EOL, $output); + } +} From 289db45f2722405122b4b6b2fc9bbe3d94f25c08 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2016 18:50:50 +0200 Subject: [PATCH 09/39] Created ListKeysCommand --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 1 + .../CLI/src/Command/Api/ListKeysCommand.php | 108 ++++++++++++++++++ .../test/Command/Api/ListKeysCommandTest.php | 62 ++++++++++ 4 files changed, 172 insertions(+) create mode 100644 module/CLI/src/Command/Api/ListKeysCommand.php create mode 100644 module/CLI/test/Command/Api/ListKeysCommandTest.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 5739228c..35624e51 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -13,6 +13,7 @@ return [ Command\Config\GenerateCharsetCommand::class, Command\Api\GenerateKeyCommand::class, Command\Api\DisableKeyCommand::class, + Command\Api\ListKeysCommand::class, ] ], diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 03e3bd2c..f3257fc6 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -19,6 +19,7 @@ return [ Command\Config\GenerateCharsetCommand::class => AnnotatedFactory::class, Command\Api\GenerateKeyCommand::class => AnnotatedFactory::class, Command\Api\DisableKeyCommand::class => AnnotatedFactory::class, + Command\Api\ListKeysCommand::class => AnnotatedFactory::class, ], ], diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php new file mode 100644 index 00000000..e6f70ec0 --- /dev/null +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -0,0 +1,108 @@ +apiKeyService = $apiKeyService; + $this->translator = $translator; + parent::__construct(null); + } + + public function configure() + { + $this->setName('api-key:list') + ->setDescription($this->translator->translate('Lists all the available API keys.')) + ->addOption( + 'enabledOnly', + null, + InputOption::VALUE_NONE, + $this->translator->translate('Tells if only enabled API keys should be returned.') + ); + } + + public function execute(InputInterface $input, OutputInterface $output) + { + $enabledOnly = $input->getOption('enabledOnly'); + $list = $this->apiKeyService->listKeys($enabledOnly); + + $table = new Table($output); + if ($enabledOnly) { + $table->setHeaders([ + $this->translator->translate('Key'), + $this->translator->translate('Expiration date'), + ]); + } else { + $table->setHeaders([ + $this->translator->translate('Key'), + $this->translator->translate('Is enabled'), + $this->translator->translate('Expiration date'), + ]); + } + + /** @var ApiKey $row */ + foreach ($list as $row) { + $key = $row->getKey(); + $expiration = $row->getExpirationDate(); + $rowData = []; + + if ($enabledOnly) { + $rowData[] = $key; + } else { + $rowData[] = $row->isEnabled() ? $this->getSuccessString($key) : $this->getErrorString($key); + $rowData[] = $row->isEnabled() ? $this->getSuccessString('+++') : $this->getErrorString('---'); + } + + $rowData[] = isset($expiration) ? $expiration->format(\DateTime::ISO8601) : '-'; + $table->addRow($rowData); + } + + $table->render(); + } + + /** + * @param string $string + * @return string + */ + protected function getErrorString($string) + { + return sprintf('%s', $string); + } + + /** + * @param string $string + * @return string + */ + protected function getSuccessString($string) + { + return sprintf('%s', $string); + } +} diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php new file mode 100644 index 00000000..a7f58257 --- /dev/null +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -0,0 +1,62 @@ +apiKeyService = $this->prophesize(ApiKeyService::class); + $command = new ListKeysCommand($this->apiKeyService->reveal(), Translator::factory([])); + $app = new Application(); + $app->add($command); + $this->commandTester = new CommandTester($command); + } + + /** + * @test + */ + public function ifEnabledOnlyIsNotProvidedEverythingIsListed() + { + $this->apiKeyService->listKeys(false)->willReturn([ + new ApiKey(), + new ApiKey(), + new ApiKey(), + ])->shouldBeCalledTimes(1); + $this->commandTester->execute([ + 'command' => 'api-key:list', + ]); + } + + /** + * @test + */ + public function ifEnabledOnlyIsProvidedOnlyThoseKeysAreListed() + { + $this->apiKeyService->listKeys(true)->willReturn([ + new ApiKey(), + new ApiKey(), + ])->shouldBeCalledTimes(1); + $this->commandTester->execute([ + 'command' => 'api-key:list', + '--enabledOnly' => true, + ]); + } +} From 1d92e87d5025aa83696851a156f727ce9586a3fa Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 10:26:34 +0200 Subject: [PATCH 10/39] Updated AuthenticateAction to use the APiKeyService instead of the RestTokenService --- module/Rest/src/Action/AuthenticateAction.php | 37 ++++++++++--------- module/Rest/src/Util/RestUtils.php | 1 + .../test/Action/AuthenticateActionTest.php | 31 +++++++--------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 7ac531df..0fcac3f9 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -4,35 +4,34 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Rest\Exception\AuthenticationException; -use Shlinkio\Shlink\Rest\Service\RestTokenService; -use Shlinkio\Shlink\Rest\Service\RestTokenServiceInterface; +use Shlinkio\Shlink\Rest\Service\ApiKeyService; +use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; use Zend\I18n\Translator\TranslatorInterface; class AuthenticateAction extends AbstractRestAction { - /** - * @var RestTokenServiceInterface - */ - private $restTokenService; /** * @var TranslatorInterface */ private $translator; + /** + * @var ApiKeyService|ApiKeyServiceInterface + */ + private $apiKeyService; /** * AuthenticateAction constructor. - * @param RestTokenServiceInterface|RestTokenService $restTokenService + * @param ApiKeyServiceInterface|ApiKeyService $apiKeyService * @param TranslatorInterface $translator * - * @Inject({RestTokenService::class, "translator"}) + * @Inject({ApiKeyService::class, "translator"}) */ - public function __construct(RestTokenServiceInterface $restTokenService, TranslatorInterface $translator) + public function __construct(ApiKeyServiceInterface $apiKeyService, TranslatorInterface $translator) { - $this->restTokenService = $restTokenService; $this->translator = $translator; + $this->apiKeyService = $apiKeyService; } /** @@ -44,7 +43,7 @@ class AuthenticateAction extends AbstractRestAction public function dispatch(Request $request, Response $response, callable $out = null) { $authData = $request->getParsedBody(); - if (! isset($authData['apiKey']) && ! isset($authData['username'], $authData['password'])) { + if (! isset($authData['apiKey'])) { return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, 'message' => $this->translator->translate( @@ -53,14 +52,16 @@ class AuthenticateAction extends AbstractRestAction ], 400); } - try { - $token = $this->restTokenService->createToken($authData['username'], $authData['password']); - return new JsonResponse(['token' => $token->getToken()]); - } catch (AuthenticationException $e) { + // Authenticate using provided API key + if (! $this->apiKeyService->check($authData['apiKey'])) { return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => $this->translator->translate('Invalid username and/or password'), + 'error' => RestUtils::INVALID_API_KEY_ERROR, + 'message' => $this->translator->translate('Provided API key does not exist or is invalid.'), ], 401); } + + // TODO Generate a JSON Web Token that will be used for authorization in next requests + + return new JsonResponse(['token' => '']); } } diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php index b67491ed..8326487e 100644 --- a/module/Rest/src/Util/RestUtils.php +++ b/module/Rest/src/Util/RestUtils.php @@ -12,6 +12,7 @@ class RestUtils const INVALID_ARGUMENT_ERROR = 'INVALID_ARGUMENT'; const INVALID_CREDENTIALS_ERROR = 'INVALID_CREDENTIALS'; const INVALID_AUTH_TOKEN_ERROR = 'INVALID_AUTH_TOKEN'; + const INVALID_API_KEY_ERROR = 'INVALID_API_KEY'; const NOT_FOUND_ERROR = 'NOT_FOUND'; const UNKNOWN_ERROR = 'UNKNOWN_ERROR'; diff --git a/module/Rest/test/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index d61da421..57522852 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -3,10 +3,8 @@ namespace ShlinkioTest\Shlink\Rest\Action; use PHPUnit_Framework_TestCase as TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Core\Entity\RestToken; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; -use Shlinkio\Shlink\Rest\Exception\AuthenticationException; -use Shlinkio\Shlink\Rest\Service\RestTokenService; +use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; @@ -20,12 +18,12 @@ class AuthenticateActionTest extends TestCase /** * @var ObjectProphecy */ - protected $tokenService; + protected $apiKeyService; public function setUp() { - $this->tokenService = $this->prophesize(RestTokenService::class); - $this->action = new AuthenticateAction($this->tokenService->reveal(), Translator::factory([])); + $this->apiKeyService = $this->prophesize(ApiKeyService::class); + $this->action = new AuthenticateAction($this->apiKeyService->reveal(), Translator::factory([])); } /** @@ -40,34 +38,31 @@ class AuthenticateActionTest extends TestCase /** * @test */ - public function properCredentialsReturnTokenInResponse() + public function properApiKeyReturnsTokenInResponse() { - $this->tokenService->createToken('foo', 'bar')->willReturn( - (new RestToken())->setToken('abc-ABC') - )->shouldBeCalledTimes(1); + $this->apiKeyService->check('foo')->willReturn(true) + ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ - 'username' => 'foo', - 'password' => 'bar', + 'apiKey' => 'foo', ]); $response = $this->action->__invoke($request, new Response()); $this->assertEquals(200, $response->getStatusCode()); $response->getBody()->rewind(); - $this->assertEquals(['token' => 'abc-ABC'], json_decode($response->getBody()->getContents(), true)); + $this->assertTrue(strpos($response->getBody()->getContents(), '"token"') > 0); } /** * @test */ - public function authenticationExceptionsReturnErrorResponse() + public function invalidApiKeyReturnsErrorResponse() { - $this->tokenService->createToken('foo', 'bar')->willThrow(new AuthenticationException()) - ->shouldBeCalledTimes(1); + $this->apiKeyService->check('foo')->willReturn(false) + ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ - 'username' => 'foo', - 'password' => 'bar', + 'apiKey' => 'foo', ]); $response = $this->action->__invoke($request, new Response()); $this->assertEquals(401, $response->getStatusCode()); From a60080b1ce6f7835716822d5936999831af4d411 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 14:44:33 +0200 Subject: [PATCH 11/39] Created JWTService and related classes --- .env.dist | 1 + composer.json | 3 +- config/autoload/app_options.global.php | 10 ++ module/Core/config/app_options.config.php | 6 + module/Core/config/dependencies.config.php | 3 + module/Core/src/Options/AppOptions.php | 97 +++++++++++++++ module/Rest/src/Action/AuthenticateAction.php | 1 + module/Rest/src/Authentication/JWTService.php | 110 ++++++++++++++++++ .../Authentication/JWTServiceInterface.php | 47 ++++++++ .../src/Exception/AuthenticationException.php | 5 + .../test/Authentication/JWTServiceTest.php | 93 +++++++++++++++ 11 files changed, 375 insertions(+), 1 deletion(-) create mode 100644 config/autoload/app_options.global.php create mode 100644 module/Core/config/app_options.config.php create mode 100644 module/Core/src/Options/AppOptions.php create mode 100644 module/Rest/src/Authentication/JWTService.php create mode 100644 module/Rest/src/Authentication/JWTServiceInterface.php create mode 100644 module/Rest/test/Authentication/JWTServiceTest.php diff --git a/.env.dist b/.env.dist index 9b175618..d56f522f 100644 --- a/.env.dist +++ b/.env.dist @@ -1,5 +1,6 @@ # Application APP_ENV= +SECRET_KEY= SHORTENED_URL_SCHEMA= SHORTENED_URL_HOSTNAME= SHORTCODE_CHARS= diff --git a/composer.json b/composer.json index 829bb949..72391371 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,8 @@ "acelaya/zsm-annotated-services": "^0.2.0", "doctrine/orm": "^2.5", "guzzlehttp/guzzle": "^6.2", - "symfony/console": "^3.0" + "symfony/console": "^3.0", + "firebase/php-jwt": "^4.0" }, "require-dev": { "phpunit/phpunit": "^5.0", diff --git a/config/autoload/app_options.global.php b/config/autoload/app_options.global.php new file mode 100644 index 00000000..4db642ce --- /dev/null +++ b/config/autoload/app_options.global.php @@ -0,0 +1,10 @@ + [ + 'name' => 'Shlink', + 'version' => '1.1.0', + 'secret_key' => env('SECRET_KEY'), + ], + +]; diff --git a/module/Core/config/app_options.config.php b/module/Core/config/app_options.config.php new file mode 100644 index 00000000..bf224541 --- /dev/null +++ b/module/Core/config/app_options.config.php @@ -0,0 +1,6 @@ + [], + +]; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 9bcacbfa..27983069 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -1,12 +1,15 @@ [ 'factories' => [ + AppOptions::class => AnnotatedFactory::class, + // Services Service\UrlShortener::class => AnnotatedFactory::class, Service\VisitsTracker::class => AnnotatedFactory::class, diff --git a/module/Core/src/Options/AppOptions.php b/module/Core/src/Options/AppOptions.php new file mode 100644 index 00000000..6ee1322c --- /dev/null +++ b/module/Core/src/Options/AppOptions.php @@ -0,0 +1,97 @@ +name; + } + + /** + * @param string $name + * @return $this + */ + protected function setName($name) + { + $this->name = $name; + return $this; + } + + /** + * @return string + */ + public function getVersion() + { + return $this->version; + } + + /** + * @param string $version + * @return $this + */ + protected function setVersion($version) + { + $this->version = $version; + return $this; + } + + /** + * @return mixed + */ + public function getSecretKey() + { + return $this->secretKey; + } + + /** + * @param mixed $secretKey + * @return $this + */ + protected function setSecretKey($secretKey) + { + $this->secretKey = $secretKey; + return $this; + } + + /** + * @return string + */ + public function __toString() + { + return sprintf('%s:v%s', $this->name, $this->version); + } +} diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 0fcac3f9..093e935f 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Firebase\JWT\JWT; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Rest\Service\ApiKeyService; diff --git a/module/Rest/src/Authentication/JWTService.php b/module/Rest/src/Authentication/JWTService.php new file mode 100644 index 00000000..bc1647c2 --- /dev/null +++ b/module/Rest/src/Authentication/JWTService.php @@ -0,0 +1,110 @@ +appOptions = $appOptions; + } + + /** + * Creates a new JSON web token por provided API key + * + * @param ApiKey $apiKey + * @param int $lifetime + * @return string + */ + public function create(ApiKey $apiKey, $lifetime = self::DEFAULT_LIFETIME) + { + $currentTimestamp = time(); + + return $this->encode([ + 'iss' => $this->appOptions->__toString(), + 'iat' => $currentTimestamp, + 'exp' => $currentTimestamp + $lifetime, + 'sub' => 'auth', + 'key' => $apiKey->getId(), // The ID is opaque. Returning the key would be insecure + ]); + } + + /** + * Refreshes a token and returns it with the new expiration + * + * @param string $jwt + * @param int $lifetime + * @return string + * @throws AuthenticationException If the token has expired + */ + public function refresh($jwt, $lifetime = self::DEFAULT_LIFETIME) + { + $payload = $this->getPayload($jwt); + $payload['exp'] = time() + $lifetime; + return $this->encode($payload); + } + + /** + * Verifies that certain JWT is valid + * + * @param string $jwt + * @return bool + */ + public function verify($jwt) + { + try { + // If no exception is thrown while decoding the token, it is considered valid + $this->decode($jwt); + return true; + } catch (\UnexpectedValueException $e) { + return false; + } + } + + /** + * Decodes certain token and returns the payload + * + * @param string $jwt + * @return array + * @throws AuthenticationException If the token has expired + */ + public function getPayload($jwt) + { + try { + return $this->decode($jwt); + } catch (\UnexpectedValueException $e) { + throw AuthenticationException::expiredJWT($e); + } + } + + /** + * @param array $data + * @return string + */ + protected function encode(array $data) + { + return JWT::encode($data, $this->appOptions->getSecretKey(), self::DEFAULT_ENCRYPTION_ALG); + } + + /** + * @param $jwt + * @return array + */ + protected function decode($jwt) + { + return (array) JWT::decode($jwt, $this->appOptions->getSecretKey(), [self::DEFAULT_ENCRYPTION_ALG]); + } +} diff --git a/module/Rest/src/Authentication/JWTServiceInterface.php b/module/Rest/src/Authentication/JWTServiceInterface.php new file mode 100644 index 00000000..278e6c67 --- /dev/null +++ b/module/Rest/src/Authentication/JWTServiceInterface.php @@ -0,0 +1,47 @@ + "%s". Password -> "%s"', $username, $password)); } + + public static function expiredJWT(\Exception $prev = null) + { + return new self('The token has expired.', -1, $prev); + } } diff --git a/module/Rest/test/Authentication/JWTServiceTest.php b/module/Rest/test/Authentication/JWTServiceTest.php new file mode 100644 index 00000000..ede0b6c6 --- /dev/null +++ b/module/Rest/test/Authentication/JWTServiceTest.php @@ -0,0 +1,93 @@ +service = new JWTService(new AppOptions([ + 'name' => 'ShlinkTest', + 'version' => '10000.3.1', + 'secret_key' => 'foo', + ])); + } + + /** + * @test + */ + public function tokenIsProperlyCreated() + { + $id = 34; + $token = $this->service->create((new ApiKey())->setId($id)); + $payload = (array) JWT::decode($token, 'foo', [JWTService::DEFAULT_ENCRYPTION_ALG]); + $this->assertGreaterThanOrEqual($payload['iat'], time()); + $this->assertGreaterThan(time(), $payload['exp']); + $this->assertEquals($id, $payload['key']); + $this->assertEquals('auth', $payload['sub']); + $this->assertEquals('ShlinkTest:v10000.3.1', $payload['iss']); + } + + /** + * @test + */ + public function refreshIncreasesExpiration() + { + $originalLifetime = 10; + $newLifetime = 30; + $originalPayload = ['exp' => time() + $originalLifetime]; + $token = JWT::encode($originalPayload, 'foo'); + $newToken = $this->service->refresh($token, $newLifetime); + $newPayload = (array) JWT::decode($newToken, 'foo', [JWTService::DEFAULT_ENCRYPTION_ALG]); + + $this->assertGreaterThan($originalPayload['exp'], $newPayload['exp']); + } + + /** + * @test + */ + public function verifyReturnsTrueWhenTheTokenIsCorrect() + { + $this->assertTrue($this->service->verify(JWT::encode([], 'foo'))); + } + + /** + * @test + */ + public function verifyReturnsFalseWhenTheTokenIsCorrect() + { + $this->assertFalse($this->service->verify('invalidToken')); + } + + /** + * @test + */ + public function getPayloadWorksWithCorrectTokens() + { + $originalPayload = [ + 'exp' => time() + 10, + 'sub' => 'testing', + ]; + $token = JWT::encode($originalPayload, 'foo'); + $this->assertEquals($originalPayload, $this->service->getPayload($token)); + } + + /** + * @test + * @expectedException \Shlinkio\Shlink\Rest\Exception\AuthenticationException + */ + public function getPayloadThrowsExceptionWithIncorrectTokens() + { + $this->service->getPayload('invalidToken'); + } +} From 9573e9f4ef8187d8838205191902e663264e3cd6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 19:13:40 +0200 Subject: [PATCH 12/39] Updated AuthenticateAction to generate and return a JWT --- module/Rest/config/dependencies.config.php | 2 ++ module/Rest/src/Action/AuthenticateAction.php | 26 ++++++++++++++----- module/Rest/src/Authentication/JWTService.php | 3 +++ module/Rest/src/Service/ApiKeyService.php | 21 ++++++++++----- .../src/Service/ApiKeyServiceInterface.php | 8 ++++++ .../test/Action/AuthenticateActionTest.php | 21 +++++++++++---- 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 8f6dbbfb..e0cc13f7 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -1,6 +1,7 @@ [ 'factories' => [ + JWTService::class => AnnotatedFactory::class, Service\RestTokenService::class => AnnotatedFactory::class, Service\ApiKeyService::class => AnnotatedFactory::class, diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 093e935f..020a0fb5 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -5,6 +5,8 @@ use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Firebase\JWT\JWT; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Shlinkio\Shlink\Rest\Authentication\JWTService; +use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Shlinkio\Shlink\Rest\Util\RestUtils; @@ -21,18 +23,27 @@ class AuthenticateAction extends AbstractRestAction * @var ApiKeyService|ApiKeyServiceInterface */ private $apiKeyService; + /** + * @var JWTServiceInterface + */ + private $jwtService; /** * AuthenticateAction constructor. * @param ApiKeyServiceInterface|ApiKeyService $apiKeyService + * @param JWTServiceInterface|JWTService $jwtService * @param TranslatorInterface $translator * - * @Inject({ApiKeyService::class, "translator"}) + * @Inject({ApiKeyService::class, JWTService::class, "translator"}) */ - public function __construct(ApiKeyServiceInterface $apiKeyService, TranslatorInterface $translator) - { + public function __construct( + ApiKeyServiceInterface $apiKeyService, + JWTServiceInterface $jwtService, + TranslatorInterface $translator + ) { $this->translator = $translator; $this->apiKeyService = $apiKeyService; + $this->jwtService = $jwtService; } /** @@ -54,15 +65,16 @@ class AuthenticateAction extends AbstractRestAction } // Authenticate using provided API key - if (! $this->apiKeyService->check($authData['apiKey'])) { + $apiKey = $this->apiKeyService->getByKey($authData['apiKey']); + if (! $apiKey->isValid()) { return new JsonResponse([ 'error' => RestUtils::INVALID_API_KEY_ERROR, 'message' => $this->translator->translate('Provided API key does not exist or is invalid.'), ], 401); } - // TODO Generate a JSON Web Token that will be used for authorization in next requests - - return new JsonResponse(['token' => '']); + // Generate a JSON Web Token that will be used for authorization in next requests + $token = $this->jwtService->create($apiKey); + return new JsonResponse(['token' => $token]); } } diff --git a/module/Rest/src/Authentication/JWTService.php b/module/Rest/src/Authentication/JWTService.php index bc1647c2..ee252606 100644 --- a/module/Rest/src/Authentication/JWTService.php +++ b/module/Rest/src/Authentication/JWTService.php @@ -1,6 +1,7 @@ em->getRepository(ApiKey::class)->findOneBy([ - 'key' => $key, - ]); + $apiKey = $this->getByKey($key); if (! isset($apiKey)) { return false; } @@ -71,9 +69,7 @@ class ApiKeyService implements ApiKeyServiceInterface public function disable($key) { /** @var ApiKey $apiKey */ - $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ - 'key' => $key, - ]); + $apiKey = $this->getByKey($key); if (! isset($apiKey)) { throw new InvalidArgumentException(sprintf('API key "%s" does not exist and can\'t be disabled', $key)); } @@ -94,4 +90,17 @@ class ApiKeyService implements ApiKeyServiceInterface $conditions = $enabledOnly ? ['enabled' => true] : []; return $this->em->getRepository(ApiKey::class)->findBy($conditions); } + + /** + * Tries to find one API key by its key string + * + * @param string $key + * @return ApiKey|null + */ + public function getByKey($key) + { + return $this->em->getRepository(ApiKey::class)->findOneBy([ + 'key' => $key, + ]); + } } diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index 84c856ce..e1b8ce53 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -36,4 +36,12 @@ interface ApiKeyServiceInterface * @return ApiKey[] */ public function listKeys($enabledOnly = false); + + /** + * Tries to find one API key by its key string + * + * @param string $key + * @return ApiKey|null + */ + public function getByKey($key); } diff --git a/module/Rest/test/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index 57522852..240b60a6 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -4,6 +4,8 @@ namespace ShlinkioTest\Shlink\Rest\Action; use PHPUnit_Framework_TestCase as TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; +use Shlinkio\Shlink\Rest\Authentication\JWTService; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; @@ -19,11 +21,20 @@ class AuthenticateActionTest extends TestCase * @var ObjectProphecy */ protected $apiKeyService; + /** + * @var ObjectProphecy + */ + protected $jwtService; public function setUp() { $this->apiKeyService = $this->prophesize(ApiKeyService::class); - $this->action = new AuthenticateAction($this->apiKeyService->reveal(), Translator::factory([])); + $this->jwtService = $this->prophesize(JWTService::class); + $this->action = new AuthenticateAction( + $this->apiKeyService->reveal(), + $this->jwtService->reveal(), + Translator::factory([]) + ); } /** @@ -40,8 +51,8 @@ class AuthenticateActionTest extends TestCase */ public function properApiKeyReturnsTokenInResponse() { - $this->apiKeyService->check('foo')->willReturn(true) - ->shouldBeCalledTimes(1); + $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->setId(5)) + ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'apiKey' => 'foo', @@ -58,8 +69,8 @@ class AuthenticateActionTest extends TestCase */ public function invalidApiKeyReturnsErrorResponse() { - $this->apiKeyService->check('foo')->willReturn(false) - ->shouldBeCalledTimes(1); + $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->setEnabled(false)) + ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'apiKey' => 'foo', From 7b0beb3b8c76eaed206ce896b8e7800fb6de80a8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 19:53:14 +0200 Subject: [PATCH 13/39] Updated CheckAuthenticationMiddleware to work with JWT and the Authorization header --- .../CheckAuthenticationMiddleware.php | 65 +++++++++++++------ module/Rest/src/Util/RestUtils.php | 1 + .../CheckAuthenticationMiddlewareTest.php | 57 ++++++++++++---- 3 files changed, 91 insertions(+), 32 deletions(-) diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index 1ad53c4b..39c670e6 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php @@ -4,9 +4,9 @@ namespace Shlinkio\Shlink\Rest\Middleware; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; -use Shlinkio\Shlink\Rest\Service\RestTokenService; -use Shlinkio\Shlink\Rest\Service\RestTokenServiceInterface; +use Shlinkio\Shlink\Rest\Authentication\JWTService; +use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; +use Shlinkio\Shlink\Rest\Exception\AuthenticationException; use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; use Zend\Expressive\Router\RouteResult; @@ -15,28 +15,28 @@ use Zend\Stratigility\MiddlewareInterface; class CheckAuthenticationMiddleware implements MiddlewareInterface { - const AUTH_TOKEN_HEADER = 'X-Auth-Token'; + const AUTHORIZATION_HEADER = 'Authorization'; - /** - * @var RestTokenServiceInterface - */ - private $restTokenService; /** * @var TranslatorInterface */ private $translator; + /** + * @var JWTServiceInterface + */ + private $jwtService; /** * CheckAuthenticationMiddleware constructor. - * @param RestTokenServiceInterface|RestTokenService $restTokenService + * @param JWTServiceInterface|JWTService $jwtService * @param TranslatorInterface $translator * - * @Inject({RestTokenService::class, "translator"}) + * @Inject({JWTService::class, "translator"}) */ - public function __construct(RestTokenServiceInterface $restTokenService, TranslatorInterface $translator) + public function __construct(JWTServiceInterface $jwtService, TranslatorInterface $translator) { - $this->restTokenService = $restTokenService; $this->translator = $translator; + $this->jwtService = $jwtService; } /** @@ -78,21 +78,46 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface } // Check that the auth header was provided, and that it belongs to a non-expired token - if (! $request->hasHeader(self::AUTH_TOKEN_HEADER)) { + if (! $request->hasHeader(self::AUTHORIZATION_HEADER)) { return $this->createTokenErrorResponse(); } - $authToken = $request->getHeaderLine(self::AUTH_TOKEN_HEADER); + // Get token making sure the an authorization type is provided + $authToken = $request->getHeaderLine(self::AUTHORIZATION_HEADER); + $authTokenParts = explode(' ', $authToken); + if (count($authTokenParts) === 1) { + return new JsonResponse([ + 'error' => RestUtils::INVALID_AUTHORIZATION_ERROR, + 'message' => sprintf($this->translator->translate( + 'You need to provide the Bearer type in the %s header.' + ), self::AUTHORIZATION_HEADER), + ], 401); + } + + // Make sure the authorization type is Bearer + list($authType, $jwt) = $authTokenParts; + if (strtolower($authType) !== 'bearer') { + return new JsonResponse([ + 'error' => RestUtils::INVALID_AUTHORIZATION_ERROR, + 'message' => sprintf($this->translator->translate( + 'Provided authorization type %s is not supported. Use Bearer instead.' + ), $authType), + ], 401); + } + try { - $restToken = $this->restTokenService->getByToken($authToken); - if ($restToken->isExpired()) { + if (! $this->jwtService->verify($jwt)) { return $this->createTokenErrorResponse(); } // Update the token expiration and continue to next middleware - $this->restTokenService->updateExpiration($restToken); - return $out($request, $response); - } catch (InvalidArgumentException $e) { + $jwt = $this->jwtService->refresh($jwt); + /** @var Response $response */ + $response = $out($request, $response); + + // Return the response with the updated token on it + return $response->withHeader(self::AUTHORIZATION_HEADER, 'Bearer ' . $jwt); + } catch (AuthenticationException $e) { return $this->createTokenErrorResponse(); } } @@ -106,7 +131,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' . 'token on every new request on the "%s" header' ), - self::AUTH_TOKEN_HEADER + self::AUTHORIZATION_HEADER ), ], 401); } diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php index 8326487e..4f4332c4 100644 --- a/module/Rest/src/Util/RestUtils.php +++ b/module/Rest/src/Util/RestUtils.php @@ -12,6 +12,7 @@ class RestUtils const INVALID_ARGUMENT_ERROR = 'INVALID_ARGUMENT'; const INVALID_CREDENTIALS_ERROR = 'INVALID_CREDENTIALS'; const INVALID_AUTH_TOKEN_ERROR = 'INVALID_AUTH_TOKEN'; + const INVALID_AUTHORIZATION_ERROR = 'INVALID_AUTHORIZATION'; const INVALID_API_KEY_ERROR = 'INVALID_API_KEY'; const NOT_FOUND_ERROR = 'NOT_FOUND'; const UNKNOWN_ERROR = 'UNKNOWN_ERROR'; diff --git a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php index 650d4d2f..5d8dce7c 100644 --- a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php @@ -4,8 +4,8 @@ namespace ShlinkioTest\Shlink\Rest\Middleware; use PHPUnit_Framework_TestCase as TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\RestToken; +use Shlinkio\Shlink\Rest\Authentication\JWTService; use Shlinkio\Shlink\Rest\Middleware\CheckAuthenticationMiddleware; -use Shlinkio\Shlink\Rest\Service\RestTokenService; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; use Zend\Expressive\Router\RouteResult; @@ -20,18 +20,18 @@ class CheckAuthenticationMiddlewareTest extends TestCase /** * @var ObjectProphecy */ - protected $tokenService; + protected $jwtService; public function setUp() { - $this->tokenService = $this->prophesize(RestTokenService::class); - $this->middleware = new CheckAuthenticationMiddleware($this->tokenService->reveal(), Translator::factory([])); + $this->jwtService = $this->prophesize(JWTService::class); + $this->middleware = new CheckAuthenticationMiddleware($this->jwtService->reveal(), Translator::factory([])); } /** * @test */ - public function someWhitelistedSituationsFallbackToNextMiddleware() + public function someWhiteListedSituationsFallbackToNextMiddleware() { $request = ServerRequestFactory::fromGlobals(); $response = new Response(); @@ -92,6 +92,40 @@ class CheckAuthenticationMiddlewareTest extends TestCase $this->assertEquals(401, $response->getStatusCode()); } + /** + * @test + */ + public function provideAnAuthorizationWithoutTypeReturnsError() + { + $authToken = 'ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRouteMatch('bar', 'foo', []) + )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); + + $response = $this->middleware->__invoke($request, new Response()); + $this->assertEquals(401, $response->getStatusCode()); + $this->assertTrue(strpos($response->getBody()->getContents(), 'You need to provide the Bearer type') > 0); + } + + /** + * @test + */ + public function provideAnAuthorizationWithWrongTypeReturnsError() + { + $authToken = 'ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRouteMatch('bar', 'foo', []) + )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Basic ' . $authToken); + + $response = $this->middleware->__invoke($request, new Response()); + $this->assertEquals(401, $response->getStatusCode()); + $this->assertTrue( + strpos($response->getBody()->getContents(), 'Provided authorization type Basic is not supported') > 0 + ); + } + /** * @test */ @@ -101,10 +135,8 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRouteMatch('bar', 'foo', []) - )->withHeader(CheckAuthenticationMiddleware::AUTH_TOKEN_HEADER, $authToken); - $this->tokenService->getByToken($authToken)->willReturn( - (new RestToken())->setExpirationDate((new \DateTime())->sub(new \DateInterval('P1D'))) - )->shouldBeCalledTimes(1); + )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Bearer ' . $authToken); + $this->jwtService->verify($authToken)->willReturn(false)->shouldBeCalledTimes(1); $response = $this->middleware->__invoke($request, new Response()); $this->assertEquals(401, $response->getStatusCode()); @@ -120,14 +152,15 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRouteMatch('bar', 'foo', []) - )->withHeader(CheckAuthenticationMiddleware::AUTH_TOKEN_HEADER, $authToken); - $this->tokenService->getByToken($authToken)->willReturn($restToken)->shouldBeCalledTimes(1); - $this->tokenService->updateExpiration($restToken)->shouldBeCalledTimes(1); + )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'bearer ' . $authToken); + $this->jwtService->verify($authToken)->willReturn(true)->shouldBeCalledTimes(1); + $this->jwtService->refresh($authToken)->willReturn($authToken)->shouldBeCalledTimes(1); $isCalled = false; $this->assertFalse($isCalled); $this->middleware->__invoke($request, new Response(), function ($req, $resp) use (&$isCalled) { $isCalled = true; + return $resp; }); $this->assertTrue($isCalled); } From 258f954a388337ca86a40f90c42b95b6491be38e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 19:57:23 +0200 Subject: [PATCH 14/39] Deleted rest token related classes --- module/Core/src/Entity/RestToken.php | 103 ------------------ module/Rest/config/dependencies.config.php | 1 - module/Rest/src/Service/RestTokenService.php | 98 ----------------- .../src/Service/RestTokenServiceInterface.php | 32 ------ .../CheckAuthenticationMiddlewareTest.php | 2 - .../test/Service/RestTokenServiceTest.php | 93 ---------------- 6 files changed, 329 deletions(-) delete mode 100644 module/Core/src/Entity/RestToken.php delete mode 100644 module/Rest/src/Service/RestTokenService.php delete mode 100644 module/Rest/src/Service/RestTokenServiceInterface.php delete mode 100644 module/Rest/test/Service/RestTokenServiceTest.php diff --git a/module/Core/src/Entity/RestToken.php b/module/Core/src/Entity/RestToken.php deleted file mode 100644 index 865c83b9..00000000 --- a/module/Core/src/Entity/RestToken.php +++ /dev/null @@ -1,103 +0,0 @@ -updateExpiration(); - $this->setRandomTokenKey(); - } - - /** - * @return \DateTime - */ - public function getExpirationDate() - { - return $this->expirationDate; - } - - /** - * @param \DateTime $expirationDate - * @return $this - */ - public function setExpirationDate($expirationDate) - { - $this->expirationDate = $expirationDate; - return $this; - } - - /** - * @return string - */ - public function getToken() - { - return $this->token; - } - - /** - * @param string $token - * @return $this - */ - public function setToken($token) - { - $this->token = $token; - return $this; - } - - /** - * @return bool - */ - public function isExpired() - { - return new \DateTime() > $this->expirationDate; - } - - /** - * Updates the expiration of the token, setting it to the default interval in the future - * @return $this - */ - public function updateExpiration() - { - return $this->setExpirationDate((new \DateTime())->add(new \DateInterval(self::DEFAULT_INTERVAL))); - } - - /** - * Sets a random unique token key for this RestToken - * @return RestToken - */ - public function setRandomTokenKey() - { - return $this->setToken($this->generateV4Uuid()); - } -} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index e0cc13f7..685de2c3 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -11,7 +11,6 @@ return [ 'dependencies' => [ 'factories' => [ JWTService::class => AnnotatedFactory::class, - Service\RestTokenService::class => AnnotatedFactory::class, Service\ApiKeyService::class => AnnotatedFactory::class, Action\AuthenticateAction::class => AnnotatedFactory::class, diff --git a/module/Rest/src/Service/RestTokenService.php b/module/Rest/src/Service/RestTokenService.php deleted file mode 100644 index b9dd4a9d..00000000 --- a/module/Rest/src/Service/RestTokenService.php +++ /dev/null @@ -1,98 +0,0 @@ -em = $em; - $this->restConfig = $restConfig; - } - - /** - * @param string $token - * @return RestToken - * @throws InvalidArgumentException - */ - public function getByToken($token) - { - $restToken = $this->em->getRepository(RestToken::class)->findOneBy([ - 'token' => $token, - ]); - if (! isset($restToken)) { - throw new InvalidArgumentException(sprintf('RestToken not found for token "%s"', $token)); - } - - return $restToken; - } - - /** - * Creates and returns a new RestToken if username and password are correct - * @param $username - * @param $password - * @return RestToken - * @throws AuthenticationException - */ - public function createToken($username, $password) - { - $this->processCredentials($username, $password); - - $restToken = new RestToken(); - $this->em->persist($restToken); - $this->em->flush(); - - return $restToken; - } - - /** - * @param string $username - * @param string $password - */ - protected function processCredentials($username, $password) - { - $configUsername = strtolower(trim($this->restConfig['username'])); - $providedUsername = strtolower(trim($username)); - $configPassword = trim($this->restConfig['password']); - $providedPassword = trim($password); - - if ($configUsername === $providedUsername && $configPassword === $providedPassword) { - return; - } - - // If credentials are not correct, throw exception - throw AuthenticationException::fromCredentials($providedUsername, $providedPassword); - } - - /** - * Updates the expiration of provided token, extending its life - * - * @param RestToken $token - */ - public function updateExpiration(RestToken $token) - { - $token->updateExpiration(); - $this->em->flush(); - } -} diff --git a/module/Rest/src/Service/RestTokenServiceInterface.php b/module/Rest/src/Service/RestTokenServiceInterface.php deleted file mode 100644 index 1e03cbaa..00000000 --- a/module/Rest/src/Service/RestTokenServiceInterface.php +++ /dev/null @@ -1,32 +0,0 @@ -setExpirationDate((new \DateTime())->add(new \DateInterval('P1D'))); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRouteMatch('bar', 'foo', []) diff --git a/module/Rest/test/Service/RestTokenServiceTest.php b/module/Rest/test/Service/RestTokenServiceTest.php deleted file mode 100644 index d4487ff1..00000000 --- a/module/Rest/test/Service/RestTokenServiceTest.php +++ /dev/null @@ -1,93 +0,0 @@ -em = $this->prophesize(EntityManager::class); - $this->service = new RestTokenService($this->em->reveal(), [ - 'username' => 'foo', - 'password' => 'bar', - ]); - } - - /** - * @test - */ - public function tokenIsCreatedIfCredentialsAreCorrect() - { - $this->em->persist(Argument::type(RestToken::class))->shouldBeCalledTimes(1); - $this->em->flush()->shouldBeCalledTimes(1); - - $token = $this->service->createToken('foo', 'bar'); - $this->assertInstanceOf(RestToken::class, $token); - $this->assertFalse($token->isExpired()); - } - - /** - * @test - * @expectedException \Shlinkio\Shlink\Rest\Exception\AuthenticationException - */ - public function exceptionIsThrownWhileCreatingTokenWithWrongCredentials() - { - $this->service->createToken('foo', 'wrong'); - } - - /** - * @test - */ - public function restTokenIsReturnedFromTokenString() - { - $authToken = 'ABC-abc'; - $theToken = new RestToken(); - $repo = $this->prophesize(EntityRepository::class); - $repo->findOneBy(['token' => $authToken])->willReturn($theToken)->shouldBeCalledTimes(1); - $this->em->getRepository(RestToken::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); - - $this->assertSame($theToken, $this->service->getByToken($authToken)); - } - - /** - * @test - * @expectedException \Shlinkio\Shlink\Common\Exception\InvalidArgumentException - */ - public function exceptionIsThrownWhenRequestingWrongToken() - { - $authToken = 'ABC-abc'; - $repo = $this->prophesize(EntityRepository::class); - $repo->findOneBy(['token' => $authToken])->willReturn(null)->shouldBeCalledTimes(1); - $this->em->getRepository(RestToken::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); - - $this->service->getByToken($authToken); - } - - /** - * @test - */ - public function updateExpirationFlushesEntityManager() - { - $token = $this->prophesize(RestToken::class); - $token->updateExpiration()->shouldBeCalledTimes(1); - $this->em->flush()->shouldBeCalledTimes(1); - - $this->service->updateExpiration($token->reveal()); - } -} From 2a089f05b17a816e61d26d291c169b583b683d23 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 20:21:38 +0200 Subject: [PATCH 15/39] Updated languages --- module/CLI/lang/es.mo | Bin 4071 -> 5096 bytes module/CLI/lang/es.po | 44 +++++++++++++++++++++++++++++++++++++++-- module/Rest/lang/es.mo | Bin 1897 -> 2200 bytes module/Rest/lang/es.po | 34 +++++++++++++++++++++++-------- 4 files changed, 68 insertions(+), 10 deletions(-) diff --git a/module/CLI/lang/es.mo b/module/CLI/lang/es.mo index b51c40cb39c8ec9d16ff4e818f2aa38a26f83482..1f88545ff8ea4ea09889369e6d732a63b4a2e971 100644 GIT binary patch delta 1982 zcmZwGUrZcD9Ki8`9zrQa3T^3MIz&X z?fboN@#pr;lA;aM_s~DAQ|c%VZs0;2$tcx{kKhw{0w?e-%;61`@AfuS9YuNn1w4Ww zO1#go75~I`%-lm)YEgA^)53#2xE1^HUVI#P<1}u;mrw#+!Z+|u?82>$O6|mlu^FF1 zS^H`1z!z~p&f`XW8#TU*EzGaJ;6?&}iw^#Xo3W`$sYAFO58yO1q+UUJ|232yS;7bK z2b2Z;jR$cfvu(%6Z~{-_0A9gu_!V|Czxt^*a0?|s7Z=%)e%ysw+)~fhqhyk2a139? zDf}A8v4d6b!!!6GmT@m$!{fMuC$WRo%TAxg#U0#S;)b%QC6wB{gYx00wg10E$?zAH zl{ZlzVyP~a!!wBYg8+43_;`PF6QPrc9O1b&Xgm>~=KE{jiM4&|)8j;HY>%;G=o?7y7WEGI=) zIE^xJ4vD2cK;Bi?QR6pQhY!)IR$3lN+kG`SO2>jQMwr|Rj_hUl_*X_AaI`K8Dqmh$$}4@+Gg zIweb+{B-0Hafnr4`lNd40jb$0x@@IvZ-3?RhMOZZllq)3>+asTTf3o+br2@nUi9L` zQAcWny|JE;!g(9{W$k*gnf0w(>B)TGldJv{YZK_&%Z?hqIPXO!@xnm6Cb5;jGJTD; z!Ag6><*uh}U>RpBqXTY1o~cjI|BYY1A=WR#U7-8{gM)A&e4z>e;C{ zeMSTxCIzca$#}keuo@?JmTxqa>lUr=$J(3IVZhpJG1uOactzjUvsOnoDMo>H9rYaH zRufOc)qK;(l8*KQeWBpx3+cS8FV+HCN!>d7RLqisvM#lyxy?L1qvyityh)tO@y1(? zhxEar(cx@mvT1+qeq?!}>Am`6Q%2`~Q?lBvt|6^T5LWA99d&g5AXRZ~Trjhq?= zWvu1e^VN|wQc$#|yWh~Im8(?JIBN225Js}1$r*|6hk4@{!Z=amak6q%-&t}sp~~%+ zt^ujB(MjlH+QCyFtaw$ zv5mMZ6|J9`*irx8ZP|*o{fuOIF5Mu$^o?F9lF3@ekJi>jT&g1nKBDXirFfZlipF0k Xdd$Yz4!NcqLm30jQE)1^TmSeMvcP;n delta 987 zcmYk*O=uHA6u|M>nhz6g^W{fsYdWd6R-sA)QK*P1wuM4!t3^=3!{P?3EDdCP5iPli zC!q{Ih#&}h5RW+uUbKP-4=r81qC<9nGqbZh^WIFpANkhP_?!+c zDoP($H`j=zR0-WSZj|y4rS{`6p2aITh7WNBzqFqBg&0Ho44%UVcHnDF;|lUq>)dwZ z5A0%?`a?wqX2MF{$6oBj7ns2%jNwPrxQ6n9P$YOSg~w@YY{yAFh1c)|E@BjyQ094$ z638bUV1Cuj(>NWYcnqg-FWy0ke4*9<9A(@Glm)Ef0sM(6vK8H_RGxMxgUWasD|i=2 z@f{w-#4Z+zgE-CnYM#m-T*fSZMS@gYD5(oG+W=-!A{<89vq_Zk>nK~diOiw87&L_a zNSCrvBA>-EoJW4@4Yx*_$_f=pNrq8mMNMN0ui^!Cv4F2oUi^h8aT6s&r9`lR36%a@ zDDOW(380A@-=c-GtML4h~?B23za@GYHf_gxk*#wS#=3eB%;P{Q%TD|IMw|3ojXKLP@wjHNl^*mL%DGk@v-b}4Nr!QTsc+G|AZOc50 z{f-uNu`pCD7!%L8>c!^E_~W+bv-X%}Ry&TGT;gfg|G4egdVESd)qiuCY%7q diff --git a/module/CLI/lang/es.po b/module/CLI/lang/es.po index 8bc5d32b..968701ea 100644 --- a/module/CLI/lang/es.po +++ b/module/CLI/lang/es.po @@ -1,8 +1,8 @@ msgid "" msgstr "" "Project-Id-Version: Shlink 1.0\n" -"POT-Creation-Date: 2016-08-01 21:21+0200\n" -"PO-Revision-Date: 2016-08-01 21:22+0200\n" +"POT-Creation-Date: 2016-08-07 20:16+0200\n" +"PO-Revision-Date: 2016-08-07 20:18+0200\n" "Last-Translator: Alejandro Celaya \n" "Language-Team: \n" "Language: es_ES\n" @@ -17,6 +17,46 @@ msgstr "" "X-Poedit-SearchPath-0: src\n" "X-Poedit-SearchPath-1: config\n" +msgid "Disables an API key." +msgstr "Desahbilita una clave de API." + +msgid "The API key to disable" +msgstr "La clave de API a deshabilitar" + +#, php-format +msgid "API key %s properly disabled" +msgstr "Clave de API %s deshabilitada correctamente" + +#, php-format +msgid "API key \"%s\" does not exist." +msgstr "La clave de API \"%s\" no existe." + +msgid "Generates a new valid API key." +msgstr "Genera una nueva clave de API válida." + +msgid "The date in which the API key should expire. Use any valid PHP format." +msgstr "" +"La fecha en la que la clave de API debe expirar. Utiliza cualquier valor " +"válido en PHP." + +msgid "Generated API key" +msgstr "Generada clave de API" + +msgid "Lists all the available API keys." +msgstr "Lista todas las claves de API disponibles." + +msgid "Tells if only enabled API keys should be returned." +msgstr "Define si sólo las claves de API habilitadas deben ser devueltas." + +msgid "Key" +msgstr "Clave" + +msgid "Expiration date" +msgstr "Fecha de caducidad" + +msgid "Is enabled" +msgstr "Está habilitada" + #, php-format msgid "" "Generates a character set sample just by shuffling the default one, \"%s\". " diff --git a/module/Rest/lang/es.mo b/module/Rest/lang/es.mo index 1bfa9aefbc137b30b9e4597348190e0ec80aacf7..915466d15c785a69440ae36b00e4dc36dce073a8 100644 GIT binary patch delta 823 zcmY+B&1+LZ6vZb=lWP6Iiui$2uhs7Kk(O%O?i%W%C?ZPnYchWpMW`DW zf*TzKH!iYsUvMXgLHrBci6H30g)4C@o_R@&9k~2v9&^tIX9vpiqu=b)( zpdO(nQD0DDQ9+2q;3@DKxB#96KY|hX1FV6EibAY_m%;G;Id~d;4;}-*gLUvPcph9X zVNOwOhzHnA;ltCi5bwY@;05q*Whnd_yn^#*@D6xp9L?ZkunK+vgYnI#vlMI{lHMxl^U`R_U6CI9^Lc4ywcJx;uK)t-7=jtJF>3?UjxAooaEJr+?P zOh5)l;US!bY4`w3@B`Y=ju4H&6&SwXg9Gps=HN9k!2Nd-<51P zD_KQ)+UH8t&pAHCs`XRsDwR2F{WOoUMYA8@OwM!W2bwb_*WHS%xzjk>SV`FCJw7q4 Qxu@m-r7Cg*{ohvf4MyZ~wg3PC diff --git a/module/Rest/lang/es.po b/module/Rest/lang/es.po index 62f76e1d..c911722c 100644 --- a/module/Rest/lang/es.po +++ b/module/Rest/lang/es.po @@ -1,8 +1,8 @@ msgid "" msgstr "" "Project-Id-Version: Shlink 1.0\n" -"POT-Creation-Date: 2016-07-27 08:53+0200\n" -"PO-Revision-Date: 2016-07-27 08:53+0200\n" +"POT-Creation-Date: 2016-08-07 20:19+0200\n" +"PO-Revision-Date: 2016-08-07 20:21+0200\n" "Last-Translator: Alejandro Celaya \n" "Language-Team: \n" "Language: es_ES\n" @@ -17,11 +17,13 @@ msgstr "" "X-Poedit-SearchPath-0: config\n" "X-Poedit-SearchPath-1: src\n" -msgid "You have to provide both \"username\" and \"password\"" -msgstr "Debes proporcionar tanto \"username\" como \"password\"" +msgid "You have to provide a valid API key under the \"apiKey\" param name." +msgstr "" +"Debes proporcionar una clave de API válida bajo el nombre de parámetro " +"\"apiKey\"." -msgid "Invalid username and/or password" -msgstr "Usuario y/o contraseña no válidos" +msgid "Provided API key does not exist or is invalid." +msgstr "La clave de API proporcionada no existe o es inválida." msgid "A URL was not provided" msgstr "No se ha proporcionado una URL" @@ -47,6 +49,16 @@ msgstr "No se ha encontrado una URL para el código corto \"%s\"" msgid "Provided short code \"%s\" has an invalid format" msgstr "El código corto proporcionado \"%s\" tiene un formato no inválido" +#, php-format +msgid "You need to provide the Bearer type in the %s header." +msgstr "Debes proporcionar el typo Bearer en la cabecera %s." + +#, php-format +msgid "Provided authorization type %s is not supported. Use Bearer instead." +msgstr "" +"El tipo de autorización proporcionado %s no está soportado. En vez de eso " +"utiliza Bearer." + #, php-format msgid "" "Missing or invalid auth token provided. Perform a new authentication request " @@ -56,8 +68,14 @@ msgstr "" "una nueva petición de autenticación y envía el token proporcionado en cada " "nueva petición en la cabecera \"%s\"" -msgid "Requested route does not exist." -msgstr "La ruta solicitada no existe." +#~ msgid "You have to provide both \"username\" and \"password\"" +#~ msgstr "Debes proporcionar tanto \"username\" como \"password\"" + +#~ msgid "Invalid username and/or password" +#~ msgstr "Usuario y/o contraseña no válidos" + +#~ msgid "Requested route does not exist." +#~ msgstr "La ruta solicitada no existe." #~ msgid "RestToken not found for token \"%s\"" #~ msgstr "No se ha encontrado un RestToken para el token \"%s\"" From 57bc681b9e6ae114c037fd5d0e3d6e646a4cb477 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 20:30:19 +0200 Subject: [PATCH 16/39] Created command to generate a random secret key string --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 1 + .../Command/Config/GenerateSecretCommand.php | 45 +++++++++++++++++++ 3 files changed, 47 insertions(+) create mode 100644 module/CLI/src/Command/Config/GenerateSecretCommand.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 35624e51..adc49fa9 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -11,6 +11,7 @@ return [ Command\GetVisitsCommand::class, Command\ProcessVisitsCommand::class, Command\Config\GenerateCharsetCommand::class, + Command\Config\GenerateSecretCommand::class, Command\Api\GenerateKeyCommand::class, Command\Api\DisableKeyCommand::class, Command\Api\ListKeysCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index f3257fc6..18e05dcb 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -17,6 +17,7 @@ return [ Command\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\Config\GenerateCharsetCommand::class => AnnotatedFactory::class, + Command\Config\GenerateSecretCommand::class => AnnotatedFactory::class, Command\Api\GenerateKeyCommand::class => AnnotatedFactory::class, Command\Api\DisableKeyCommand::class => AnnotatedFactory::class, Command\Api\ListKeysCommand::class => AnnotatedFactory::class, diff --git a/module/CLI/src/Command/Config/GenerateSecretCommand.php b/module/CLI/src/Command/Config/GenerateSecretCommand.php new file mode 100644 index 00000000..bef5c86a --- /dev/null +++ b/module/CLI/src/Command/Config/GenerateSecretCommand.php @@ -0,0 +1,45 @@ +translator = $translator; + parent::__construct(null); + } + + public function configure() + { + $this->setName('config:generate-secret') + ->setDescription($this->translator->translate( + 'Generates a random secret string that can be used for JWT token encryption' + )); + } + + public function execute(InputInterface $input, OutputInterface $output) + { + $secret = $this->generateRandomString(32); + $output->writeln($this->translator->translate('Secret key:') . sprintf(' %s', $secret)); + } +} From 80d8c32881aff29eed9839a6e47fc7845046d9cf Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2016 20:47:43 +0200 Subject: [PATCH 17/39] Removed rest auth env vars --- .env.dist | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.env.dist b/.env.dist index d56f522f..9ecb9fe0 100644 --- a/.env.dist +++ b/.env.dist @@ -13,7 +13,3 @@ CLI_LOCALE= DB_USER= DB_PASSWORD= DB_NAME= - -# Rest authentication -REST_USER= -REST_PASSWORD= From a65003803b748fc81e95d7e6f8ff1ea6a19485ae Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 09:36:52 +0200 Subject: [PATCH 18/39] Updated namespace for Shortcode CLI commands --- module/CLI/config/cli.config.php | 8 ++++---- module/CLI/config/dependencies.config.php | 9 ++++----- .../Command/{ => Shortcode}/GenerateShortcodeCommand.php | 4 ++-- .../CLI/src/Command/{ => Shortcode}/GetVisitsCommand.php | 4 ++-- .../Command/{ => Shortcode}/ListShortcodesCommand.php | 4 ++-- .../src/Command/{ => Shortcode}/ResolveUrlCommand.php | 4 ++-- module/CLI/test/Command/GenerateShortcodeCommandTest.php | 2 +- module/CLI/test/Command/GetVisitsCommandTest.php | 2 +- module/CLI/test/Command/ListShortcodesCommandTest.php | 2 +- module/CLI/test/Command/ResolveUrlCommandTest.php | 2 +- 10 files changed, 20 insertions(+), 21 deletions(-) rename module/CLI/src/Command/{ => Shortcode}/GenerateShortcodeCommand.php (97%) rename module/CLI/src/Command/{ => Shortcode}/GetVisitsCommand.php (97%) rename module/CLI/src/Command/{ => Shortcode}/ListShortcodesCommand.php (96%) rename module/CLI/src/Command/{ => Shortcode}/ResolveUrlCommand.php (96%) diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index adc49fa9..699d4275 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -5,10 +5,10 @@ return [ 'cli' => [ 'commands' => [ - Command\GenerateShortcodeCommand::class, - Command\ResolveUrlCommand::class, - Command\ListShortcodesCommand::class, - Command\GetVisitsCommand::class, + Command\Shortcode\GenerateShortcodeCommand::class, + Command\Shortcode\ResolveUrlCommand::class, + Command\Shortcode\ListShortcodesCommand::class, + Command\Shortcode\GetVisitsCommand::class, Command\ProcessVisitsCommand::class, Command\Config\GenerateCharsetCommand::class, Command\Config\GenerateSecretCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 18e05dcb..682204b4 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -10,11 +10,10 @@ return [ 'factories' => [ Application::class => ApplicationFactory::class, - Command\GenerateShortcodeCommand::class => AnnotatedFactory::class, - Command\ResolveUrlCommand::class => AnnotatedFactory::class, - Command\ListShortcodesCommand::class => AnnotatedFactory::class, - Command\GetVisitsCommand::class => AnnotatedFactory::class, - Command\ProcessVisitsCommand::class => AnnotatedFactory::class, + Command\Shortcode\GenerateShortcodeCommand::class => AnnotatedFactory::class, + Command\Shortcode\ResolveUrlCommand::class => AnnotatedFactory::class, + Command\Shortcode\ListShortcodesCommand::class => AnnotatedFactory::class, + Command\Shortcode\GetVisitsCommand::class => AnnotatedFactory::class, Command\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\Config\GenerateCharsetCommand::class => AnnotatedFactory::class, Command\Config\GenerateSecretCommand::class => AnnotatedFactory::class, diff --git a/module/CLI/src/Command/GenerateShortcodeCommand.php b/module/CLI/src/Command/Shortcode/GenerateShortcodeCommand.php similarity index 97% rename from module/CLI/src/Command/GenerateShortcodeCommand.php rename to module/CLI/src/Command/Shortcode/GenerateShortcodeCommand.php index f02c110b..2830fb40 100644 --- a/module/CLI/src/Command/GenerateShortcodeCommand.php +++ b/module/CLI/src/Command/Shortcode/GenerateShortcodeCommand.php @@ -1,5 +1,5 @@ Date: Mon, 8 Aug 2016 09:38:50 +0200 Subject: [PATCH 19/39] Updated namespace for Visit CLI commands --- module/CLI/config/cli.config.php | 2 +- module/CLI/config/dependencies.config.php | 2 +- module/CLI/src/Command/{ => Visit}/ProcessVisitsCommand.php | 6 +++--- module/CLI/test/Command/ProcessVisitsCommandTest.php | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename module/CLI/src/Command/{ => Visit}/ProcessVisitsCommand.php (94%) diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 699d4275..1244e259 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -9,7 +9,7 @@ return [ Command\Shortcode\ResolveUrlCommand::class, Command\Shortcode\ListShortcodesCommand::class, Command\Shortcode\GetVisitsCommand::class, - Command\ProcessVisitsCommand::class, + Command\Visit\ProcessVisitsCommand::class, Command\Config\GenerateCharsetCommand::class, Command\Config\GenerateSecretCommand::class, Command\Api\GenerateKeyCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 682204b4..ebc607c8 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -14,7 +14,7 @@ return [ Command\Shortcode\ResolveUrlCommand::class => AnnotatedFactory::class, Command\Shortcode\ListShortcodesCommand::class => AnnotatedFactory::class, Command\Shortcode\GetVisitsCommand::class => AnnotatedFactory::class, - Command\ProcessVisitsCommand::class => AnnotatedFactory::class, + Command\Visit\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\Config\GenerateCharsetCommand::class => AnnotatedFactory::class, Command\Config\GenerateSecretCommand::class => AnnotatedFactory::class, Command\Api\GenerateKeyCommand::class => AnnotatedFactory::class, diff --git a/module/CLI/src/Command/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php similarity index 94% rename from module/CLI/src/Command/ProcessVisitsCommand.php rename to module/CLI/src/Command/Visit/ProcessVisitsCommand.php index e9f95a7b..195d891c 100644 --- a/module/CLI/src/Command/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -1,5 +1,5 @@ Date: Mon, 8 Aug 2016 09:46:40 +0200 Subject: [PATCH 20/39] Updated status returned in REST endpoints to be 404 when something is not found --- module/Rest/src/Action/GetVisitsAction.php | 9 ++++++--- module/Rest/src/Action/ResolveUrlAction.php | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/module/Rest/src/Action/GetVisitsAction.php b/module/Rest/src/Action/GetVisitsAction.php index bd78adbb..7a911789 100644 --- a/module/Rest/src/Action/GetVisitsAction.php +++ b/module/Rest/src/Action/GetVisitsAction.php @@ -25,7 +25,7 @@ class GetVisitsAction extends AbstractRestAction /** * GetVisitsAction constructor. - * @param VisitsTrackerInterface|VisitsTracker $visitsTracker + * @param VisitsTrackerInterface $visitsTracker * @param TranslatorInterface $translator * * @Inject({VisitsTracker::class, "translator"}) @@ -59,8 +59,11 @@ class GetVisitsAction extends AbstractRestAction } catch (InvalidArgumentException $e) { return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf($this->translator->translate('Provided short code "%s" is invalid'), $shortCode), - ], 400); + 'message' => sprintf( + $this->translator->translate('Provided short code "%s" does not exist'), + $shortCode + ), + ], 404); } catch (\Exception $e) { return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, diff --git a/module/Rest/src/Action/ResolveUrlAction.php b/module/Rest/src/Action/ResolveUrlAction.php index c1783834..ac703b38 100644 --- a/module/Rest/src/Action/ResolveUrlAction.php +++ b/module/Rest/src/Action/ResolveUrlAction.php @@ -50,8 +50,8 @@ class ResolveUrlAction extends AbstractRestAction if (! isset($longUrl)) { return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => sprintf($this->translator->translate('No URL found for shortcode "%s"'), $shortCode), - ], 400); + 'message' => sprintf($this->translator->translate('No URL found for short code "%s"'), $shortCode), + ], 404); } return new JsonResponse([ From f49e9064cdf4cbd885088ce7dabc0ccf828de066 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 10:02:52 +0200 Subject: [PATCH 21/39] Added cache adapter to the UrlShortener service to cache shortcode-url maps --- module/Core/src/Action/RedirectAction.php | 6 ++-- module/Core/src/Service/UrlShortener.php | 24 ++++++++++++-- module/Core/test/Service/UrlShortenerTest.php | 33 ++++++++++++++++--- .../Rest/test/Action/GetVisitsActionTest.php | 2 +- .../Rest/test/Action/ResolveUrlActionTest.php | 2 +- 5 files changed, 56 insertions(+), 11 deletions(-) diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 031aa0fe..626ae27e 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -18,14 +18,14 @@ class RedirectAction implements MiddlewareInterface */ private $urlShortener; /** - * @var VisitsTracker|VisitsTrackerInterface + * @var VisitsTrackerInterface */ private $visitTracker; /** * RedirectMiddleware constructor. - * @param UrlShortenerInterface|UrlShortener $urlShortener - * @param VisitsTrackerInterface|VisitsTracker $visitTracker + * @param UrlShortenerInterface $urlShortener + * @param VisitsTrackerInterface $visitTracker * * @Inject({UrlShortener::class, VisitsTracker::class}) */ diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 01a8d4b6..356667e1 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Core\Service; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Doctrine\Common\Cache\Cache; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\ORMException; use GuzzleHttp\ClientInterface; @@ -28,23 +29,30 @@ class UrlShortener implements UrlShortenerInterface * @var string */ private $chars; + /** + * @var Cache + */ + private $cache; /** * UrlShortener constructor. * @param ClientInterface $httpClient * @param EntityManagerInterface $em + * @param Cache $cache * @param string $chars * - * @Inject({"httpClient", "em", "config.url_shortener.shortcode_chars"}) + * @Inject({"httpClient", "em", Cache::class, "config.url_shortener.shortcode_chars"}) */ public function __construct( ClientInterface $httpClient, EntityManagerInterface $em, + Cache $cache, $chars = self::DEFAULT_CHARS ) { $this->httpClient = $httpClient; $this->em = $em; $this->chars = empty($chars) ? self::DEFAULT_CHARS : $chars; + $this->cache = $cache; } /** @@ -140,6 +148,11 @@ class UrlShortener implements UrlShortenerInterface */ public function shortCodeToUrl($shortCode) { + // Check if the short code => URL map is already cached + if ($this->cache->contains($shortCode)) { + return $this->cache->fetch($shortCode); + } + // Validate short code format if (! preg_match('|[' . $this->chars . "]+|", $shortCode)) { throw InvalidShortCodeException::fromShortCode($shortCode, $this->chars); @@ -149,6 +162,13 @@ class UrlShortener implements UrlShortenerInterface $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ 'shortCode' => $shortCode, ]); - return isset($shortUrl) ? $shortUrl->getOriginalUrl() : null; + // Cache the shortcode + if (isset($shortUrl)) { + $url = $shortUrl->getOriginalUrl(); + $this->cache->save($shortCode, $url); + return $url; + } + + return null; } } diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 8298a8cc..90886016 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -1,6 +1,8 @@ findOneBy(Argument::any())->willReturn(null); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->urlShortener = new UrlShortener($this->httpClient->reveal(), $this->em->reveal()); + $this->cache = new ArrayCache(); + + $this->urlShortener = new UrlShortener($this->httpClient->reveal(), $this->em->reveal(), $this->cache); } /** @@ -112,16 +120,19 @@ class UrlShortenerTest extends TestCase public function shortCodeIsProperlyParsed() { // 12C1c -> 10 + $shortCode = '12C1c'; $shortUrl = new ShortUrl(); - $shortUrl->setShortCode('12C1c') + $shortUrl->setShortCode($shortCode) ->setOriginalUrl('expected_url'); $repo = $this->prophesize(ObjectRepository::class); - $repo->findOneBy(['shortCode' => '12C1c'])->willReturn($shortUrl); + $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $url = $this->urlShortener->shortCodeToUrl('12C1c'); + $this->assertFalse($this->cache->contains($shortCode)); + $url = $this->urlShortener->shortCodeToUrl($shortCode); $this->assertEquals($shortUrl->getOriginalUrl(), $url); + $this->assertTrue($this->cache->contains($shortCode)); } /** @@ -132,4 +143,18 @@ class UrlShortenerTest extends TestCase { $this->urlShortener->shortCodeToUrl('&/('); } + + /** + * @test + */ + public function cachedShortCodeDoesNotHitDatabase() + { + $shortCode = '12C1c'; + $expectedUrl = 'expected_url'; + $this->cache->save($shortCode, $expectedUrl); + $this->em->getRepository(ShortUrl::class)->willReturn(null)->shouldBeCalledTimes(0); + + $url = $this->urlShortener->shortCodeToUrl($shortCode); + $this->assertEquals($expectedUrl, $url); + } } diff --git a/module/Rest/test/Action/GetVisitsActionTest.php b/module/Rest/test/Action/GetVisitsActionTest.php index 901c549e..7e222269 100644 --- a/module/Rest/test/Action/GetVisitsActionTest.php +++ b/module/Rest/test/Action/GetVisitsActionTest.php @@ -59,7 +59,7 @@ class GetVisitsActionTest extends TestCase ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), new Response() ); - $this->assertEquals(400, $response->getStatusCode()); + $this->assertEquals(404, $response->getStatusCode()); } /** diff --git a/module/Rest/test/Action/ResolveUrlActionTest.php b/module/Rest/test/Action/ResolveUrlActionTest.php index 0bd3bded..07db655a 100644 --- a/module/Rest/test/Action/ResolveUrlActionTest.php +++ b/module/Rest/test/Action/ResolveUrlActionTest.php @@ -39,7 +39,7 @@ class ResolveUrlActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->__invoke($request, new Response()); - $this->assertEquals(400, $response->getStatusCode()); + $this->assertEquals(404, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_ARGUMENT_ERROR) > 0); } From 63e867cf4b810702af84636eda5d21acfc3f1708 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 10:08:34 +0200 Subject: [PATCH 22/39] Updated vendor name on error pages --- module/Core/templates/core/layout/default.html.twig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/templates/core/layout/default.html.twig b/module/Core/templates/core/layout/default.html.twig index aaec0aab..6f93dfdd 100644 --- a/module/Core/templates/core/layout/default.html.twig +++ b/module/Core/templates/core/layout/default.html.twig @@ -27,7 +27,7 @@
{% block footer %}

- © {{ "now" | date("Y") }} by Alejandro Celaya. + © {{ "now" | date("Y") }} Shlink

{% endblock %} From cff9b7c0b546128a455278d8090d229fc982a43a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 11:17:14 +0200 Subject: [PATCH 23/39] Deleted docs which are now in Shlink's website --- config/autoload/logger.global.php | 12 ++ data/docs/installation.md | 19 -- data/docs/rest.md | 289 ------------------------------ data/log/.gitignore | 2 + 4 files changed, 14 insertions(+), 308 deletions(-) create mode 100644 config/autoload/logger.global.php delete mode 100644 data/docs/installation.md delete mode 100644 data/docs/rest.md create mode 100644 data/log/.gitignore diff --git a/config/autoload/logger.global.php b/config/autoload/logger.global.php new file mode 100644 index 00000000..d7f246bc --- /dev/null +++ b/config/autoload/logger.global.php @@ -0,0 +1,12 @@ + [ + 'loggers' => [ + 'Shlink' => [ + 'handlers' => [], + ], + ], + ], + +]; diff --git a/data/docs/installation.md b/data/docs/installation.md deleted file mode 100644 index 67e241fc..00000000 --- a/data/docs/installation.md +++ /dev/null @@ -1,19 +0,0 @@ -### Installation steps - -- Define ENV vars in apache or nginx: - - SHORTENED_URL_SCHEMA: http|https - - SHORTENED_URL_HOSTNAME: Short domain - - SHORTCODE_CHARS: The char set used to generate short codes (defaults to **123456789bcdfghjkmnpqrstvwxyzBCDFGHJKLMNPQRSTVWXYZ**, but a new one can be generated with the `config:generate-charset` command) - - DB_USER: MySQL database user - - DB_PASSWORD: MySQL database password - - REST_USER: Username for REST authentication - - REST_PASSWORD: Password for REST authentication - - DB_NAME: MySQL database name (defaults to **shlink**) - - DEFAULT_LOCALE: Language in which web requests (browser and REST) will be returned if no `Accept-Language` header is sent (defaults to **en**) - - CLI_LOCALE: Language in which console command messages will be displayed (defaults to **en**) -- Create database (`vendor/bin/doctrine orm:schema-tool:create`) -- Add write permissions to `data` directory -- Create doctrine proxies (`vendor/bin/doctrine orm:generate-proxies`) -- Create symlink to bin/cli as `shlink` in /usr/local/bin (linux only. Optional) - -Supported languages: es and en diff --git a/data/docs/rest.md b/data/docs/rest.md deleted file mode 100644 index bbc9a565..00000000 --- a/data/docs/rest.md +++ /dev/null @@ -1,289 +0,0 @@ - -# REST API documentation - -## Error management - -Statuses: - -* 400 -> controlled error -* 401 -> authentication error -* 500 -> unexpected error - -[TODO] - -## Authentication - -Once you have called to the authentication endpoint for the first time (see below) yopu will get an authentication token. - -You will have to send that token in the `X-Auth-Token` header on any later request or you will get an authentication error. - -## Language - -In order to set the application language, you have to pass it by using the `Accept-Language` header. - -If not provided or provided language is not supported, english (en_US) will be used. - -## Endpoints - -#### Authenticate - -**REQUEST** - -* `POST` -> `/rest/authenticate` -* Params: - * username: `string` - * password: `string` - -**SUCCESS RESPONSE** - -```json -{ - "token": "9f741eb0-33d7-4c56-b8f7-3719e9929946" -} -``` - -**ERROR RESPONSE** - -```json -{ - "error": "INVALID_ARGUMENT", - "message": "You have to provide both \"username\" and \"password\"" -} -``` - -Posible errors: - -* **INVALID_ARGUMENT**: Username or password were not provided. -* **INVALID_CREDENTIALS**: Username or password are incorrect. - - -#### Create shortcode - -**REQUEST** - -* `POST` -> `/rest/short-codes` -* Params: - * longUrl: `string` -> The URL to shorten -* Headers: - * X-Auth-Token: `string` -> The token provided in the authentication request - -**SUCCESS RESPONSE** - -```json -{ - "longUrl": "https://www.facebook.com/something/something", - "shortUrl": "https://doma.in/rY9Kr", - "shortCode": "rY9Kr" -} -``` - -**ERROR RESPONSE** - -```json -{ - "error": "INVALID_URL", - "message": "Provided URL \"wfwef\" is invalid. Try with a different one." -} -``` - -Posible errors: - -* **INVALID_ARGUMENT**: The longUrl was not provided. -* **INVALID_URL**: Provided longUrl has an invalid format or does not resolve. -* **UNKNOWN_ERROR**: Something unexpected happened. - - -#### Resolve URL - -**REQUEST** - -* `GET` -> `/rest/short-codes/{shortCode}` -* Route params: - * shortCode: `string` -> The short code we want to resolve -* Headers: - * X-Auth-Token: `string` -> The token provided in the authentication request - -**SUCCESS RESPONSE** - -```json -{ - "longUrl": "https://www.facebook.com/something/something" -} -``` - -**ERROR RESPONSE** - -```json -{ - "error": "INVALID_SHORTCODE", - "message": "Provided short code \"abc123\" has an invalid format" -} -``` - -Posible errors: - -* **INVALID_ARGUMENT**: No longUrl was found for provided shortCode. -* **INVALID_SHORTCODE**: Provided shortCode does not match the character set used by the app to generate short codes. -* **UNKNOWN_ERROR**: Something unexpected happened. - - -#### List shortened URLs - -**REQUEST** - -* `GET` -> `/rest/short-codes` -* Query params: - * page: `integer` -> The page to list. Defaults to 1 if not provided. -* Headers: - * X-Auth-Token: `string` -> The token provided in the authentication request - -**SUCCESS RESPONSE** - -```json -{ - "shortUrls": { - "data": [ - { - "shortCode": "abc123", - "originalUrl": "http://www.alejandrocelaya.com", - "dateCreated": "2016-04-30T18:01:47+0200", - "visitsCount": 4 - }, - { - "shortCode": "def456", - "originalUrl": "http://www.alejandrocelaya.com/en", - "dateCreated": "2016-04-30T18:03:43+0200", - "visitsCount": 0 - }, - { - "shortCode": "ghi789", - "originalUrl": "http://www.alejandrocelaya.com/es", - "dateCreated": "2016-04-30T18:10:38+0200", - "visitsCount": 0 - }, - { - "shortCode": "jkl987", - "originalUrl": "http://www.alejandrocelaya.com/es/", - "dateCreated": "2016-04-30T18:10:57+0200", - "visitsCount": 0 - }, - { - "shortCode": "mno654", - "originalUrl": "http://blog.alejandrocelaya.com/2016/04/09/improving-zend-service-manager-workflow-with-annotations/", - "dateCreated": "2016-04-30T19:21:05+0200", - "visitsCount": 1 - }, - { - "shortCode": "pqr321", - "originalUrl": "http://www.google.com", - "dateCreated": "2016-05-01T11:19:53+0200", - "visitsCount": 0 - }, - { - "shortCode": "stv159", - "originalUrl": "http://www.acelaya.com", - "dateCreated": "2016-06-12T17:49:21+0200", - "visitsCount": 0 - }, - { - "shortCode": "wxy753", - "originalUrl": "http://www.atomic-reader.com", - "dateCreated": "2016-06-12T17:50:27+0200", - "visitsCount": 0 - }, - { - "shortCode": "zab852", - "originalUrl": "http://foo.com", - "dateCreated": "2016-07-03T09:07:36+0200", - "visitsCount": 0 - }, - { - "shortCode": "cde963", - "originalUrl": "https://www.facebook.com.com", - "dateCreated": "2016-07-03T09:12:35+0200", - "visitsCount": 0 - } - ], - "pagination": { - "currentPage": 4, - "pagesCount": 15 - } - } -} -``` - -**ERROR RESPONSE** - -```json -{ - "error": "UNKNOWN_ERROR", - "message": "Unexpected error occured" -} -``` - -Posible errors: - -* **UNKNOWN_ERROR**: Something unexpected happened. - - -#### Get visits - -**REQUEST** - -* `GET` -> `/rest/short-codes/{shortCode}/visits` -* Route params: - * shortCode: `string` -> The shortCode from which we eant to get the visits. -* Query params: - * startDate: `string` -> If provided, only visits older that this date will be returned - * endDate: `string` -> If provided, only visits newer that this date will be returned -* Headers: - * X-Auth-Token: `string` -> The token provided in the authentication request - -**SUCCESS RESPONSE** - -```json -{ - "shortUrls": { - "data": [ - { - "referer": null, - "date": "2016-06-18T09:32:22+0200", - "remoteAddr": "127.0.0.1", - "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36" - }, - { - "referer": null, - "date": "2016-04-30T19:20:06+0200", - "remoteAddr": "127.0.0.1", - "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36" - }, - { - "referer": "google.com", - "date": "2016-04-30T19:19:57+0200", - "remoteAddr": "1.2.3.4", - "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36" - }, - { - "referer": null, - "date": "2016-04-30T19:17:35+0200", - "remoteAddr": "127.0.0.1", - "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36" - } - ], - } -} -``` - -**ERROR RESPONSE** - -```json -{ - "error": "INVALID_ARGUMENT", - "message": "Provided short code \"abc123\" is invalid" -} -``` - -Posible errors: - -* **INVALID_ARGUMENT**: The shortcode does not belong to any short URL -* **UNKNOWN_ERROR**: Something unexpected happened. diff --git a/data/log/.gitignore b/data/log/.gitignore new file mode 100644 index 00000000..d6b7ef32 --- /dev/null +++ b/data/log/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore From b7f3c332e44c08dbacb859ae474967c123c6ca2a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 11:56:19 +0200 Subject: [PATCH 24/39] Created Logger factory and logger config, and added logger dependencies --- composer.json | 4 +- config/autoload/logger.global.php | 22 ++++++++++- config/autoload/logger.local.php.dist | 14 +++++++ module/Common/config/dependencies.config.php | 9 ++++- module/Common/src/Factory/LoggerFactory.php | 39 ++++++++++++++++++++ 5 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 config/autoload/logger.local.php.dist create mode 100644 module/Common/src/Factory/LoggerFactory.php diff --git a/composer.json b/composer.json index 72391371..9ea805ba 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,9 @@ "doctrine/orm": "^2.5", "guzzlehttp/guzzle": "^6.2", "symfony/console": "^3.0", - "firebase/php-jwt": "^4.0" + "firebase/php-jwt": "^4.0", + "monolog/monolog": "^1.21", + "theorchard/monolog-cascade": "^0.4" }, "require-dev": { "phpunit/phpunit": "^5.0", diff --git a/config/autoload/logger.global.php b/config/autoload/logger.global.php index d7f246bc..2a56bdad 100644 --- a/config/autoload/logger.global.php +++ b/config/autoload/logger.global.php @@ -1,10 +1,30 @@ [ + 'formatters' => [ + 'dashed' => [ + 'format' => '[%datetime%] %channel%.%level_name% - %message% %context%' . PHP_EOL, + 'include_stacktraces' => true, + ], + ], + + 'handlers' => [ + 'rotating_file_handler' => [ + 'class' => RotatingFileHandler::class, + 'level' => Logger::INFO, + 'filename' => 'data/log/shlink_log.log', + 'max_files' => 30, + 'formatter' => 'dashed', + ], + ], + 'loggers' => [ 'Shlink' => [ - 'handlers' => [], + 'handlers' => ['rotating_file_handler'], ], ], ], diff --git a/config/autoload/logger.local.php.dist b/config/autoload/logger.local.php.dist new file mode 100644 index 00000000..951a3af0 --- /dev/null +++ b/config/autoload/logger.local.php.dist @@ -0,0 +1,14 @@ + [ + 'handlers' => [ + 'rotating_file_handler' => [ + 'level' => Logger::DEBUG, + ], + ], + ], + +]; diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index bcd30faa..ea670d57 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -2,9 +2,11 @@ use Acelaya\ZsmAnnotatedServices\Factory\V3\AnnotatedFactory; use Doctrine\Common\Cache\Cache; use Doctrine\ORM\EntityManager; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\ErrorHandler; use Shlinkio\Shlink\Common\Factory\CacheFactory; use Shlinkio\Shlink\Common\Factory\EntityManagerFactory; +use Shlinkio\Shlink\Common\Factory\LoggerFactory; use Shlinkio\Shlink\Common\Factory\TranslatorFactory; use Shlinkio\Shlink\Common\Middleware\LocaleMiddleware; use Shlinkio\Shlink\Common\Service\IpLocationResolver; @@ -19,11 +21,15 @@ return [ EntityManager::class => EntityManagerFactory::class, GuzzleHttp\Client::class => InvokableFactory::class, Cache::class => CacheFactory::class, - IpLocationResolver::class => AnnotatedFactory::class, + LoggerInterface::class => LoggerFactory::class, + 'Logger_Shlink' => LoggerFactory::class, + Translator::class => TranslatorFactory::class, TranslatorExtension::class => AnnotatedFactory::class, LocaleMiddleware::class => AnnotatedFactory::class, + IpLocationResolver::class => AnnotatedFactory::class, + ErrorHandler\ContentBasedErrorHandler::class => AnnotatedFactory::class, ErrorHandler\ErrorHandlerManager::class => ErrorHandler\ErrorHandlerManagerFactory::class, ], @@ -31,6 +37,7 @@ return [ 'em' => EntityManager::class, 'httpClient' => GuzzleHttp\Client::class, 'translator' => Translator::class, + 'logger' => LoggerInterface::class, AnnotatedFactory::CACHE_SERVICE => Cache::class, ], ], diff --git a/module/Common/src/Factory/LoggerFactory.php b/module/Common/src/Factory/LoggerFactory.php new file mode 100644 index 00000000..d42b2f01 --- /dev/null +++ b/module/Common/src/Factory/LoggerFactory.php @@ -0,0 +1,39 @@ +has('config') ? $container->get('config') : []; + Cascade::fileConfig(isset($config['logger']) ? $config['logger'] : ['loggers' => []]); + + // Compose requested logger name + $loggerName = isset($options) & isset($options['logger_name']) ? $options['logger_name'] : 'Logger'; + $nameParts = explode('_', $requestedName); + if (count($nameParts) > 1) { + $loggerName = $nameParts[1]; + } + + return Cascade::getLogger($loggerName); + } +} From fff058f44b9d2c5074bea3ec34033add7a5ddf84 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 12:07:04 +0200 Subject: [PATCH 25/39] Created LoggerFactoryTest --- module/Common/config/dependencies.config.php | 2 + .../Common/test/Factory/LoggerFactoryTest.php | 54 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 module/Common/test/Factory/LoggerFactoryTest.php diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index ea670d57..b0cd571a 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -2,6 +2,7 @@ use Acelaya\ZsmAnnotatedServices\Factory\V3\AnnotatedFactory; use Doctrine\Common\Cache\Cache; use Doctrine\ORM\EntityManager; +use Monolog\Logger; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\ErrorHandler; use Shlinkio\Shlink\Common\Factory\CacheFactory; @@ -38,6 +39,7 @@ return [ 'httpClient' => GuzzleHttp\Client::class, 'translator' => Translator::class, 'logger' => LoggerInterface::class, + Logger::class => LoggerInterface::class, AnnotatedFactory::CACHE_SERVICE => Cache::class, ], ], diff --git a/module/Common/test/Factory/LoggerFactoryTest.php b/module/Common/test/Factory/LoggerFactoryTest.php new file mode 100644 index 00000000..bf292a1f --- /dev/null +++ b/module/Common/test/Factory/LoggerFactoryTest.php @@ -0,0 +1,54 @@ +factory = new LoggerFactory(); + } + + /** + * @test + */ + public function serviceIsCreated() + { + /** @var Logger $instance */ + $instance = $this->factory->__invoke(new ServiceManager(), ''); + $this->assertInstanceOf(LoggerInterface::class, $instance); + $this->assertEquals('Logger', $instance->getName()); + } + + /** + * @test + */ + public function nameIsSetFromOptions() + { + /** @var Logger $instance */ + $instance = $this->factory->__invoke(new ServiceManager(), '', ['logger_name' => 'Foo']); + $this->assertInstanceOf(LoggerInterface::class, $instance); + $this->assertEquals('Foo', $instance->getName()); + } + + /** + * @test + */ + public function serviceNameOverwritesOptionsLoggerName() + { + /** @var Logger $instance */ + $instance = $this->factory->__invoke(new ServiceManager(), 'Logger_Shlink', ['logger_name' => 'Foo']); + $this->assertInstanceOf(LoggerInterface::class, $instance); + $this->assertEquals('Shlink', $instance->getName()); + } +} From 34753ca7d3e13477b153040818400b8d8d831155 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 12:33:58 +0200 Subject: [PATCH 26/39] Added logger to classes that catch errors in order to log them --- module/Core/src/Action/RedirectAction.php | 18 +++++++++++++++--- module/Core/src/Service/UrlShortener.php | 2 +- module/Rest/src/Action/AbstractRestAction.php | 12 ++++++++++++ .../Rest/src/Action/CreateShortcodeAction.php | 12 +++++++++--- module/Rest/src/Action/GetVisitsAction.php | 16 ++++++++++++---- .../Rest/src/Action/ListShortcodesAction.php | 14 +++++++++++--- module/Rest/src/Action/ResolveUrlAction.php | 12 ++++++++++-- .../CheckAuthenticationMiddleware.php | 18 +++++++++++++++--- 8 files changed, 85 insertions(+), 19 deletions(-) diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 626ae27e..8aa3ddb7 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -4,6 +4,8 @@ namespace Shlinkio\Shlink\Core\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -21,18 +23,27 @@ class RedirectAction implements MiddlewareInterface * @var VisitsTrackerInterface */ private $visitTracker; + /** + * @var null|LoggerInterface + */ + private $logger; /** * RedirectMiddleware constructor. * @param UrlShortenerInterface $urlShortener * @param VisitsTrackerInterface $visitTracker + * @param LoggerInterface|null $logger * - * @Inject({UrlShortener::class, VisitsTracker::class}) + * @Inject({UrlShortener::class, VisitsTracker::class, "Logger_Shlink"}) */ - public function __construct(UrlShortenerInterface $urlShortener, VisitsTrackerInterface $visitTracker) - { + public function __construct( + UrlShortenerInterface $urlShortener, + VisitsTrackerInterface $visitTracker, + LoggerInterface $logger = null + ) { $this->urlShortener = $urlShortener; $this->visitTracker = $visitTracker; + $this->logger = $logger ?: new NullLogger(); } /** @@ -81,6 +92,7 @@ class RedirectAction implements MiddlewareInterface return new RedirectResponse($longUrl); } catch (\Exception $e) { // In case of error, dispatch 404 error + $this->logger->error('Error redirecting to long URL.' . PHP_EOL . $e); return $this->notFoundResponse($request, $response, $out); } } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 356667e1..30b700cd 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -99,7 +99,7 @@ class UrlShortener implements UrlShortenerInterface $this->em->close(); } - throw new RuntimeException('An error occured while persisting the short URL', -1, $e); + throw new RuntimeException('An error occurred while persisting the short URL', -1, $e); } } diff --git a/module/Rest/src/Action/AbstractRestAction.php b/module/Rest/src/Action/AbstractRestAction.php index 587e1a93..4acbe2ba 100644 --- a/module/Rest/src/Action/AbstractRestAction.php +++ b/module/Rest/src/Action/AbstractRestAction.php @@ -3,10 +3,22 @@ namespace Shlinkio\Shlink\Rest\Action; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Zend\Stratigility\MiddlewareInterface; abstract class AbstractRestAction implements MiddlewareInterface { + /** + * @var LoggerInterface + */ + protected $logger; + + public function __construct(LoggerInterface $logger = null) + { + $this->logger = $logger ?: new NullLogger(); + } + /** * Process an incoming request and/or response. * diff --git a/module/Rest/src/Action/CreateShortcodeAction.php b/module/Rest/src/Action/CreateShortcodeAction.php index 29aa1108..5dd1221e 100644 --- a/module/Rest/src/Action/CreateShortcodeAction.php +++ b/module/Rest/src/Action/CreateShortcodeAction.php @@ -4,6 +4,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; @@ -33,14 +34,17 @@ class CreateShortcodeAction extends AbstractRestAction * @param UrlShortenerInterface|UrlShortener $urlShortener * @param TranslatorInterface $translator * @param array $domainConfig + * @param LoggerInterface|null $logger * - * @Inject({UrlShortener::class, "translator", "config.url_shortener.domain"}) + * @Inject({UrlShortener::class, "translator", "config.url_shortener.domain", "Logger_Shlink"}) */ public function __construct( UrlShortenerInterface $urlShortener, TranslatorInterface $translator, - array $domainConfig + array $domainConfig, + LoggerInterface $logger = null ) { + parent::__construct($logger); $this->urlShortener = $urlShortener; $this->translator = $translator; $this->domainConfig = $domainConfig; @@ -75,14 +79,16 @@ class CreateShortcodeAction extends AbstractRestAction 'shortCode' => $shortCode, ]); } catch (InvalidUrlException $e) { + $this->logger->warning('Provided Invalid URL.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), 'message' => sprintf( - $this->translator->translate('Provided URL "%s" is invalid. Try with a different one.'), + $this->translator->translate('Provided URL %s is invalid. Try with a different one.'), $longUrl ), ], 400); } catch (\Exception $e) { + $this->logger->error('Unexpected error creating shortcode.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, 'message' => $this->translator->translate('Unexpected error occurred'), diff --git a/module/Rest/src/Action/GetVisitsAction.php b/module/Rest/src/Action/GetVisitsAction.php index 7a911789..0d78bcc1 100644 --- a/module/Rest/src/Action/GetVisitsAction.php +++ b/module/Rest/src/Action/GetVisitsAction.php @@ -4,6 +4,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -27,11 +28,16 @@ class GetVisitsAction extends AbstractRestAction * GetVisitsAction constructor. * @param VisitsTrackerInterface $visitsTracker * @param TranslatorInterface $translator + * @param LoggerInterface $logger * - * @Inject({VisitsTracker::class, "translator"}) + * @Inject({VisitsTracker::class, "translator", "Logger_Shlink"}) */ - public function __construct(VisitsTrackerInterface $visitsTracker, TranslatorInterface $translator) - { + public function __construct( + VisitsTrackerInterface $visitsTracker, + TranslatorInterface $translator, + LoggerInterface $logger = null + ) { + parent::__construct($logger); $this->visitsTracker = $visitsTracker; $this->translator = $translator; } @@ -57,14 +63,16 @@ class GetVisitsAction extends AbstractRestAction ] ]); } catch (InvalidArgumentException $e) { + $this->logger->warning('Provided nonexistent shortcode'. PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), 'message' => sprintf( - $this->translator->translate('Provided short code "%s" does not exist'), + $this->translator->translate('Provided short code %s does not exist'), $shortCode ), ], 404); } catch (\Exception $e) { + $this->logger->error('Unexpected error while parsing short code'. PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, 'message' => $this->translator->translate('Unexpected error occurred'), diff --git a/module/Rest/src/Action/ListShortcodesAction.php b/module/Rest/src/Action/ListShortcodesAction.php index 3d0d9613..1cd376bc 100644 --- a/module/Rest/src/Action/ListShortcodesAction.php +++ b/module/Rest/src/Action/ListShortcodesAction.php @@ -4,6 +4,8 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; @@ -28,11 +30,16 @@ class ListShortcodesAction extends AbstractRestAction * ListShortcodesAction constructor. * @param ShortUrlServiceInterface|ShortUrlService $shortUrlService * @param TranslatorInterface $translator + * @param LoggerInterface $logger * - * @Inject({ShortUrlService::class, "translator"}) + * @Inject({ShortUrlService::class, "translator", "Logger_Shlink"}) */ - public function __construct(ShortUrlServiceInterface $shortUrlService, TranslatorInterface $translator) - { + public function __construct( + ShortUrlServiceInterface $shortUrlService, + TranslatorInterface $translator, + LoggerInterface $logger = null + ) { + parent::__construct($logger); $this->shortUrlService = $shortUrlService; $this->translator = $translator; } @@ -50,6 +57,7 @@ class ListShortcodesAction extends AbstractRestAction $shortUrls = $this->shortUrlService->listShortUrls(isset($query['page']) ? $query['page'] : 1); return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls)]); } catch (\Exception $e) { + $this->logger->error('Unexpected error while listing short URLs.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, 'message' => $this->translator->translate('Unexpected error occurred'), diff --git a/module/Rest/src/Action/ResolveUrlAction.php b/module/Rest/src/Action/ResolveUrlAction.php index ac703b38..c99e233a 100644 --- a/module/Rest/src/Action/ResolveUrlAction.php +++ b/module/Rest/src/Action/ResolveUrlAction.php @@ -4,6 +4,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; @@ -26,11 +27,16 @@ class ResolveUrlAction extends AbstractRestAction * ResolveUrlAction constructor. * @param UrlShortenerInterface|UrlShortener $urlShortener * @param TranslatorInterface $translator + * @param LoggerInterface $logger * * @Inject({UrlShortener::class, "translator"}) */ - public function __construct(UrlShortenerInterface $urlShortener, TranslatorInterface $translator) - { + public function __construct( + UrlShortenerInterface $urlShortener, + TranslatorInterface $translator, + LoggerInterface $logger = null + ) { + parent::__construct($logger); $this->urlShortener = $urlShortener; $this->translator = $translator; } @@ -58,6 +64,7 @@ class ResolveUrlAction extends AbstractRestAction 'longUrl' => $longUrl, ]); } catch (InvalidShortCodeException $e) { + $this->logger->warning('Provided short code with invalid format.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), 'message' => sprintf( @@ -66,6 +73,7 @@ class ResolveUrlAction extends AbstractRestAction ), ], 400); } catch (\Exception $e) { + $this->logger->error('Unexpected error while resolving the URL behind a short code.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, 'message' => $this->translator->translate('Unexpected error occurred'), diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index 39c670e6..53f6cbe9 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php @@ -4,6 +4,8 @@ namespace Shlinkio\Shlink\Rest\Middleware; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Rest\Authentication\JWTService; use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Exception\AuthenticationException; @@ -25,18 +27,27 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface * @var JWTServiceInterface */ private $jwtService; + /** + * @var LoggerInterface + */ + private $logger; /** * CheckAuthenticationMiddleware constructor. * @param JWTServiceInterface|JWTService $jwtService * @param TranslatorInterface $translator + * @param LoggerInterface $logger * - * @Inject({JWTService::class, "translator"}) + * @Inject({JWTService::class, "translator", "Logger_Shlink"}) */ - public function __construct(JWTServiceInterface $jwtService, TranslatorInterface $translator) - { + public function __construct( + JWTServiceInterface $jwtService, + TranslatorInterface $translator, + LoggerInterface $logger = null + ) { $this->translator = $translator; $this->jwtService = $jwtService; + $this->logger = $logger ?: new NullLogger(); } /** @@ -118,6 +129,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface // Return the response with the updated token on it return $response->withHeader(self::AUTHORIZATION_HEADER, 'Bearer ' . $jwt); } catch (AuthenticationException $e) { + $this->logger->warning('Tried to access API with an invalid JWT.' . PHP_EOL . $e); return $this->createTokenErrorResponse(); } } From 73a236b3d02c0ed92ac3fa6a5c47c2a4bcec8264 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 08:52:06 +0200 Subject: [PATCH 27/39] Updated VisitsTracker so that the track method expects a Request object to be provided --- module/Core/src/Action/RedirectAction.php | 2 +- module/Core/src/Service/VisitsTracker.php | 7 ++++--- module/Core/src/Service/VisitsTrackerInterface.php | 6 ++++-- module/Core/test/Service/VisitsTrackerTest.php | 3 ++- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 8aa3ddb7..a86a98c4 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -85,7 +85,7 @@ class RedirectAction implements MiddlewareInterface } // Track visit to this short code - $this->visitTracker->track($shortCode); + $this->visitTracker->track($shortCode, $request); // Return a redirect response to the long URL. // Use a temporary redirect to make sure browsers always hit the server for analytics purposes diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 87187aad..6ae38e83 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -3,6 +3,7 @@ namespace Shlinkio\Shlink\Core\Service; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Doctrine\ORM\EntityManagerInterface; +use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; @@ -31,11 +32,11 @@ class VisitsTracker implements VisitsTrackerInterface * Tracks a new visit to provided short code, using an array of data to look up information * * @param string $shortCode - * @param array $visitorData Defaults to global $_SERVER + * @param ServerRequestInterface $request */ - public function track($shortCode, array $visitorData = null) + public function track($shortCode, ServerRequestInterface $request) { - $visitorData = $visitorData ?: $_SERVER; + $visitorData = $request->getServerParams(); /** @var ShortUrl $shortUrl */ $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index cec254d3..6aecabe4 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -1,6 +1,7 @@ em->persist(Argument::any())->shouldBeCalledTimes(1); $this->em->flush()->shouldBeCalledTimes(1); - $this->visitsTracker->track($shortCode); + $this->visitsTracker->track($shortCode, ServerRequestFactory::fromGlobals()); } /** From 7c5d8cf244924ad468892a5081650705c272a87b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 09:13:39 +0200 Subject: [PATCH 28/39] Fixed VisitsTracker to take into account the X-Forwarded-For header in case the server is behind a load balabncer or proxy --- module/Core/src/Service/VisitsTracker.php | 25 +++++++++++-------- .../Core/test/Service/VisitsTrackerTest.php | 23 +++++++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 6ae38e83..94647086 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -36,8 +36,6 @@ class VisitsTracker implements VisitsTrackerInterface */ public function track($shortCode, ServerRequestInterface $request) { - $visitorData = $request->getServerParams(); - /** @var ShortUrl $shortUrl */ $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ 'shortCode' => $shortCode, @@ -45,22 +43,27 @@ class VisitsTracker implements VisitsTrackerInterface $visit = new Visit(); $visit->setShortUrl($shortUrl) - ->setUserAgent($this->getArrayValue($visitorData, 'HTTP_USER_AGENT')) - ->setReferer($this->getArrayValue($visitorData, 'HTTP_REFERER')) - ->setRemoteAddr($this->getArrayValue($visitorData, 'REMOTE_ADDR')); + ->setUserAgent($request->getHeaderLine('User-Agent')) + ->setReferer($request->getHeaderLine('Referer')) + ->setRemoteAddr($this->findOutRemoteAddr($request)); $this->em->persist($visit); $this->em->flush(); } /** - * @param array $array - * @param $key - * @param null $default - * @return mixed|null + * @param ServerRequestInterface $request + * @return string */ - protected function getArrayValue(array $array, $key, $default = null) + protected function findOutRemoteAddr(ServerRequestInterface $request) { - return isset($array[$key]) ? $array[$key] : $default; + $forwardedFor = $request->getHeaderLine('X-Forwarded-For'); + if (empty($forwardedFor)) { + $serverParams = $request->getServerParams(); + return isset($serverParams['REMOTE_ADDR']) ? $serverParams['REMOTE_ADDR'] : null; + } + + $ips = explode(',', $forwardedFor); + return $ips[0]; } /** diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 12dcf013..557b715f 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -45,6 +45,29 @@ class VisitsTrackerTest extends TestCase $this->visitsTracker->track($shortCode, ServerRequestFactory::fromGlobals()); } + /** + * @test + */ + public function trackUsesForwardedForHeaderIfPresent() + { + $shortCode = '123ABC'; + $test = $this; + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl()); + + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); + $this->em->persist(Argument::any())->will(function ($args) use ($test) { + /** @var Visit $visit */ + $visit = $args[0]; + $test->assertEquals('4.3.2.1', $visit->getRemoteAddr()); + })->shouldBeCalledTimes(1); + $this->em->flush()->shouldBeCalledTimes(1); + + $this->visitsTracker->track($shortCode, ServerRequestFactory::fromGlobals( + ['REMOTE_ADDR' => '1.2.3.4'] + )->withHeader('X-Forwarded-For', '4.3.2.1,99.99.99.99')); + } + /** * @test */ From 99b7c77997b07b1c0f8ea46c83a7f03f9155d00d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 10:24:42 +0200 Subject: [PATCH 29/39] Created action to generate QR codes --- composer.json | 3 +- module/Common/src/Response/QrCodeResponse.php | 35 ++++++ module/Core/config/dependencies.config.php | 5 +- module/Core/config/routes.config.php | 10 +- module/Core/src/Action/QrCodeAction.php | 113 ++++++++++++++++++ 5 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 module/Common/src/Response/QrCodeResponse.php create mode 100644 module/Core/src/Action/QrCodeAction.php diff --git a/composer.json b/composer.json index 9ea805ba..1f9bf7bd 100644 --- a/composer.json +++ b/composer.json @@ -28,7 +28,8 @@ "symfony/console": "^3.0", "firebase/php-jwt": "^4.0", "monolog/monolog": "^1.21", - "theorchard/monolog-cascade": "^0.4" + "theorchard/monolog-cascade": "^0.4", + "endroid/qrcode": "^1.7" }, "require-dev": { "phpunit/phpunit": "^5.0", diff --git a/module/Common/src/Response/QrCodeResponse.php b/module/Common/src/Response/QrCodeResponse.php new file mode 100644 index 00000000..e19e4578 --- /dev/null +++ b/module/Common/src/Response/QrCodeResponse.php @@ -0,0 +1,35 @@ +createBody($qrCode), + $status, + $this->injectContentType($qrCode->getContentType(), $headers) + ); + } + + /** + * Create the message body. + * + * @param QrCode $qrCode + * @return StreamInterface + */ + private function createBody(QrCode $qrCode) + { + $body = new Stream('php://temp', 'wb+'); + $body->write($qrCode->get()); + $body->rewind(); + return $body; + } +} diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 27983069..eb6168f9 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -1,6 +1,6 @@ AnnotatedFactory::class, // Middleware - RedirectAction::class => AnnotatedFactory::class, + Action\RedirectAction::class => AnnotatedFactory::class, + Action\QrCodeAction::class => AnnotatedFactory::class, ], ], diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index 4d9a85e5..9b499f71 100644 --- a/module/Core/config/routes.config.php +++ b/module/Core/config/routes.config.php @@ -1,5 +1,5 @@ 'long-url-redirect', 'path' => '/{shortCode}', - 'middleware' => RedirectAction::class, + 'middleware' => Action\RedirectAction::class, + 'allowed_methods' => ['GET'], + ], + [ + 'name' => 'short-url-qr-code', + 'path' => '/qr/{shortCode}[/{size:[0-9]+}]', + 'middleware' => Action\QrCodeAction::class, 'allowed_methods' => ['GET'], ], ], diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php new file mode 100644 index 00000000..5d0549eb --- /dev/null +++ b/module/Core/src/Action/QrCodeAction.php @@ -0,0 +1,113 @@ +router = $router; + $this->urlShortener = $urlShortener; + $this->logger = $logger ?: new NullLogger(); + } + + /** + * Process an incoming request and/or response. + * + * Accepts a server-side request and a response instance, and does + * something with them. + * + * If the response is not complete and/or further processing would not + * interfere with the work done in the middleware, or if the middleware + * wants to delegate to another process, it can use the `$out` callable + * if present. + * + * If the middleware does not return a value, execution of the current + * request is considered complete, and the response instance provided will + * be considered the response to return. + * + * Alternately, the middleware may return a response instance. + * + * Often, middleware will `return $out();`, with the assumption that a + * later middleware will return a response. + * + * @param Request $request + * @param Response $response + * @param null|callable $out + * @return null|Response + */ + public function __invoke(Request $request, Response $response, callable $out = null) + { + // Make sure the short URL exists for this short code + $shortCode = $request->getAttribute('shortCode'); + try { + $shortUrl = $this->urlShortener->shortCodeToUrl($shortCode); + if (! isset($shortUrl)) { + return $out($request, $response->withStatus(404), 'Not Found'); + } + } catch (InvalidShortCodeException $e) { + $this->logger->warning('Tried to create a QR code with an invalid short code' . PHP_EOL . $e); + return $out($request, $response->withStatus(404), 'Not Found'); + } + + $path = $this->router->generateUri('long-url-redirect', ['shortCode' => $shortCode]); + $size = $this->getSizeParam($request); + + $qrCode = new QrCode($request->getUri()->withPath($path)->withQuery('')); + $qrCode->setSize($size) + ->setPadding(0); + return new QrCodeResponse($qrCode); + } + + /** + * @param Request $request + * @return int + */ + protected function getSizeParam(Request $request) + { + $size = intval($request->getAttribute('size', 300)); + if ($size < 50) { + return 50; + } elseif ($size > 1000) { + return 1000; + } + + return $size; + } +} From 8eb279fd288d1345bf966e9cdc5bdad9fca23109 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 13:32:33 +0200 Subject: [PATCH 30/39] Updated UrlShortener to namespace the cache entries --- module/Core/src/Service/UrlShortener.php | 7 ++++--- module/Core/test/Service/UrlShortenerTest.php | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 30b700cd..ed87b604 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -148,9 +148,10 @@ class UrlShortener implements UrlShortenerInterface */ public function shortCodeToUrl($shortCode) { + $cacheKey = sprintf('%s_longUrl', $shortCode); // Check if the short code => URL map is already cached - if ($this->cache->contains($shortCode)) { - return $this->cache->fetch($shortCode); + if ($this->cache->contains($cacheKey)) { + return $this->cache->fetch($cacheKey); } // Validate short code format @@ -165,7 +166,7 @@ class UrlShortener implements UrlShortenerInterface // Cache the shortcode if (isset($shortUrl)) { $url = $shortUrl->getOriginalUrl(); - $this->cache->save($shortCode, $url); + $this->cache->save($cacheKey, $url); return $url; } diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 90886016..4cf7da1d 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -129,10 +129,10 @@ class UrlShortenerTest extends TestCase $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->assertFalse($this->cache->contains($shortCode)); + $this->assertFalse($this->cache->contains($shortCode . '_longUrl')); $url = $this->urlShortener->shortCodeToUrl($shortCode); $this->assertEquals($shortUrl->getOriginalUrl(), $url); - $this->assertTrue($this->cache->contains($shortCode)); + $this->assertTrue($this->cache->contains($shortCode . '_longUrl')); } /** @@ -151,7 +151,7 @@ class UrlShortenerTest extends TestCase { $shortCode = '12C1c'; $expectedUrl = 'expected_url'; - $this->cache->save($shortCode, $expectedUrl); + $this->cache->save($shortCode . '_longUrl', $expectedUrl); $this->em->getRepository(ShortUrl::class)->willReturn(null)->shouldBeCalledTimes(0); $url = $this->urlShortener->shortCodeToUrl($shortCode); From 18084433c7969e39f34fd360842622b4a08ad556 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 13:41:30 +0200 Subject: [PATCH 31/39] Created middleware to cache generated QR codes --- module/Common/src/Factory/CacheFactory.php | 1 + module/Core/config/dependencies.config.php | 2 + module/Core/config/routes.config.php | 6 +- .../src/Middleware/QrCodeCacheMiddleware.php | 73 +++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 module/Core/src/Middleware/QrCodeCacheMiddleware.php diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index c866b980..710c5962 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -3,6 +3,7 @@ namespace Shlinkio\Shlink\Common\Factory; use Doctrine\Common\Cache\ApcuCache; use Doctrine\Common\Cache\ArrayCache; +use Doctrine\Common\Cache\FilesystemCache; use Interop\Container\ContainerInterface; use Interop\Container\Exception\ContainerException; use Zend\ServiceManager\Exception\ServiceNotCreatedException; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index eb6168f9..6a5596ee 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -1,6 +1,7 @@ AnnotatedFactory::class, Action\QrCodeAction::class => AnnotatedFactory::class, + Middleware\QrCodeCacheMiddleware::class => AnnotatedFactory::class, ], ], diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index 9b499f71..9b0c071f 100644 --- a/module/Core/config/routes.config.php +++ b/module/Core/config/routes.config.php @@ -1,5 +1,6 @@ 'short-url-qr-code', 'path' => '/qr/{shortCode}[/{size:[0-9]+}]', - 'middleware' => Action\QrCodeAction::class, + 'middleware' => [ + Middleware\QrCodeCacheMiddleware::class, + Action\QrCodeAction::class, + ], 'allowed_methods' => ['GET'], ], ], diff --git a/module/Core/src/Middleware/QrCodeCacheMiddleware.php b/module/Core/src/Middleware/QrCodeCacheMiddleware.php new file mode 100644 index 00000000..d0f132bb --- /dev/null +++ b/module/Core/src/Middleware/QrCodeCacheMiddleware.php @@ -0,0 +1,73 @@ +cache = $cache; + } + + /** + * Process an incoming request and/or response. + * + * Accepts a server-side request and a response instance, and does + * something with them. + * + * If the response is not complete and/or further processing would not + * interfere with the work done in the middleware, or if the middleware + * wants to delegate to another process, it can use the `$out` callable + * if present. + * + * If the middleware does not return a value, execution of the current + * request is considered complete, and the response instance provided will + * be considered the response to return. + * + * Alternately, the middleware may return a response instance. + * + * Often, middleware will `return $out();`, with the assumption that a + * later middleware will return a response. + * + * @param Request $request + * @param Response $response + * @param null|callable $out + * @return null|Response + */ + public function __invoke(Request $request, Response $response, callable $out = null) + { + $cacheKey = $request->getUri()->getPath(); + + // If this QR code is already cached, just return it + if ($this->cache->contains($cacheKey)) { + $qrData = $this->cache->fetch($cacheKey); + $response->getBody()->write($qrData['body']); + return $response->withHeader('Content-Type', $qrData['content-type']); + } + + // If not, call the next middleware and cache it + /** @var Response $resp */ + $resp = $out($request, $response); + $this->cache->save($cacheKey, [ + 'body' => $resp->getBody()->__toString(), + 'content-type' => $resp->getHeaderLine('Content-Type'), + ]); + return $resp; + } +} From 12410e82d824f1f1db8fb1f851729e4afbbfa7c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 14:18:20 +0200 Subject: [PATCH 32/39] Created tests for QrCode middlewares --- module/Core/test/Action/QrCodeActionTest.php | 93 +++++++++++++++++++ .../Middleware/QrCodeCacheMiddlewareTest.php | 69 ++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 module/Core/test/Action/QrCodeActionTest.php create mode 100644 module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php new file mode 100644 index 00000000..d4924724 --- /dev/null +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -0,0 +1,93 @@ +prophesize(RouterInterface::class); + $router->generateUri(Argument::cetera())->willReturn('/foo/bar'); + + $this->urlShortener = $this->prophesize(UrlShortener::class); + + $this->action = new QrCodeAction($router->reveal(), $this->urlShortener->reveal()); + } + + /** + * @test + */ + public function aNonexistentShortCodeWillReturnNotFoundResponse() + { + $shortCode = 'abc123'; + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); + + $resp = $this->action->__invoke( + ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), + new Response(), + function ($req, $resp) { + return $resp; + } + ); + $this->assertEquals(404, $resp->getStatusCode()); + } + + /** + * @test + */ + public function anInvalidShortCodeWillReturnNotFoundResponse() + { + $shortCode = 'abc123'; + $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) + ->shouldBeCalledTimes(1); + + $resp = $this->action->__invoke( + ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), + new Response(), + function ($req, $resp) { + return $resp; + } + ); + $this->assertEquals(404, $resp->getStatusCode()); + } + + /** + * @test + */ + public function aCorrectRequestReturnsTheQrCodeResponse() + { + $shortCode = 'abc123'; + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(new ShortUrl())->shouldBeCalledTimes(1); + + $resp = $this->action->__invoke( + ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), + new Response(), + function ($req, $resp) { + return $resp; + } + ); + + $this->assertInstanceOf(QrCodeResponse::class, $resp); + $this->assertEquals(200, $resp->getStatusCode()); + } +} diff --git a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php new file mode 100644 index 00000000..64aac3e0 --- /dev/null +++ b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php @@ -0,0 +1,69 @@ +cache = new ArrayCache(); + $this->middleware = new QrCodeCacheMiddleware($this->cache); + } + + /** + * @test + */ + public function noCachedPathFallbacksToNextMiddleware() + { + $isCalled = false; + $this->middleware->__invoke( + ServerRequestFactory::fromGlobals(), + new Response(), + function ($req, $resp) use (&$isCalled) { + $isCalled = true; + return $resp; + } + ); + $this->assertTrue($isCalled); + } + + /** + * @test + */ + public function cachedPathReturnsCacheContent() + { + $isCalled = false; + $uri = (new Uri())->withPath('/foo'); + $this->cache->save('/foo', ['body' => 'the body', 'content-type' => 'image/png']); + + $resp = $this->middleware->__invoke( + ServerRequestFactory::fromGlobals()->withUri($uri), + new Response(), + function ($req, $resp) use (&$isCalled) { + $isCalled = true; + return $resp; + } + ); + + $this->assertFalse($isCalled); + $resp->getBody()->rewind(); + $this->assertEquals('the body', $resp->getBody()->getContents()); + $this->assertEquals('image/png', $resp->getHeaderLine('Content-Type')); + } +} From 90cef7d4d904f318c8b6b8b03ccdd9cc9bc932d4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 14:19:46 +0200 Subject: [PATCH 33/39] Removed unused import --- module/Common/src/Factory/CacheFactory.php | 1 - 1 file changed, 1 deletion(-) diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index 710c5962..c866b980 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -3,7 +3,6 @@ namespace Shlinkio\Shlink\Common\Factory; use Doctrine\Common\Cache\ApcuCache; use Doctrine\Common\Cache\ArrayCache; -use Doctrine\Common\Cache\FilesystemCache; use Interop\Container\ContainerInterface; use Interop\Container\Exception\ContainerException; use Zend\ServiceManager\Exception\ServiceNotCreatedException; From 090479fa62b09290690c1dd34b03c6fcd367895b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 17:58:47 +0200 Subject: [PATCH 34/39] Improved CacheFactory supporting more adapters --- module/Common/src/Factory/CacheFactory.php | 48 ++++++++++++++++--- .../Common/test/Factory/CacheFactoryTest.php | 44 +++++++++++++++-- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index c866b980..905ebf56 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -1,8 +1,7 @@ resolveCacheAdapter($config['cache']); } // If the adapter has not been set in config, create one based on environment - return env('APP_ENV', 'pro') === 'pro' ? new ApcuCache() : new ArrayCache(); + return env('APP_ENV', 'pro') === 'pro' ? new Cache\ApcuCache() : new Cache\ArrayCache(); + } + + /** + * @param array $cacheConfig + * @return Cache\Cache + */ + protected function resolveCacheAdapter(array $cacheConfig) + { + switch ($cacheConfig['adapter']) { + case Cache\ArrayCache::class: + case Cache\ApcuCache::class: + return new $cacheConfig['adapter'](); + case Cache\FilesystemCache::class: + case Cache\PhpFileCache::class: + return new $cacheConfig['adapter']($cacheConfig['options']['dir']); + case Cache\MemcachedCache::class: + $memcached = new \Memcached(); + $servers = isset($cacheConfig['options']['servers']) ? $cacheConfig['options']['servers'] : []; + + foreach ($servers as $server) { + if (! isset($server['host'])) { + continue; + } + $port = isset($server['port']) ? intval($server['port']) : 11211; + + $memcached->addServer($server['host'], $port); + } + + $cache = new Cache\MemcachedCache(); + $cache->setMemcached($memcached); + return $cache; + default: + return new Cache\ArrayCache(); + } } } diff --git a/module/Common/test/Factory/CacheFactoryTest.php b/module/Common/test/Factory/CacheFactoryTest.php index 2e938dfa..607849ac 100644 --- a/module/Common/test/Factory/CacheFactoryTest.php +++ b/module/Common/test/Factory/CacheFactoryTest.php @@ -4,6 +4,8 @@ namespace ShlinkioTest\Shlink\Common\Factory; use Doctrine\Common\Cache\ApcuCache; use Doctrine\Common\Cache\ArrayCache; use Doctrine\Common\Cache\FilesystemCache; +use Doctrine\Common\Cache\MemcachedCache; +use Doctrine\Common\Cache\RedisCache; use PHPUnit_Framework_TestCase as TestCase; use Shlinkio\Shlink\Common\Factory\CacheFactory; use Zend\ServiceManager\ServiceManager; @@ -61,15 +63,51 @@ class CacheFactoryTest extends TestCase public function invalidAdapterDefinedInConfigFallbacksToEnvironment() { putenv('APP_ENV=pro'); - $instance = $this->factory->__invoke($this->createSM(FilesystemCache::class), ''); + $instance = $this->factory->__invoke($this->createSM(RedisCache::class), ''); $this->assertInstanceOf(ApcuCache::class, $instance); } - private function createSM($cacheAdapter = null) + /** + * @test + */ + public function filesystemCacheAdaptersReadDirOption() + { + $dir = sys_get_temp_dir(); + /** @var FilesystemCache $instance */ + $instance = $this->factory->__invoke($this->createSM(FilesystemCache::class, ['dir' => $dir]), ''); + $this->assertInstanceOf(FilesystemCache::class, $instance); + $this->assertEquals($dir, $instance->getDirectory()); + } + + /** + * @test + */ + public function memcachedCacheAdaptersReadServersOption() + { + $servers = [ + [ + 'host' => '1.2.3.4', + 'port' => 123 + ], + [ + 'host' => '4.3.2.1', + 'port' => 321 + ], + ]; + /** @var MemcachedCache $instance */ + $instance = $this->factory->__invoke($this->createSM(MemcachedCache::class, ['servers' => $servers]), ''); + $this->assertInstanceOf(MemcachedCache::class, $instance); + $this->assertEquals($servers, $instance->getMemcached()->getServerList()); + } + + private function createSM($cacheAdapter = null, array $options = []) { return new ServiceManager(['services' => [ 'config' => isset($cacheAdapter) ? [ - 'cache' => ['adapter' => $cacheAdapter], + 'cache' => [ + 'adapter' => $cacheAdapter, + 'options' => $options, + ], ] : [], ]]); } From 3140ab2ad7a7144cc9c1762106f8fe7afb753436 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 18:17:28 +0200 Subject: [PATCH 35/39] Updated shlink website link to use https --- module/Core/templates/core/layout/default.html.twig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/templates/core/layout/default.html.twig b/module/Core/templates/core/layout/default.html.twig index 6f93dfdd..4b405c5b 100644 --- a/module/Core/templates/core/layout/default.html.twig +++ b/module/Core/templates/core/layout/default.html.twig @@ -27,7 +27,7 @@
{% block footer %}

- © {{ "now" | date("Y") }} Shlink + © {{ "now" | date("Y") }} Shlink

{% endblock %} From 5913550eec2523d1b8df860dbacdd03f3d92357f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 18:29:47 +0200 Subject: [PATCH 36/39] Fixed build when memcached is not enabled in PHP 7.1 --- module/Common/src/Factory/CacheFactory.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index 905ebf56..1add99dd 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -59,6 +59,10 @@ class CacheFactory implements FactoryInterface case Cache\PhpFileCache::class: return new $cacheConfig['adapter']($cacheConfig['options']['dir']); case Cache\MemcachedCache::class: + if (! class_exists(\Memcached::class)) { + return new Cache\ArrayCache(); + } + $memcached = new \Memcached(); $servers = isset($cacheConfig['options']['servers']) ? $cacheConfig['options']['servers'] : []; From 69cc30bce7697d51315d392123a3754bb61faa12 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 18:38:48 +0200 Subject: [PATCH 37/39] Allowed failures on PHP 7.1 environments --- .travis.yml | 5 +++++ module/Common/src/Factory/CacheFactory.php | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 52312334..0f92960b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,3 +23,8 @@ after_script: - php ocular.phar code-coverage:upload --format=php-clover build/clover.xml sudo: false + +matrix: + fast_finish: true + allow_failures: + - php: 7.1 diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index 1add99dd..905ebf56 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -59,10 +59,6 @@ class CacheFactory implements FactoryInterface case Cache\PhpFileCache::class: return new $cacheConfig['adapter']($cacheConfig['options']['dir']); case Cache\MemcachedCache::class: - if (! class_exists(\Memcached::class)) { - return new Cache\ArrayCache(); - } - $memcached = new \Memcached(); $servers = isset($cacheConfig['options']['servers']) ? $cacheConfig['options']['servers'] : []; From 39d2f5a38f4c04a612de3f39a78dd860edadf195 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 18:50:14 +0200 Subject: [PATCH 38/39] Created travis config file to enable memcached extension --- .travis-php.ini | 1 + .travis.yml | 7 ++----- 2 files changed, 3 insertions(+), 5 deletions(-) create mode 100644 .travis-php.ini diff --git a/.travis-php.ini b/.travis-php.ini new file mode 100644 index 00000000..c9a2ff0c --- /dev/null +++ b/.travis-php.ini @@ -0,0 +1 @@ +extension="memcached.so" \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index 0f92960b..83d37309 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,8 @@ php: - 7 - 7.1 +before_install: phpenv config-add .travis-php.ini + before_script: - composer self-update - composer install --no-interaction @@ -23,8 +25,3 @@ after_script: - php ocular.phar code-coverage:upload --format=php-clover build/clover.xml sudo: false - -matrix: - fast_finish: true - allow_failures: - - php: 7.1 From 43c6b56e4280b735ecb4524095e76d1eee27395f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 18:56:01 +0200 Subject: [PATCH 39/39] Fixed memcached test while comparing servers --- module/Common/test/Factory/CacheFactoryTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Common/test/Factory/CacheFactoryTest.php b/module/Common/test/Factory/CacheFactoryTest.php index 607849ac..b7fc49a8 100644 --- a/module/Common/test/Factory/CacheFactoryTest.php +++ b/module/Common/test/Factory/CacheFactoryTest.php @@ -97,7 +97,7 @@ class CacheFactoryTest extends TestCase /** @var MemcachedCache $instance */ $instance = $this->factory->__invoke($this->createSM(MemcachedCache::class, ['servers' => $servers]), ''); $this->assertInstanceOf(MemcachedCache::class, $instance); - $this->assertEquals($servers, $instance->getMemcached()->getServerList()); + $this->assertEquals(count($servers), count($instance->getMemcached()->getServerList())); } private function createSM($cacheAdapter = null, array $options = [])