From 4077c81f48ddd4ad5641229e3001e638106218b8 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Fri, 11 Jun 2021 14:26:17 +0200 Subject: [PATCH 01/34] OpenId-Connect-Bundle: Upgraded to itk-dev/openid-connect 2.0.0 --- composer.json | 2 +- src/Controller/LoginController.php | 23 +++++------ src/DependencyInjection/Configuration.php | 2 +- .../ItkDevOpenIdConnectExtension.php | 13 ++++--- src/Resources/config/services.yaml | 24 ++++++++++-- src/Security/OpenIdLoginAuthenticator.php | 38 +++++++++++-------- 6 files changed, 66 insertions(+), 36 deletions(-) diff --git a/composer.json b/composer.json index bc6800b..fb51f1d 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,7 @@ "symfony/framework-bundle": "^5.2", "doctrine/orm": "^2.8", "symfony/security-bundle": "^5.2", - "itk-dev/openid-connect": "^1.0", + "itk-dev/openid-connect": "^2.0", "symfony/yaml": "^5.2" }, "require-dev": { diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index dda9f0a..ebfcc31 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -2,6 +2,7 @@ namespace ItkDev\OpenIdConnectBundle\Controller; +use ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException; use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -10,29 +11,29 @@ class LoginController extends AbstractController { - /** - * @var array + * @var OpenIdConfigurationProvider */ - private $openIdProviderOptions; + private $provider; - public function __construct(array $openIdProviderOptions) + public function __construct(OpenIdConfigurationProvider $provider) { - $this->openIdProviderOptions = $openIdProviderOptions; + $this->provider = $provider; } /** - * @param SessionInterface $session - * @return Response + * @throws ItkOpenIdConnectException */ public function login(SessionInterface $session): Response { - $provider = new OpenIdConfigurationProvider($this->openIdProviderOptions); + $nonce = $this->provider->generateNonce(); + $state = $this->provider->generateState(); - $authUrl = $provider->getAuthorizationUrl(); + // Save to session + $session->set('oauth2state', $state); + $session->set('oauth2nonce', $nonce); - // Set a oauth2state to avoid CSRF check it in authenticator - $session->set('oauth2state', $provider->getState()); + $authUrl = $this->provider->getAuthorizationUrl(['state' => $state, 'nonce' => $nonce]); return new RedirectResponse($authUrl); } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 59622d8..3a75c7a 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -12,7 +12,7 @@ class Configuration implements ConfigurationInterface */ public function getConfigTreeBuilder(): TreeBuilder { - $treeBuilder = new TreeBuilder('itk_dev_open_id_connect'); + $treeBuilder = new TreeBuilder('itkdev_open_id_connect'); // Specify which variables must be configured in itk_dev_openid_connect file // That is client_id, client_secret, discovery url and cache path diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index 85eb301..c279092 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -2,6 +2,7 @@ namespace ItkDev\OpenIdConnectBundle\DependencyInjection; +use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use ItkDev\OpenIdConnectBundle\Controller\LoginController; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -9,6 +10,7 @@ use Symfony\Component\DependencyInjection\Loader\FileLoader; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; +use Symfony\Component\DependencyInjection\Reference; class ItkDevOpenIdConnectExtension extends Extension { @@ -23,16 +25,17 @@ public function load(array $configs, ContainerBuilder $container) $configuration = new Configuration(); $config = $this->processConfiguration($configuration, $configs); - $newConfig = [ - 'urlConfiguration' => $config['open_id_provider_options']['configuration_url'], + $providerConfig = [ + 'openIDConnectMetadataUrl' => $config['open_id_provider_options']['configuration_url'], 'clientId' => $config['open_id_provider_options']['client_id'], 'clientSecret' => $config['open_id_provider_options']['client_secret'], - 'cachePath' => $config['open_id_provider_options']['cache_path'], + 'cacheItemPool' => new Reference($config['open_id_provider_options']['cache_path']), 'redirectUri' => $config['open_id_provider_options']['callback_uri'], ]; - $definition = $container->getDefinition(LoginController::class); - $definition->replaceArgument('$openIdProviderOptions', $newConfig); + $definition = $container->getDefinition(OpenIdConfigurationProvider::class); + $definition->replaceArgument('$options', $providerConfig); + $definition->replaceArgument('$collaborators', []); } /** diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index 157f62a..61981ca 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -7,7 +7,25 @@ services: # makes classes in src/ available to be used as services # this creates a service per class whose id is the fully-qualified class name - ItkDev\OpenIdConnectBundle\Controller\LoginController: - public: true + + ItkDev\OpenIdConnectBundle\: + resource: '../../../src/*' + exclude: + - '../../../src/DependencyInjection/' + - '../../../src/Kernel.php' + - '../../../src/Tests/' + + + ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider: + arguments: + $options: ~ + $collaborators: ~ + + #ItkDev\OpenIdConnectBundle\Controller\LoginController: + # arguments: + # $provider: ~ + + ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator: arguments: - $openIdProviderOptions: ~ \ No newline at end of file + $provider: ~ + $session: ~ \ No newline at end of file diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php index 53ef97a..9141ab4 100644 --- a/src/Security/OpenIdLoginAuthenticator.php +++ b/src/Security/OpenIdLoginAuthenticator.php @@ -2,6 +2,9 @@ namespace ItkDev\OpenIdConnectBundle\Security; +use ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException; +use ItkDev\OpenIdConnect\Exception\ValidationException; +use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; @@ -17,9 +20,14 @@ abstract class OpenIdLoginAuthenticator extends AbstractGuardAuthenticator */ private $session; + /** + * @var OpenIdConfigurationProvider + */ + private $provider; - public function __construct(SessionInterface $session) + public function __construct(OpenIdConfigurationProvider $provider, SessionInterface $session) { + $this->provider = $provider; $this->session = $session; } @@ -29,27 +37,27 @@ public function supports(Request $request) return $request->query->has('state') && $request->query->has('id_token'); } + /** + * @throws ValidationException + */ public function getCredentials(Request $request) { - // Make sure state and oauth2sate are the same + // Make sure state and oauth2state are the same if ($request->query->get('state') !== $this->session->get('oauth2state')) { $this->session->remove('oauth2state'); - throw new \RuntimeException('Invalid state'); + throw new ValidationException('Invalid state'); } - - // Retrieve id_token and decode it - // @see https://tools.ietf.org/html/rfc7519 - $idToken = $request->query->get('id_token'); - if (null === $idToken) { - throw new \RuntimeException('Id token not found'); - } - - if (!is_string($idToken)) { - throw new \RuntimeException('Id token not type string'); + try { + $claims = $this->provider->validateIdToken($request->query->get('id_token'), $this->session->get('oauth2nonce')); + // Authentication successful + } catch (ItkOpenIdConnectException $exception) { + // Handle failed authentication + throw new ValidationException('Token validation failed.'); + } finally { + $this->session->remove('oauth2nonce'); } - [$jose, $payload, $signature] = array_map('base64_decode', explode('.', $idToken)); - return json_decode($payload, true); + return (array) $claims; } abstract public function getUser($credentials, UserProviderInterface $userProvider); From b2f69a043b436c9312c0dca50198d5c2f8d680be Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Fri, 11 Jun 2021 14:55:29 +0200 Subject: [PATCH 02/34] OpenId-Connect-Bundle: Explicitly declared services --- src/Resources/config/services.yaml | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index 61981ca..c7a07f5 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -5,25 +5,14 @@ services: autowire: true # Automatically injects dependencies in your services. autoconfigure: true # Automatically registers your services as commands, event subscribers, etc. - # makes classes in src/ available to be used as services - # this creates a service per class whose id is the fully-qualified class name - - ItkDev\OpenIdConnectBundle\: - resource: '../../../src/*' - exclude: - - '../../../src/DependencyInjection/' - - '../../../src/Kernel.php' - - '../../../src/Tests/' - - ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider: arguments: $options: ~ $collaborators: ~ - #ItkDev\OpenIdConnectBundle\Controller\LoginController: - # arguments: - # $provider: ~ + ItkDev\OpenIdConnectBundle\Controller\LoginController: + arguments: + $provider: '@ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider' ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator: arguments: From 90d294d80a2b37c17a31e7abfe143fe0c69c50bf Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Fri, 11 Jun 2021 14:58:31 +0200 Subject: [PATCH 03/34] OpenId-Connect-Bundle: Consistency in service configuration --- src/Resources/config/services.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index c7a07f5..93e31dd 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -16,5 +16,5 @@ services: ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator: arguments: - $provider: ~ + $provider: '@ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider' $session: ~ \ No newline at end of file From 793a615f37ac105ba856c04ec80f8a08425216b6 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Mon, 14 Jun 2021 09:46:29 +0200 Subject: [PATCH 04/34] OpenId-Connect-Bundle: Made check for idToken being string during validation --- src/Security/OpenIdLoginAuthenticator.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php index 9141ab4..5cc903e 100644 --- a/src/Security/OpenIdLoginAuthenticator.php +++ b/src/Security/OpenIdLoginAuthenticator.php @@ -48,7 +48,17 @@ public function getCredentials(Request $request) throw new ValidationException('Invalid state'); } try { - $claims = $this->provider->validateIdToken($request->query->get('id_token'), $this->session->get('oauth2nonce')); + $idToken = $request->query->get('id_token'); + + if (null === $idToken){ + throw new ValidationException('Id token not found.'); + } + + if (!is_string($idToken)) { + throw new ValidationException('Id token not type string'); + } + + $claims = $this->provider->validateIdToken($idToken, $this->session->get('oauth2nonce')); // Authentication successful } catch (ItkOpenIdConnectException $exception) { // Handle failed authentication From 3428810e6959dc602aaf6007da1b666b044a04fd Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Mon, 14 Jun 2021 14:21:49 +0200 Subject: [PATCH 05/34] OpenId-Connect-Bundle: Made naming consistent and updated README --- README.md | 8 ++++---- src/DependencyInjection/Configuration.php | 4 ++-- .../ItkDevOpenIdConnectExtension.php | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 3ec6f5a..7c4be9d 100644 --- a/README.md +++ b/README.md @@ -31,8 +31,8 @@ In `/config/packages/` you need the following `itk_dev_openid_connect.yaml` file for configuring OpenId Connect variables ```yaml -itk_dev_open_id_connect: - open_id_provider_options: +itkdev_openid_connect: + openid_provider_options: configuration_url: 'https://.../openid-configuration..' # url to OpenId Discovery document client_id: 'client_id' # Client id assigned by authorizer client_secret: 'client_secret' # Client password assigned by authorizer @@ -44,8 +44,8 @@ In `/config/routes/` you need a similar `itk_dev_openid_connect.yaml` file for configuring the routing ```yaml -itk_dev_openid_connect: - resource: "@ItkDevOpenIdConnectBundle/config/routes.yaml" +itkdev_openid_connect: + resource: "@ItkDevOpenIdConnectBundle/src/Resources/config/routes.yaml" prefix: "/openidconnect" # Prefix for bundle routes ``` diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3a75c7a..7b6588c 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -12,7 +12,7 @@ class Configuration implements ConfigurationInterface */ public function getConfigTreeBuilder(): TreeBuilder { - $treeBuilder = new TreeBuilder('itkdev_open_id_connect'); + $treeBuilder = new TreeBuilder('itkdev_openid_connect'); // Specify which variables must be configured in itk_dev_openid_connect file // That is client_id, client_secret, discovery url and cache path @@ -20,7 +20,7 @@ public function getConfigTreeBuilder(): TreeBuilder $treeBuilder->getRootNode() ->children() - ->arrayNode('open_id_provider_options') + ->arrayNode('openid_provider_options') ->isRequired() ->children() ->scalarNode('configuration_url') diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index c279092..36b092b 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -26,11 +26,11 @@ public function load(array $configs, ContainerBuilder $container) $config = $this->processConfiguration($configuration, $configs); $providerConfig = [ - 'openIDConnectMetadataUrl' => $config['open_id_provider_options']['configuration_url'], - 'clientId' => $config['open_id_provider_options']['client_id'], - 'clientSecret' => $config['open_id_provider_options']['client_secret'], - 'cacheItemPool' => new Reference($config['open_id_provider_options']['cache_path']), - 'redirectUri' => $config['open_id_provider_options']['callback_uri'], + 'openIDConnectMetadataUrl' => $config['openid_provider_options']['configuration_url'], + 'clientId' => $config['openid_provider_options']['client_id'], + 'clientSecret' => $config['openid_provider_options']['client_secret'], + 'cacheItemPool' => new Reference($config['openid_provider_options']['cache_path']), + 'redirectUri' => $config['openid_provider_options']['callback_uri'], ]; $definition = $container->getDefinition(OpenIdConfigurationProvider::class); From 41b3eb8519ef8566281e082c8553a8bf457d0c34 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 10:22:12 +0200 Subject: [PATCH 06/34] OpenId-Connect-Bundle: Add leeway and update tests --- README.md | 19 ++++++++-------- src/Resources/config/routes.yaml | 2 +- src/Resources/config/services.yaml | 3 +++ src/Security/OpenIdLoginAuthenticator.php | 10 ++++++--- tests/ItkDevOpenIdConnectBundleTest.php | 22 +++++++++++++------ ...ItkDevOpenIdConnectBundleTestingKernel.php | 17 ++++++++------ tests/config/framework.yml | 20 +++++++++++++++++ tests/config/itkdev_openid_connect.yml | 7 ++++++ 8 files changed, 73 insertions(+), 27 deletions(-) create mode 100644 tests/config/framework.yml create mode 100644 tests/config/itkdev_openid_connect.yml diff --git a/README.md b/README.md index 7c4be9d..a6289fb 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ the bundle authenticator, `OpenIdLoginAuthenticator`. ### Variable configuration -In `/config/packages/` you need the following `itk_dev_openid_connect.yaml` +In `/config/packages/` you need the following `itkdev_openid_connect.yaml` file for configuring OpenId Connect variables ```yaml @@ -41,7 +41,7 @@ itkdev_openid_connect: ``` In `/config/routes/` you need a similar -`itk_dev_openid_connect.yaml` file for configuring the routing +`itkdev_openid_connect.yaml` file for configuring the routing ```yaml itkdev_openid_connect: @@ -136,11 +136,12 @@ class TestAuthenticator extends OpenIdLoginAuthenticator */ private $entityManager; - public function __construct(EntityManagerInterface $entityManager, SessionInterface $session, UrlGeneratorInterface $router) + public function __construct(EntityManagerInterface $entityManager, SessionInterface $session, UrlGeneratorInterface $router, OpenIdConfigurationProvider $provider) { $this->router = $router; $this->entityManager = $entityManager; - parent::__construct($session); + // Set leeway directly or configure via .env (must be positive) + parent::__construct($provider, $session, $leeway); } public function getUser($credentials, UserProviderInterface $userProvider) @@ -148,19 +149,19 @@ class TestAuthenticator extends OpenIdLoginAuthenticator $name = $credentials['name']; $email = $credentials['upn']; - //Check if user exists already - if not create a user + // Check if user exists already - if not create a user $user = $this->entityManager->getRepository(User::class) ->findOneBy(['email'=> $email]); if (null === $user) { // Create the new user $user = new User(); } - // Update/set names here + // Update/set user properties $user->setName($name); $user->setEmail($email); - // persist and flush user to database - // If no change persist will recognize this + // Persist and flush user to database + // If no change, persist will recognize this $this->entityManager->persist($user); $this->entityManager->flush(); @@ -174,7 +175,7 @@ class TestAuthenticator extends OpenIdLoginAuthenticator public function start(Request $request, AuthenticationException $authException = null) { - return new RedirectResponse($this->router->generate('itk_dev_openid_connect_login')); + return new RedirectResponse($this->router->generate('itkdev_openid_connect_login')); } } ``` diff --git a/src/Resources/config/routes.yaml b/src/Resources/config/routes.yaml index 1b78813..4aa609d 100644 --- a/src/Resources/config/routes.yaml +++ b/src/Resources/config/routes.yaml @@ -1,3 +1,3 @@ -itk_dev_openid_connect_login: +itkdev_openid_connect_login: path: /login controller: ItkDev\OpenIdConnectBundle\Controller\LoginController::login \ No newline at end of file diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index 93e31dd..dbaad1b 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -6,15 +6,18 @@ services: autoconfigure: true # Automatically registers your services as commands, event subscribers, etc. ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider: + public: true arguments: $options: ~ $collaborators: ~ ItkDev\OpenIdConnectBundle\Controller\LoginController: + public: true arguments: $provider: '@ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider' ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator: + public: true arguments: $provider: '@ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider' $session: ~ \ No newline at end of file diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php index 5cc903e..fbff427 100644 --- a/src/Security/OpenIdLoginAuthenticator.php +++ b/src/Security/OpenIdLoginAuthenticator.php @@ -25,10 +25,13 @@ abstract class OpenIdLoginAuthenticator extends AbstractGuardAuthenticator */ private $provider; - public function __construct(OpenIdConfigurationProvider $provider, SessionInterface $session) + private $leeway; + + public function __construct(OpenIdConfigurationProvider $provider, SessionInterface $session, int $leeway = 0) { $this->provider = $provider; $this->session = $session; + $this->leeway = $leeway; } public function supports(Request $request) @@ -47,6 +50,7 @@ public function getCredentials(Request $request) $this->session->remove('oauth2state'); throw new ValidationException('Invalid state'); } + try { $idToken = $request->query->get('id_token'); @@ -58,11 +62,11 @@ public function getCredentials(Request $request) throw new ValidationException('Id token not type string'); } - $claims = $this->provider->validateIdToken($idToken, $this->session->get('oauth2nonce')); + $claims = $this->provider->validateIdToken($idToken, $this->session->get('oauth2nonce'), $this->leeway); // Authentication successful } catch (ItkOpenIdConnectException $exception) { // Handle failed authentication - throw new ValidationException('Token validation failed.'); + throw new ValidationException($exception->getMessage()); } finally { $this->session->remove('oauth2nonce'); } diff --git a/tests/ItkDevOpenIdConnectBundleTest.php b/tests/ItkDevOpenIdConnectBundleTest.php index afb6b0e..19e63cc 100644 --- a/tests/ItkDevOpenIdConnectBundleTest.php +++ b/tests/ItkDevOpenIdConnectBundleTest.php @@ -2,8 +2,11 @@ namespace ItkDev\OpenIdConnectBundle\Tests; +use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use ItkDev\OpenIdConnectBundle\Controller\LoginController; +use ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator; use PHPUnit\Framework\TestCase; +use Symfony\Component\Cache\Adapter\ArrayAdapter; /** * Class ItkDevOpenIdConnectBundleTest @@ -16,20 +19,25 @@ class ItkDevOpenIdConnectBundleTest extends TestCase public function testServiceWiring() { $kernel = new ItkDevOpenIdConnectBundleTestingKernel([ - 'open_id_provider_options' => [ - 'configuration_url' => 'https://provider.com/openid-configuration', - 'client_id' => 'test_id', - 'client_secret' => 'test_secret', - 'cache_path' => 'test_path', - 'callback_uri' => 'https://app.com/callback_uri' - ] + __DIR__.'/config/framework.yml', + __DIR__.'/config/itkdev_openid_connect.yml', ]); $kernel->boot(); $container = $kernel->getContainer(); + // LoginController service $this->assertTrue($container->has(LoginController::class)); $controller = $container->get(LoginController::class); $this->assertInstanceOf(LoginController::class, $controller); + + // OpenIdConfigurationProvider service + $this->assertTrue($container->has(OpenIdConfigurationProvider::class)); + + $provider = $container->get(OpenIdConfigurationProvider::class); + $this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider); + + // OpenIdLoginAuthenticator service + $this->assertTrue($container->has(OpenIdLoginAuthenticator::class)); } } diff --git a/tests/ItkDevOpenIdConnectBundleTestingKernel.php b/tests/ItkDevOpenIdConnectBundleTestingKernel.php index fbeb691..74c611b 100644 --- a/tests/ItkDevOpenIdConnectBundleTestingKernel.php +++ b/tests/ItkDevOpenIdConnectBundleTestingKernel.php @@ -8,6 +8,7 @@ namespace ItkDev\OpenIdConnectBundle\Tests; use ItkDev\OpenIdConnectBundle\ItkDevOpenIdConnectBundle; +use Symfony\Bundle\FrameworkBundle\FrameworkBundle; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpClient\CurlHttpClient; @@ -19,12 +20,11 @@ */ class ItkDevOpenIdConnectBundleTestingKernel extends Kernel { - private $config; + private $pathToConfigs; - public function __construct(array $config) + public function __construct(array $pathToConfigs) { - $this->config = $config; - + $this->pathToConfigs = $pathToConfigs; parent::__construct('test', true); } @@ -35,6 +35,7 @@ public function registerBundles() { return [ new ItkDevOpenIdConnectBundle(), + new FrameworkBundle(), ]; } @@ -43,8 +44,10 @@ public function registerBundles() */ public function registerContainerConfiguration(LoaderInterface $loader) { - $loader->load(function (ContainerBuilder $containerBuilder) { - $containerBuilder->loadFromExtension('itkdev_openid_connect', $this->config); - }); + foreach ($this->pathToConfigs as $path) { + if (file_exists($path)) { + $loader->load($path); + } + } } } diff --git a/tests/config/framework.yml b/tests/config/framework.yml new file mode 100644 index 0000000..8a3124e --- /dev/null +++ b/tests/config/framework.yml @@ -0,0 +1,20 @@ +framework: + cache: + pools: + cache.array: + default_lifetime: 0 + adapters: + - cache.adapter.array + test: true + session: + storage_id: session.storage.mock_file + form: false + csrf_protection: false + validation: + enabled: false + esi: + enabled: false + workflows: + enabled: false + translator: + enabled: false diff --git a/tests/config/itkdev_openid_connect.yml b/tests/config/itkdev_openid_connect.yml new file mode 100644 index 0000000..7168cfe --- /dev/null +++ b/tests/config/itkdev_openid_connect.yml @@ -0,0 +1,7 @@ +itkdev_openid_connect: + openid_provider_options: + configuration_url: 'https://provider.com/openid-configuration' + client_id: 'test_id' + client_secret: 'test_secret' + cache_path: 'cache.array' + callback_uri: 'https://app.com/callback_uri' \ No newline at end of file From 5598a85c3836976b6bda8b17acac2b8b010e5a37 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 10:25:32 +0200 Subject: [PATCH 07/34] OpenId-Connect-Bundle: Applied codind standards --- src/Security/OpenIdLoginAuthenticator.php | 2 +- tests/ItkDevOpenIdConnectBundleTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php index fbff427..fdc7613 100644 --- a/src/Security/OpenIdLoginAuthenticator.php +++ b/src/Security/OpenIdLoginAuthenticator.php @@ -54,7 +54,7 @@ public function getCredentials(Request $request) try { $idToken = $request->query->get('id_token'); - if (null === $idToken){ + if (null === $idToken) { throw new ValidationException('Id token not found.'); } diff --git a/tests/ItkDevOpenIdConnectBundleTest.php b/tests/ItkDevOpenIdConnectBundleTest.php index 19e63cc..4a3e0cf 100644 --- a/tests/ItkDevOpenIdConnectBundleTest.php +++ b/tests/ItkDevOpenIdConnectBundleTest.php @@ -19,8 +19,8 @@ class ItkDevOpenIdConnectBundleTest extends TestCase public function testServiceWiring() { $kernel = new ItkDevOpenIdConnectBundleTestingKernel([ - __DIR__.'/config/framework.yml', - __DIR__.'/config/itkdev_openid_connect.yml', + __DIR__ . '/config/framework.yml', + __DIR__ . '/config/itkdev_openid_connect.yml', ]); $kernel->boot(); $container = $kernel->getContainer(); From 19c81023977689c3e5f2ab28db527cab0b7adce5 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 10:40:58 +0200 Subject: [PATCH 08/34] OpenId-Connect-Bundle: Updated CHANGELOG --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42045a7..664f3a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,4 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - This CHANGELOG file to hopefully serve as an evolving example of a standardized open source project CHANGELOG. - PHP-CS-Fixer -- Markdownlint \ No newline at end of file +- Markdownlint +- Test Suite +- Psalm setup for static analysis +- Code formatting +- OpenID Connect Bundle: Upgraded from + `itk-dev/openid-connect` 1.0.0 to 2.1.0 \ No newline at end of file From 47c023dc58680e1c98819df6aefcaf561a7baf43 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 10:45:14 +0200 Subject: [PATCH 09/34] OpenId-Connect-Bundle: Fixed small mistakes and coding standards in README --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a6289fb..304199f 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ docker compose exec phpfpm ./vendor/bin/phpunit ### Psalm static analysis -Where using [Psalm](https://psalm.dev/) for static analysis. To run +Where using [Psalm](https://psalm.dev/) for static analysis. To run psalm do ```shell @@ -231,7 +231,7 @@ the coding standard for the project. ```shell docker run -v ${PWD}:/app itkdev/yarn:latest install - docker run -v ${PWD}:/app itkdev/yarn:latest check-coding-standards + docker run -v ${PWD}:/app itkdev/yarn:latest coding-standards-check ``` ### Apply Coding Standards @@ -248,7 +248,7 @@ To attempt to automatically fix coding style ```shell docker run -v ${PWD}:/app itkdev/yarn:latest install - docker run -v ${PWD}:/app itkdev/yarn:latest check-coding-standards + docker run -v ${PWD}:/app itkdev/yarn:latest coding-standards-apply ``` ## CI From ca5fc4721928bbe096a97db68663121d9aa6f597 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 16 Jun 2021 09:47:52 +0200 Subject: [PATCH 10/34] OpenId-Connect-Bundle: Updated ExampleAuthenticator in README --- README.md | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 304199f..f61dcac 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ security: main: guard: authenticators: - - App\Security\TestAuthenticator + - App\Security\ExampleAuthenticator ``` #### Example authenticator functions @@ -125,7 +125,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\User\UserProviderInterface; -class TestAuthenticator extends OpenIdLoginAuthenticator +class ExampleAuthenticator extends OpenIdLoginAuthenticator { /** * @var UrlGeneratorInterface @@ -136,16 +136,16 @@ class TestAuthenticator extends OpenIdLoginAuthenticator */ private $entityManager; - public function __construct(EntityManagerInterface $entityManager, SessionInterface $session, UrlGeneratorInterface $router, OpenIdConfigurationProvider $provider) + public function __construct(EntityManagerInterface $entityManager, int $leeway, SessionInterface $session, UrlGeneratorInterface $router, OpenIdConfigurationProvider $provider) { $this->router = $router; $this->entityManager = $entityManager; - // Set leeway directly or configure via .env (must be positive) parent::__construct($provider, $session, $leeway); } public function getUser($credentials, UserProviderInterface $userProvider) { + // Extract properties from credentials $name = $credentials['name']; $email = $credentials['upn']; @@ -153,16 +153,14 @@ class TestAuthenticator extends OpenIdLoginAuthenticator $user = $this->entityManager->getRepository(User::class) ->findOneBy(['email'=> $email]); if (null === $user) { - // Create the new user + // Create the new user and persist it $user = new User(); + $this->entityManager->persist($user); } // Update/set user properties $user->setName($name); $user->setEmail($email); - // Persist and flush user to database - // If no change, persist will recognize this - $this->entityManager->persist($user); $this->entityManager->flush(); return $user; @@ -180,6 +178,22 @@ class TestAuthenticator extends OpenIdLoginAuthenticator } ``` +For this example we have bound `$leeway` via `.env` +and `services.yaml`: + +```text +###> itk-dev/openid-connect-bundle ### +ITK_DEV_LEEWAY=10 +###< itk-dev/openid-connect-bundle ### +``` + +```yaml +services: + _defaults: + bind: + $leeway: '%env(ITK_DEV_LEEWAY)%' +``` + ## Changes for Symfony 6.0 In Symfony 6.0 a new security system is From 98c6ccf5e0f22665ccba6d900163382bbc69b1ac Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 16 Jun 2021 09:50:24 +0200 Subject: [PATCH 11/34] OpenId-Connect-Bundle: Made sure to remove oauth2state from session --- src/Security/OpenIdLoginAuthenticator.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php index fdc7613..692a00e 100644 --- a/src/Security/OpenIdLoginAuthenticator.php +++ b/src/Security/OpenIdLoginAuthenticator.php @@ -46,8 +46,10 @@ public function supports(Request $request) public function getCredentials(Request $request) { // Make sure state and oauth2state are the same - if ($request->query->get('state') !== $this->session->get('oauth2state')) { - $this->session->remove('oauth2state'); + $oauth2state = $this->session->get('oauth2state'); + $this->session->remove('oauth2state'); + + if ($request->query->get('state') !== $oauth2state) { throw new ValidationException('Invalid state'); } From 4cbeada92f2dd6b5a079b03d8a1c7f11b1a58050 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 13:39:21 +0200 Subject: [PATCH 12/34] OpenId-Connect-Bundle: Added CLI login logic --- composer.json | 4 +- src/Command/UserLoginCommand.php | 79 +++++++++++++++ src/DependencyInjection/Configuration.php | 12 +++ .../ItkDevOpenIdConnectExtension.php | 12 +++ src/Resources/config/services.yaml | 10 +- src/Security/LoginTokenAuthenticator.php | 97 +++++++++++++++++++ src/Util/CliLoginHelper.php | 82 ++++++++++++++++ 7 files changed, 294 insertions(+), 2 deletions(-) create mode 100644 src/Command/UserLoginCommand.php create mode 100644 src/Security/LoginTokenAuthenticator.php create mode 100644 src/Util/CliLoginHelper.php diff --git a/composer.json b/composer.json index fb51f1d..591ee08 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,9 @@ "doctrine/orm": "^2.8", "symfony/security-bundle": "^5.2", "itk-dev/openid-connect": "^2.0", - "symfony/yaml": "^5.2" + "symfony/yaml": "^5.2", + "symfony/uid": "^5.2", + "symfony/cache": "^5.2" }, "require-dev": { "phpunit/phpunit": "^9.5", diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php new file mode 100644 index 0000000..1a6365f --- /dev/null +++ b/src/Command/UserLoginCommand.php @@ -0,0 +1,79 @@ +cliLoginHelper = $cliLoginHelper; + $this->cliLoginRedirectRoute = $cliLoginRedirectRoute; + $this->urlGenerator = $urlGenerator; + $this->userProvider = $userProvider; + + parent::__construct(); + } + + protected function configure() + { + $this + ->setDescription(self::$defaultDescription) + ->addArgument('username', InputArgument::REQUIRED, 'Username'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $username = $input->getArgument('username'); + + // Check if username is registered in User database + try { + $this->userProvider->loadUserByUsername($username); + } catch (UsernameNotFoundException $e) { + throw new \Exception('User does not exist'); + } + + // Create token via CliLoginHelper + + if (!is_string($username)) { + throw new \Exception('Username is not string type'); + } + + $token = $this->cliLoginHelper->createToken($username); + + //Generate absolute url for login + $loginPage = $this->urlGenerator->generate($this->cliLoginRedirectRoute, [ + 'loginToken' => $token, + ], UrlGeneratorInterface::ABSOLUTE_URL); + + $io->writeln($loginPage); + + return Command::SUCCESS; + } +} \ No newline at end of file diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 7b6588c..02ffc09 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -20,6 +20,18 @@ public function getConfigTreeBuilder(): TreeBuilder $treeBuilder->getRootNode() ->children() + ->arrayNode('cli_login_options') + ->isRequired() + ->children() + ->scalarNode('cli_redirect') + ->info('Return route for CLI login') + ->cannotBeEmpty()->end() + ->scalarNode('cache_pool') + ->info('Method for caching') + ->defaultValue('cache.app') + ->cannotBeEmpty()->end() + ->end() + ->end() ->arrayNode('openid_provider_options') ->isRequired() ->children() diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index 36b092b..f74965f 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -3,7 +3,10 @@ namespace ItkDev\OpenIdConnectBundle\DependencyInjection; use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; +use ItkDev\OpenIdConnectBundle\Command\UserLoginCommand; use ItkDev\OpenIdConnectBundle\Controller\LoginController; +use ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator; +use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\Extension; @@ -36,6 +39,15 @@ public function load(array $configs, ContainerBuilder $container) $definition = $container->getDefinition(OpenIdConfigurationProvider::class); $definition->replaceArgument('$options', $providerConfig); $definition->replaceArgument('$collaborators', []); + + $definition = $container->getDefinition(CliLoginHelper::class); + $definition->addArgument(new Reference($config['cli_login_options']['cache_pool'])); + + $definition = $container->getDefinition(UserLoginCommand::class); + $definition->replaceArgument('$cliLoginRedirectRoute', $config['cli_login_options']['cli_redirect']); + + $definition = $container->getDefinition(LoginTokenAuthenticator::class); + $definition->replaceArgument('$cliLoginRedirectRoute', $config['cli_login_options']['cli_redirect']); } /** diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index dbaad1b..e6f87bf 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -20,4 +20,12 @@ services: public: true arguments: $provider: '@ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider' - $session: ~ \ No newline at end of file + $session: ~ + + ItkDev\OpenIdConnectBundle\Util\CliLoginHelper: + + ItkDev\OpenIdConnectBundle\Command\UserLoginCommand: + $cliLoginRedirectRoute: ~ + + ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator: + $cliLoginRedirectRoute: ~ \ No newline at end of file diff --git a/src/Security/LoginTokenAuthenticator.php b/src/Security/LoginTokenAuthenticator.php new file mode 100644 index 0000000..5b08b5f --- /dev/null +++ b/src/Security/LoginTokenAuthenticator.php @@ -0,0 +1,97 @@ +cliLoginHelper = $cliLoginHelper; + $this->cliLoginRedirectRoute = $cliLoginRedirectRoute; + $this->router = $router; + } + + public function supports(Request $request) + { + return $request->query->has('loginToken'); + } + + public function getCredentials(Request $request) + { + return $request->query->get('loginToken'); + } + + public function getUser($credentials, UserProviderInterface $userProvider) + { + if (null === $credentials) { + // The token header was empty, authentication fails with HTTP Status + // Code 401 "Unauthorized" + return null; + } + + // Get username from CliHelperLogin + $username = $this->cliLoginHelper->getUsername($credentials); + + try { + $user = $userProvider->loadUserByUsername($username); + } catch (UsernameNotFoundException $e) { + throw new \Exception('Token correct but user not found'); + } + + return $user; + } + + public function checkCredentials($credentials, UserInterface $user) + { + // No credentials to check since loginToken login + return true; + } + + public function onAuthenticationFailure(Request $request, AuthenticationException $exception) + { + // Throw (telling) error + throw new AuthenticationException('Error occurred validating login token'); + } + + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $providerKey) + { + return new RedirectResponse($this->router->generate($this->cliLoginRedirectRoute)); + } + + public function start(Request $request, AuthenticationException $authException = null) + { + // Only way to start the CLI login flow should be via CMD and URL + throw new AuthenticationException('Authentication needed to access this URI/resource.'); + } + + public function supportsRememberMe() + { + return false; + } +} \ No newline at end of file diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php new file mode 100644 index 0000000..a8beb23 --- /dev/null +++ b/src/Util/CliLoginHelper.php @@ -0,0 +1,82 @@ +cache = $cache; + $this->itkNamespace = 'itk-dev-cli-login'; + } + + public function createToken(string $username): string + { + $encodedUsername = $this->encodeKey($username); + $token = Uuid::v4()->toBase32(); + + // Add username => token to make sure no username has more than one token + $revCacheItem = $this->cache->getItem($encodedUsername); + if ($revCacheItem->isHit()) { + return $revCacheItem->get(); + } + $revCacheItem->set($token); + $this->cache->save($revCacheItem); + + // Add token => username + $cacheItem = $this->cache->getItem($token); + $cacheItem->set($encodedUsername); + $this->cache->save($cacheItem); + + return $token; + } + + + public function getUsername(string $token): ?string + { + // check if token exists in cache + $usernameItem = $this->cache->getItem($token); + if (!$usernameItem->isHit()) { + // username does not exist in the cache + throw new \Exception('Token does not exist'); + } + + $username = $usernameItem->get(); + + // delete both entries from cache + $this->cache->delete($token); + $this->cache->delete($username); + + return $this->decodeKey($username); + } + + public function encodeKey(string $key): string + { + // Add namespace to key before encoding + return base64_encode($this->itkNamespace.$key); + } + + public function decodeKey(string $encodedKey): string + { + // Decode encoded key + $decodedKeyWithNamespace = base64_decode($encodedKey); + + // Remove namespace + $key = str_replace($this->itkNamespace, '', $decodedKeyWithNamespace); + + return $key; + } +} \ No newline at end of file From 28fc2a6b50b94146f63dd7295b2def441fa20c27 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 13:40:22 +0200 Subject: [PATCH 13/34] OpenId-Connect-Bundle: Updated CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 664f3a0..6f18e2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,4 +18,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Psalm setup for static analysis - Code formatting - OpenID Connect Bundle: Upgraded from - `itk-dev/openid-connect` 1.0.0 to 2.1.0 \ No newline at end of file + `itk-dev/openid-connect` 1.0.0 to 2.1.0 +- OpenId Connect Bundle: Added CLI login feature. \ No newline at end of file From 4ea6a5e46d8619a1d63141705635394a2b4f5493 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 15:19:30 +0200 Subject: [PATCH 14/34] OpenId-Connect-Bundle: Using psr6 cache during CLI login and tests --- .../ItkDevOpenIdConnectExtension.php | 2 +- src/Resources/config/services.yaml | 13 ++- src/Util/CliLoginHelper.php | 12 +- tests/Util/CliLoginHelperTest.php | 106 ++++++++++++++++++ tests/config/itkdev_openid_connect.yml | 3 + 5 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 tests/Util/CliLoginHelperTest.php diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index f74965f..b623d86 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -41,7 +41,7 @@ public function load(array $configs, ContainerBuilder $container) $definition->replaceArgument('$collaborators', []); $definition = $container->getDefinition(CliLoginHelper::class); - $definition->addArgument(new Reference($config['cli_login_options']['cache_pool'])); + $definition->replaceArgument('$cache', new Reference($config['cli_login_options']['cache_pool'])); $definition = $container->getDefinition(UserLoginCommand::class); $definition->replaceArgument('$cliLoginRedirectRoute', $config['cli_login_options']['cli_redirect']); diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index e6f87bf..ef454e9 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -23,9 +23,18 @@ services: $session: ~ ItkDev\OpenIdConnectBundle\Util\CliLoginHelper: + public: true + arguments: + $cache: ~ ItkDev\OpenIdConnectBundle\Command\UserLoginCommand: - $cliLoginRedirectRoute: ~ + public: true + arguments: + $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' + $cliLoginRedirectRoute: ~ ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator: - $cliLoginRedirectRoute: ~ \ No newline at end of file + public: true + arguments: + $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' + $cliLoginRedirectRoute: ~ diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index a8beb23..f76884e 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -4,11 +4,12 @@ use Symfony\Component\Uid\Uuid; use Symfony\Contracts\Cache\CacheInterface; +use Psr\Cache\CacheItemPoolInterface; class CliLoginHelper { /** - * @var CacheInterface + * @var CacheItemPoolInterface */ private $cache; @@ -17,7 +18,7 @@ class CliLoginHelper */ private $itkNamespace; - public function __construct(CacheInterface $cache) + public function __construct(CacheItemPoolInterface $cache) { $this->cache = $cache; $this->itkNamespace = 'itk-dev-cli-login'; @@ -43,8 +44,7 @@ public function createToken(string $username): string return $token; } - - + public function getUsername(string $token): ?string { // check if token exists in cache @@ -57,8 +57,8 @@ public function getUsername(string $token): ?string $username = $usernameItem->get(); // delete both entries from cache - $this->cache->delete($token); - $this->cache->delete($username); + $this->cache->deleteItem($token); + $this->cache->deleteItem($username); return $this->decodeKey($username); } diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php new file mode 100644 index 0000000..159a049 --- /dev/null +++ b/tests/Util/CliLoginHelperTest.php @@ -0,0 +1,106 @@ +toBase32(); + + // Encode the username + $encodedUsername = $cliHelper->encodeKey($randomUsername); + + // Decode the encoded username + $decodedUsername = $cliHelper->decodeKey($encodedUsername); + + // Assert equals + $this->assertEquals($randomUsername, $decodedUsername); + } + + public function testThrowExceptionIfTokenDoesNotExist() + { + // Expect an exception to be thrown + $this->expectException(\Exception::class); + + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Call the function that should throw an error + $username = $cliHelper->getUsername('random_gibberish_token'); + } + + public function testReuseSetTokenRatherThanRemake() + { + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Create and set a token for user test_user + $testUser = 'test_user'; + $token = $cliHelper->createToken($testUser); + + // Set another token + $token2 = $cliHelper->createToken($testUser); + + // Test that they are the same + $this->assertEquals($token, $token2); + } + + public function testTokenIsRemovedAfterUse() + { + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Create and set a token for user test_user + $testUser = 'test_user'; + $token = $cliHelper->createToken($testUser); + + // Get username from token created + $username = $cliHelper->getUsername($token); + + // Expect an exception to be thrown the second time we call getUsername + $this->expectException(\Exception::class); + + //Try again, to ensure its gone + $username = $cliHelper->getUsername($token); + } + + public function testCreateTokenAndGetUsername() + { + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Create and set a token for user test_user + $testUser = 'test_user'; + $token = $cliHelper->createToken($testUser); + + // Get username from token created + $username = $cliHelper->getUsername($token); + + // Check that we get correct username back + $this->assertEquals($testUser, $username); + } +} \ No newline at end of file diff --git a/tests/config/itkdev_openid_connect.yml b/tests/config/itkdev_openid_connect.yml index 7168cfe..af17822 100644 --- a/tests/config/itkdev_openid_connect.yml +++ b/tests/config/itkdev_openid_connect.yml @@ -1,4 +1,7 @@ itkdev_openid_connect: + cli_login_options: + cli_redirect: 'cli_redirect_test' + cache_pool: 'cache.array' openid_provider_options: configuration_url: 'https://provider.com/openid-configuration' client_id: 'test_id' From 93b314dbbf79d3203ed47e88ceb2db25aba64a35 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 16 Jun 2021 11:14:39 +0200 Subject: [PATCH 15/34] OpenId-Connect-Bundle: Refactored exceptions and fixed some psalm errors --- src/Command/UserLoginCommand.php | 24 ++++++++++++++----- src/Exception/CacheException.php | 10 ++++++++ .../ItkOpenIdConnectBundleException.php | 8 +++++++ src/Exception/TokenNotFoundException.php | 10 ++++++++ src/Exception/UserDoesNotExistException.php | 10 ++++++++ .../UsernameDoesNotExistException.php | 10 ++++++++ src/Security/LoginTokenAuthenticator.php | 18 ++++++++++++-- src/Util/CliLoginHelper.php | 12 +++++++--- tests/Util/CliLoginHelperTest.php | 7 +++--- 9 files changed, 94 insertions(+), 15 deletions(-) create mode 100644 src/Exception/CacheException.php create mode 100644 src/Exception/ItkOpenIdConnectBundleException.php create mode 100644 src/Exception/TokenNotFoundException.php create mode 100644 src/Exception/UserDoesNotExistException.php create mode 100644 src/Exception/UsernameDoesNotExistException.php diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 1a6365f..5863b9a 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -2,6 +2,9 @@ namespace ItkDev\OpenIdConnectBundle\Command; +use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; +use ItkDev\OpenIdConnectBundle\Exception\UserDoesNotExistException; +use ItkDev\OpenIdConnectBundle\Exception\UsernameDoesNotExistException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -16,6 +19,7 @@ class UserLoginCommand extends Command { protected static $defaultName = 'itk-dev:openid-connect:login'; protected static $defaultDescription = 'Get login url for user'; + /** * @var UrlGeneratorInterface */ @@ -26,8 +30,14 @@ class UserLoginCommand extends Command */ private $cliLoginHelper; + /** + * @var UserProviderInterface + */ private $userProvider; + /** + * @var string + */ private $cliLoginRedirectRoute; public function __construct(CliLoginHelper $cliLoginHelper, string $cliLoginRedirectRoute, UrlGeneratorInterface $urlGenerator, UserProviderInterface $userProvider) @@ -47,24 +57,26 @@ protected function configure() ->addArgument('username', InputArgument::REQUIRED, 'Username'); } + /** + * @throws ItkOpenIdConnectBundleException + */ protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); $username = $input->getArgument('username'); + if (!is_string($username)) { + throw new UsernameDoesNotExistException('Username is not type string.'); + } + // Check if username is registered in User database try { $this->userProvider->loadUserByUsername($username); } catch (UsernameNotFoundException $e) { - throw new \Exception('User does not exist'); + throw new UserDoesNotExistException('User does not exist.'); } // Create token via CliLoginHelper - - if (!is_string($username)) { - throw new \Exception('Username is not string type'); - } - $token = $this->cliLoginHelper->createToken($username); //Generate absolute url for login diff --git a/src/Exception/CacheException.php b/src/Exception/CacheException.php new file mode 100644 index 0000000..99bcddd --- /dev/null +++ b/src/Exception/CacheException.php @@ -0,0 +1,10 @@ +query->get('loginToken'); } + /** + * @throws ItkOpenIdConnectBundleException + */ public function getUser($credentials, UserProviderInterface $userProvider) { if (null === $credentials) { @@ -56,12 +62,20 @@ public function getUser($credentials, UserProviderInterface $userProvider) } // Get username from CliHelperLogin - $username = $this->cliLoginHelper->getUsername($credentials); + try { + $username = $this->cliLoginHelper->getUsername($credentials); + } catch (ItkOpenIdConnectBundleException $exception) { + throw new UsernameDoesNotExistException($exception->getMessage()); + } + + if (null === $username) { + throw new UsernameDoesNotExistException('null is not a valid Username.'); + } try { $user = $userProvider->loadUserByUsername($username); } catch (UsernameNotFoundException $e) { - throw new \Exception('Token correct but user not found'); + throw new UserDoesNotExistException('Token correct but user not found'); } return $user; diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index f76884e..c63b939 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -2,8 +2,9 @@ namespace ItkDev\OpenIdConnectBundle\Util; +use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; +use ItkDev\OpenIdConnectBundle\Exception\TokenNotFoundException; use Symfony\Component\Uid\Uuid; -use Symfony\Contracts\Cache\CacheInterface; use Psr\Cache\CacheItemPoolInterface; class CliLoginHelper @@ -44,14 +45,19 @@ public function createToken(string $username): string return $token; } - + + + /** + * @throws ItkOpenIdConnectBundleException + */ public function getUsername(string $token): ?string { // check if token exists in cache $usernameItem = $this->cache->getItem($token); + if (!$usernameItem->isHit()) { // username does not exist in the cache - throw new \Exception('Token does not exist'); + throw new TokenNotFoundException('Token does not exist'); } $username = $usernameItem->get(); diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php index 159a049..240adbe 100644 --- a/tests/Util/CliLoginHelperTest.php +++ b/tests/Util/CliLoginHelperTest.php @@ -2,12 +2,11 @@ namespace ItkDev\OpenIdConnectBundle\Tests\Util; +use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use PHPUnit\Framework\TestCase; use Symfony\Component\Cache\Adapter\ArrayAdapter; -use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Component\Uid\Uuid; -use Symfony\Contracts\Cache\CacheInterface; class CliLoginHelperTest extends TestCase { @@ -32,7 +31,7 @@ public function testEncodeAndDecode() public function testThrowExceptionIfTokenDoesNotExist() { // Expect an exception to be thrown - $this->expectException(\Exception::class); + $this->expectException(ItkOpenIdConnectBundleException::class); // Testing it works with one adapter $cache = new ArrayAdapter(); @@ -79,7 +78,7 @@ public function testTokenIsRemovedAfterUse() $username = $cliHelper->getUsername($token); // Expect an exception to be thrown the second time we call getUsername - $this->expectException(\Exception::class); + $this->expectException(ItkOpenIdConnectBundleException::class); //Try again, to ensure its gone $username = $cliHelper->getUsername($token); From ba3b8ca7a2abfcf23f9bb7054c235ff526f2fc2c Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 16 Jun 2021 11:34:48 +0200 Subject: [PATCH 16/34] OpenId-Connect-Bundle: Applied coding standards --- src/Command/UserLoginCommand.php | 4 ++-- src/DependencyInjection/Configuration.php | 4 ++-- src/DependencyInjection/ItkDevOpenIdConnectExtension.php | 5 +---- src/Exception/CacheException.php | 4 +--- src/Exception/ItkOpenIdConnectBundleException.php | 2 +- src/Exception/TokenNotFoundException.php | 4 +--- src/Exception/UserDoesNotExistException.php | 4 +--- src/Exception/UsernameDoesNotExistException.php | 4 +--- src/Security/LoginTokenAuthenticator.php | 2 +- src/Util/CliLoginHelper.php | 4 ++-- tests/Util/CliLoginHelperTest.php | 2 +- 11 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 5863b9a..9a1aabd 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -50,7 +50,7 @@ public function __construct(CliLoginHelper $cliLoginHelper, string $cliLoginRedi parent::__construct(); } - protected function configure() + protected function configure(): void { $this ->setDescription(self::$defaultDescription) @@ -88,4 +88,4 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Command::SUCCESS; } -} \ No newline at end of file +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 02ffc09..7da4bd3 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -39,7 +39,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->info('URL to OpenId Discovery Document') ->validate() ->ifTrue( - function ($value) { + function (string $value) { return !filter_var($value, FILTER_VALIDATE_URL); } ) @@ -60,7 +60,7 @@ function ($value) { ->info('Callback URI registered at identity provider') ->validate() ->ifTrue( - function ($value) { + function (string $value) { return !filter_var($value, FILTER_VALIDATE_URL); } ) diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index b623d86..97f2360 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -4,14 +4,11 @@ use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use ItkDev\OpenIdConnectBundle\Command\UserLoginCommand; -use ItkDev\OpenIdConnectBundle\Controller\LoginController; use ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\Extension; -use Symfony\Component\DependencyInjection\Loader\FileLoader; -use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; use Symfony\Component\DependencyInjection\Reference; @@ -20,7 +17,7 @@ class ItkDevOpenIdConnectExtension extends Extension /** * {@inheritdoc} */ - public function load(array $configs, ContainerBuilder $container) + public function load(array $configs, ContainerBuilder $container): void { $loader = new YamlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.yaml'); diff --git a/src/Exception/CacheException.php b/src/Exception/CacheException.php index 99bcddd..a0d6acc 100644 --- a/src/Exception/CacheException.php +++ b/src/Exception/CacheException.php @@ -1,10 +1,8 @@ itkNamespace.$key); + return base64_encode($this->itkNamespace . $key); } public function decodeKey(string $encodedKey): string @@ -85,4 +85,4 @@ public function decodeKey(string $encodedKey): string return $key; } -} \ No newline at end of file +} diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php index 240adbe..c893336 100644 --- a/tests/Util/CliLoginHelperTest.php +++ b/tests/Util/CliLoginHelperTest.php @@ -102,4 +102,4 @@ public function testCreateTokenAndGetUsername() // Check that we get correct username back $this->assertEquals($testUser, $username); } -} \ No newline at end of file +} From ebfb52054d0095bc1c0863f565c6a56701c75d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ture=20Gj=C3=B8rup?= Date: Thu, 17 Jun 2021 11:00:54 +0200 Subject: [PATCH 17/34] TEAMMANAGE-2026: Added tests --- src/Controller/LoginController.php | 3 +- tests/Controller/LoginControllerTest.php | 54 ++++++++++++++++ .../Security/OpenIdLoginAuthenticatorTest.php | 61 +++++++++++++++++++ tests/{ => Security}/TestAuthenticator.php | 5 +- 4 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 tests/Controller/LoginControllerTest.php create mode 100644 tests/Security/OpenIdLoginAuthenticatorTest.php rename tests/{ => Security}/TestAuthenticator.php (94%) diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index ebfcc31..f10038b 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -6,7 +6,6 @@ use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\RedirectResponse; -use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\SessionInterface; class LoginController extends AbstractController @@ -24,7 +23,7 @@ public function __construct(OpenIdConfigurationProvider $provider) /** * @throws ItkOpenIdConnectException */ - public function login(SessionInterface $session): Response + public function login(SessionInterface $session): RedirectResponse { $nonce = $this->provider->generateNonce(); $state = $this->provider->generateState(); diff --git a/tests/Controller/LoginControllerTest.php b/tests/Controller/LoginControllerTest.php new file mode 100644 index 0000000..0099d76 --- /dev/null +++ b/tests/Controller/LoginControllerTest.php @@ -0,0 +1,54 @@ +createMock(OpenIdConfigurationProvider::class); + $mockProvider + ->expects($this->exactly(1)) + ->method('generateNonce') + ->willReturn('1234'); + $mockProvider + ->expects($this->exactly(1)) + ->method('generateState') + ->willReturn('abcd'); + $mockProvider + ->expects($this->exactly(1)) + ->method('getAuthorizationUrl') + ->with(['state' => 'abcd', 'nonce' => '1234']) + ->willReturn('https://test.com'); + + $this->loginController = new LoginController($mockProvider); + } + + public function testLogin(): void + { + $mockSession = $this->createMock(SessionInterface::class); + $mockSession + ->expects($this->exactly(2)) + ->method('set') + ->withConsecutive( + [$this->equalTo('oauth2state'), $this->equalTo('abcd')], + [$this->equalTo('oauth2nonce'), $this->equalTo('1234')] + ); + + $response = $this->loginController->login($mockSession); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('https://test.com', $response->getTargetUrl()); + } + + +} diff --git a/tests/Security/OpenIdLoginAuthenticatorTest.php b/tests/Security/OpenIdLoginAuthenticatorTest.php new file mode 100644 index 0000000..72d021e --- /dev/null +++ b/tests/Security/OpenIdLoginAuthenticatorTest.php @@ -0,0 +1,61 @@ +createMock(OpenIdConfigurationProvider::class); + $mockSession = $this->createMock(SessionInterface::class); + + $this->authenticator = new TestAuthenticator($mockProvider, $mockSession); + } + + public function testSupports(): void + { + $mockRequest = $this->createMock(Request::class); + $mockRequest->query = new InputBag(); + + $this->assertFalse($this->authenticator->supports($mockRequest)); + + $mockRequest->query->set('state', 'abcd'); + $this->assertFalse($this->authenticator->supports($mockRequest)); + + $mockRequest->query->set('id_token', 'xyz'); + $this->assertTrue($this->authenticator->supports($mockRequest)); + } + + public function testCheckCredentials(): void + { + $stubUser = $this->createStub(UserInterface::class); + + $this->assertTrue($this->authenticator->checkCredentials(null, $stubUser)); + } + + public function testOnAuthenticationFailure(): void + { + $this->expectException(AuthenticationException::class); + + $stubRequest = $this->createStub(Request::class); + $exception = new AuthenticationException(); + + $this->authenticator->onAuthenticationFailure($stubRequest, $exception); + } + + public function testSupportsRememberMe():void + { + $this->assertFalse($this->authenticator->supportsRememberMe()); + } +} diff --git a/tests/TestAuthenticator.php b/tests/Security/TestAuthenticator.php similarity index 94% rename from tests/TestAuthenticator.php rename to tests/Security/TestAuthenticator.php index a870c18..b4731ce 100644 --- a/tests/TestAuthenticator.php +++ b/tests/Security/TestAuthenticator.php @@ -1,6 +1,6 @@ Date: Thu, 17 Jun 2021 11:05:22 +0200 Subject: [PATCH 18/34] TEAMMANAGE-2026: Code style fixes --- tests/Controller/LoginControllerTest.php | 2 -- tests/Security/OpenIdLoginAuthenticatorTest.php | 2 +- tests/Security/TestAuthenticator.php | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/Controller/LoginControllerTest.php b/tests/Controller/LoginControllerTest.php index 0099d76..757a2f5 100644 --- a/tests/Controller/LoginControllerTest.php +++ b/tests/Controller/LoginControllerTest.php @@ -49,6 +49,4 @@ public function testLogin(): void $this->assertInstanceOf(RedirectResponse::class, $response); $this->assertSame('https://test.com', $response->getTargetUrl()); } - - } diff --git a/tests/Security/OpenIdLoginAuthenticatorTest.php b/tests/Security/OpenIdLoginAuthenticatorTest.php index 72d021e..8f00d8e 100644 --- a/tests/Security/OpenIdLoginAuthenticatorTest.php +++ b/tests/Security/OpenIdLoginAuthenticatorTest.php @@ -54,7 +54,7 @@ public function testOnAuthenticationFailure(): void $this->authenticator->onAuthenticationFailure($stubRequest, $exception); } - public function testSupportsRememberMe():void + public function testSupportsRememberMe(): void { $this->assertFalse($this->authenticator->supportsRememberMe()); } diff --git a/tests/Security/TestAuthenticator.php b/tests/Security/TestAuthenticator.php index b4731ce..5d1373e 100644 --- a/tests/Security/TestAuthenticator.php +++ b/tests/Security/TestAuthenticator.php @@ -24,4 +24,4 @@ public function start(Request $request, AuthenticationException $authException = { // TODO: Implement start() method. } -} \ No newline at end of file +} From eec3d49b4921d92500fb2ed370aedcb86fa7e41f Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 13:39:21 +0200 Subject: [PATCH 19/34] OpenId-Connect-Bundle: Added CLI login logic --- composer.json | 4 +- src/Command/UserLoginCommand.php | 79 +++++++++++++++ src/DependencyInjection/Configuration.php | 12 +++ .../ItkDevOpenIdConnectExtension.php | 12 +++ src/Resources/config/services.yaml | 10 +- src/Security/LoginTokenAuthenticator.php | 97 +++++++++++++++++++ src/Util/CliLoginHelper.php | 82 ++++++++++++++++ 7 files changed, 294 insertions(+), 2 deletions(-) create mode 100644 src/Command/UserLoginCommand.php create mode 100644 src/Security/LoginTokenAuthenticator.php create mode 100644 src/Util/CliLoginHelper.php diff --git a/composer.json b/composer.json index fb51f1d..591ee08 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,9 @@ "doctrine/orm": "^2.8", "symfony/security-bundle": "^5.2", "itk-dev/openid-connect": "^2.0", - "symfony/yaml": "^5.2" + "symfony/yaml": "^5.2", + "symfony/uid": "^5.2", + "symfony/cache": "^5.2" }, "require-dev": { "phpunit/phpunit": "^9.5", diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php new file mode 100644 index 0000000..1a6365f --- /dev/null +++ b/src/Command/UserLoginCommand.php @@ -0,0 +1,79 @@ +cliLoginHelper = $cliLoginHelper; + $this->cliLoginRedirectRoute = $cliLoginRedirectRoute; + $this->urlGenerator = $urlGenerator; + $this->userProvider = $userProvider; + + parent::__construct(); + } + + protected function configure() + { + $this + ->setDescription(self::$defaultDescription) + ->addArgument('username', InputArgument::REQUIRED, 'Username'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $username = $input->getArgument('username'); + + // Check if username is registered in User database + try { + $this->userProvider->loadUserByUsername($username); + } catch (UsernameNotFoundException $e) { + throw new \Exception('User does not exist'); + } + + // Create token via CliLoginHelper + + if (!is_string($username)) { + throw new \Exception('Username is not string type'); + } + + $token = $this->cliLoginHelper->createToken($username); + + //Generate absolute url for login + $loginPage = $this->urlGenerator->generate($this->cliLoginRedirectRoute, [ + 'loginToken' => $token, + ], UrlGeneratorInterface::ABSOLUTE_URL); + + $io->writeln($loginPage); + + return Command::SUCCESS; + } +} \ No newline at end of file diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 7b6588c..02ffc09 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -20,6 +20,18 @@ public function getConfigTreeBuilder(): TreeBuilder $treeBuilder->getRootNode() ->children() + ->arrayNode('cli_login_options') + ->isRequired() + ->children() + ->scalarNode('cli_redirect') + ->info('Return route for CLI login') + ->cannotBeEmpty()->end() + ->scalarNode('cache_pool') + ->info('Method for caching') + ->defaultValue('cache.app') + ->cannotBeEmpty()->end() + ->end() + ->end() ->arrayNode('openid_provider_options') ->isRequired() ->children() diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index 36b092b..f74965f 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -3,7 +3,10 @@ namespace ItkDev\OpenIdConnectBundle\DependencyInjection; use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; +use ItkDev\OpenIdConnectBundle\Command\UserLoginCommand; use ItkDev\OpenIdConnectBundle\Controller\LoginController; +use ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator; +use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\Extension; @@ -36,6 +39,15 @@ public function load(array $configs, ContainerBuilder $container) $definition = $container->getDefinition(OpenIdConfigurationProvider::class); $definition->replaceArgument('$options', $providerConfig); $definition->replaceArgument('$collaborators', []); + + $definition = $container->getDefinition(CliLoginHelper::class); + $definition->addArgument(new Reference($config['cli_login_options']['cache_pool'])); + + $definition = $container->getDefinition(UserLoginCommand::class); + $definition->replaceArgument('$cliLoginRedirectRoute', $config['cli_login_options']['cli_redirect']); + + $definition = $container->getDefinition(LoginTokenAuthenticator::class); + $definition->replaceArgument('$cliLoginRedirectRoute', $config['cli_login_options']['cli_redirect']); } /** diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index dbaad1b..e6f87bf 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -20,4 +20,12 @@ services: public: true arguments: $provider: '@ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider' - $session: ~ \ No newline at end of file + $session: ~ + + ItkDev\OpenIdConnectBundle\Util\CliLoginHelper: + + ItkDev\OpenIdConnectBundle\Command\UserLoginCommand: + $cliLoginRedirectRoute: ~ + + ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator: + $cliLoginRedirectRoute: ~ \ No newline at end of file diff --git a/src/Security/LoginTokenAuthenticator.php b/src/Security/LoginTokenAuthenticator.php new file mode 100644 index 0000000..5b08b5f --- /dev/null +++ b/src/Security/LoginTokenAuthenticator.php @@ -0,0 +1,97 @@ +cliLoginHelper = $cliLoginHelper; + $this->cliLoginRedirectRoute = $cliLoginRedirectRoute; + $this->router = $router; + } + + public function supports(Request $request) + { + return $request->query->has('loginToken'); + } + + public function getCredentials(Request $request) + { + return $request->query->get('loginToken'); + } + + public function getUser($credentials, UserProviderInterface $userProvider) + { + if (null === $credentials) { + // The token header was empty, authentication fails with HTTP Status + // Code 401 "Unauthorized" + return null; + } + + // Get username from CliHelperLogin + $username = $this->cliLoginHelper->getUsername($credentials); + + try { + $user = $userProvider->loadUserByUsername($username); + } catch (UsernameNotFoundException $e) { + throw new \Exception('Token correct but user not found'); + } + + return $user; + } + + public function checkCredentials($credentials, UserInterface $user) + { + // No credentials to check since loginToken login + return true; + } + + public function onAuthenticationFailure(Request $request, AuthenticationException $exception) + { + // Throw (telling) error + throw new AuthenticationException('Error occurred validating login token'); + } + + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $providerKey) + { + return new RedirectResponse($this->router->generate($this->cliLoginRedirectRoute)); + } + + public function start(Request $request, AuthenticationException $authException = null) + { + // Only way to start the CLI login flow should be via CMD and URL + throw new AuthenticationException('Authentication needed to access this URI/resource.'); + } + + public function supportsRememberMe() + { + return false; + } +} \ No newline at end of file diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php new file mode 100644 index 0000000..a8beb23 --- /dev/null +++ b/src/Util/CliLoginHelper.php @@ -0,0 +1,82 @@ +cache = $cache; + $this->itkNamespace = 'itk-dev-cli-login'; + } + + public function createToken(string $username): string + { + $encodedUsername = $this->encodeKey($username); + $token = Uuid::v4()->toBase32(); + + // Add username => token to make sure no username has more than one token + $revCacheItem = $this->cache->getItem($encodedUsername); + if ($revCacheItem->isHit()) { + return $revCacheItem->get(); + } + $revCacheItem->set($token); + $this->cache->save($revCacheItem); + + // Add token => username + $cacheItem = $this->cache->getItem($token); + $cacheItem->set($encodedUsername); + $this->cache->save($cacheItem); + + return $token; + } + + + public function getUsername(string $token): ?string + { + // check if token exists in cache + $usernameItem = $this->cache->getItem($token); + if (!$usernameItem->isHit()) { + // username does not exist in the cache + throw new \Exception('Token does not exist'); + } + + $username = $usernameItem->get(); + + // delete both entries from cache + $this->cache->delete($token); + $this->cache->delete($username); + + return $this->decodeKey($username); + } + + public function encodeKey(string $key): string + { + // Add namespace to key before encoding + return base64_encode($this->itkNamespace.$key); + } + + public function decodeKey(string $encodedKey): string + { + // Decode encoded key + $decodedKeyWithNamespace = base64_decode($encodedKey); + + // Remove namespace + $key = str_replace($this->itkNamespace, '', $decodedKeyWithNamespace); + + return $key; + } +} \ No newline at end of file From 70d92a8462c215d5461b6d8bb58fb56f055a1b04 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 13:40:22 +0200 Subject: [PATCH 20/34] OpenId-Connect-Bundle: Updated CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 664f3a0..6f18e2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,4 +18,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Psalm setup for static analysis - Code formatting - OpenID Connect Bundle: Upgraded from - `itk-dev/openid-connect` 1.0.0 to 2.1.0 \ No newline at end of file + `itk-dev/openid-connect` 1.0.0 to 2.1.0 +- OpenId Connect Bundle: Added CLI login feature. \ No newline at end of file From 9e65eb161ba6b026a4b168fa96be536316e138ba Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 15 Jun 2021 15:19:30 +0200 Subject: [PATCH 21/34] OpenId-Connect-Bundle: Using psr6 cache during CLI login and tests --- .../ItkDevOpenIdConnectExtension.php | 2 +- src/Resources/config/services.yaml | 13 ++- src/Util/CliLoginHelper.php | 12 +- tests/Util/CliLoginHelperTest.php | 106 ++++++++++++++++++ tests/config/itkdev_openid_connect.yml | 3 + 5 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 tests/Util/CliLoginHelperTest.php diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index f74965f..b623d86 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -41,7 +41,7 @@ public function load(array $configs, ContainerBuilder $container) $definition->replaceArgument('$collaborators', []); $definition = $container->getDefinition(CliLoginHelper::class); - $definition->addArgument(new Reference($config['cli_login_options']['cache_pool'])); + $definition->replaceArgument('$cache', new Reference($config['cli_login_options']['cache_pool'])); $definition = $container->getDefinition(UserLoginCommand::class); $definition->replaceArgument('$cliLoginRedirectRoute', $config['cli_login_options']['cli_redirect']); diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index e6f87bf..ef454e9 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -23,9 +23,18 @@ services: $session: ~ ItkDev\OpenIdConnectBundle\Util\CliLoginHelper: + public: true + arguments: + $cache: ~ ItkDev\OpenIdConnectBundle\Command\UserLoginCommand: - $cliLoginRedirectRoute: ~ + public: true + arguments: + $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' + $cliLoginRedirectRoute: ~ ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator: - $cliLoginRedirectRoute: ~ \ No newline at end of file + public: true + arguments: + $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' + $cliLoginRedirectRoute: ~ diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index a8beb23..f76884e 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -4,11 +4,12 @@ use Symfony\Component\Uid\Uuid; use Symfony\Contracts\Cache\CacheInterface; +use Psr\Cache\CacheItemPoolInterface; class CliLoginHelper { /** - * @var CacheInterface + * @var CacheItemPoolInterface */ private $cache; @@ -17,7 +18,7 @@ class CliLoginHelper */ private $itkNamespace; - public function __construct(CacheInterface $cache) + public function __construct(CacheItemPoolInterface $cache) { $this->cache = $cache; $this->itkNamespace = 'itk-dev-cli-login'; @@ -43,8 +44,7 @@ public function createToken(string $username): string return $token; } - - + public function getUsername(string $token): ?string { // check if token exists in cache @@ -57,8 +57,8 @@ public function getUsername(string $token): ?string $username = $usernameItem->get(); // delete both entries from cache - $this->cache->delete($token); - $this->cache->delete($username); + $this->cache->deleteItem($token); + $this->cache->deleteItem($username); return $this->decodeKey($username); } diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php new file mode 100644 index 0000000..159a049 --- /dev/null +++ b/tests/Util/CliLoginHelperTest.php @@ -0,0 +1,106 @@ +toBase32(); + + // Encode the username + $encodedUsername = $cliHelper->encodeKey($randomUsername); + + // Decode the encoded username + $decodedUsername = $cliHelper->decodeKey($encodedUsername); + + // Assert equals + $this->assertEquals($randomUsername, $decodedUsername); + } + + public function testThrowExceptionIfTokenDoesNotExist() + { + // Expect an exception to be thrown + $this->expectException(\Exception::class); + + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Call the function that should throw an error + $username = $cliHelper->getUsername('random_gibberish_token'); + } + + public function testReuseSetTokenRatherThanRemake() + { + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Create and set a token for user test_user + $testUser = 'test_user'; + $token = $cliHelper->createToken($testUser); + + // Set another token + $token2 = $cliHelper->createToken($testUser); + + // Test that they are the same + $this->assertEquals($token, $token2); + } + + public function testTokenIsRemovedAfterUse() + { + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Create and set a token for user test_user + $testUser = 'test_user'; + $token = $cliHelper->createToken($testUser); + + // Get username from token created + $username = $cliHelper->getUsername($token); + + // Expect an exception to be thrown the second time we call getUsername + $this->expectException(\Exception::class); + + //Try again, to ensure its gone + $username = $cliHelper->getUsername($token); + } + + public function testCreateTokenAndGetUsername() + { + // Testing it works with one adapter + $cache = new ArrayAdapter(); + + // Create CliLoginHelper + $cliHelper = new CliLoginHelper($cache); + + // Create and set a token for user test_user + $testUser = 'test_user'; + $token = $cliHelper->createToken($testUser); + + // Get username from token created + $username = $cliHelper->getUsername($token); + + // Check that we get correct username back + $this->assertEquals($testUser, $username); + } +} \ No newline at end of file diff --git a/tests/config/itkdev_openid_connect.yml b/tests/config/itkdev_openid_connect.yml index 7168cfe..af17822 100644 --- a/tests/config/itkdev_openid_connect.yml +++ b/tests/config/itkdev_openid_connect.yml @@ -1,4 +1,7 @@ itkdev_openid_connect: + cli_login_options: + cli_redirect: 'cli_redirect_test' + cache_pool: 'cache.array' openid_provider_options: configuration_url: 'https://provider.com/openid-configuration' client_id: 'test_id' From 09d43138bf52b21b738ece1ed577cb7fc9ffbc45 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 16 Jun 2021 11:14:39 +0200 Subject: [PATCH 22/34] OpenId-Connect-Bundle: Refactored exceptions and fixed some psalm errors --- src/Command/UserLoginCommand.php | 24 ++++++++++++++----- src/Exception/CacheException.php | 10 ++++++++ .../ItkOpenIdConnectBundleException.php | 8 +++++++ src/Exception/TokenNotFoundException.php | 10 ++++++++ src/Exception/UserDoesNotExistException.php | 10 ++++++++ .../UsernameDoesNotExistException.php | 10 ++++++++ src/Security/LoginTokenAuthenticator.php | 18 ++++++++++++-- src/Util/CliLoginHelper.php | 12 +++++++--- tests/Util/CliLoginHelperTest.php | 7 +++--- 9 files changed, 94 insertions(+), 15 deletions(-) create mode 100644 src/Exception/CacheException.php create mode 100644 src/Exception/ItkOpenIdConnectBundleException.php create mode 100644 src/Exception/TokenNotFoundException.php create mode 100644 src/Exception/UserDoesNotExistException.php create mode 100644 src/Exception/UsernameDoesNotExistException.php diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 1a6365f..5863b9a 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -2,6 +2,9 @@ namespace ItkDev\OpenIdConnectBundle\Command; +use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; +use ItkDev\OpenIdConnectBundle\Exception\UserDoesNotExistException; +use ItkDev\OpenIdConnectBundle\Exception\UsernameDoesNotExistException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -16,6 +19,7 @@ class UserLoginCommand extends Command { protected static $defaultName = 'itk-dev:openid-connect:login'; protected static $defaultDescription = 'Get login url for user'; + /** * @var UrlGeneratorInterface */ @@ -26,8 +30,14 @@ class UserLoginCommand extends Command */ private $cliLoginHelper; + /** + * @var UserProviderInterface + */ private $userProvider; + /** + * @var string + */ private $cliLoginRedirectRoute; public function __construct(CliLoginHelper $cliLoginHelper, string $cliLoginRedirectRoute, UrlGeneratorInterface $urlGenerator, UserProviderInterface $userProvider) @@ -47,24 +57,26 @@ protected function configure() ->addArgument('username', InputArgument::REQUIRED, 'Username'); } + /** + * @throws ItkOpenIdConnectBundleException + */ protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); $username = $input->getArgument('username'); + if (!is_string($username)) { + throw new UsernameDoesNotExistException('Username is not type string.'); + } + // Check if username is registered in User database try { $this->userProvider->loadUserByUsername($username); } catch (UsernameNotFoundException $e) { - throw new \Exception('User does not exist'); + throw new UserDoesNotExistException('User does not exist.'); } // Create token via CliLoginHelper - - if (!is_string($username)) { - throw new \Exception('Username is not string type'); - } - $token = $this->cliLoginHelper->createToken($username); //Generate absolute url for login diff --git a/src/Exception/CacheException.php b/src/Exception/CacheException.php new file mode 100644 index 0000000..99bcddd --- /dev/null +++ b/src/Exception/CacheException.php @@ -0,0 +1,10 @@ +query->get('loginToken'); } + /** + * @throws ItkOpenIdConnectBundleException + */ public function getUser($credentials, UserProviderInterface $userProvider) { if (null === $credentials) { @@ -56,12 +62,20 @@ public function getUser($credentials, UserProviderInterface $userProvider) } // Get username from CliHelperLogin - $username = $this->cliLoginHelper->getUsername($credentials); + try { + $username = $this->cliLoginHelper->getUsername($credentials); + } catch (ItkOpenIdConnectBundleException $exception) { + throw new UsernameDoesNotExistException($exception->getMessage()); + } + + if (null === $username) { + throw new UsernameDoesNotExistException('null is not a valid Username.'); + } try { $user = $userProvider->loadUserByUsername($username); } catch (UsernameNotFoundException $e) { - throw new \Exception('Token correct but user not found'); + throw new UserDoesNotExistException('Token correct but user not found'); } return $user; diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index f76884e..c63b939 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -2,8 +2,9 @@ namespace ItkDev\OpenIdConnectBundle\Util; +use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; +use ItkDev\OpenIdConnectBundle\Exception\TokenNotFoundException; use Symfony\Component\Uid\Uuid; -use Symfony\Contracts\Cache\CacheInterface; use Psr\Cache\CacheItemPoolInterface; class CliLoginHelper @@ -44,14 +45,19 @@ public function createToken(string $username): string return $token; } - + + + /** + * @throws ItkOpenIdConnectBundleException + */ public function getUsername(string $token): ?string { // check if token exists in cache $usernameItem = $this->cache->getItem($token); + if (!$usernameItem->isHit()) { // username does not exist in the cache - throw new \Exception('Token does not exist'); + throw new TokenNotFoundException('Token does not exist'); } $username = $usernameItem->get(); diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php index 159a049..240adbe 100644 --- a/tests/Util/CliLoginHelperTest.php +++ b/tests/Util/CliLoginHelperTest.php @@ -2,12 +2,11 @@ namespace ItkDev\OpenIdConnectBundle\Tests\Util; +use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use PHPUnit\Framework\TestCase; use Symfony\Component\Cache\Adapter\ArrayAdapter; -use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Component\Uid\Uuid; -use Symfony\Contracts\Cache\CacheInterface; class CliLoginHelperTest extends TestCase { @@ -32,7 +31,7 @@ public function testEncodeAndDecode() public function testThrowExceptionIfTokenDoesNotExist() { // Expect an exception to be thrown - $this->expectException(\Exception::class); + $this->expectException(ItkOpenIdConnectBundleException::class); // Testing it works with one adapter $cache = new ArrayAdapter(); @@ -79,7 +78,7 @@ public function testTokenIsRemovedAfterUse() $username = $cliHelper->getUsername($token); // Expect an exception to be thrown the second time we call getUsername - $this->expectException(\Exception::class); + $this->expectException(ItkOpenIdConnectBundleException::class); //Try again, to ensure its gone $username = $cliHelper->getUsername($token); From b1efd4e6345047da747acf95f1db4bb71a9732fa Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 16 Jun 2021 11:34:48 +0200 Subject: [PATCH 23/34] OpenId-Connect-Bundle: Applied coding standards --- src/Command/UserLoginCommand.php | 4 ++-- src/DependencyInjection/Configuration.php | 4 ++-- src/DependencyInjection/ItkDevOpenIdConnectExtension.php | 5 +---- src/Exception/CacheException.php | 4 +--- src/Exception/ItkOpenIdConnectBundleException.php | 2 +- src/Exception/TokenNotFoundException.php | 4 +--- src/Exception/UserDoesNotExistException.php | 4 +--- src/Exception/UsernameDoesNotExistException.php | 4 +--- src/Security/LoginTokenAuthenticator.php | 2 +- src/Util/CliLoginHelper.php | 4 ++-- tests/Util/CliLoginHelperTest.php | 2 +- 11 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 5863b9a..9a1aabd 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -50,7 +50,7 @@ public function __construct(CliLoginHelper $cliLoginHelper, string $cliLoginRedi parent::__construct(); } - protected function configure() + protected function configure(): void { $this ->setDescription(self::$defaultDescription) @@ -88,4 +88,4 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Command::SUCCESS; } -} \ No newline at end of file +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 02ffc09..7da4bd3 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -39,7 +39,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->info('URL to OpenId Discovery Document') ->validate() ->ifTrue( - function ($value) { + function (string $value) { return !filter_var($value, FILTER_VALIDATE_URL); } ) @@ -60,7 +60,7 @@ function ($value) { ->info('Callback URI registered at identity provider') ->validate() ->ifTrue( - function ($value) { + function (string $value) { return !filter_var($value, FILTER_VALIDATE_URL); } ) diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index b623d86..97f2360 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -4,14 +4,11 @@ use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use ItkDev\OpenIdConnectBundle\Command\UserLoginCommand; -use ItkDev\OpenIdConnectBundle\Controller\LoginController; use ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\Extension; -use Symfony\Component\DependencyInjection\Loader\FileLoader; -use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; use Symfony\Component\DependencyInjection\Reference; @@ -20,7 +17,7 @@ class ItkDevOpenIdConnectExtension extends Extension /** * {@inheritdoc} */ - public function load(array $configs, ContainerBuilder $container) + public function load(array $configs, ContainerBuilder $container): void { $loader = new YamlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.yaml'); diff --git a/src/Exception/CacheException.php b/src/Exception/CacheException.php index 99bcddd..a0d6acc 100644 --- a/src/Exception/CacheException.php +++ b/src/Exception/CacheException.php @@ -1,10 +1,8 @@ itkNamespace.$key); + return base64_encode($this->itkNamespace . $key); } public function decodeKey(string $encodedKey): string @@ -85,4 +85,4 @@ public function decodeKey(string $encodedKey): string return $key; } -} \ No newline at end of file +} diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php index 240adbe..c893336 100644 --- a/tests/Util/CliLoginHelperTest.php +++ b/tests/Util/CliLoginHelperTest.php @@ -102,4 +102,4 @@ public function testCreateTokenAndGetUsername() // Check that we get correct username back $this->assertEquals($testUser, $username); } -} \ No newline at end of file +} From 76ed7a2a33d2f20afaace891505a97e701b41503 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Thu, 9 Sep 2021 12:29:05 +0200 Subject: [PATCH 24/34] OpenId-Connect-Bundle: Explicitly add UrlGeneratorInterface and UserProivderInterface as services --- src/Resources/config/services.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index ef454e9..84662ce 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -32,9 +32,17 @@ services: arguments: $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' $cliLoginRedirectRoute: ~ + $urlGenerator: '@Symfony\Component\Routing\Generator\UrlGeneratorInterface' + $userProvider: '@Symfony\Component\Security\Core\User\UserProviderInterface' ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator: public: true arguments: $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' $cliLoginRedirectRoute: ~ + + Symfony\Component\Routing\Generator\UrlGeneratorInterface: + public: true + + Symfony\Component\Security\Core\User\UserProviderInterface: + public: true \ No newline at end of file From 26ae5d321d7f638c91fae0f514adf919f1d5f457 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Thu, 9 Sep 2021 12:44:29 +0200 Subject: [PATCH 25/34] OpenId-Connect-Bundle: Added CLI login documentation --- README.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/README.md b/README.md index f61dcac..38f9d66 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,19 @@ It is not necessary to add a prefix to the bundle routes, but in case you want i.e. another `/login` route, it makes distinguishing between them easier. + +### CLI login + +In order to use the CLI login feature the following +environment variable must be set: + +```shell +DEFAULT_URI= +``` + +See [Symfony documentation](https://symfony.com/doc/current/routing.html#generating-urls-in-commands) +for more information. + ### Creating the Authenticator The bundle handles the extraction of credentials received from the authorizer - @@ -194,6 +207,28 @@ services: $leeway: '%env(ITK_DEV_LEEWAY)%' ``` +## Sign in from command line + +Rather than signing in via OpenId Connect, you can get +a sign in url from the command line by providing a username. +Make sure to configure `DEFAULT_URI`. Run + +```shell +bin/console itk-dev:openid-connect:login +``` + +Or + +```shell +bin/console itk-dev:openid-connect:login --help +``` + +for details. + +Be aware that a login token only can be used once +before it is removed, and if you used email as your user provider property +the email goes into the `username` argument. + ## Changes for Symfony 6.0 In Symfony 6.0 a new security system is From c424c26420b6ecc237126bcfe03b63d58c10718a Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Fri, 10 Sep 2021 13:01:45 +0200 Subject: [PATCH 26/34] OpenId-Connect-Bundle: Added documentation and docblocks --- src/Command/UserLoginCommand.php | 21 +++++++++++++-- src/Controller/LoginController.php | 7 +++++ src/Security/LoginTokenAuthenticator.php | 4 ++- src/Security/OpenIdLoginAuthenticator.php | 3 +++ src/Util/CliLoginHelper.php | 32 ++++++++++++++++++----- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 9a1aabd..1b30202 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -2,10 +2,10 @@ namespace ItkDev\OpenIdConnectBundle\Command; -use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; use ItkDev\OpenIdConnectBundle\Exception\UserDoesNotExistException; use ItkDev\OpenIdConnectBundle\Exception\UsernameDoesNotExistException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; +use Psr\Cache\InvalidArgumentException; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -40,6 +40,15 @@ class UserLoginCommand extends Command */ private $cliLoginRedirectRoute; + /** + * UserLoginCommand constructor. + * + * @param CliLoginHelper $cliLoginHelper + * @param string $cliLoginRedirectRoute + * @param UrlGeneratorInterface $urlGenerator + * @param UserProviderInterface $userProvider + */ + public function __construct(CliLoginHelper $cliLoginHelper, string $cliLoginRedirectRoute, UrlGeneratorInterface $urlGenerator, UserProviderInterface $userProvider) { $this->cliLoginHelper = $cliLoginHelper; @@ -58,7 +67,15 @@ protected function configure(): void } /** - * @throws ItkOpenIdConnectBundleException + * Executes the CLI login url generation. + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int + * @throws InvalidArgumentException + * @throws UserDoesNotExistException + * @throws UsernameDoesNotExistException */ protected function execute(InputInterface $input, OutputInterface $output): int { diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index f10038b..d6e892c 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -8,6 +8,9 @@ use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Session\SessionInterface; +/** + * Login Controller class. + */ class LoginController extends AbstractController { /** @@ -21,6 +24,10 @@ public function __construct(OpenIdConfigurationProvider $provider) } /** + * Login method redirecting to authorizer. + * + * @param SessionInterface $session + * @return RedirectResponse * @throws ItkOpenIdConnectException */ public function login(SessionInterface $session): RedirectResponse diff --git a/src/Security/LoginTokenAuthenticator.php b/src/Security/LoginTokenAuthenticator.php index 4ca4155..3e32cbd 100644 --- a/src/Security/LoginTokenAuthenticator.php +++ b/src/Security/LoginTokenAuthenticator.php @@ -16,6 +16,9 @@ use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Guard\AbstractGuardAuthenticator; +/** + * Authenticator class for CLI login. + */ class LoginTokenAuthenticator extends AbstractGuardAuthenticator { /** @@ -89,7 +92,6 @@ public function checkCredentials($credentials, UserInterface $user) public function onAuthenticationFailure(Request $request, AuthenticationException $exception) { - // Throw (telling) error throw new AuthenticationException('Error occurred validating login token'); } diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php index 692a00e..9229837 100644 --- a/src/Security/OpenIdLoginAuthenticator.php +++ b/src/Security/OpenIdLoginAuthenticator.php @@ -13,6 +13,9 @@ use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Guard\AbstractGuardAuthenticator; +/** + * Authenticator for OpenId Connect login. + */ abstract class OpenIdLoginAuthenticator extends AbstractGuardAuthenticator { /** diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index 185da02..dd4a54a 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -2,11 +2,14 @@ namespace ItkDev\OpenIdConnectBundle\Util; -use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; use ItkDev\OpenIdConnectBundle\Exception\TokenNotFoundException; +use Psr\Cache\InvalidArgumentException; use Symfony\Component\Uid\Uuid; use Psr\Cache\CacheItemPoolInterface; +/** + * Helper class for CLI login. + */ class CliLoginHelper { /** @@ -25,6 +28,13 @@ public function __construct(CacheItemPoolInterface $cache) $this->itkNamespace = 'itk-dev-cli-login'; } + /** + * Creates login token for CLI login. + * + * @param string $username + * @return string + * @throws InvalidArgumentException + */ public function createToken(string $username): string { $encodedUsername = $this->encodeKey($username); @@ -48,36 +58,46 @@ public function createToken(string $username): string /** - * @throws ItkOpenIdConnectBundleException + * Gets username from login token. + * + * @param string $token + * @return string|null + * @throws InvalidArgumentException + * @throws TokenNotFoundException */ public function getUsername(string $token): ?string { - // check if token exists in cache $usernameItem = $this->cache->getItem($token); if (!$usernameItem->isHit()) { - // username does not exist in the cache throw new TokenNotFoundException('Token does not exist'); } $username = $usernameItem->get(); - // delete both entries from cache + // Delete both entries from cache $this->cache->deleteItem($token); $this->cache->deleteItem($username); return $this->decodeKey($username); } + /** + * @param string $key + * @return string + */ public function encodeKey(string $key): string { // Add namespace to key before encoding return base64_encode($this->itkNamespace . $key); } + /** + * @param string $encodedKey + * @return string + */ public function decodeKey(string $encodedKey): string { - // Decode encoded key $decodedKeyWithNamespace = base64_decode($encodedKey); // Remove namespace From 5f9a3616b4a1306688da83d0fd3e07abc69c6990 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 15 Sep 2021 12:43:40 +0200 Subject: [PATCH 27/34] OpenId-Connect-Bundle: Added SecurityBundle to test kernel to resolve service issues --- src/Resources/config/services.yaml | 10 +--------- tests/ItkDevOpenIdConnectBundleTest.php | 1 + tests/ItkDevOpenIdConnectBundleTestingKernel.php | 2 ++ tests/config/framework.yml | 2 ++ tests/config/security.yml | 13 +++++++++++++ 5 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 tests/config/security.yml diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index 84662ce..9b9968f 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -32,17 +32,9 @@ services: arguments: $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' $cliLoginRedirectRoute: ~ - $urlGenerator: '@Symfony\Component\Routing\Generator\UrlGeneratorInterface' - $userProvider: '@Symfony\Component\Security\Core\User\UserProviderInterface' ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator: public: true arguments: $cliLoginHelper: '@ItkDev\OpenIdConnectBundle\Util\CliLoginHelper' - $cliLoginRedirectRoute: ~ - - Symfony\Component\Routing\Generator\UrlGeneratorInterface: - public: true - - Symfony\Component\Security\Core\User\UserProviderInterface: - public: true \ No newline at end of file + $cliLoginRedirectRoute: ~ \ No newline at end of file diff --git a/tests/ItkDevOpenIdConnectBundleTest.php b/tests/ItkDevOpenIdConnectBundleTest.php index 4a3e0cf..2877e8f 100644 --- a/tests/ItkDevOpenIdConnectBundleTest.php +++ b/tests/ItkDevOpenIdConnectBundleTest.php @@ -20,6 +20,7 @@ public function testServiceWiring() { $kernel = new ItkDevOpenIdConnectBundleTestingKernel([ __DIR__ . '/config/framework.yml', + __DIR__ . '/config/security.yml', __DIR__ . '/config/itkdev_openid_connect.yml', ]); $kernel->boot(); diff --git a/tests/ItkDevOpenIdConnectBundleTestingKernel.php b/tests/ItkDevOpenIdConnectBundleTestingKernel.php index 74c611b..0b44312 100644 --- a/tests/ItkDevOpenIdConnectBundleTestingKernel.php +++ b/tests/ItkDevOpenIdConnectBundleTestingKernel.php @@ -9,6 +9,7 @@ use ItkDev\OpenIdConnectBundle\ItkDevOpenIdConnectBundle; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; +use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpClient\CurlHttpClient; @@ -35,6 +36,7 @@ public function registerBundles() { return [ new ItkDevOpenIdConnectBundle(), + new SecurityBundle(), new FrameworkBundle(), ]; } diff --git a/tests/config/framework.yml b/tests/config/framework.yml index 8a3124e..1727dd7 100644 --- a/tests/config/framework.yml +++ b/tests/config/framework.yml @@ -18,3 +18,5 @@ framework: enabled: false translator: enabled: false + router: + resource: ~ diff --git a/tests/config/security.yml b/tests/config/security.yml new file mode 100644 index 0000000..da73cb5 --- /dev/null +++ b/tests/config/security.yml @@ -0,0 +1,13 @@ +security: + providers: + test_users: + memory: + users: + admin: { password: 'test', roles: ['ROLE_ADMIN'] } + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true + lazy: true \ No newline at end of file From bb25c7ee8a7fb4a5a4acb732d3ad1d5b2bc06fa5 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 15 Sep 2021 12:45:12 +0200 Subject: [PATCH 28/34] OpenId-Connect-Bundle: Applied coding standards --- src/Command/UserLoginCommand.php | 3 --- src/Util/CliLoginHelper.php | 2 -- 2 files changed, 5 deletions(-) diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 1b30202..9ba28df 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -5,7 +5,6 @@ use ItkDev\OpenIdConnectBundle\Exception\UserDoesNotExistException; use ItkDev\OpenIdConnectBundle\Exception\UsernameDoesNotExistException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; -use Psr\Cache\InvalidArgumentException; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -73,7 +72,6 @@ protected function configure(): void * @param OutputInterface $output * * @return int - * @throws InvalidArgumentException * @throws UserDoesNotExistException * @throws UsernameDoesNotExistException */ @@ -85,7 +83,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int if (!is_string($username)) { throw new UsernameDoesNotExistException('Username is not type string.'); } - // Check if username is registered in User database try { $this->userProvider->loadUserByUsername($username); diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index dd4a54a..7e6b62d 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -33,7 +33,6 @@ public function __construct(CacheItemPoolInterface $cache) * * @param string $username * @return string - * @throws InvalidArgumentException */ public function createToken(string $username): string { @@ -62,7 +61,6 @@ public function createToken(string $username): string * * @param string $token * @return string|null - * @throws InvalidArgumentException * @throws TokenNotFoundException */ public function getUsername(string $token): ?string From a6356af28ecc3efbdde2e406f00cc8b0b271e814 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 15 Sep 2021 13:04:48 +0200 Subject: [PATCH 29/34] OpenId-Connect-Bundle: Removed unnecessary comments and unused imports --- .../ItkDevOpenIdConnectExtension.php | 4 ++- .../ItkOpenIdConnectBundleException.php | 4 ++- src/Util/CliLoginHelper.php | 1 - tests/ItkDevOpenIdConnectBundleTest.php | 1 - ...ItkDevOpenIdConnectBundleTestingKernel.php | 5 ++-- .../Security/OpenIdLoginAuthenticatorTest.php | 1 - tests/Util/CliLoginHelperTest.php | 26 ------------------- 7 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index 97f2360..7a94d70 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -2,6 +2,7 @@ namespace ItkDev\OpenIdConnectBundle\DependencyInjection; +use Exception; use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use ItkDev\OpenIdConnectBundle\Command\UserLoginCommand; use ItkDev\OpenIdConnectBundle\Security\LoginTokenAuthenticator; @@ -16,6 +17,7 @@ class ItkDevOpenIdConnectExtension extends Extension { /** * {@inheritdoc} + * @throws Exception */ public function load(array $configs, ContainerBuilder $container): void { @@ -50,7 +52,7 @@ public function load(array $configs, ContainerBuilder $container): void /** * {@inheritdoc} */ - public function getAlias() + public function getAlias(): string { return 'itkdev_openid_connect'; } diff --git a/src/Exception/ItkOpenIdConnectBundleException.php b/src/Exception/ItkOpenIdConnectBundleException.php index 3db5493..8bf4f14 100644 --- a/src/Exception/ItkOpenIdConnectBundleException.php +++ b/src/Exception/ItkOpenIdConnectBundleException.php @@ -2,7 +2,9 @@ namespace ItkDev\OpenIdConnectBundle\Exception; -abstract class ItkOpenIdConnectBundleException extends \Exception +use Exception; + +abstract class ItkOpenIdConnectBundleException extends Exception { } diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index 7e6b62d..8b79095 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -3,7 +3,6 @@ namespace ItkDev\OpenIdConnectBundle\Util; use ItkDev\OpenIdConnectBundle\Exception\TokenNotFoundException; -use Psr\Cache\InvalidArgumentException; use Symfony\Component\Uid\Uuid; use Psr\Cache\CacheItemPoolInterface; diff --git a/tests/ItkDevOpenIdConnectBundleTest.php b/tests/ItkDevOpenIdConnectBundleTest.php index 2877e8f..66210a5 100644 --- a/tests/ItkDevOpenIdConnectBundleTest.php +++ b/tests/ItkDevOpenIdConnectBundleTest.php @@ -6,7 +6,6 @@ use ItkDev\OpenIdConnectBundle\Controller\LoginController; use ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator; use PHPUnit\Framework\TestCase; -use Symfony\Component\Cache\Adapter\ArrayAdapter; /** * Class ItkDevOpenIdConnectBundleTest diff --git a/tests/ItkDevOpenIdConnectBundleTestingKernel.php b/tests/ItkDevOpenIdConnectBundleTestingKernel.php index 0b44312..3023394 100644 --- a/tests/ItkDevOpenIdConnectBundleTestingKernel.php +++ b/tests/ItkDevOpenIdConnectBundleTestingKernel.php @@ -7,14 +7,12 @@ namespace ItkDev\OpenIdConnectBundle\Tests; +use Exception; use ItkDev\OpenIdConnectBundle\ItkDevOpenIdConnectBundle; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\Loader\LoaderInterface; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\HttpClient\CurlHttpClient; use Symfony\Component\HttpKernel\Kernel; -use Symfony\Contracts\HttpClient\HttpClientInterface; /** * Class ItkDevOpenIdConnectBundleTestingKernel. @@ -43,6 +41,7 @@ public function registerBundles() /** * {@inheritdoc} + * @throws Exception */ public function registerContainerConfiguration(LoaderInterface $loader) { diff --git a/tests/Security/OpenIdLoginAuthenticatorTest.php b/tests/Security/OpenIdLoginAuthenticatorTest.php index 8f00d8e..edfa437 100644 --- a/tests/Security/OpenIdLoginAuthenticatorTest.php +++ b/tests/Security/OpenIdLoginAuthenticatorTest.php @@ -3,7 +3,6 @@ namespace ItkDev\OpenIdConnectBundle\Tests\Security; use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; -use ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\InputBag; use Symfony\Component\HttpFoundation\Request; diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php index c893336..791dfb4 100644 --- a/tests/Util/CliLoginHelperTest.php +++ b/tests/Util/CliLoginHelperTest.php @@ -13,93 +13,67 @@ class CliLoginHelperTest extends TestCase public function testEncodeAndDecode() { $cache = new ArrayAdapter(); - // Create CliLoginHelper $cliHelper = new CliLoginHelper($cache); $randomUsername = Uuid::v4()->toBase32(); - // Encode the username $encodedUsername = $cliHelper->encodeKey($randomUsername); - - // Decode the encoded username $decodedUsername = $cliHelper->decodeKey($encodedUsername); - // Assert equals $this->assertEquals($randomUsername, $decodedUsername); } public function testThrowExceptionIfTokenDoesNotExist() { - // Expect an exception to be thrown $this->expectException(ItkOpenIdConnectBundleException::class); - // Testing it works with one adapter $cache = new ArrayAdapter(); - // Create CliLoginHelper $cliHelper = new CliLoginHelper($cache); - // Call the function that should throw an error $username = $cliHelper->getUsername('random_gibberish_token'); } public function testReuseSetTokenRatherThanRemake() { - // Testing it works with one adapter $cache = new ArrayAdapter(); - // Create CliLoginHelper $cliHelper = new CliLoginHelper($cache); - // Create and set a token for user test_user $testUser = 'test_user'; $token = $cliHelper->createToken($testUser); - - // Set another token $token2 = $cliHelper->createToken($testUser); - // Test that they are the same $this->assertEquals($token, $token2); } public function testTokenIsRemovedAfterUse() { - // Testing it works with one adapter $cache = new ArrayAdapter(); - // Create CliLoginHelper $cliHelper = new CliLoginHelper($cache); - // Create and set a token for user test_user $testUser = 'test_user'; $token = $cliHelper->createToken($testUser); - // Get username from token created $username = $cliHelper->getUsername($token); - // Expect an exception to be thrown the second time we call getUsername $this->expectException(ItkOpenIdConnectBundleException::class); - //Try again, to ensure its gone $username = $cliHelper->getUsername($token); } public function testCreateTokenAndGetUsername() { - // Testing it works with one adapter $cache = new ArrayAdapter(); - // Create CliLoginHelper $cliHelper = new CliLoginHelper($cache); - // Create and set a token for user test_user $testUser = 'test_user'; $token = $cliHelper->createToken($testUser); - // Get username from token created $username = $cliHelper->getUsername($token); - // Check that we get correct username back $this->assertEquals($testUser, $username); } } From 79a5f468ae1d17ce0ce8dd5f1fea53085d2ecd09 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Thu, 16 Sep 2021 10:36:34 +0200 Subject: [PATCH 30/34] OpenId-Connect-Bundle: Used Command:FAILURE instead of throwing exception --- src/Command/UserLoginCommand.php | 12 +++++------- src/Exception/UserDoesNotExistException.php | 8 -------- src/Exception/UsernameDoesNotExistException.php | 8 -------- 3 files changed, 5 insertions(+), 23 deletions(-) delete mode 100644 src/Exception/UserDoesNotExistException.php delete mode 100644 src/Exception/UsernameDoesNotExistException.php diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 9ba28df..f9d1212 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -2,8 +2,6 @@ namespace ItkDev\OpenIdConnectBundle\Command; -use ItkDev\OpenIdConnectBundle\Exception\UserDoesNotExistException; -use ItkDev\OpenIdConnectBundle\Exception\UsernameDoesNotExistException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -47,7 +45,6 @@ class UserLoginCommand extends Command * @param UrlGeneratorInterface $urlGenerator * @param UserProviderInterface $userProvider */ - public function __construct(CliLoginHelper $cliLoginHelper, string $cliLoginRedirectRoute, UrlGeneratorInterface $urlGenerator, UserProviderInterface $userProvider) { $this->cliLoginHelper = $cliLoginHelper; @@ -72,8 +69,6 @@ protected function configure(): void * @param OutputInterface $output * * @return int - * @throws UserDoesNotExistException - * @throws UsernameDoesNotExistException */ protected function execute(InputInterface $input, OutputInterface $output): int { @@ -81,13 +76,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int $username = $input->getArgument('username'); if (!is_string($username)) { - throw new UsernameDoesNotExistException('Username is not type string.'); + $io->error('Username is not type string'); + return Command::FAILURE; } // Check if username is registered in User database try { $this->userProvider->loadUserByUsername($username); } catch (UsernameNotFoundException $e) { - throw new UserDoesNotExistException('User does not exist.'); + $io->error('User does not exist'); + return Command::FAILURE; } // Create token via CliLoginHelper @@ -98,6 +95,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int 'loginToken' => $token, ], UrlGeneratorInterface::ABSOLUTE_URL); + //$io->success($loginPage); $io->writeln($loginPage); return Command::SUCCESS; diff --git a/src/Exception/UserDoesNotExistException.php b/src/Exception/UserDoesNotExistException.php deleted file mode 100644 index 96bcdf0..0000000 --- a/src/Exception/UserDoesNotExistException.php +++ /dev/null @@ -1,8 +0,0 @@ - Date: Thu, 16 Sep 2021 10:37:03 +0200 Subject: [PATCH 31/34] OpenId-Connect-Bundle: Used class constant for itk namespace --- src/Util/CliLoginHelper.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index 8b79095..14c4584 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -11,20 +11,16 @@ */ class CliLoginHelper { + private const ITK_NAMESPACE = 'itk-dev-cli-login'; + /** * @var CacheItemPoolInterface */ private $cache; - /** - * @var string - */ - private $itkNamespace; - public function __construct(CacheItemPoolInterface $cache) { $this->cache = $cache; - $this->itkNamespace = 'itk-dev-cli-login'; } /** @@ -86,7 +82,7 @@ public function getUsername(string $token): ?string public function encodeKey(string $key): string { // Add namespace to key before encoding - return base64_encode($this->itkNamespace . $key); + return base64_encode(self::ITK_NAMESPACE . $key); } /** @@ -98,7 +94,7 @@ public function decodeKey(string $encodedKey): string $decodedKeyWithNamespace = base64_decode($encodedKey); // Remove namespace - $key = str_replace($this->itkNamespace, '', $decodedKeyWithNamespace); + $key = str_replace(self::ITK_NAMESPACE, '', $decodedKeyWithNamespace); return $key; } From b1c8c6d35bc6a7cacb570a1ff1e1ab3e6fcb8052 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Thu, 16 Sep 2021 10:38:16 +0200 Subject: [PATCH 32/34] OpenId-Connect-Bundle: Removed unused code --- src/Command/UserLoginCommand.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index f9d1212..e3f3ff8 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -95,7 +95,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int 'loginToken' => $token, ], UrlGeneratorInterface::ABSOLUTE_URL); - //$io->success($loginPage); $io->writeln($loginPage); return Command::SUCCESS; From c90cfe4b9fac007a1d115c56d039bab96eec4f7a Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Thu, 16 Sep 2021 11:17:15 +0200 Subject: [PATCH 33/34] OpenId-Connect-Bundle: try catch around cache calls --- psalm.xml | 7 +++++++ src/Util/CliLoginHelper.php | 32 +++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/psalm.xml b/psalm.xml index d58dc09..6044bcd 100644 --- a/psalm.xml +++ b/psalm.xml @@ -13,4 +13,11 @@ + + + + + + + diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index 14c4584..d98c406 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -2,7 +2,9 @@ namespace ItkDev\OpenIdConnectBundle\Util; +use ItkDev\OpenIdConnectBundle\Exception\CacheException; use ItkDev\OpenIdConnectBundle\Exception\TokenNotFoundException; +use Psr\Cache\InvalidArgumentException; use Symfony\Component\Uid\Uuid; use Psr\Cache\CacheItemPoolInterface; @@ -28,6 +30,7 @@ public function __construct(CacheItemPoolInterface $cache) * * @param string $username * @return string + * @throws CacheException */ public function createToken(string $username): string { @@ -35,7 +38,12 @@ public function createToken(string $username): string $token = Uuid::v4()->toBase32(); // Add username => token to make sure no username has more than one token - $revCacheItem = $this->cache->getItem($encodedUsername); + try { + $revCacheItem = $this->cache->getItem($encodedUsername); + } catch (InvalidArgumentException $e) { + throw new CacheException($e->getMessage()); + } + if ($revCacheItem->isHit()) { return $revCacheItem->get(); } @@ -43,7 +51,12 @@ public function createToken(string $username): string $this->cache->save($revCacheItem); // Add token => username - $cacheItem = $this->cache->getItem($token); + try { + $cacheItem = $this->cache->getItem($token); + } catch (InvalidArgumentException $e) { + throw new CacheException($e->getMessage()); + } + $cacheItem->set($encodedUsername); $this->cache->save($cacheItem); @@ -57,10 +70,15 @@ public function createToken(string $username): string * @param string $token * @return string|null * @throws TokenNotFoundException + * @throws CacheException */ public function getUsername(string $token): ?string { - $usernameItem = $this->cache->getItem($token); + try { + $usernameItem = $this->cache->getItem($token); + } catch (InvalidArgumentException $e) { + throw new CacheException($e->getMessage()); + } if (!$usernameItem->isHit()) { throw new TokenNotFoundException('Token does not exist'); @@ -69,8 +87,12 @@ public function getUsername(string $token): ?string $username = $usernameItem->get(); // Delete both entries from cache - $this->cache->deleteItem($token); - $this->cache->deleteItem($username); + try { + $this->cache->deleteItem($token); + $this->cache->deleteItem($username); + } catch (InvalidArgumentException $e) { + throw new CacheException($e->getMessage()); + } return $this->decodeKey($username); } From 200a5d6ee3f0f062877f3eb7907e83ebcdb52ada Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Thu, 16 Sep 2021 11:25:35 +0200 Subject: [PATCH 34/34] OpenId-Connect-Bundle: Readded custom exceptions --- src/Command/UserLoginCommand.php | 2 ++ src/Exception/UserDoesNotExistException.php | 8 ++++++++ src/Exception/UsernameDoesNotExistException.php | 8 ++++++++ src/Security/LoginTokenAuthenticator.php | 12 ++++++++---- 4 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 src/Exception/UserDoesNotExistException.php create mode 100644 src/Exception/UsernameDoesNotExistException.php diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index e3f3ff8..dd6819e 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -2,6 +2,7 @@ namespace ItkDev\OpenIdConnectBundle\Command; +use ItkDev\OpenIdConnectBundle\Exception\CacheException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -69,6 +70,7 @@ protected function configure(): void * @param OutputInterface $output * * @return int + * @throws CacheException */ protected function execute(InputInterface $input, OutputInterface $output): int { diff --git a/src/Exception/UserDoesNotExistException.php b/src/Exception/UserDoesNotExistException.php new file mode 100644 index 0000000..96bcdf0 --- /dev/null +++ b/src/Exception/UserDoesNotExistException.php @@ -0,0 +1,8 @@ +cliLoginHelper->getUsername($credentials); - } catch (ItkOpenIdConnectBundleException $exception) { - throw new UsernameDoesNotExistException($exception->getMessage()); + } catch (CacheException | TokenNotFoundException $e) { + throw $e; } if (null === $username) {