From 6a533579b7312b412ad00d9968c0b9f1876de60b Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Fri, 31 Jan 2020 19:47:34 +0100 Subject: [PATCH] added support for saml login (#1408) --- composer.json | 2 + composer.lock | 149 +++++++++++++- config/packages/security.yaml | 8 +- config/services-saml.yaml | 32 +++ config/services.yaml | 3 +- src/DependencyInjection/AppExtension.php | 3 +- .../Compiler/TwigContextCompilerPass.php | 3 + src/DependencyInjection/Configuration.php | 191 ++++++++++++++++++ src/Entity/User.php | 42 ++++ .../ResetPasswordSubscriber.php | 44 ++++ src/Kernel.php | 15 ++ src/Ldap/FormLoginLdapFactory.php | 23 +-- src/Ldap/LdapUserHydrator.php | 3 +- src/Ldap/LdapUserProvider.php | 7 +- src/Migrations/Version20200125123942.php | 55 +++++ src/Repository/UserRepository.php | 29 ++- src/Saml/Controller/SamlController.php | 86 ++++++++ src/Saml/Logout/SamlLogoutHandler.php | 56 +++++ src/Saml/Provider/SamlProvider.php | 90 +++++++++ src/Saml/SamlAuth.php | 26 +++ src/Saml/SamlTokenFactory.php | 28 +++ .../SamlAuthenticationSuccessHandler.php | 30 +++ src/Saml/Security/SamlFactory.php | 77 +++++++ src/Saml/User/SamlUserFactory.php | 116 +++++++++++ src/Security/DoctrineUserProvider.php | 77 +++++++ src/Voter/UserVoter.php | 4 + symfony.lock | 9 + .../FOSUserBundle/Security/login.html.twig | 9 + .../DependencyInjection/AppExtensionTest.php | 3 + .../DependencyInjection/ConfigurationTest.php | 52 +++++ tests/Entity/UserTest.php | 112 +++++++--- .../ResetPasswordSubscriberTest.php | 89 ++++++++ tests/Ldap/FormLoginLdapFactoryTest.php | 4 +- tests/Ldap/LdapManagerTest.php | 4 +- tests/Ldap/LdapUserProviderTest.php | 34 ++++ tests/Mocks/Saml/SamlAuthFactory.php | 84 ++++++++ tests/Saml/Controller/SamlControllerTest.php | 45 +++++ tests/Saml/Logout/SamlLogoutHandlerTest.php | 62 ++++++ tests/Saml/Provider/SamlProviderTest.php | 111 ++++++++++ tests/Saml/SamlTokenFactoryTest.php | 37 ++++ .../SamlAuthenticationSuccessHandlerTest.php | 104 ++++++++++ tests/Saml/Security/SamlFactoryTest.php | 45 +++++ tests/Saml/User/SamlUserFactoryTest.php | 184 +++++++++++++++++ tests/Security/DoctrineUserProviderTest.php | 126 ++++++++++++ tests/Security/TestUserEntity.php | 16 ++ tests/Voter/UserVoterTest.php | 26 +++ var/data/kimai_test.sqlite | Bin 770048 -> 770048 bytes 47 files changed, 2278 insertions(+), 77 deletions(-) create mode 100644 config/services-saml.yaml create mode 100644 src/EventSubscriber/ResetPasswordSubscriber.php create mode 100644 src/Migrations/Version20200125123942.php create mode 100644 src/Saml/Controller/SamlController.php create mode 100644 src/Saml/Logout/SamlLogoutHandler.php create mode 100644 src/Saml/Provider/SamlProvider.php create mode 100644 src/Saml/SamlAuth.php create mode 100644 src/Saml/SamlTokenFactory.php create mode 100644 src/Saml/Security/SamlAuthenticationSuccessHandler.php create mode 100644 src/Saml/Security/SamlFactory.php create mode 100644 src/Saml/User/SamlUserFactory.php create mode 100644 src/Security/DoctrineUserProvider.php create mode 100644 tests/EventSubscriber/ResetPasswordSubscriberTest.php create mode 100644 tests/Mocks/Saml/SamlAuthFactory.php create mode 100644 tests/Saml/Controller/SamlControllerTest.php create mode 100644 tests/Saml/Logout/SamlLogoutHandlerTest.php create mode 100644 tests/Saml/Provider/SamlProviderTest.php create mode 100644 tests/Saml/SamlTokenFactoryTest.php create mode 100644 tests/Saml/Security/SamlAuthenticationSuccessHandlerTest.php create mode 100644 tests/Saml/Security/SamlFactoryTest.php create mode 100644 tests/Saml/User/SamlUserFactoryTest.php create mode 100644 tests/Security/DoctrineUserProviderTest.php create mode 100644 tests/Security/TestUserEntity.php diff --git a/composer.json b/composer.json index 7d1b1db090..273fb39a67 100644 --- a/composer.json +++ b/composer.json @@ -25,6 +25,7 @@ "jms/metadata": "^2.0", "jms/serializer-bundle": "^3.2", "kevinpapst/adminlte-bundle": "^3.0", + "hslavich/oneloginsaml-bundle": "^1.4", "kimai/kimai2-composer": "^0.1", "laravolt/avatar": "^3.0", "league/csv": "^9.4", @@ -32,6 +33,7 @@ "nelmio/api-doc-bundle": "^3.2", "nelmio/cors-bundle": "^1.5", "ocramius/proxy-manager": "^2.1.1", + "onelogin/php-saml": "^3.4", "phpoffice/phpspreadsheet": "^1.10", "phpoffice/phpword": "^0.17", "psr/log": "^1.1", diff --git a/composer.lock b/composer.lock index 2e64f1f8f5..55a2dd0cf5 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c4e30ced9c0035e4b1185044f9645fcb", + "content-hash": "2fee31e8ce28cbf8048bae137c7e3f31", "packages": [ { "name": "beberlei/doctrineextensions", @@ -2555,6 +2555,55 @@ ], "time": "2017-01-10T10:39:54+00:00" }, + { + "name": "hslavich/oneloginsaml-bundle", + "version": "v1.4.1", + "source": { + "type": "git", + "url": "https://github.com/hslavich/OneloginSamlBundle.git", + "reference": "7a8e7e7f0bbf30bc0fcb662d52a4549b0eba3828" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/hslavich/OneloginSamlBundle/zipball/7a8e7e7f0bbf30bc0fcb662d52a4549b0eba3828", + "reference": "7a8e7e7f0bbf30bc0fcb662d52a4549b0eba3828", + "shasum": "" + }, + "require": { + "onelogin/php-saml": "^3.0", + "symfony/framework-bundle": "~2.3|~3.0|^4.0", + "symfony/security-bundle": "~2.3|~3.0|^4.0" + }, + "require-dev": { + "doctrine/orm": "~2.3", + "phpunit/phpunit": "~5.7", + "satooshi/php-coveralls": "~1.0", + "symfony/phpunit-bridge": "~2.7|~3.0|^4.0" + }, + "type": "symfony-bundle", + "autoload": { + "psr-4": { + "Hslavich\\OneloginSamlBundle\\": "" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "hslavich", + "email": "hernan.slavich@gmail.com" + } + ], + "description": "OneLogin SAML Bundle for Symfony2", + "keywords": [ + "SSO", + "onelogin", + "saml" + ], + "time": "2019-02-05T14:15:37+00:00" + }, { "name": "illuminate/cache", "version": "v6.13.1", @@ -3993,6 +4042,56 @@ ], "time": "2019-08-10T08:37:15+00:00" }, + { + "name": "onelogin/php-saml", + "version": "3.4.1", + "source": { + "type": "git", + "url": "https://github.com/onelogin/php-saml.git", + "reference": "5fbf3486704ac9835b68184023ab54862c95f213" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/onelogin/php-saml/zipball/5fbf3486704ac9835b68184023ab54862c95f213", + "reference": "5fbf3486704ac9835b68184023ab54862c95f213", + "shasum": "" + }, + "require": { + "php": ">=5.4", + "robrichards/xmlseclibs": ">=3.0.4" + }, + "require-dev": { + "pdepend/pdepend": "^2.5.0", + "php-coveralls/php-coveralls": "^1.0.2 || ^2.0", + "phploc/phploc": "^2.1 || ^3.0 || ^4.0", + "phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.5 || ^7.1", + "sebastian/phpcpd": "^2.0 || ^3.0 || ^4.0", + "squizlabs/php_codesniffer": "^3.1.1" + }, + "suggest": { + "ext-curl": "Install curl lib to be able to use the IdPMetadataParser for parsing remote XMLs", + "ext-gettext": "Install gettext and php5-gettext libs to handle translations", + "ext-openssl": "Install openssl lib in order to handle with x509 certs (require to support sign and encryption)" + }, + "type": "library", + "autoload": { + "psr-4": { + "OneLogin\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "OneLogin PHP SAML Toolkit", + "homepage": "https://developers.onelogin.com/saml/php", + "keywords": [ + "SAML2", + "onelogin", + "saml" + ], + "time": "2019-11-25T17:30:07+00:00" + }, { "name": "pagerfanta/pagerfanta", "version": "v2.1.3", @@ -4832,6 +4931,44 @@ "description": "A polyfill for getallheaders.", "time": "2019-03-08T08:55:37+00:00" }, + { + "name": "robrichards/xmlseclibs", + "version": "3.0.4", + "source": { + "type": "git", + "url": "https://github.com/robrichards/xmlseclibs.git", + "reference": "0a53d3c3aa87564910cae4ed01416441d3ae0db5" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/robrichards/xmlseclibs/zipball/0a53d3c3aa87564910cae4ed01416441d3ae0db5", + "reference": "0a53d3c3aa87564910cae4ed01416441d3ae0db5", + "shasum": "" + }, + "require": { + "ext-openssl": "*", + "php": ">= 5.4" + }, + "type": "library", + "autoload": { + "psr-4": { + "RobRichards\\XMLSecLibs\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "description": "A PHP library for XML Security", + "homepage": "https://github.com/robrichards/xmlseclibs", + "keywords": [ + "security", + "signature", + "xml", + "xmldsig" + ], + "time": "2019-11-05T11:44:22+00:00" + }, { "name": "sensio/framework-extra-bundle", "version": "v5.5.3", @@ -8672,16 +8809,16 @@ }, { "name": "symfony/webpack-encore-bundle", - "version": "v1.7.2", + "version": "v1.7.3", "source": { "type": "git", "url": "https://github.com/symfony/webpack-encore-bundle.git", - "reference": "787c2fdedde57788013339f05719c82ce07b6058" + "reference": "5c0f659eceae87271cce54bbdfb05ed8ec9007bd" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/webpack-encore-bundle/zipball/787c2fdedde57788013339f05719c82ce07b6058", - "reference": "787c2fdedde57788013339f05719c82ce07b6058", + "url": "https://api.github.com/repos/symfony/webpack-encore-bundle/zipball/5c0f659eceae87271cce54bbdfb05ed8ec9007bd", + "reference": "5c0f659eceae87271cce54bbdfb05ed8ec9007bd", "shasum": "" }, "require": { @@ -8721,7 +8858,7 @@ } ], "description": "Integration with your Symfony app & Webpack Encore!", - "time": "2019-11-26T14:48:41+00:00" + "time": "2020-01-31T15:31:59+00:00" }, { "name": "symfony/yaml", diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 3c07d11ea5..03f8f58a5d 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -5,11 +5,11 @@ security: providers: chain_provider: chain: - providers: [fos_userbundle] + providers: [kimai_internal] kimai_ldap: id: App\Ldap\LdapUserProvider - fos_userbundle: - id: fos_user.user_provider.username_email + kimai_internal: + id: App\Security\DoctrineUserProvider firewalls: dev: @@ -54,6 +54,8 @@ security: ROLE_SUPER_ADMIN: ROLE_ADMIN access_control: + - { path: '^/auth/saml/login', roles: IS_AUTHENTICATED_ANONYMOUSLY } + - { path: '^/auth/saml/metadata', roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: '^/(%app_locales%)$', role: IS_AUTHENTICATED_ANONYMOUSLY } - { path: '^/(%app_locales%)/login', role: IS_AUTHENTICATED_ANONYMOUSLY } - { path: '^/(%app_locales%)/register', role: IS_AUTHENTICATED_ANONYMOUSLY } diff --git a/config/services-saml.yaml b/config/services-saml.yaml new file mode 100644 index 0000000000..6942ea749c --- /dev/null +++ b/config/services-saml.yaml @@ -0,0 +1,32 @@ + +services: + + # ================================================================================ + # SAML + # ================================================================================ + + App\Saml\SamlAuth: + alias: onelogin_auth + + OneLogin\Saml2\Auth: + alias: onelogin_auth + + onelogin_auth: + class: App\Saml\SamlAuth + arguments: ['@request_stack', '%kimai.saml.connection%'] + + App\Saml\User\SamlUserFactory: + arguments: ['%kimai.saml%'] + + kimai.saml_listener: + class: Hslavich\OneloginSamlBundle\Security\Firewall\SamlListener + parent: security.authentication.listener.abstract + abstract: true + calls: + - [setOneLoginAuth, ["@onelogin_auth"]] + + App\Saml\Provider\SamlProvider: + arguments: ['@App\Repository\UserRepository', '', '@App\Saml\SamlTokenFactory', '@App\Saml\User\SamlUserFactory'] + + App\Saml\Security\SamlAuthenticationSuccessHandler: + parent: security.authentication.success_handler diff --git a/config/services.yaml b/config/services.yaml index 05f40fa7eb..c7d79ab670 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -146,8 +146,7 @@ services: # LDAP # ================================================================================ - kimai_ldap.security.authentication.provider: - class: App\Ldap\LdapAuthenticationProvider + App\Ldap\LdapAuthenticationProvider: arguments: ['@App\Security\UserChecker', '', '', '', '@App\Configuration\LdapConfiguration', '%security.authentication.hide_user_not_found%'] # ================================================================================ diff --git a/src/DependencyInjection/AppExtension.php b/src/DependencyInjection/AppExtension.php index 914dc44377..b7d3ed674c 100644 --- a/src/DependencyInjection/AppExtension.php +++ b/src/DependencyInjection/AppExtension.php @@ -65,7 +65,8 @@ public function load(array $configs, ContainerBuilder $container) $this->createPermissionParameter($config['permissions'], $container); $this->createThemeParameter($config['theme'], $container); $this->createUserParameter($config['user'], $container); - + $container->setParameter('kimai.saml', $config['saml']); + $container->setParameter('kimai.saml.connection', $config['saml']['connection']); $container->setParameter('kimai.timesheet', $config['timesheet']); $container->setParameter('kimai.timesheet.rates', $config['timesheet']['rates']); $container->setParameter('kimai.timesheet.rounding', $config['timesheet']['rounding']); diff --git a/src/DependencyInjection/Compiler/TwigContextCompilerPass.php b/src/DependencyInjection/Compiler/TwigContextCompilerPass.php index 841682973f..72f7f4b585 100644 --- a/src/DependencyInjection/Compiler/TwigContextCompilerPass.php +++ b/src/DependencyInjection/Compiler/TwigContextCompilerPass.php @@ -29,6 +29,9 @@ public function process(ContainerBuilder $container) $theme = $container->getDefinition(ThemeConfiguration::class); $twig->addMethodCall('addGlobal', ['kimai_context', $theme]); + $saml = $container->getParameter('kimai.saml'); + $twig->addMethodCall('addGlobal', ['saml', $saml]); + if ($container->hasDefinition('twig.loader.native_filesystem')) { $definition = $container->getDefinition('twig.loader.native_filesystem'); diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index dc0f28d5f5..5146e142bb 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -66,6 +66,7 @@ public function getConfigTreeBuilder() ->append($this->getDefaultsNode()) ->append($this->getPermissionsNode()) ->append($this->getLdapNode()) + ->append($this->getSamlNode()) ->end() ->end(); @@ -666,4 +667,194 @@ protected function getLdapNode() return $node; } + + protected function getSamlNode() + { + $builder = new TreeBuilder('saml'); + /** @var ArrayNodeDefinition $node */ + $node = $builder->getRootNode(); + + $node + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('activate') + ->defaultFalse() + ->end() + ->scalarNode('title') + ->defaultValue('Login with SAML') + ->end() + ->arrayNode('roles') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('attribute') + ->defaultNull() + ->end() + ->arrayNode('mapping') + ->defaultValue([]) + ->arrayPrototype() + ->children() + ->scalarNode('saml')->isRequired()->cannotBeEmpty()->end() + ->scalarNode('kimai')->isRequired()->cannotBeEmpty()->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->arrayNode('mapping') + ->defaultValue([]) + ->arrayPrototype() + ->children() + ->scalarNode('saml')->isRequired()->cannotBeEmpty()->end() + ->scalarNode('kimai')->isRequired()->cannotBeEmpty()->end() + ->end() + ->end() + ->end() + ->arrayNode('connection') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('baseurl')->end() + ->booleanNode('strict')->end() + ->booleanNode('debug')->end() + ->arrayNode('idp') + ->children() + ->scalarNode('entityId')->end() + ->scalarNode('x509cert')->end() + ->arrayNode('singleSignOnService') + ->children() + ->scalarNode('url')->end() + ->scalarNode('binding')->end() + ->end() + ->end() + ->arrayNode('singleLogoutService') + ->children() + ->scalarNode('url')->end() + ->scalarNode('binding')->end() + ->end() + ->end() + ->scalarNode('certFingerprint')->end() + ->scalarNode('certFingerprintAlgorithm')->end() + ->arrayNode('x509certMulti') + ->children() + ->arrayNode('signing') + ->prototype('scalar')->end() + ->end() + ->arrayNode('encryption') + ->prototype('scalar')->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->arrayNode('sp') + ->children() + ->scalarNode('entityId')->end() + ->scalarNode('NameIDFormat')->end() + ->scalarNode('x509cert')->end() + ->scalarNode('privateKey')->end() + ->arrayNode('assertionConsumerService') + ->children() + ->scalarNode('url')->end() + ->scalarNode('binding')->end() + ->end() + ->end() + ->arrayNode('attributeConsumingService') + ->children() + ->scalarNode('serviceName')->end() + ->scalarNode('serviceDescription')->end() + ->arrayNode('requestedAttributes') + ->prototype('array') + ->children() + ->scalarNode('name')->end() + ->booleanNode('isRequired')->defaultValue(false)->end() + ->scalarNode('nameFormat')->end() + ->scalarNode('friendlyName')->end() + ->arrayNode('attributeValue')->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->arrayNode('singleLogoutService') + ->children() + ->scalarNode('url')->end() + ->scalarNode('binding')->end() + ->end() + ->end() + ->end() + ->end() + ->arrayNode('security') + ->children() + ->booleanNode('nameIdEncrypted')->end() + ->booleanNode('authnRequestsSigned')->end() + ->booleanNode('logoutRequestSigned')->end() + ->booleanNode('logoutResponseSigned')->end() + ->booleanNode('wantMessagesSigned')->end() + ->booleanNode('wantAssertionsSigned')->end() + ->booleanNode('wantAssertionsEncrypted')->end() + ->booleanNode('wantNameId')->end() + ->booleanNode('wantNameIdEncrypted')->end() + ->variableNode('requestedAuthnContext') + ->validate() + ->ifTrue(function ($v) { + return !is_bool($v) && !is_array($v); + }) + ->thenInvalid('Must be an array or a bool.') + ->end() + ->end() + ->booleanNode('signMetadata')->end() + ->booleanNode('wantXMLValidation')->end() + ->booleanNode('lowercaseUrlencoding')->end() + ->scalarNode('signatureAlgorithm')->end() + ->scalarNode('digestAlgorithm')->end() + ->scalarNode('entityManagerName')->end() + ->end() + ->end() + ->arrayNode('contactPerson') + ->children() + ->arrayNode('technical') + ->children() + ->scalarNode('givenName')->end() + ->scalarNode('emailAddress')->end() + ->end() + ->end() + ->arrayNode('support') + ->children() + ->scalarNode('givenName')->end() + ->scalarNode('emailAddress')->end() + ->end() + ->end() + ->end() + ->end() + ->arrayNode('organization') + ->prototype('array') + ->children() + ->scalarNode('name')->end() + ->scalarNode('displayname')->end() + ->scalarNode('url')->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->validate() + ->ifTrue(static function ($v) { + if (true !== $v['activate']) { + return false; + } + $found = false; + foreach ($v['mapping'] as $mapping) { + if ($mapping['kimai'] === 'email') { + $found = true; + } + } + + return !$found; + }) + ->thenInvalid('You need to configure a SAML mapping for the email attribute.') + ->end() + ; + + return $node; + } } diff --git a/src/Entity/User.php b/src/Entity/User.php index 6e23d53228..ec7d9b9870 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -38,6 +38,10 @@ class User extends BaseUser implements UserInterface public const DEFAULT_ROLE = self::ROLE_USER; public const DEFAULT_LANGUAGE = 'en'; + public const AUTH_INTERNAL = 'kimai'; + public const AUTH_LDAP = 'ldap'; + public const AUTH_SAML = 'saml'; + /** * @var int * @@ -111,6 +115,13 @@ class User extends BaseUser implements UserInterface */ private $teams; + /** + * @var string + * + * @ORM\Column(name="auth", type="string", length=20, nullable=true) + */ + private $auth = self::AUTH_INTERNAL; + /** * User constructor. */ @@ -308,6 +319,10 @@ public function getPreferenceValue($name, $default = null) */ public function addPreference(UserPreference $preference): User { + if (null === $this->preferences) { + $this->preferences = new ArrayCollection(); + } + $this->preferences->add($preference); $preference->setUser($this); @@ -372,6 +387,33 @@ public function getDisplayName(): ?string return $this->getUsername(); } + public function getAuth(): ?string + { + return $this->auth; + } + + public function setAuth(string $auth): User + { + $this->auth = $auth; + + return $this; + } + + public function isSamlUser(): bool + { + return $this->auth === self::AUTH_SAML; + } + + public function isLdapUser(): bool + { + return $this->auth === self::AUTH_LDAP; + } + + public function isInternalUser(): bool + { + return $this->auth === null || $this->auth === self::AUTH_INTERNAL; + } + /** * @return string */ diff --git a/src/EventSubscriber/ResetPasswordSubscriber.php b/src/EventSubscriber/ResetPasswordSubscriber.php new file mode 100644 index 0000000000..5f83e37fcf --- /dev/null +++ b/src/EventSubscriber/ResetPasswordSubscriber.php @@ -0,0 +1,44 @@ + ['onInitializeResetPassword', 200] + ]; + } + + public function onInitializeResetPassword(GetResponseNullableUserEvent $event) + { + $user = $event->getUser(); + if (!($user instanceof User)) { + return; + } + + // that is not nice :-D + if (!$user->isInternalUser()) { + throw new AccessDeniedHttpException( + sprintf('The user "%s" tried to reset the password, but it is registered as "%s" auth-type.', $user->getUsername(), $user->getAuth()) + ); + } + } +} diff --git a/src/Kernel.php b/src/Kernel.php index 1e957ab290..693af9cc54 100644 --- a/src/Kernel.php +++ b/src/Kernel.php @@ -23,6 +23,7 @@ use App\Invoice\RendererInterface as InvoiceRendererInterface; use App\Ldap\FormLoginLdapFactory; use App\Plugin\PluginInterface; +use App\Saml\Security\SamlFactory; use App\Timesheet\CalculatorInterface as TimesheetCalculator; use App\Timesheet\Rounding\RoundingInterface; use App\Timesheet\TrackingMode\TrackingModeInterface; @@ -85,6 +86,7 @@ protected function build(ContainerBuilder $container) /** @var SecurityExtension $extension */ $extension = $container->getExtension('security'); $extension->addSecurityListenerFactory(new FormLoginLdapFactory()); + $extension->addSecurityListenerFactory(new SamlFactory()); } public function registerBundles() @@ -174,6 +176,7 @@ protected function configureContainer(ContainerBuilder $container, LoaderInterfa } $loader->load($confDir . '/packages/local' . self::CONFIG_EXTS, 'glob'); $loader->load($confDir . '/services' . self::CONFIG_EXTS, 'glob'); + $loader->load($confDir . '/services-*' . self::CONFIG_EXTS, 'glob'); $loader->load($confDir . '/services_' . $this->environment . self::CONFIG_EXTS, 'glob'); $container->addCompilerPass(new DoctrineCompilerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -1000); @@ -189,6 +192,7 @@ protected function configureRoutes(RouteCollectionBuilder $routes) // some routes are based on app configs and will be imported manually $this->configureFosUserRoutes($routes); + $this->configureSamlRoutes($routes); // load bundle specific route files if (is_dir($confDir . '/routes/')) { @@ -229,4 +233,15 @@ protected function configureFosUserRoutes(RouteCollectionBuilder $routes) ); } } + + protected function configureSamlRoutes(RouteCollectionBuilder $routes) + { + $saml = $this->getContainer()->getParameter('kimai.saml'); + + if (!$saml['activate']) { + return; + } + + $routes->import('../src/Saml/Controller/SamlController.php', '/auth', 'annotation'); + } } diff --git a/src/Ldap/FormLoginLdapFactory.php b/src/Ldap/FormLoginLdapFactory.php index 9eab591df8..2cec0b8a6e 100644 --- a/src/Ldap/FormLoginLdapFactory.php +++ b/src/Ldap/FormLoginLdapFactory.php @@ -20,12 +20,12 @@ */ class FormLoginLdapFactory implements SecurityFactoryInterface { - public function create(ContainerBuilder $container, $id, $config, $userProviderId, $defaultEntryPointId) + public function create(ContainerBuilder $container, $id, $config, $userProviderId, $defaultEntryPoint) { $authProviderId = $this->createAuthProvider($container, $id, $userProviderId); $listenerId = $this->createListener($container, $id, $config); - return [$authProviderId, $listenerId, $defaultEntryPointId]; + return [$authProviderId, $listenerId, $defaultEntryPoint]; } public function getPosition() @@ -44,11 +44,10 @@ public function addConfiguration(NodeDefinition $node) protected function createAuthProvider(ContainerBuilder $container, $id, $userProviderId) { - $provider = 'kimai_ldap.security.authentication.provider'; - $providerId = $provider . '.' . $id; + $providerId = 'security.authentication.provider.kimai_ldap.' . $id; $container - ->setDefinition($providerId, new ChildDefinition($provider)) + ->setDefinition($providerId, new ChildDefinition(LdapAuthenticationProvider::class)) ->replaceArgument(1, $id) ->replaceArgument(2, new Reference($userProviderId)) ; @@ -58,14 +57,14 @@ protected function createAuthProvider(ContainerBuilder $container, $id, $userPro protected function createListener(ContainerBuilder $container, $id, $config) { - $listenerId = 'security.authentication.listener.form'; + $listener = 'security.authentication.listener.form'; + $listenerId = $listener . '.' . $id; - $listener = new ChildDefinition($listenerId); - $listener->replaceArgument(4, $id); - $listener->replaceArgument(5, $config); - - $listenerId .= '.' . $id; - $container->setDefinition($listenerId, $listener); + $container + ->setDefinition($listenerId, new ChildDefinition($listener)) + ->replaceArgument(4, $id) + ->replaceArgument(5, $config) + ; return $listenerId; } diff --git a/src/Ldap/LdapUserHydrator.php b/src/Ldap/LdapUserHydrator.php index 3099eab08a..2b002ee11d 100644 --- a/src/Ldap/LdapUserHydrator.php +++ b/src/Ldap/LdapUserHydrator.php @@ -71,8 +71,9 @@ public function hydrateUser(User $user, array $ldapEntry) $user->setEmail($user->getUsername()); } - // prevent that users will define a password for the internal account + // fill them after hydrating account, so they can't be overwritten $user->setPassword(''); + $user->setAuth(User::AUTH_LDAP); $user->setPreferenceValue('ldap.dn', $ldapEntry['dn']); } diff --git a/src/Ldap/LdapUserProvider.php b/src/Ldap/LdapUserProvider.php index debc3f54da..f8fbb87acd 100644 --- a/src/Ldap/LdapUserProvider.php +++ b/src/Ldap/LdapUserProvider.php @@ -73,12 +73,17 @@ public function refreshUser(UserInterface $user) throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_class($user))); } - if (null === $user->getPreferenceValue('ldap.dn')) { + if (!$user->isLdapUser() && null === $user->getPreferenceValue('ldap.dn')) { throw new UnsupportedUserException(sprintf('Account "%s" is not a registered LDAP user.', $user->getUsername())); } try { $this->ldapManager->updateUser($user); + + // updating old LDAP accounts + if (!$user->isLdapUser() && null !== $user->getPreferenceValue('ldap.dn')) { + $user->setAuth(User::AUTH_LDAP); + } } catch (LdapDriverException $ex) { throw new UnsupportedUserException(sprintf('Failed to refresh user "%s", probably DN is expired.', $user->getUsername())); } diff --git a/src/Migrations/Version20200125123942.php b/src/Migrations/Version20200125123942.php new file mode 100644 index 0000000000..4526af2e73 --- /dev/null +++ b/src/Migrations/Version20200125123942.php @@ -0,0 +1,55 @@ +isPlatformSqlite()) { + // does fail if we use transactions, as tables are re-created and foreign keys would fail + return false; + } + + return true; + } + + public function up(Schema $schema): void + { + $users = $schema->getTable('kimai2_users'); + $users->addColumn('auth', 'string', ['notnull' => false, 'length' => 20]); + } + + public function down(Schema $schema): void + { + $users = $schema->getTable('kimai2_users'); + $users->dropColumn('auth'); + } +} diff --git a/src/Repository/UserRepository.php b/src/Repository/UserRepository.php index 3f003dc11a..5817203aea 100644 --- a/src/Repository/UserRepository.php +++ b/src/Repository/UserRepository.php @@ -52,21 +52,16 @@ public function saveUser(User $user) */ public function getUserById($id): ?User { - try { - return $this->createQueryBuilder('u') - ->select('u', 'p', 't', 'tu', 'tl') - ->leftJoin('u.preferences', 'p') - ->leftJoin('u.teams', 't') - ->leftJoin('t.users', 'tu') - ->leftJoin('t.teamlead', 'tl') - ->where('u.id = :id') - ->setParameter('id', $id) - ->getQuery() - ->getSingleResult(); - } catch (\Exception $ex) { - } - - return null; + return $this->createQueryBuilder('u') + ->select('u', 'p', 't', 'tu', 'tl') + ->leftJoin('u.preferences', 'p') + ->leftJoin('u.teams', 't') + ->leftJoin('t.users', 'tu') + ->leftJoin('t.teamlead', 'tl') + ->where('u.id = :id') + ->setParameter('id', $id) + ->getQuery() + ->getOneOrNullResult(); } /** @@ -122,7 +117,7 @@ public function findByQuery(UserQuery $query) /** * @param string $username - * @return mixed|null|\Symfony\Component\Security\Core\User\UserInterface + * @return null|User * @throws \Doctrine\ORM\NoResultException * @throws \Doctrine\ORM\NonUniqueResultException */ @@ -138,7 +133,7 @@ public function loadUserByUsername($username) ->orWhere('u.email = :username') ->setParameter('username', $username) ->getQuery() - ->getSingleResult(); + ->getOneOrNullResult(); } public function getQueryBuilderForFormType(UserFormTypeQuery $query): QueryBuilder diff --git a/src/Saml/Controller/SamlController.php b/src/Saml/Controller/SamlController.php new file mode 100644 index 0000000000..9352ca5cab --- /dev/null +++ b/src/Saml/Controller/SamlController.php @@ -0,0 +1,86 @@ +oneLoginAuth = $oneLoginAuth; + } + + /** + * @Route(path="/login", name="saml_login") + */ + public function loginAction(Request $request) + { + $session = $request->getSession(); + $authErrorKey = Security::AUTHENTICATION_ERROR; + + if ($request->attributes->has($authErrorKey)) { + $error = $request->attributes->get($authErrorKey); + } elseif (null !== $session && $session->has($authErrorKey)) { + $error = $session->get($authErrorKey); + $session->remove($authErrorKey); + } else { + $error = null; + } + + if ($error) { + throw new \RuntimeException($error->getMessage()); + } + + $this->oneLoginAuth->login($session->get('_security.main.target_path')); + } + + /** + * @Route(path="/metadata", name="saml_metadata") + */ + public function metadataAction() + { + $metadata = $this->oneLoginAuth->getSettings()->getSPMetadata(); + + $response = new Response($metadata); + $response->headers->set('Content-Type', 'xml'); + + return $response; + } + + /** + * @Route(path="/acs", name="saml_acs") + */ + public function assertionConsumerServiceAction() + { + throw new \RuntimeException('You must configure the check path in your firewall.'); + } + + /** + * @Route(path="/logout", name="saml_logout") + */ + public function logoutAction() + { + throw new \RuntimeException('You must configure the logout path in your firewall.'); + } +} diff --git a/src/Saml/Logout/SamlLogoutHandler.php b/src/Saml/Logout/SamlLogoutHandler.php new file mode 100644 index 0000000000..ce9690ce31 --- /dev/null +++ b/src/Saml/Logout/SamlLogoutHandler.php @@ -0,0 +1,56 @@ +samlAuth = $samlAuth; + } + + /** + * This method is called by the LogoutListener when a user has requested + * to be logged out. Usually, you would unset session variables, or remove + * cookies, etc. + * + * @param Request $request + * @param Response $response + * @param TokenInterface $token + */ + public function logout(Request $request, Response $response, TokenInterface $token) + { + if (!$token instanceof SamlTokenInterface) { + return; + } + + try { + $this->samlAuth->processSLO(); + } catch (Error $e) { + if (!empty($this->samlAuth->getSLOurl())) { + $sessionIndex = $token->hasAttribute('sessionIndex') ? $token->getAttribute('sessionIndex') : null; + $this->samlAuth->logout(null, [], $token->getUsername(), $sessionIndex); + } + } + } +} diff --git a/src/Saml/Provider/SamlProvider.php b/src/Saml/Provider/SamlProvider.php new file mode 100644 index 0000000000..4059e4cdaa --- /dev/null +++ b/src/Saml/Provider/SamlProvider.php @@ -0,0 +1,90 @@ +repository = $repository; + $this->userProvider = $userProvider; + $this->tokenFactory = $tokenFactory; + $this->userFactory = $userFactory; + } + + public function authenticate(TokenInterface $token) + { + $user = null; + + /** @var ChainUserProvider $p */ + $p = $this->userProvider; + + try { + $user = $this->userProvider->loadUserByUsername($token->getUsername()); + } catch (UsernameNotFoundException $e) { + } + + try { + if (null === $user) { + $user = $this->userFactory->createUser($token); + } else { + $this->userFactory->hydrateUser($user, $token); + } + + $this->repository->saveUser($user); + } catch (\Exception $ex) { + throw new AuthenticationException( + sprintf('Failed creating or hydrating user "%s": %s', $token->getUsername(), $ex->getMessage()) + ); + } + + if ($user) { + $authenticatedToken = $this->tokenFactory->createToken($user, $token->getAttributes(), $user->getRoles()); + $authenticatedToken->setAuthenticated(true); + + return $authenticatedToken; + } + + throw new AuthenticationException('The authentication failed.'); + } + + public function supports(TokenInterface $token) + { + return $token instanceof SamlTokenInterface; + } +} diff --git a/src/Saml/SamlAuth.php b/src/Saml/SamlAuth.php new file mode 100644 index 0000000000..b6ffd29921 --- /dev/null +++ b/src/Saml/SamlAuth.php @@ -0,0 +1,26 @@ +getMasterRequest() && $request->getMasterRequest()->isFromTrustedProxy()) { + Utils::setProxyVars(true); + } + } +} diff --git a/src/Saml/SamlTokenFactory.php b/src/Saml/SamlTokenFactory.php new file mode 100644 index 0000000000..6be5049e03 --- /dev/null +++ b/src/Saml/SamlTokenFactory.php @@ -0,0 +1,28 @@ +setUser($user); + $token->setAttributes($attributes); + + return $token; + } +} diff --git a/src/Saml/Security/SamlAuthenticationSuccessHandler.php b/src/Saml/Security/SamlAuthenticationSuccessHandler.php new file mode 100644 index 0000000000..0374d76c50 --- /dev/null +++ b/src/Saml/Security/SamlAuthenticationSuccessHandler.php @@ -0,0 +1,30 @@ +options['always_use_default_target_path']) { + return $this->options['default_target_path']; + } + + $relayState = $request->get('RelayState'); + if (null !== $relayState && $relayState !== $this->httpUtils->generateUri($request, $this->options['login_path'])) { + return $relayState; + } + + return parent::determineTargetUrl($request); + } +} diff --git a/src/Saml/Security/SamlFactory.php b/src/Saml/Security/SamlFactory.php new file mode 100644 index 0000000000..70fcb94e0f --- /dev/null +++ b/src/Saml/Security/SamlFactory.php @@ -0,0 +1,77 @@ +addOption('check_path', 'saml_acs'); + $this->addOption('failure_path', 'fos_user_security_login'); + $this->addOption('success_handler', SamlAuthenticationSuccessHandler::class); + $this->defaultFailureHandlerOptions['login_path'] = 'saml_login'; + } + + protected function isRememberMeAware($config) + { + return false; + } + + public function getPosition() + { + return 'pre_auth'; + } + + public function getKey() + { + return 'kimai_saml'; + } + + protected function getListenerId() + { + return 'kimai.saml_listener'; + } + + protected function createAuthProvider(ContainerBuilder $container, $id, $config, $userProviderId) + { + $providerId = 'security.authentication.provider.saml.' . $id; + $definition = $container->setDefinition($providerId, new ChildDefinition(SamlProvider::class)); + $definition->replaceArgument(1, new Reference($userProviderId)); + + return $providerId; + } + + protected function createListener($container, $id, $config, $userProvider) + { + $listenerId = parent::createListener($container, $id, $config, $userProvider); + $this->createLogoutHandler($container, $id, $config); + + return $listenerId; + } + + private function createLogoutHandler(ContainerBuilder $container, $id, $config) + { + if ($container->hasDefinition('security.logout_listener.' . $id)) { + $logoutListener = $container->getDefinition('security.logout_listener.' . $id); + + $container + ->setDefinition(SamlLogoutHandler::class, new ChildDefinition('saml.security.http.logout')) + ->replaceArgument(2, array_intersect_key($config, $this->options)); + $logoutListener->addMethodCall('addHandler', [new Reference(SamlLogoutHandler::class)]); + } + } +} diff --git a/src/Saml/User/SamlUserFactory.php b/src/Saml/User/SamlUserFactory.php new file mode 100644 index 0000000000..4ab1c35487 --- /dev/null +++ b/src/Saml/User/SamlUserFactory.php @@ -0,0 +1,116 @@ +mapping = $attributes['mapping']; + $this->groupAttribute = $attributes['roles']['attribute']; + $this->groupMapping = $attributes['roles']['mapping']; + } + + public function createUser(SamlTokenInterface $token) + { + $user = new User(); + $user->setEnabled(true); + $user->setUsername($token->getUsername()); + + $this->hydrateUser($user, $token); + + return $user; + } + + public function hydrateUser(User $user, SamlTokenInterface $token): void + { + // extract user roles from a special saml attribute + if (!empty($this->groupAttribute) && $token->hasAttribute($this->groupAttribute)) { + $groupMap = []; + foreach ($this->groupMapping as $mapping) { + $field = $mapping['kimai']; + $attribute = $mapping['saml']; + $groupMap[$attribute] = $field; + } + + $roles = []; + $samlGroups = $token->getAttribute($this->groupAttribute); + foreach ($samlGroups as $groupName) { + if (array_key_exists($groupName, $groupMap)) { + $roles[] = $groupMap[$groupName]; + } + } + $user->setRoles($roles); + } + + foreach ($this->mapping as $mapping) { + $field = $mapping['kimai']; + $attribute = $mapping['saml']; + $value = $this->getPropertyValue($token, $attribute); + $setter = 'set' . ucfirst($field); + if (method_exists($user, $setter)) { + $user->$setter($value); + } else { + throw new \RuntimeException('Invalid mapping field given: ' . $field); + } + } + + // fill them after hydrating account, so they can't be overwritten + $user->setUsername($token->getUsername()); + $user->setPassword(''); + $user->setAuth(User::AUTH_SAML); + } + + private function getPropertyValue(SamlTokenInterface $token, $attribute) + { + $results = []; + $attributes = $token->getAttributes(); + + $parts = explode(' ', $attribute); + foreach ($parts as $part) { + if (empty(trim($part))) { + continue; + } + if ($part[0] === '$') { + $key = substr($part, 1); + if (!isset($attributes[$key])) { + throw new \RuntimeException('Missing user attribute: ' . $key); + } + + $results[] = $attributes[$key][0]; + } else { + $results[] = $part; + } + } + + if (!empty($results)) { + return implode(' ', $results); + } + + return $attribute; + } +} diff --git a/src/Security/DoctrineUserProvider.php b/src/Security/DoctrineUserProvider.php new file mode 100644 index 0000000000..852cc4689e --- /dev/null +++ b/src/Security/DoctrineUserProvider.php @@ -0,0 +1,77 @@ +repository = $repository; + } + + /** + * {@inheritdoc} + */ + public function loadUserByUsername($username) + { + $user = null; + + try { + /** @var User $user */ + $user = $this->repository->loadUserByUsername($username); + } catch (\Exception $ex) { + } + + if (null === $user) { + throw new UsernameNotFoundException(sprintf('User "%s" not found.', $username)); + } + + return $user; + } + + /** + * {@inheritdoc} + */ + public function refreshUser(SecurityUserInterface $user) + { + if (!$user instanceof User) { + throw new UnsupportedUserException(sprintf('Expected an instance of %s, but got "%s".', User::class, get_class($user))); + } + + /** @var User $reloadedUser */ + $reloadedUser = $this->repository->getUserById($user->getId()); + + if (null === $reloadedUser) { + throw new UsernameNotFoundException(sprintf('User with ID "%s" could not be reloaded.', $user->getId())); + } + + return $reloadedUser; + } + + /** + * {@inheritdoc} + */ + public function supportsClass($class) + { + return $class === User::class || $class === 'App\Entity\User'; + } +} diff --git a/src/Voter/UserVoter.php b/src/Voter/UserVoter.php index 2714f3856b..596d24dcb0 100644 --- a/src/Voter/UserVoter.php +++ b/src/Voter/UserVoter.php @@ -67,6 +67,10 @@ protected function voteOnAttribute($attribute, $subject, TokenInterface $token) } return $this->hasRolePermission($user, 'delete_user'); + } elseif ($attribute === 'password') { + if (!$subject->isInternalUser()) { + return false; + } } $permission = $attribute; diff --git a/symfony.lock b/symfony.lock index e1011295cd..99c400a48a 100644 --- a/symfony.lock +++ b/symfony.lock @@ -186,6 +186,9 @@ "hoa/zformat": { "version": "1.17.01.10" }, + "hslavich/oneloginsaml-bundle": { + "version": "v1.4.1" + }, "illuminate/cache": { "version": "v6.0.4" }, @@ -273,6 +276,9 @@ "ocramius/proxy-manager": { "version": "2.1.1" }, + "onelogin/php-saml": { + "version": "3.4.1" + }, "pagerfanta/pagerfanta": { "version": "v1.0.5" }, @@ -369,6 +375,9 @@ "ralouphie/getallheaders": { "version": "3.0.3" }, + "robrichards/xmlseclibs": { + "version": "3.0.4" + }, "sebastian/code-unit-reverse-lookup": { "version": "1.0.1" }, diff --git a/templates/bundles/FOSUserBundle/Security/login.html.twig b/templates/bundles/FOSUserBundle/Security/login.html.twig index ab34340c83..4478476fdd 100644 --- a/templates/bundles/FOSUserBundle/Security/login.html.twig +++ b/templates/bundles/FOSUserBundle/Security/login.html.twig @@ -20,4 +20,13 @@ {{ encore_entry_script_tags('app') }} {% set event = trigger(constant('App\\Event\\ThemeEvent::JAVASCRIPT')) %} {{ event.content|raw }} +{% endblock %} + +{% block login_social_auth %} + {% if saml.activate %} + + {{ saml.title|trans }} + +
+ {% endif %} {% endblock %} \ No newline at end of file diff --git a/tests/DependencyInjection/AppExtensionTest.php b/tests/DependencyInjection/AppExtensionTest.php index 46c0115362..8cd6ab17bd 100644 --- a/tests/DependencyInjection/AppExtensionTest.php +++ b/tests/DependencyInjection/AppExtensionTest.php @@ -57,6 +57,9 @@ protected function getMinConfig() 'data_dir' => '/tmp/', 'plugin_dir' => '/tmp/', 'timesheet' => [], + 'saml' => [ + 'connection' => [] + ] ] ]; } diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 29d476f5bd..e90e8b796a 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -181,6 +181,46 @@ public function testValidateLdapAccountFilterFormatInvalidParenthesisCounter() $this->assertConfig($config, []); } + public function testValidateSamlIsMissingMappingForEmail() + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid configuration for path "kimai.saml": You need to configure a SAML mapping for the email attribute.'); + + $config = $this->getMinConfig(); + $config['saml'] = [ + 'activate' => true, + 'mapping' => [], + ]; + + $this->assertConfig($config, []); + } + + public function testValidateSamlDoesNotTriggerOnDeactivatedSaml() + { + $finalizedConfig = $this->getCompiledConfig($this->getMinConfig()); + $config = $this->getMinConfig(); + $config['saml'] = [ + 'activate' => false, + 'mapping' => [], + ]; + + $this->assertConfig($config, $finalizedConfig); + } + + public function testValidateSamlDoesNotTriggerWhenEmailMappingExists() + { + $config = $this->getMinConfig(); + $config['saml'] = [ + 'activate' => true, + 'mapping' => [ + ['saml' => 'email', 'kimai' => 'email'] + ], + ]; + $finalizedConfig = $this->getCompiledConfig($config); + + $this->assertConfig($config, $finalizedConfig); + } + public function testDefaultLdapSettings() { $finalizedConfig = $this->getCompiledConfig($this->getMinConfig()); @@ -349,6 +389,18 @@ public function testFullDefaultConfig() 'userDnAttribute' => 'member', 'groups' => [], ], + ], + 'saml' => [ + 'activate' => false, + 'title' => 'Login with SAML', + 'roles' => [ + 'attribute' => null, + 'mapping' => [] + ], + 'mapping' => [], + 'connection' => [ + 'organization' => [] + ], ] ]; diff --git a/tests/Entity/UserTest.php b/tests/Entity/UserTest.php index 3e3348c411..cb76d7ed55 100644 --- a/tests/Entity/UserTest.php +++ b/tests/Entity/UserTest.php @@ -24,23 +24,54 @@ public function testDefaultValues() { $user = new User(); $this->assertInstanceOf(ArrayCollection::class, $user->getPreferences()); - $this->assertNull($user->getTitle()); - $this->assertNull($user->getDisplayName()); - $this->assertNull($user->getAvatar()); - $this->assertNull($user->getAlias()); - $this->assertNull($user->getId()); - $this->assertNull($user->getApiToken()); - $this->assertNull($user->getPlainApiToken()); - $this->assertEquals(User::DEFAULT_LANGUAGE, $user->getLocale()); + self::assertNull($user->getTitle()); + self::assertNull($user->getDisplayName()); + self::assertNull($user->getAvatar()); + self::assertNull($user->getAlias()); + self::assertNull($user->getId()); + self::assertNull($user->getApiToken()); + self::assertNull($user->getPlainApiToken()); + self::assertEquals(User::DEFAULT_LANGUAGE, $user->getLocale()); $user->setAvatar('https://www.gravatar.com/avatar/00000000000000000000000000000000?d=retro&f=y'); - $this->assertEquals('https://www.gravatar.com/avatar/00000000000000000000000000000000?d=retro&f=y', $user->getAvatar()); + self::assertEquals('https://www.gravatar.com/avatar/00000000000000000000000000000000?d=retro&f=y', $user->getAvatar()); + $user->setApiToken('nbvfdswe34567ujko098765rerfghbgvfcdsert'); - $this->assertEquals('nbvfdswe34567ujko098765rerfghbgvfcdsert', $user->getApiToken()); + self::assertEquals('nbvfdswe34567ujko098765rerfghbgvfcdsert', $user->getApiToken()); + $user->setPlainApiToken('https://www.gravatar.com/avatar/nbvfdswe34567ujko098765rerfghbgvfcdsert'); - $this->assertEquals('https://www.gravatar.com/avatar/nbvfdswe34567ujko098765rerfghbgvfcdsert', $user->getPlainApiToken()); + self::assertEquals('https://www.gravatar.com/avatar/nbvfdswe34567ujko098765rerfghbgvfcdsert', $user->getPlainApiToken()); + $user->setTitle('Mr. Code Blaster'); - $this->assertEquals('Mr. Code Blaster', $user->getTitle()); + self::assertEquals('Mr. Code Blaster', $user->getTitle()); + } + + public function testAuth() + { + $user = new User(); + + self::assertEquals(User::AUTH_INTERNAL, $user->getAuth()); + self::assertFalse($user->isLdapUser()); + self::assertFalse($user->isSamlUser()); + self::assertTrue($user->isInternalUser()); + + $user->setAuth(User::AUTH_LDAP); + self::assertEquals(User::AUTH_LDAP, $user->getAuth()); + self::assertTrue($user->isLdapUser()); + self::assertFalse($user->isSamlUser()); + self::assertFalse($user->isInternalUser()); + + $user->setAuth(User::AUTH_SAML); + self::assertEquals(User::AUTH_SAML, $user->getAuth()); + self::assertFalse($user->isLdapUser()); + self::assertTrue($user->isSamlUser()); + self::assertFalse($user->isInternalUser()); + + $user->setAuth(User::AUTH_INTERNAL); + self::assertEquals(User::AUTH_INTERNAL, $user->getAuth()); + self::assertFalse($user->isLdapUser()); + self::assertFalse($user->isSamlUser()); + self::assertTrue($user->isInternalUser()); } public function testDatetime() @@ -48,30 +79,30 @@ public function testDatetime() $date = new \DateTime('+1 day'); $user = new User(); $user->setRegisteredAt($date); - $this->assertEquals($date, $user->getRegisteredAt()); + self::assertEquals($date, $user->getRegisteredAt()); } public function testPreferences() { $user = new User(); - $this->assertNull($user->getPreference('test')); - $this->assertNull($user->getPreferenceValue('test')); - $this->assertEquals('foo', $user->getPreferenceValue('test', 'foo')); + self::assertNull($user->getPreference('test')); + self::assertNull($user->getPreferenceValue('test')); + self::assertEquals('foo', $user->getPreferenceValue('test', 'foo')); $preference = new UserPreference(); $preference ->setName('test') ->setValue('foobar'); $user->addPreference($preference); - $this->assertEquals('foobar', $user->getPreferenceValue('test', 'foo')); - $this->assertEquals($preference, $user->getPreference('test')); + self::assertEquals('foobar', $user->getPreferenceValue('test', 'foo')); + self::assertEquals($preference, $user->getPreference('test')); $user->setPreferenceValue('test', 'Hello World'); - $this->assertEquals('Hello World', $user->getPreferenceValue('test', 'foo')); + self::assertEquals('Hello World', $user->getPreferenceValue('test', 'foo')); - $this->assertNull($user->getPreferenceValue('test2')); + self::assertNull($user->getPreferenceValue('test2')); $user->setPreferenceValue('test2', 'I like rain'); - $this->assertEquals('I like rain', $user->getPreferenceValue('test2')); + self::assertEquals('I like rain', $user->getPreferenceValue('test2')); } public function testDisplayName() @@ -79,28 +110,28 @@ public function testDisplayName() $user = new User(); $user->setUsername('bar'); - $this->assertEquals('bar', $user->getDisplayName()); - $this->assertEquals('bar', $user->getUsername()); - $this->assertEquals('bar', (string) $user); + self::assertEquals('bar', $user->getDisplayName()); + self::assertEquals('bar', $user->getUsername()); + self::assertEquals('bar', (string) $user); $user->setAlias('foo'); - $this->assertEquals('foo', $user->getAlias()); - $this->assertEquals('bar', $user->getUsername()); - $this->assertEquals('foo', $user->getDisplayName()); - $this->assertEquals('foo', (string) $user); + self::assertEquals('foo', $user->getAlias()); + self::assertEquals('bar', $user->getUsername()); + self::assertEquals('foo', $user->getDisplayName()); + self::assertEquals('foo', (string) $user); } public function testGetLocale() { $sut = new User(); - $this->assertEquals(User::DEFAULT_LANGUAGE, $sut->getLocale()); + self::assertEquals(User::DEFAULT_LANGUAGE, $sut->getLocale()); $language = new UserPreference(); $language->setName(UserPreference::LOCALE); $language->setValue('fr'); $sut->addPreference($language); - $this->assertEquals('fr', $sut->getLocale()); + self::assertEquals('fr', $sut->getLocale()); } public function testTeams() @@ -142,4 +173,25 @@ public function testRoles() $sut->addRole(User::ROLE_TEAMLEAD); self::assertTrue($sut->isTeamlead()); } + + public function testPreferencesCollectionIsCreatedOnBrokenUser() + { + // this code is only used in some rare edge cases, maybe even only in development ... + // lets keep it, as it occured during the work on SAML authentication + $sut = new User(); + + $preference = new UserPreference(); + $preference + ->setName('test') + ->setValue('foobar'); + + $property = new \ReflectionProperty(User::class, 'preferences'); + $property->setAccessible(true); + $property->setValue($sut, null); + + // make sure that addPreference will work, even if the internal collection was set to null + $sut->addPreference($preference); + + self::assertEquals('foobar', $sut->getPreferenceValue('test')); + } } diff --git a/tests/EventSubscriber/ResetPasswordSubscriberTest.php b/tests/EventSubscriber/ResetPasswordSubscriberTest.php new file mode 100644 index 0000000000..6a03f3250c --- /dev/null +++ b/tests/EventSubscriber/ResetPasswordSubscriberTest.php @@ -0,0 +1,89 @@ +assertArrayHasKey(FOSUserEvents::RESETTING_SEND_EMAIL_INITIALIZE, $events); + $methodName = $events[FOSUserEvents::RESETTING_SEND_EMAIL_INITIALIZE][0]; + $this->assertTrue(method_exists(ResetPasswordSubscriber::class, $methodName)); + } + + public function testUnknownUserTypeIsIgnored() + { + $user = new TestUserEntity(); + $user->setUsername('foo@bar'); + + $request = $this->createMock(Request::class); + $event = new GetResponseNullableUserEvent($user, $request); + + $sut = new ResetPasswordSubscriber(); + $sut->onInitializeResetPassword($event); + + self::assertNull($event->getResponse()); + } + + public function testInternalAuthTypeIsIgnored() + { + $user = new User(); + $user->setUsername('foo@bar'); + + $request = $this->createMock(Request::class); + $event = new GetResponseNullableUserEvent($user, $request); + + $sut = new ResetPasswordSubscriber(); + $sut->onInitializeResetPassword($event); + + self::assertNull($event->getResponse()); + } + + /** + * @dataProvider getAuthTypeData + */ + public function testNonInternalAuthTypeThrowsAccessDeniedException(string $authType) + { + $this->expectException(AccessDeniedHttpException::class); + $this->expectExceptionMessage(sprintf('The user "foo@bar" tried to reset the password, but it is registered as "%s" auth-type.', $authType)); + + $user = new User(); + $user->setUsername('foo@bar'); + $user->setAuth($authType); + + $request = $this->createMock(Request::class); + $event = new GetResponseNullableUserEvent($user, $request); + + $sut = new ResetPasswordSubscriber(); + $sut->onInitializeResetPassword($event); + } + + public function getAuthTypeData() + { + return [ + [User::AUTH_SAML], + [User::AUTH_LDAP], + ]; + } +} diff --git a/tests/Ldap/FormLoginLdapFactoryTest.php b/tests/Ldap/FormLoginLdapFactoryTest.php index f4d821c9d1..f0c7e64659 100644 --- a/tests/Ldap/FormLoginLdapFactoryTest.php +++ b/tests/Ldap/FormLoginLdapFactoryTest.php @@ -33,12 +33,12 @@ public function testCreate() $result = $sut->create($container, 'test', ['foo' => 'bar'], 'fosuserbundle', 'secured_area'); self::assertEquals([ - 'kimai_ldap.security.authentication.provider.test', + 'security.authentication.provider.kimai_ldap.test', 'security.authentication.listener.form.test', 'secured_area' ], $result); - $definition = $container->getDefinition('kimai_ldap.security.authentication.provider.test'); + $definition = $container->getDefinition('security.authentication.provider.kimai_ldap.test'); self::assertInstanceOf(ChildDefinition::class, $definition); self::assertEquals('test', $definition->getArguments()['index_1']); diff --git a/tests/Ldap/LdapManagerTest.php b/tests/Ldap/LdapManagerTest.php index c855c2fbef..72cc0ae09d 100644 --- a/tests/Ldap/LdapManagerTest.php +++ b/tests/Ldap/LdapManagerTest.php @@ -315,7 +315,7 @@ public function testUpdateUserOnValidResultWithEmptyRoleBaseDn() $userOrig = clone $user; $sut->updateUser($user); - self::assertEquals($userOrig->setEmail('foobar'), $user); + self::assertEquals($userOrig->setEmail('foobar')->setAuth(User::AUTH_LDAP), $user); self::assertEquals($user->getPreferenceValue('ldap.dn'), 'blub-updated'); } @@ -446,7 +446,7 @@ public function testUpdateUserOnValidResultWithRolesResult(array $expectedUsers, $user = (new User())->setUsername('Karl-Heinz'); $user->setPreferenceValue('ldap.dn', 'blub'); $userOrig = clone $user; - $userOrig->setEmail('Karl-Heinz')->setRoles(['ROLE_TEAMLEAD', 'ROLE_ADMIN']); + $userOrig->setEmail('Karl-Heinz')->setRoles(['ROLE_TEAMLEAD', 'ROLE_ADMIN'])->setAuth(User::AUTH_LDAP); $sut->updateUser($user); self::assertEquals($userOrig, $user); diff --git a/tests/Ldap/LdapUserProviderTest.php b/tests/Ldap/LdapUserProviderTest.php index fd15577398..2e3b99668c 100644 --- a/tests/Ldap/LdapUserProviderTest.php +++ b/tests/Ldap/LdapUserProviderTest.php @@ -10,9 +10,11 @@ namespace App\Tests\Ldap; use App\Entity\User; +use App\Ldap\LdapDriverException; use App\Ldap\LdapManager; use App\Ldap\LdapUserProvider; use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; /** @@ -51,6 +53,7 @@ public function testRefreshUserReturnsUser() $user = new User(); $user->setUsername('foobar'); $user->setPreferenceValue('ldap.dn', 'sdfdsf'); + self::assertFalse($user->isLdapUser()); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['updateUser'])->getMock(); @@ -59,5 +62,36 @@ public function testRefreshUserReturnsUser() self::assertInstanceOf(User::class, $actual); self::assertSame($user, $actual); + self::assertTrue($user->isLdapUser()); + } + + public function testRefreshUserThrowsExceptionOnNonLdapUser() + { + $this->expectException(UnsupportedUserException::class); + $this->expectExceptionMessage('Account "foobar" is not a registered LDAP user.'); + + $user = new User(); + $user->setUsername('foobar'); + + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['updateUser'])->getMock(); + + $sut = new LdapUserProvider($manager); + $actual = $sut->refreshUser($user); + } + + public function testRefreshUserThrowsExceptionOnBrokenUpdateUser() + { + $this->expectException(UnsupportedUserException::class); + $this->expectExceptionMessage('Failed to refresh user "foobar", probably DN is expired.'); + + $user = new User(); + $user->setUsername('foobar'); + $user->setPreferenceValue('ldap.dn', 'sdfdsf'); + + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['updateUser'])->getMock(); + $manager->expects($this->once())->method('updateUser')->willThrowException(new LdapDriverException('blub')); + + $sut = new LdapUserProvider($manager); + $actual = $sut->refreshUser($user); } } diff --git a/tests/Mocks/Saml/SamlAuthFactory.php b/tests/Mocks/Saml/SamlAuthFactory.php new file mode 100644 index 0000000000..8223c71ed3 --- /dev/null +++ b/tests/Mocks/Saml/SamlAuthFactory.php @@ -0,0 +1,84 @@ + [ + 'entityId' => 'https://accounts.google.com/o/saml2?idpid=', + 'singleSignOnService' => [ + 'url' => 'https://accounts.google.com/o/saml2/idp?idpid=', + 'binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect', + ], + 'x509cert' => 'asdf', + ], + 'sp' => [ + 'entityId' => 'https://127.0.0.1:8010/auth/saml/metadata', + 'assertionConsumerService' => [ + 'url' => 'https://127.0.0.1:8010/auth/saml/acs', + 'binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST', + ], + 'singleLogoutService' => [ + 'url' => 'https://127.0.0.1:8010/auth/saml/logout', + 'binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect', + ], + 'privateKey' => '' + ], + 'strict' => true, + 'debug' => true, + 'security' => [ + 'nameIdEncrypted' => false, + 'authnRequestsSigned' => false, + 'logoutRequestSigned' => false, + 'logoutResponseSigned' => false, + 'wantMessagesSigned' => false, + 'wantAssertionsSigned' => false, + 'wantNameIdEncrypted' => false, + 'requestedAuthnContext' => true, + 'signMetadata' => false, + 'wantXMLValidation' => true, + 'signatureAlgorithm' => 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256', + 'digestAlgorithm' => 'http://www.w3.org/2001/04/xmlenc#sha256', + ], + 'contactPerson' => [ + 'technical' => [ + 'givenName' => 'Kimai Admin', + 'emailAddress' => 'kimai-tech@example.com', + ], + 'support' => [ + 'givenName' => 'Kimai Support', + 'emailAddress' => 'kimai-support@example.com', + ] + ], + 'organization' => [ + 'en' => [ + 'name' => 'Kimai', + 'displayname' => 'Kimai', + 'url' => 'https://www.kimai.org', + ] + ] + ]; + } + + $requestStack = new RequestStack(); + $requestStack->push(new Request()); + + return new SamlAuth($requestStack, $connection); + } +} diff --git a/tests/Saml/Controller/SamlControllerTest.php b/tests/Saml/Controller/SamlControllerTest.php new file mode 100644 index 0000000000..6f3b248d86 --- /dev/null +++ b/tests/Saml/Controller/SamlControllerTest.php @@ -0,0 +1,45 @@ +create(); + } + + public function testAssertionConsumerServiceAction() + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('You must configure the check path in your firewall.'); + + $oauth = $this->getAuth(); + $sut = new SamlController($oauth); + $sut->assertionConsumerServiceAction(); + } + + public function testLogoutAction() + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('You must configure the logout path in your firewall.'); + + $oauth = $this->getAuth(); + $sut = new SamlController($oauth); + $sut->logoutAction(); + } +} diff --git a/tests/Saml/Logout/SamlLogoutHandlerTest.php b/tests/Saml/Logout/SamlLogoutHandlerTest.php new file mode 100644 index 0000000000..a4b5cb3d51 --- /dev/null +++ b/tests/Saml/Logout/SamlLogoutHandlerTest.php @@ -0,0 +1,62 @@ +getMockBuilder(SamlAuth::class)->disableOriginalConstructor()->getMock(); + $auth->expects($this->once())->method('processSLO')->willThrowException(new Error('blub')); + $auth->expects($this->once())->method('getSLOurl')->willReturn(''); + + $request = new Request(); + $response = new Response(); + $token = new SamlToken([]); + + $sut = new SamlLogoutHandler($auth); + $sut->logout($request, $response, $token); + } + + public function testLogoutWithLogoutUrl() + { + $auth = $this->getMockBuilder(SamlAuth::class)->disableOriginalConstructor()->getMock(); + $auth->expects($this->once())->method('processSLO')->willThrowException(new Error('blub')); + $auth->expects($this->once())->method('getSLOurl')->willReturn('/logout'); + $auth->expects($this->once())->method('logout')->willReturnCallback(function () { + $args = func_get_args(); + self::assertEquals(null, $args[0]); + self::assertEquals([], $args[1]); + self::assertEquals('tony', $args[2]); + self::assertEquals('foo-bar', $args[3]); + }); + + $request = new Request(); + $response = new Response(); + $token = new SamlToken([]); + $token->setUser((new User())->setUsername('tony')); + $token->setAttribute('sessionIndex', 'foo-bar'); + + $sut = new SamlLogoutHandler($auth); + $sut->logout($request, $response, $token); + } +} diff --git a/tests/Saml/Provider/SamlProviderTest.php b/tests/Saml/Provider/SamlProviderTest.php new file mode 100644 index 0000000000..bf814e1246 --- /dev/null +++ b/tests/Saml/Provider/SamlProviderTest.php @@ -0,0 +1,111 @@ + [ + ['saml' => '$Email', 'kimai' => 'email'], + ['saml' => '$title', 'kimai' => 'title'], + ], + 'roles' => [ + 'attribute' => '', + 'mapping' => [] + ] + ]; + } + + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + if ($loadUser !== false) { + $repository->expects($this->once())->method('loadUserByUsername')->willReturn($loadUser); + } + $userProvider = new ChainUserProvider([new DoctrineUserProvider($repository)]); + $provider = new SamlProvider($repository, $userProvider, new SamlTokenFactory(), new SamlUserFactory($mapping)); + + return $provider; + } + + public function testSupportsToken() + { + $sut = $this->getSamlProvider(); + self::assertFalse($sut->supports(new AnonymousToken('ads', 'ads'))); + self::assertFalse($sut->supports(new UsernamePasswordToken('ads', 'ads', 'asd'))); + self::assertTrue($sut->supports(new SamlToken([]))); + } + + public function testAuthenticateHydratesUser() + { + $user = new User(); + $user->setAuth(User::AUTH_SAML); + + $token = new SamlToken([]); + $token->setUser('foo1@example.com'); + $token->setAttributes([ + 'Email' => ['foo@example.com'], + 'title' => ['Tralalala'], + ]); + self::assertFalse($token->isAuthenticated()); + + $sut = $this->getSamlProvider(null, $user); + $authToken = $sut->authenticate($token); + + self::assertTrue($authToken->isAuthenticated()); + + /** @var User $tokenUser */ + $tokenUser = $authToken->getUser(); + + self::assertSame($user, $tokenUser); + self::assertEquals('foo1@example.com', $tokenUser->getUsername()); + self::assertEquals('Tralalala', $tokenUser->getTitle()); + self::assertEquals('foo@example.com', $tokenUser->getEmail()); + } + + public function testAuthenticatCreatesNewUser() + { + $token = new SamlToken([]); + $token->setUser('foo1@example.com'); + $token->setAttributes([ + 'Email' => ['foo@example.com'], + 'title' => ['Tralalala'], + ]); + self::assertFalse($token->isAuthenticated()); + + $sut = $this->getSamlProvider(null); + $authToken = $sut->authenticate($token); + + self::assertTrue($authToken->isAuthenticated()); + + /** @var User $tokenUser */ + $tokenUser = $authToken->getUser(); + + self::assertEquals('foo1@example.com', $tokenUser->getUsername()); + self::assertEquals('Tralalala', $tokenUser->getTitle()); + self::assertEquals('foo@example.com', $tokenUser->getEmail()); + } +} diff --git a/tests/Saml/SamlTokenFactoryTest.php b/tests/Saml/SamlTokenFactoryTest.php new file mode 100644 index 0000000000..bce068815d --- /dev/null +++ b/tests/Saml/SamlTokenFactoryTest.php @@ -0,0 +1,37 @@ +setUsername('foobar'); + + $factory = new SamlTokenFactory(); + $sut = $factory->createToken($user, ['foo' => 'bar', 'bar' => 'world'], ['ROLE_ADMIN', 'ROLE_TEST']); + + self::assertInstanceOf(SamlToken::class, $sut); + self::assertEquals('bar', $sut->getAttribute('foo')); + self::assertEquals('world', $sut->getAttribute('bar')); + self::assertEquals(['ROLE_ADMIN', 'ROLE_TEST'], $sut->getRoleNames()); + self::assertSame($user, $sut->getUser()); + self::assertEquals('foobar', $sut->getUsername()); + } +} diff --git a/tests/Saml/Security/SamlAuthenticationSuccessHandlerTest.php b/tests/Saml/Security/SamlAuthenticationSuccessHandlerTest.php new file mode 100644 index 0000000000..ebe268ec0a --- /dev/null +++ b/tests/Saml/Security/SamlAuthenticationSuccessHandlerTest.php @@ -0,0 +1,104 @@ +getUrlGenerator()); + $handler = new SamlAuthenticationSuccessHandler($httpUtils, ['always_use_default_target_path' => true]); + $defaultTargetPath = $httpUtils->generateUri($this->getRequest('/sso/login'), $this->getOption($handler, 'default_target_path', '/')); + $response = $handler->onAuthenticationSuccess($this->getRequest('/login', 'http://localhost/relayed'), $this->getSamlToken()); + $this->assertTrue($response->isRedirect($defaultTargetPath)); + } + + public function testRelayState() + { + $handler = new SamlAuthenticationSuccessHandler(new HttpUtils($this->getUrlGenerator()), ['always_use_default_target_path' => false]); + $response = $handler->onAuthenticationSuccess($this->getRequest('/sso/login', 'http://localhost/relayed'), $this->getSamlToken()); + $this->assertTrue($response->isRedirect('http://localhost/relayed')); + } + + public function testWithoutRelayState() + { + $httpUtils = new HttpUtils($this->getUrlGenerator()); + $handler = new SamlAuthenticationSuccessHandler($httpUtils, ['always_use_default_target_path' => false]); + $defaultTargetPath = $httpUtils->generateUri($this->getRequest('/sso/login'), $this->getOption($handler, 'default_target_path', '/')); + $response = $handler->onAuthenticationSuccess($this->getRequest(), $this->getSamlToken()); + $this->assertTrue($response->isRedirect($defaultTargetPath)); + } + + public function testRelayStateLoop() + { + $httpUtils = new HttpUtils($this->getUrlGenerator()); + $handler = new SamlAuthenticationSuccessHandler($httpUtils, ['always_use_default_target_path' => false]); + $loginPath = $httpUtils->generateUri($this->getRequest('/sso/login'), $this->getOption($handler, 'login_path', '/login')); + $response = $handler->onAuthenticationSuccess($this->getRequest($loginPath), $this->getSamlToken()); + $this->assertTrue(!$response->isRedirect($loginPath)); + } + + private function getUrlGenerator() + { + $urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock(); + $urlGenerator + ->expects($this->any()) + ->method('generate') + ->will($this->returnCallback(function ($name) { + return (string) $name; + })) + ; + + return $urlGenerator; + } + + private function getRequest($path = '/', $relayState = null) + { + $params = []; + if (null !== $relayState) { + $params['RelayState'] = $relayState; + } + + return Request::create($path, 'get', $params); + } + + private function getSamlToken() + { + $token = new SamlToken([]); + $token->setAttributes(['foo' => 'bar']); + $token->setUser('admin'); + + return $token; + } + + private function getOption($handler, $name, $default = null) + { + $reflection = new \ReflectionObject($handler); + $options = $reflection->getProperty('options'); + $options->setAccessible(true); + $arr = $options->getValue($handler); + if (!is_array($arr) || !isset($arr[$name])) { + return $default; + } + + return $arr[$name]; + } +} diff --git a/tests/Saml/Security/SamlFactoryTest.php b/tests/Saml/Security/SamlFactoryTest.php new file mode 100644 index 0000000000..b948c643b2 --- /dev/null +++ b/tests/Saml/Security/SamlFactoryTest.php @@ -0,0 +1,45 @@ +getKey()); + self::assertEquals('pre_auth', $sut->getPosition()); + } + + public function testCreate() + { + $container = new ContainerBuilder(); + $sut = new SamlFactory(); + $result = $sut->create($container, 'test', ['foo' => 'bar', 'login_path' => null, 'use_forward' => null], 'fosuserbundle', 'secured_area'); + + self::assertEquals([ + 'security.authentication.provider.saml.test', + 'kimai.saml_listener.test', + 'secured_area' + ], $result); + + $definition = $container->getDefinition('security.authentication.provider.saml.test'); + self::assertInstanceOf(ChildDefinition::class, $definition); + self::assertCount(1, $definition->getArguments()); + } +} diff --git a/tests/Saml/User/SamlUserFactoryTest.php b/tests/Saml/User/SamlUserFactoryTest.php new file mode 100644 index 0000000000..6d0177c098 --- /dev/null +++ b/tests/Saml/User/SamlUserFactoryTest.php @@ -0,0 +1,184 @@ +expectException(\RuntimeException::class); + $this->expectExceptionMessage('Missing user attribute: title'); + + $mapping = [ + 'mapping' => [ + ['saml' => '$Email', 'kimai' => 'email'], + ['saml' => '$title', 'kimai' => 'title'], + ], + 'roles' => [ + 'attribute' => '', + 'mapping' => [] + ] + ]; + + $attributes = [ + 'Email' => ['test@example.com'], + ]; + + $token = new SamlToken(); + $token->setAttributes($attributes); + + $sut = new SamlUserFactory($mapping); + $user = $sut->createUser($token); + } + + public function testCreateUserThrowsExceptionOnMissingAttributeInMultiple() + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Missing user attribute: test'); + + $mapping = [ + 'mapping' => [ + ['saml' => '$Email $test', 'kimai' => 'email'], + ], + 'roles' => [ + 'attribute' => '', + 'mapping' => [] + ] + ]; + + $attributes = [ + 'Email' => ['test@example.com'], + ]; + + $token = new SamlToken(); + $token->setAttributes($attributes); + + $sut = new SamlUserFactory($mapping); + $user = $sut->createUser($token); + } + + public function testCreateUserThrowsExceptionOnInvalidMapping() + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Invalid mapping field given: foo'); + + $mapping = [ + 'mapping' => [ + ['saml' => '$Email', 'kimai' => 'email'], + ['saml' => '$Email', 'kimai' => 'foo'], + ], + 'roles' => [ + 'attribute' => '', + 'mapping' => [] + ] + ]; + + $attributes = [ + 'Email' => ['test@example.com'], + ]; + + $token = new SamlToken(); + $token->setAttributes($attributes); + + $sut = new SamlUserFactory($mapping); + $user = $sut->createUser($token); + } + + public function testCreateUser() + { + $mapping = [ + 'mapping' => [ + ['saml' => '$avatar', 'kimai' => 'avatar'], + ['saml' => '$Email', 'kimai' => 'email'], + ['saml' => 'A static super title', 'kimai' => 'title'], + // double space between "$LastName $FOOO" on purpose!!! + ['saml' => '$FirstName $LastName $FOOO me', 'kimai' => 'alias'], + ], + 'roles' => [ + 'attribute' => 'RoLeS', + 'mapping' => [ + ['saml' => 'fooobar', 'kimai' => 'ROLE_ADMIN'], + ['saml' => 'ROLE_1', 'kimai' => 'ROLE_TEAMLEAD'], + ['saml' => 'ROLE_2', 'kimai' => 'ROLE_2'], + ] + ] + ]; + + $attributes = [ + 'RoLeS' => ['ROLE_1', 'ROLE_2', 'ROLE_3'], + 'Email' => ['test@example.com'], + 'FOOO' => ['test', 'test2'], + 'FirstName' => ['Kevin'], + 'LastName' => ['Papst'], + 'avatar' => ['http://www.example.com/test.jpg'], + ]; + + $token = new SamlToken(); + $token->setUser('foo@example.com'); + $token->setAttributes($attributes); + + $sut = new SamlUserFactory($mapping); + $user = $sut->createUser($token); + + self::assertInstanceOf(User::class, $user); + self::assertTrue($user->isEnabled()); + self::assertEquals('', $user->getPassword()); + self::assertEquals('test@example.com', $user->getEmail()); + self::assertEquals('foo@example.com', $user->getUsername()); + self::assertEquals('A static super title', $user->getTitle()); + self::assertEquals('Kevin Papst test me', $user->getAlias()); + self::assertEquals(['ROLE_TEAMLEAD', 'ROLE_2', 'ROLE_USER'], $user->getRoles()); + } + + public function testCreateUserDoesOverwriteUsername() + { + $mapping = [ + 'mapping' => [ + ['saml' => '$avatar', 'kimai' => 'avatar'], + ['saml' => '$Email', 'kimai' => 'email'], + ['saml' => 'A static super title', 'kimai' => 'title'], + ['saml' => 'Mr. T', 'kimai' => 'username'], + ], + 'roles' => [ + 'attribute' => null, + 'mapping' => [] + ] + ]; + + $attributes = [ + 'Email' => ['test@example.com'], + 'FOOO' => ['test', 'test2'], + 'avatar' => ['http://www.example.com/test.jpg'], + ]; + + $token = new SamlToken(); + $token->setUser('foo@example.com'); + $token->setAttributes($attributes); + + $sut = new SamlUserFactory($mapping); + $user = $sut->createUser($token); + + self::assertInstanceOf(User::class, $user); + self::assertTrue($user->isEnabled()); + self::assertEquals('', $user->getPassword()); + self::assertEquals('test@example.com', $user->getEmail()); + self::assertEquals('foo@example.com', $user->getUsername()); + self::assertEquals('A static super title', $user->getTitle()); + self::assertEquals(['ROLE_USER'], $user->getRoles()); + } +} diff --git a/tests/Security/DoctrineUserProviderTest.php b/tests/Security/DoctrineUserProviderTest.php new file mode 100644 index 0000000000..085d149c10 --- /dev/null +++ b/tests/Security/DoctrineUserProviderTest.php @@ -0,0 +1,126 @@ +expectException(UsernameNotFoundException::class); + $this->expectExceptionMessage('User "test" not found.'); + + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + $repository->expects($this->once())->method('loadUserByUsername')->willReturn(null); + + $sut = new DoctrineUserProvider($repository); + $sut->loadUserByUsername('test'); + } + + public function testLoadUserByUsernameReturnsUser() + { + $user = new User(); + $user->setUsername('foobar'); + + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + $repository->expects($this->once())->method('loadUserByUsername')->willReturn($user); + + $sut = new DoctrineUserProvider($repository); + $actual = $sut->loadUserByUsername('test'); + + self::assertInstanceOf(User::class, $actual); + self::assertSame($user, $actual); + } + + public function testSupportsClass() + { + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + + $sut = new DoctrineUserProvider($repository); + + self::assertTrue($sut->supportsClass(User::class)); + self::assertTrue($sut->supportsClass('App\Entity\User')); + self::assertFalse($sut->supportsClass(UserInterface::class)); + self::assertFalse($sut->supportsClass(SamlUserInterface::class)); + self::assertFalse($sut->supportsClass(\FOS\UserBundle\Model\User::class)); + self::assertFalse($sut->supportsClass(TestUserEntity::class)); + } + + public function testRefreshUserThrowsExceptionOnUnsupportedUserClass() + { + $this->expectException(UnsupportedUserException::class); + $this->expectExceptionMessage('Expected an instance of App\Entity\User, but got "App\Tests\Security\TestUserEntity".'); + + $user = new TestUserEntity(); + $user->setUsername('foobar'); + + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + + $sut = new DoctrineUserProvider($repository); + $actual = $sut->refreshUser($user); + } + + public function testRefreshUserThrowsExceptionOnNonFoundUser() + { + $this->expectException(UsernameNotFoundException::class); + $this->expectExceptionMessage('User with ID "" could not be reloaded'); + + $user = new User(); + $user->setUsername('foobar'); + + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + $repository->expects($this->once())->method('getUserById')->willReturn(null); + + $sut = new DoctrineUserProvider($repository); + $actual = $sut->refreshUser($user); + } + + public function testRefreshUserThrowsNoExceptionOnLdapUser() + { + $user = new User(); + $user->setUsername('foobar'); + $user->setAuth(User::AUTH_LDAP); + + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + $repository->expects($this->once())->method('getUserById')->willReturn($user); + + $sut = new DoctrineUserProvider($repository); + $actual = $sut->refreshUser($user); + + self::assertSame($user, $actual); + } + + public function testRefreshUserReturnsUser() + { + $user = new User(); + $user->setUsername('foobar'); + + $repository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock(); + $repository->expects($this->once())->method('getUserById')->willReturn($user); + + $sut = new DoctrineUserProvider($repository); + $actual = $sut->refreshUser($user); + + self::assertInstanceOf(User::class, $actual); + self::assertSame($user, $actual); + self::assertTrue($user->isInternalUser()); + } +} diff --git a/tests/Security/TestUserEntity.php b/tests/Security/TestUserEntity.php new file mode 100644 index 0000000000..cc3e9baf8f --- /dev/null +++ b/tests/Security/TestUserEntity.php @@ -0,0 +1,16 @@ +setUsername('admin'); + $user->addRole('ROLE_SUPER_ADMIN'); + + $subject = new User(); + $subject->setUsername('foo'); + $subject->addRole('ROLE_USER'); + $subject->setAuth($authType); + + $this->testVote($user, $subject, 'password', $result); + } + + public function getTestDataForAuthType() + { + return [ + [User::AUTH_LDAP, VoterInterface::ACCESS_DENIED], + [User::AUTH_INTERNAL, VoterInterface::ACCESS_GRANTED], + [User::AUTH_SAML, VoterInterface::ACCESS_DENIED], + ]; + } } diff --git a/var/data/kimai_test.sqlite b/var/data/kimai_test.sqlite index a00aaead623186d2f811bee83774ebf6de83adb8..e64ef10fe411eb124be3f99d53fe0b88607475dd 100644 GIT binary patch delta 284 zcmZo@&~IqapCBzbfq{Wx4-hK>aW(@3qxVD|W5x*^6PEo~o5{d8m0>2IIPV-@Ii6KK zmfX*oCvnGdP3H3B?BL|!C}a0zTh693Sy3UEtF=s+U0hX_v0Hy~8K2*FVIIaJCh^45 zk_?3~#~^19#~=+O15E`NS2xE{pUHw8;*6SGcQ7(@Z&u{VX51#g$fCe3%Tq9&-GI@O ziKk$*qJkcenu;v5rm>NMk%57sk*T4Pv89O-h^K2{sB2`ZU}R`zY++?+ynUJtV;(cJ zEbp!9><&QPytjb5XYfvc>&wW+3DYOr4&(wc6A&{4F$)m00x=s9vjZ^)5OV@C*Y>x* H+`>fwC~-{D delta 203 zcmZo@&~IqapCB#R$H2g_2Z)t`IGcfi(QBfPF=OAxgk}HLb};Zw<=)M0%XNs$h4TVu zJSQ*14vrS~o$MuS>)7;I=dk26pJn!%tf?n3-jG-%e+D0E?~Voqo%g nk*oc?FC!2$0WmWWvj8zG5VHX>I}mdKF((jnZU64e%~=Ehlh!{3