From 0c0e9c2f7182473ed895121305253258e8307919 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Fri, 7 Jun 2019 22:48:39 +0200 Subject: [PATCH] LDAP authentication support (#815) --- .editorconfig | 10 +- .github/FUNDING.yml | 3 +- .github/PULL_REQUEST_TEMPLATE.md | 2 +- .scrutinizer.yml | 1 + .travis.yml | 4 +- README.md | 78 ++- composer.json | 32 +- composer.lock | 116 ++++- config/packages/kimai.yaml | 9 + config/packages/security.yaml | 12 +- config/services.yaml | 46 +- src/Configuration/LdapConfiguration.php | 43 ++ src/DependencyInjection/AppExtension.php | 23 +- src/DependencyInjection/Configuration.php | 139 +++++- src/Entity/Activity.php | 64 +-- src/Entity/Configuration.php | 14 +- src/Entity/Customer.php | 238 ++------- src/Entity/InvoiceTemplate.php | 107 +---- src/Entity/Project.php | 88 +--- src/Entity/Tag.php | 22 +- src/Entity/Timesheet.php | 63 +-- src/Entity/User.php | 102 ++-- src/Form/Type/UserRoleType.php | 19 +- src/Kernel.php | 6 + src/Ldap/FormLoginLdapFactory.php | 72 +++ src/Ldap/LdapAuthenticationProvider.php | 131 +++++ src/Ldap/LdapDriver.php | 129 +++++ src/Ldap/LdapDriverException.php | 18 + src/Ldap/LdapManager.php | 165 +++++++ src/Ldap/LdapUserHydrator.php | 144 ++++++ src/Ldap/LdapUserProvider.php | 120 +++++ src/Ldap/SanitizingException.php | 40 ++ src/Ldap/ZendLdap.php | 29 ++ src/Model/InvoiceModel.php | 41 +- src/Model/TimesheetStatistic.php | 27 +- src/Repository/Query/ActivityQuery.php | 20 +- src/Repository/Query/BaseQuery.php | 6 +- src/Repository/Query/ProjectQuery.php | 6 +- src/Repository/Query/TimesheetQuery.php | 12 +- src/Repository/Query/UserQuery.php | 4 +- src/Repository/TimesheetRepository.php | 4 +- src/Security/AclDecisionManager.php | 19 - src/Security/RolePermissionManager.php | 5 - src/Security/RoleService.php | 38 ++ src/Security/UserChecker.php | 18 +- src/Voter/AbstractVoter.php | 10 - src/Voter/UserVoter.php | 2 +- symfony.lock | 18 +- tests/API/APIControllerBaseTest.php | 52 +- tests/API/Model/I18nTest.php | 2 +- tests/Command/InstallCommandTest.php | 3 +- tests/Configuration/LdapConfigurationTest.php | 52 ++ tests/Controller/ActivityControllerTest.php | 3 +- tests/Controller/ControllerBaseTest.php | 30 +- tests/Controller/ProjectControllerTest.php | 3 +- tests/Controller/TimesheetControllerTest.php | 4 +- .../TimesheetTeamControllerTest.php | 4 +- tests/Controller/UserControllerTest.php | 2 +- tests/DataFixtures/TagFixtures.php | 6 +- tests/DataFixtures/TimesheetFixtures.php | 14 +- .../DependencyInjection/AppExtensionTest.php | 316 ++++++++++++ .../DependencyInjection/ConfigurationTest.php | 246 ++++++++++ .../SqliteSessionInitSubscriberTest.php | 2 +- tests/Entity/AbstractEntityTest.php | 4 +- tests/Entity/ActivityTest.php | 2 +- tests/Entity/CustomerTest.php | 8 +- tests/Entity/ProjectTest.php | 16 +- tests/Entity/UserTest.php | 7 + tests/Event/ConfigureAdminMenuEventTest.php | 2 +- tests/Event/DashboardEventTest.php | 2 +- tests/Event/ThemeEventTest.php | 2 +- tests/Event/UserPreferenceEventTest.php | 2 +- .../TagArrayToStringTransformerTest.php | 5 +- ...torTest.php => AbstractCalculatorTest.php} | 0 .../DateNumberGeneratorTest.php | 2 +- .../Invoice/Renderer/AbstractRendererTest.php | 6 +- tests/Invoice/Renderer/DebugRenderer.php | 4 +- tests/Ldap/FormLoginLdapFactoryTest.php | 50 ++ tests/Ldap/LdapAuthenticationProviderTest.php | 238 +++++++++ tests/Ldap/LdapDriverExceptionTest.php | 28 ++ tests/Ldap/LdapDriverTest.php | 71 +++ tests/Ldap/LdapManagerTest.php | 454 ++++++++++++++++++ tests/Ldap/LdapUserHydratorTest.php | 189 ++++++++ tests/Ldap/LdapUserProviderTest.php | 102 ++++ tests/Ldap/SanitizingExceptionTest.php | 31 ++ tests/Ldap/ZendLdapTest.php | 48 ++ tests/Repository/AbstractRepositoryTest.php | 2 +- tests/Repository/TagRepositoryTest.php | 5 + tests/Repository/TimesheetRepositoryTest.php | 13 + tests/Security/RoleServiceTest.php | 34 ++ tests/Security/UserCheckerTest.php | 48 +- tests/Timesheet/Rounding/CeilRoundingTest.php | 2 +- .../Rounding/ClosestRoundingTest.php | 2 +- .../Rounding/DefaultRoundingTest.php | 2 +- .../Timesheet/Rounding/FloorRoundingTest.php | 2 +- tests/Twig/ExtensionsTest.php | 1 - tests/Voter/AbstractVoterTest.php | 12 +- tests/Voter/TimesheetVoterTest.php | 4 +- tests/phpstan.neon | 10 + 99 files changed, 3542 insertions(+), 926 deletions(-) create mode 100644 src/Configuration/LdapConfiguration.php create mode 100644 src/Ldap/FormLoginLdapFactory.php create mode 100644 src/Ldap/LdapAuthenticationProvider.php create mode 100644 src/Ldap/LdapDriver.php create mode 100644 src/Ldap/LdapDriverException.php create mode 100644 src/Ldap/LdapManager.php create mode 100644 src/Ldap/LdapUserHydrator.php create mode 100644 src/Ldap/LdapUserProvider.php create mode 100644 src/Ldap/SanitizingException.php create mode 100644 src/Ldap/ZendLdap.php create mode 100644 src/Security/RoleService.php create mode 100644 tests/Configuration/LdapConfigurationTest.php create mode 100644 tests/DependencyInjection/AppExtensionTest.php create mode 100644 tests/DependencyInjection/ConfigurationTest.php rename tests/Invoice/Calculator/{AbstractInvoiceCalculatorTest.php => AbstractCalculatorTest.php} (100%) create mode 100644 tests/Ldap/FormLoginLdapFactoryTest.php create mode 100644 tests/Ldap/LdapAuthenticationProviderTest.php create mode 100644 tests/Ldap/LdapDriverExceptionTest.php create mode 100644 tests/Ldap/LdapDriverTest.php create mode 100644 tests/Ldap/LdapManagerTest.php create mode 100644 tests/Ldap/LdapUserHydratorTest.php create mode 100644 tests/Ldap/LdapUserProviderTest.php create mode 100644 tests/Ldap/SanitizingExceptionTest.php create mode 100644 tests/Ldap/ZendLdapTest.php create mode 100644 tests/Security/RoleServiceTest.php create mode 100644 tests/phpstan.neon diff --git a/.editorconfig b/.editorconfig index db974a9e..a0e669a2 100644 --- a/.editorconfig +++ b/.editorconfig @@ -7,4 +7,12 @@ end_of_line = LF [*.php] indent_style = space -indent_size = 4 \ No newline at end of file +indent_size = 4 + +[*.yaml] +indent_style = space +indent_size = 4 + +[*.yml] +indent_style = space +indent_size = 4 diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml index bcc929b1..69f417ba 100644 --- a/.github/FUNDING.yml +++ b/.github/FUNDING.yml @@ -1 +1,2 @@ -github: kevinpapst +github: [kevinpapst] +custom: https://www.kimai.org/donate/ diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4ab04b20..ceea6f92 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,6 +7,6 @@ A clear and concise description of what this pull request adds or changes. - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist -- [ ] I verified that my code applies to the guidelines (`composer code-check`) +- [ ] I verified that my code applies to the guidelines (`composer kimai:code-check`) - [ ] I updated the documentation (see [here](https://github.com/kimai/www.kimai.org/tree/master/_documentation)) - [ ] I agree that this code is used in Kimai and will be published under the [MIT license](https://github.com/kevinpapst/kimai2/blob/master/LICENSE) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index f18cd8cd..b16e2c35 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -10,6 +10,7 @@ build: tests: override: - js-scrutinizer-run + - php-scrutinizer-run tests: tests: override: diff --git a/.travis.yml b/.travis.yml index 3a96cd2c..81077278 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,8 +31,8 @@ install: - composer install script: - - composer codestyle - - composer phpstan + - composer kimai:codestyle + - composer kimai:phpstan - if [[ $CODECOVERAGE == 1 ]]; then vendor/bin/phpunit tests/ --coverage-clover=coverage.xml; else vendor/bin/phpunit tests/; fi; - cp tests/.env.dist.sqlite .env - bin/console doctrine:database:create -n diff --git a/README.md b/README.md index ddc4292a..0d9f7a43 100644 --- a/README.md +++ b/README.md @@ -1,72 +1,68 @@ -# Kimai 2 - Time-tracking made easy - -Kimai - the open source time-tracker application with a mobile-first approach (read more at the [official website](https://www.kimai.org)). +# Kimai 2 - online time-tracker [![Latest Stable Version](https://poser.pugx.org/kevinpapst/kimai2/v/stable)](https://packagist.org/packages/kevinpapst/kimai2) [![License](https://poser.pugx.org/kevinpapst/kimai2/license)](https://packagist.org/packages/kevinpapst/kimai2) [![Travis Status](https://travis-ci.org/kevinpapst/kimai2.svg?branch=master)](https://travis-ci.org/kevinpapst/kimai2) [![Code Coverage](https://codecov.io/gh/kevinpapst/kimai2/branch/master/graph/badge.svg)](https://codecov.io/gh/kevinpapst/kimai2) +[![Gitter](https://badges.gitter.im/kimai2/support.svg)](https://gitter.im/kimai2/support) + +Kimai is a free, open source and online time-tracking software designed for small businesses and freelancers. +It is built with modern technologies such as Symfony, Bootstrap, RESTful API, responsive and mobile-ready etc. ## Introduction -This is the _reloaded_ version of the open source timetracker Kimai. -It is built from scratch and doesn't share any source code with its [predecessor](http://www.kimai.org). -But it adapts the same ideas and a clean & simple UI for your time-tracking experience. - -By now it is in an almost-stable development phase, usable and with most advanced features from Kimai 1. -You can even [import your data](https://www.kimai.org/documentation/migration-v1.html) and start testing and using it today. - -Kimai is a [multi-language application](https://www.kimai.org/documentation/translations.html) and already translated to english, german, italian, french, spanish, russian, arabic, hungarian, portuguese (brazilian), swedish and japanese. +- [Home](https://www.kimai.org) - The house of Kimai +- [Blog](https://www.kimai.org/blog/) - Get the latest news +- [Documentation](https://www.kimai.org/documentation/) - Learn how to use +- [Translations](https://www.kimai.org/documentation/translations.html) - Kimai in your language +- [Migration](https://www.kimai.org/documentation/migration-v1.html) - Import data from v1 ### Requirements -- PHP 7.2 or higher (test your system compatibility with the [requirements-checker](http://symfony.com/doc/current/reference/requirements.html)) -- The PHP extensions [xml](http://php.net/manual/en/book.xml.php), [mbstring](http://php.net/manual/en/book.mbstring.php), [gd](http://php.net/manual/en/book.image.php), [intl](https://php.net/manual/en/book.intl.php), [zip](https://php.net/manual/en/book.zip.php) and [PDO](https://php.net/manual/en/book.pdo.php) with either [pdo_sqlite](https://php.net/manual/en/ref.pdo-sqlite.php) or [pdo_mysql](https://php.net/manual/en/ref.pdo-mysql.php) enabled -- If you use MariaDB, make sure its at least v10.2.7 (see [FAQ](https://www.kimai.org/documentation/faq.html)) -- A modern browser, Kimai v2 might be broken on old browsers like IE 10 +- PHP 7.2 or higher +- Database (MySQL, MariaDB, SQLite) +- Webserver (nginx, Apache) +- A modern browser +- [Other libraries](https://www.kimai.org/download/) -## Documentation +### About -Looking for more information about Kimai 2? Check out our detailed [documentation](https://www.kimai.org/documentation/). +This new version of the open source timetracker Kimai. It is in a stable development phase, usable in production and +with most advanced features from Kimai 1 and many new ones: -### Installation +JSON API, invoicing, data exports, multi-timer and punch-in punch-out mode, tagging, multi-user and multi-timezones, +LDAP and built-in authentication, customizable role permissions, responsive and usable on your mobile device, +hourly and fixed rates, advanced filtering, support for plugins and many more. -There are multiple ways to install Kimai, all of them described in the [installation docu](https://www.kimai.org/documentation/installation.html): +## Installation -- [Recommended installation with GIT and Composer](https://www.kimai.org/documentation/installation.html#recommended-setup) -- [Development setup](https://www.kimai.org/documentation/installation.html#development-installation) -- [Docker](https://www.kimai.org/documentation/docker.html) -- [1-click installations](https://www.kimai.org/documentation/installation.html#hosting-and-1-click-installations) -- [FTP](https://www.kimai.org/documentation/installation.html#ftp-installation) +- [Recommended setup](https://www.kimai.org/documentation/installation.html#recommended-setup) - with Git and Composer +- [Docker](https://www.kimai.org/documentation/docker.html) - containerized +- [Development](https://www.kimai.org/documentation/installation.html#development-installation) - on your local machine +- [1-click installer](https://www.kimai.org/documentation/installation.html#hosting-and-1-click-installations) - hosted environments +- [FTP](https://www.kimai.org/documentation/installation.html#ftp-installation) - unfortunately still widely used ### Updating Kimai -Read the following documentations before you start your upgrade: +- [Update Kimai](https://www.kimai.org/documentation/updates.html) - the documentation +- [UPGRADING guide](UPGRADING.md) - version specific steps -- The [update documentation](https://www.kimai.org/documentation/updates.html) -- The version specific [UPGRADING guide](UPGRADING.md) -- The [release information](https://github.com/kevinpapst/kimai2/releases) +### Plugins + +- [Plugin marketplace](https://www.kimai.org/store/) - users find existing plugins here +- [Developer documentation](https://www.kimai.org/documentation/developers.html) - how to create a plugin ## Roadmap and releases You can see a rough development roadmap in the [Milestones](https://github.com/kevinpapst/kimai2/milestones) sections. -It is open for changes and input from the community, your [ideas and questions](https://github.com/kevinpapst/kimai2/issues) are welcome! +It is open for changes and input from the community, your [ideas and questions](https://github.com/kevinpapst/kimai2/issues) are welcome. > Kimai 2 uses a rolling release concept for delivering updates. -> You can upgrade Kimai at any time, you don't need to wait for the next official release. Read the [upgrade docs](UPGRADING.md) first! +> You can upgrade Kimai at any time, you don't need to wait for the next official release. -Release versions will be created on a regular base (approx. 1 per month) and you can use these tags if you are familiar with Git, -but we cannot provide support for any specific version (especially older ones). +Release versions will be created on a regular base (approx. every second month) and you can use these tags if you are familiar with git. Every code change, whether it's a new feature or a bug fix, will be done on the master branch. -I have to do it this way, as I develop Kimai in my free time and want to put most -effort into the software instead of backporting changes for old versions. - -## Plugins for Kimai 2 - -Kimai 2 was built on top of the Symfony framework and can be extended in various ways: - -- Users looking for existing plugins go to our [plugin marketplace](https://www.kimai.org/store/) -- Developer start with the [developer documentation](https://www.kimai.org/documentation/developers.html) +I have to do it this way, as I develop Kimai in my free time and want to put my effort into the software instead of backporting changes for old versions. ## Credits diff --git a/composer.json b/composer.json index b439e27e..06a2849e 100644 --- a/composer.json +++ b/composer.json @@ -13,10 +13,10 @@ "php": "^7.2", "ext-gd": "*", "ext-intl": "*", + "ext-json": "*", "ext-mbstring": "*", "ext-pdo": "*", "ext-zip": "*", - "ext-json": "*", "beberlei/doctrineextensions": "^1.2", "doctrine/doctrine-fixtures-bundle": "^3.0", "erusev/parsedown": "^1.6", @@ -34,6 +34,7 @@ "ocramius/proxy-manager": "2.1.1", "phpoffice/phpspreadsheet": "^1.4", "phpoffice/phpword": "^0.15.0", + "psr/log": "^1.1", "sensio/framework-extra-bundle": "^5.2", "symfony/asset": "^4.0", "symfony/console": "^4.0", @@ -48,7 +49,7 @@ "symfony/monolog-bundle": "^3.1", "symfony/orm-pack": "^1.0", "symfony/profiler-pack": "^1.0", - "symfony/security-bundle": "^4.0", + "symfony/security-bundle": "~4.2.0", "symfony/security-csrf": "^4.0", "symfony/serializer": "^4.0", "symfony/swiftmailer-bundle": "^3.1", @@ -59,13 +60,15 @@ "symfony/webpack-encore-bundle": "^1.5", "symfony/yaml": "^4.0", "twig/extensions": "^1.5", - "white-october/pagerfanta-bundle": "^1.1" + "white-october/pagerfanta-bundle": "^1.1", + "zendframework/zend-ldap": "^2.10" }, "require-dev": { "dama/doctrine-test-bundle": "^5.0", "friendsofphp/php-cs-fixer": "^2.10", "phpstan/phpstan": "^0.11.7", "phpstan/phpstan-doctrine": "^0.11.4", + "phpstan/phpstan-phpunit": "^0.11.2", "phpstan/phpstan-symfony": "^0.11.6", "phpunit/phpunit": "^7.0", "symfony/browser-kit": "^4.0", @@ -110,17 +113,20 @@ "post-update-cmd": [ "@auto-scripts" ], - "code-check": [ - "@codestyle", - "@phpstan", - "@tests" + "kimai:code-check": [ + "@kimai:codestyle", + "@kimai:phpstan", + "@kimai:tests" ], - "tests": "vendor/bin/phpunit tests/", - "tests-unit": "vendor/bin/phpunit --exclude-group integration tests/", - "tests-integration": "vendor/bin/phpunit --group integration tests/", - "phpstan": "vendor/bin/phpstan analyse src --level=3", - "codestyle": "vendor/bin/php-cs-fixer fix --dry-run --verbose --show-progress=none", - "codestyle-fix": "vendor/bin/php-cs-fixer fix" + "kimai:tests": "vendor/bin/phpunit tests/", + "kimai:tests-unit": "vendor/bin/phpunit --exclude-group integration tests/", + "kimai:tests-integration": "vendor/bin/phpunit --group integration tests/", + "kimai:phpstan": [ + "vendor/bin/phpstan analyse src -c phpstan.neon --level=3", + "vendor/bin/phpstan analyse tests -c tests/phpstan.neon --level=4" + ], + "kimai:codestyle": "vendor/bin/php-cs-fixer fix --dry-run --verbose --show-progress=none", + "kimai:codestyle-fix": "vendor/bin/php-cs-fixer fix" }, "conflict": { "symfony/symfony": "*" diff --git a/composer.lock b/composer.lock index 8dd0042e..c6b98f86 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": "a25872c6e7ee3cc2df923ec8a26b8265", + "content-hash": "ed199b91b3e5eb0b9d2a1eb00ea11cba", "packages": [ { "name": "beberlei/DoctrineExtensions", @@ -8618,6 +8618,59 @@ ], "time": "2018-04-25T15:33:34+00:00" }, + { + "name": "zendframework/zend-ldap", + "version": "2.10.0", + "source": { + "type": "git", + "url": "https://github.com/zendframework/zend-ldap.git", + "reference": "b63c7884a08d3a6bda60ebcf7d6238cf8ad89f49" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/zendframework/zend-ldap/zipball/b63c7884a08d3a6bda60ebcf7d6238cf8ad89f49", + "reference": "b63c7884a08d3a6bda60ebcf7d6238cf8ad89f49", + "shasum": "" + }, + "require": { + "ext-ldap": "*", + "php": "^5.6 || ^7.0" + }, + "require-dev": { + "php-mock/php-mock-phpunit": "^1.1.2 || ^2.1.1", + "phpunit/phpunit": "^5.7.27 || ^6.5.8 || ^7.1.2", + "zendframework/zend-coding-standard": "~1.0.0", + "zendframework/zend-config": "^2.5", + "zendframework/zend-eventmanager": "^2.6.3 || ^3.0.1", + "zendframework/zend-stdlib": "^2.7 || ^3.0" + }, + "suggest": { + "zendframework/zend-eventmanager": "Zend\\EventManager component" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.10.x-dev", + "dev-develop": "2.11.x-dev" + } + }, + "autoload": { + "psr-4": { + "Zend\\Ldap\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "description": "Provides support for LDAP operations including but not limited to binding, searching and modifying entries in an LDAP directory", + "keywords": [ + "ZendFramework", + "ldap", + "zf" + ], + "time": "2018-07-05T05:05:12+00:00" + }, { "name": "zircote/swagger-php", "version": "2.0.14", @@ -9910,6 +9963,63 @@ "description": "Doctrine extensions for PHPStan", "time": "2019-05-29T06:01:20+00:00" }, + { + "name": "phpstan/phpstan-phpunit", + "version": "0.11.2", + "source": { + "type": "git", + "url": "https://github.com/phpstan/phpstan-phpunit.git", + "reference": "fbf2ad56c3b13189d29655e226c9b1da47c2fad9" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/phpstan/phpstan-phpunit/zipball/fbf2ad56c3b13189d29655e226c9b1da47c2fad9", + "reference": "fbf2ad56c3b13189d29655e226c9b1da47c2fad9", + "shasum": "" + }, + "require": { + "nikic/php-parser": "^4.0", + "php": "~7.1", + "phpstan/phpdoc-parser": "^0.3", + "phpstan/phpstan": "^0.11.4" + }, + "conflict": { + "phpunit/phpunit": "<7.0" + }, + "require-dev": { + "consistence/coding-standard": "^3.0.1", + "dealerdirect/phpcodesniffer-composer-installer": "^0.4.4", + "jakub-onderka/php-parallel-lint": "^1.0", + "phing/phing": "^2.16.0", + "phpstan/phpstan-strict-rules": "^0.11", + "phpunit/phpunit": "^7.0", + "satooshi/php-coveralls": "^1.0", + "slevomat/coding-standard": "^4.5.2" + }, + "type": "phpstan-extension", + "extra": { + "branch-alias": { + "dev-master": "0.11-dev" + }, + "phpstan": { + "includes": [ + "extension.neon", + "rules.neon" + ] + } + }, + "autoload": { + "psr-4": { + "PHPStan\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "PHPUnit extensions and rules for PHPStan", + "time": "2019-05-17T17:50:16+00:00" + }, { "name": "phpstan/phpstan-symfony", "version": "0.11.6", @@ -11206,10 +11316,10 @@ "php": "^7.2", "ext-gd": "*", "ext-intl": "*", + "ext-json": "*", "ext-mbstring": "*", "ext-pdo": "*", - "ext-zip": "*", - "ext-json": "*" + "ext-zip": "*" }, "platform-dev": [] } diff --git a/config/packages/kimai.yaml b/config/packages/kimai.yaml index c798b050..4357a3d2 100644 --- a/config/packages/kimai.yaml +++ b/config/packages/kimai.yaml @@ -314,3 +314,12 @@ kimai: data_dir: '%kernel.project_dir%/var/data' plugin_dir: '%kernel.project_dir%/var/plugins' # -------------------------------------------------------------------------------- + + +# -------------------------------------------------------------------------------- +# LDAP +# Connect to your companies directory server, see https://www.kimai.org/documentation/ldap.html +# -------------------------------------------------------------------------------- +# ldap: +# active: true +# -------------------------------------------------------------------------------- diff --git a/config/packages/security.yaml b/config/packages/security.yaml index d4b2f9d5..09f80d42 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -3,6 +3,11 @@ security: App\Entity\User: bcrypt providers: + chain_provider: + chain: + providers: [fos_userbundle, kimai_ldap] + kimai_ldap: + id: App\Ldap\LdapUserProvider fos_userbundle: id: fos_user.user_provider.username_email @@ -26,8 +31,13 @@ security: lifetime: 604800 path: / + # activate the LdapAuthenticationProvider + kimai_ldap: ~ + + # activate all configured user provider + provider: chain_provider + form_login: - provider: fos_userbundle check_path: fos_user_security_check login_path: fos_user_security_login csrf_token_generator: security.csrf.token_manager diff --git a/config/services.yaml b/config/services.yaml index 412d4652..571f4eab 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -13,7 +13,7 @@ services: # fetching services directly from the container via $container->get() won't work. # The best practice is to be explicit about your dependencies anyway. bind: - $projectDirectory: "%kernel.project_dir%" + $projectDirectory: '%kernel.project_dir%' # makes classes in src/ available to be used as services # this creates a service per class whose id is the fully-qualified class name @@ -36,34 +36,37 @@ services: App\Repository\WidgetRepository: arguments: - $widgets: "%kimai.widgets%" + $widgets: '%kimai.widgets%' App\Controller\DashboardController: arguments: - $dashboard: "%kimai.dashboard%" + $dashboard: '%kimai.dashboard%' App\Configuration\LanguageFormattings: - arguments: ["%kimai.languages%"] + arguments: ['%kimai.languages%'] App\Configuration\TimesheetConfiguration: arguments: - $settings: "%kimai.timesheet%" + $settings: '%kimai.timesheet%' App\Configuration\CalendarConfiguration: arguments: - $settings: "%kimai.calendar%" + $settings: '%kimai.calendar%' App\Configuration\FormConfiguration: arguments: - $settings: "%kimai.defaults%" + $settings: '%kimai.defaults%' App\Configuration\SystemConfiguration: arguments: - $settings: "%kimai.config%" + $settings: '%kimai.config%' App\Configuration\ThemeConfiguration: arguments: - $settings: "%kimai.theme%" + $settings: '%kimai.theme%' + + App\Configuration\LdapConfiguration: + arguments: ['%kimai.ldap%'] App\Utils\MPdfConverter: arguments: ['%kernel.cache_dir%'] @@ -95,11 +98,8 @@ services: # FORMS # ================================================================================ - App\Form\Type\UserRoleType: - arguments: ["%security.role_hierarchy.roles%"] - App\Form\Type\LanguageType: - arguments: ["%app_locales%"] + arguments: ['%app_locales%'] # ================================================================================ # THEME @@ -113,17 +113,31 @@ services: # ================================================================================ App\Timesheet\Calculator\DurationCalculator: - arguments: ["%kimai.timesheet.rounding%"] + arguments: ['%kimai.timesheet.rounding%'] App\Timesheet\Calculator\RateCalculator: - arguments: ["%kimai.timesheet.rates%"] + arguments: ['%kimai.timesheet.rates%'] # ================================================================================ # SECURITY & VOTER # ================================================================================ + App\Security\RoleService: + arguments: ['%security.role_hierarchy.roles%'] + App\Security\RolePermissionManager: - arguments: ["%kimai.permissions%"] + arguments: ['%kimai.permissions%'] + + # ================================================================================ + # LDAP + # ================================================================================ + + Zend\Ldap\Ldap: + class: App\Ldap\ZendLdap + + kimai_ldap.security.authentication.provider: + class: App\Ldap\LdapAuthenticationProvider + arguments: ['@App\Security\UserChecker', '', '', '', '@App\Configuration\LdapConfiguration', '%security.authentication.hide_user_not_found%'] # ================================================================================ # REPOSITORIES diff --git a/src/Configuration/LdapConfiguration.php b/src/Configuration/LdapConfiguration.php new file mode 100644 index 00000000..124e88f8 --- /dev/null +++ b/src/Configuration/LdapConfiguration.php @@ -0,0 +1,43 @@ +settings = $settings; + } + + public function isActivated(): bool + { + return (bool) $this->settings['active']; + } + + public function getRoleParameters(): array + { + return (array) $this->settings['role']; + } + + public function getUserParameters(): array + { + return (array) $this->settings['user']; + } + + public function getConnectionParameters(): array + { + return (array) $this->settings['connection']; + } +} diff --git a/src/DependencyInjection/AppExtension.php b/src/DependencyInjection/AppExtension.php index 34280e27..757af2af 100644 --- a/src/DependencyInjection/AppExtension.php +++ b/src/DependencyInjection/AppExtension.php @@ -29,12 +29,12 @@ class AppExtension extends Extension $config = $this->processConfiguration($configuration, $configs); } catch (InvalidConfigurationException $e) { trigger_error('Found invalid "kimai" configuration: ' . $e->getMessage()); - $config = []; + throw $e; } // @deprecated since 0.9, duration_only will be removed with 1.0 if (isset($config['timesheet']['duration_only'])) { - trigger_error('Configuration "kimai.timesheet.duration_only" is deprecated, please remove it', E_USER_DEPRECATED); + @trigger_error('Configuration "kimai.timesheet.duration_only" is deprecated, please remove it', E_USER_DEPRECATED); if (true === $config['timesheet']['duration_only'] && 'duration_only' !== $config['timesheet']['mode']) { trigger_error('Found ambiguous configuration. Please remove "kimai.timesheet.duration_only" and set "kimai.timesheet.mode" instead.'); } @@ -60,6 +60,25 @@ class AppExtension extends Extension $container->setParameter('kimai.timesheet', $config['timesheet']); $container->setParameter('kimai.timesheet.rates', $config['timesheet']['rates']); $container->setParameter('kimai.timesheet.rounding', $config['timesheet']['rounding']); + + $this->setLdapParameter($config['ldap'], $container); + } + + protected function setLdapParameter(array $config, ContainerBuilder $container) + { + if (!isset($config['connection']['baseDn'])) { + $config['connection']['baseDn'] = $config['user']['baseDn']; + } + + if (empty($config['connection']['accountFilterFormat']) && $config['connection']['bindRequiresDn']) { + $filter = ''; + if (!empty($config['user']['filter'])) { + $filter = $config['user']['filter']; + } + $config['connection']['accountFilterFormat'] = '(&' . $filter . '(' . $config['user']['usernameAttribute'] . '=%s))'; + } + + $container->setParameter('kimai.ldap', $config); } /** diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index ea25790d..fe193d16 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -62,6 +62,7 @@ class Configuration implements ConfigurationInterface ->append($this->getWidgetsNode()) ->append($this->getDefaultsNode()) ->append($this->getPermissionsNode()) + ->append($this->getLdapNode()) ->end() ->end(); @@ -100,7 +101,7 @@ class Configuration implements ConfigurationInterface ->requiresAtLeastOneElement() ->useAttributeAsKey('key') ->isRequired() - ->prototype('scalar')->end() + ->scalarPrototype()->end() ->defaultValue([]) ->end() ->integerNode('begin') @@ -141,7 +142,7 @@ class Configuration implements ConfigurationInterface ->requiresAtLeastOneElement() ->useAttributeAsKey('key') ->isRequired() - ->prototype('scalar')->end() + ->scalarPrototype()->end() ->defaultValue([]) ->end() ->floatNode('factor') @@ -258,7 +259,7 @@ class Configuration implements ConfigurationInterface ->children() ->arrayNode('days') ->requiresAtLeastOneElement() - ->prototype('integer')->end() + ->integerPrototype()->end() ->defaultValue([1, 2, 3, 4, 5]) ->end() ->scalarNode('begin')->defaultValue('08:00')->end() @@ -449,7 +450,7 @@ class Configuration implements ConfigurationInterface ->arrayPrototype() ->useAttributeAsKey('key') ->isRequired() - ->prototype('scalar')->end() + ->scalarPrototype()->end() ->defaultValue([]) ->end() ->end() @@ -459,7 +460,7 @@ class Configuration implements ConfigurationInterface ->arrayPrototype() ->useAttributeAsKey('key') ->isRequired() - ->prototype('scalar')->end() + ->scalarPrototype()->end() ->defaultValue([]) ->end() ->end() @@ -469,7 +470,7 @@ class Configuration implements ConfigurationInterface ->arrayPrototype() ->useAttributeAsKey('key') ->isRequired() - ->prototype('scalar')->end() + ->scalarPrototype()->end() ->defaultValue([]) ->end() ->defaultValue([ @@ -484,4 +485,130 @@ class Configuration implements ConfigurationInterface return $node; } + + protected function getLdapNode() + { + $treeBuilder = new TreeBuilder('ldap'); + $node = $treeBuilder->getRootNode(); + + $node + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('active')->defaultFalse()->end() + ->arrayNode('connection') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('host')->defaultNull()->end() + ->scalarNode('port')->defaultValue(389)->end() + ->scalarNode('useStartTls')->defaultFalse()->end() + ->scalarNode('useSsl')->defaultFalse()->end() + ->scalarNode('username')->end() + ->scalarNode('password')->end() + ->scalarNode('bindRequiresDn')->defaultTrue()->end() + ->scalarNode('baseDn')->end() + ->scalarNode('accountCanonicalForm')->end() + ->scalarNode('accountDomainName')->end() + ->scalarNode('accountDomainNameShort')->end() + ->scalarNode('accountFilterFormat') + ->defaultNull() + ->validate() + ->ifTrue(static function ($v) { + if (empty($v)) { + return false; + } + if ($v[0] !== '(' || (substr_count($v, '(') !== substr_count($v, ')'))) { + return true; + } + + return (substr_count($v, '%s') !== 1); + }) + ->thenInvalid('The accountFilterFormat must be enclosed by a matching number of parentheses "()" and contain one "%%s" replacer for the username') + ->end() + ->end() + ->scalarNode('allowEmptyPassword')->end() + ->scalarNode('optReferrals')->end() + ->scalarNode('tryUsernameSplit')->end() + ->scalarNode('networkTimeout')->end() + ->end() + ->validate() + ->ifTrue(static function ($v) { + return $v['useSsl'] && $v['useStartTls']; + }) + ->thenInvalid('The ldap.connection.useSsl and ldap.connection.useStartTls options are mutually exclusive.') + ->end() + ->end() + ->arrayNode('user') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('baseDn')->defaultNull()->end() + ->scalarNode('filter') + ->defaultValue('') + ->validate() + ->ifTrue(static function ($v) { + if (empty($v)) { + return false; + } + if ($v[0] !== '(' || (substr_count($v, '(') !== substr_count($v, ')'))) { + return true; + } + + return (stripos($v, '%s') !== false); + }) + ->thenInvalid('The ldap.user.filter must be enclosed by a matching number of parentheses "()" and must NOT contain a "%%s" replacer') + ->end() + ->end() + ->scalarNode('usernameAttribute')->defaultValue('uid')->end() + ->arrayNode('attributes') + ->defaultValue([]) + ->arrayPrototype() + ->children() + ->scalarNode('ldap_attr')->isRequired()->cannotBeEmpty()->end() + ->scalarNode('user_method')->isRequired()->cannotBeEmpty()->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->arrayNode('role') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('baseDn')->defaultNull()->end() + ->scalarNode('filter')->end() + ->scalarNode('usernameAttribute')->defaultValue('dn')->end() + ->scalarNode('nameAttribute')->defaultValue('cn')->end() + ->scalarNode('userDnAttribute')->defaultValue('member')->end() + ->arrayNode('groups') + ->defaultValue([]) + ->arrayPrototype() + ->children() + ->scalarNode('ldap_value')->isRequired()->cannotBeEmpty()->end() + ->scalarNode('role')->isRequired()->cannotBeEmpty()->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->validate() + ->ifTrue(static function ($v) { + return $v['active'] && !extension_loaded('ldap'); + }) + ->thenInvalid('LDAP is activated, but the LDAP PHP extension is not loaded.') + ->end() + ->validate() + ->ifTrue(static function ($v) { + return $v['active'] && empty($v['connection']['host']); + }) + ->thenInvalid('The "ldap.connection.host" config must be set if LDAP is activated.') + ->end() + ->validate() + ->ifTrue(static function ($v) { + return $v['active'] && empty($v['user']['baseDn']); + }) + ->thenInvalid('The "ldap.user.baseDn" config must be set if LDAP is activated.') + ->end() + ; + + return $node; + } } diff --git a/src/Entity/Activity.php b/src/Entity/Activity.php index 41412e98..0b6ca670 100644 --- a/src/Entity/Activity.php +++ b/src/Entity/Activity.php @@ -9,6 +9,8 @@ namespace App\Entity; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Validator\Constraints as Assert; @@ -28,7 +30,7 @@ class Activity private $id; /** - * @var Project + * @var Project|null * * @ORM\ManyToOne(targetEntity="App\Entity\Project", inversedBy="activities") * @ORM\JoinColumn(onDelete="CASCADE") @@ -60,7 +62,7 @@ class Activity private $visible = true; /** - * @var Timesheet[] + * @var Timesheet[]|ArrayCollection * * @ORM\OneToMany(targetEntity="App\Entity\Timesheet", mappedBy="activity") */ @@ -70,94 +72,68 @@ class Activity use RatesTrait; use ColorTrait; - /** - * @return int - */ - public function getId() + public function __construct() + { + $this->timesheets = new ArrayCollection(); + } + + public function getId(): ?int { return $this->id; } /** - * @return Timesheet[] + * @return Collection */ - public function getTimesheets(): array + public function getTimesheets(): Collection { return $this->timesheets; } - /** - * @return Project - */ - public function getProject() + public function getProject(): ?Project { return $this->project; } - /** - * @param Project $project - * @return Activity - */ - public function setProject($project) + public function setProject(?Project $project): Activity { $this->project = $project; return $this; } - /** - * @param string $name - * @return Activity - */ - public function setName($name) + public function setName(string $name): Activity { $this->name = $name; return $this; } - /** - * @return string - */ - public function getName() + public function getName(): ?string { return $this->name; } - /** - * @param string $comment - * @return Activity - */ - public function setComment($comment) + public function setComment(?string $comment): Activity { $this->comment = $comment; return $this; } - /** - * @return string - */ - public function getComment() + public function getComment(): ?string { return $this->comment; } - /** - * @param bool $visible - * @return Activity - */ - public function setVisible($visible) + public function setVisible(bool $visible): Activity { $this->visible = $visible; return $this; } - /** - * @return bool - */ - public function getVisible() + public function getVisible(): bool { return $this->visible; } diff --git a/src/Entity/Configuration.php b/src/Entity/Configuration.php index d7f5109a..6f559dbf 100644 --- a/src/Entity/Configuration.php +++ b/src/Entity/Configuration.php @@ -48,10 +48,7 @@ class Configuration */ private $value; - /** - * @return int - */ - public function getId() + public function getId(): ?int { return $this->id; } @@ -76,14 +73,17 @@ class Configuration } /** - * Given $value will not be serialized before its stored, so it should be a scalar type. + * Given $value will not be serialized before its stored, so it should be a scalar type + * that can be casted to string. * - * @param mixed $value + * @param string|null|int|bool $value * @return Configuration */ public function setValue($value): Configuration { - $this->value = $value; + if (null !== $value) { + $this->value = (string) $value; + } return $this; } diff --git a/src/Entity/Customer.php b/src/Entity/Customer.php index b76d27f2..3fe37ea7 100644 --- a/src/Entity/Customer.php +++ b/src/Entity/Customer.php @@ -9,6 +9,8 @@ namespace App\Entity; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Validator\Constraints as Assert; @@ -53,7 +55,7 @@ class Customer private $comment; /** - * @var Project[] + * @var Project[]|ArrayCollection * * @ORM\OneToMany(targetEntity="App\Entity\Project", mappedBy="customer") */ @@ -151,362 +153,200 @@ class Customer use RatesTrait; use ColorTrait; - /** - * @return int - */ - public function getId() + public function __construct() + { + $this->projects = new ArrayCollection(); + } + + public function getId(): ?int { return $this->id; } - /** - * Set name - * - * @param string $name - * @return Customer - */ - public function setName($name) + public function setName(string $name): Customer { $this->name = $name; return $this; } - /** - * Get name - * - * @return string - */ public function getName(): ?string { return $this->name; } - /** - * @param string $number - * @return Customer - */ - public function setNumber(string $number) + public function setNumber(?string $number): Customer { $this->number = $number; return $this; } - /** - * @return string - */ public function getNumber(): ?string { return $this->number; } - /** - * Set comment - * - * @param string $comment - * @return Customer - */ - public function setComment($comment) + public function setComment(?string $comment): Customer { $this->comment = $comment; return $this; } - /** - * Get comment - * - * @return string - */ - public function getComment() + public function getComment(): ?string { return $this->comment; } - /** - * Set visible - * - * @param bool $visible - * @return Customer - */ - public function setVisible($visible) + public function setVisible(bool $visible): Customer { $this->visible = $visible; return $this; } - /** - * Get visible - * - * @return bool - */ - public function getVisible() + public function getVisible(): bool { return $this->visible; } - /** - * Set company - * - * @param string $company - * @return Customer - */ - public function setCompany($company) + public function setCompany(?string $company): Customer { $this->company = $company; return $this; } - /** - * Get company - * - * @return string - */ - public function getCompany() + public function getCompany(): ?string { return $this->company; } - /** - * Set contact - * - * @param string $contact - * @return Customer - */ - public function setContact($contact) + public function setContact(?string $contact): Customer { $this->contact = $contact; return $this; } - /** - * Get contact - * - * @return string - */ - public function getContact() + public function getContact(): ?string { return $this->contact; } - /** - * @param string $address - * @return Customer - */ - public function setAddress($address) + public function setAddress(?string $address): Customer { $this->address = $address; return $this; } - /** - * @return string - */ - public function getAddress() + public function getAddress(): ?string { return $this->address; } - /** - * Set country - * - * @param string $country - * @return Customer - */ - public function setCountry($country) + public function setCountry(string $country): Customer { $this->country = $country; return $this; } - /** - * Get country - * - * @return string - */ - public function getCountry() + public function getCountry(): ?string { return $this->country; } - /** - * @param string $currency - * @return Customer - */ - public function setCurrency($currency) + public function setCurrency(string $currency): Customer { $this->currency = $currency; return $this; } - /** - * @return string - */ - public function getCurrency() + public function getCurrency(): string { return $this->currency; } - /** - * Set phone - * - * @param string $phone - * @return Customer - */ - public function setPhone($phone) + public function setPhone(?string $phone): Customer { $this->phone = $phone; return $this; } - /** - * Get phone - * - * @return string - */ - public function getPhone() + public function getPhone(): ?string { return $this->phone; } - /** - * Set fax - * - * @param string $fax - * @return Customer - */ - public function setFax($fax) + public function setFax(?string $fax): Customer { $this->fax = $fax; return $this; } - /** - * Get fax - * - * @return string - */ - public function getFax() + public function getFax(): ?string { return $this->fax; } - /** - * Set mobile - * - * @param string $mobile - * @return Customer - */ - public function setMobile($mobile) + public function setMobile(?string $mobile): Customer { $this->mobile = $mobile; return $this; } - /** - * Get mobile - * - * @return string - */ - public function getMobile() + public function getMobile(): ?string { return $this->mobile; } - /** - * Set mail - * - * @param string $mail - * @return Customer - */ - public function setEmail($mail) + public function setEmail(?string $mail): Customer { $this->email = $mail; return $this; } - /** - * Get mail - * - * @return string - */ - public function getEmail() + public function getEmail(): ?string { return $this->email; } - /** - * Set homepage - * - * @param string $homepage - * @return Customer - */ - public function setHomepage($homepage) + public function setHomepage(?string $homepage): Customer { $this->homepage = $homepage; return $this; } - /** - * Get homepage - * - * @return string - */ - public function getHomepage() + public function getHomepage(): ?string { return $this->homepage; } - /** - * Set timezone - * - * @param string $timezone - * @return Customer - */ - public function setTimezone($timezone) + public function setTimezone(string $timezone): Customer { $this->timezone = $timezone; return $this; } - /** - * Get timezone - * - * @return string - */ - public function getTimezone() + public function getTimezone(): ?string { return $this->timezone; } /** - * @param Project[] $projects - * @return Customer + * @return Collection */ - public function setProjects($projects) - { - $this->projects = $projects; - - return $this; - } - - /** - * @return Project[] - */ - public function getProjects() + public function getProjects(): Collection { return $this->projects; } diff --git a/src/Entity/InvoiceTemplate.php b/src/Entity/InvoiceTemplate.php index 4685e8ed..cd30f04c 100644 --- a/src/Entity/InvoiceTemplate.php +++ b/src/Entity/InvoiceTemplate.php @@ -109,202 +109,125 @@ class InvoiceTemplate */ private $paymentTerms; - /** - * @return int - */ - public function getId() + public function getId(): ?int { return $this->id; } - /** - * Set name - * - * @param string $name - * @return $this - */ - public function setName($name) + public function setName(string $name): InvoiceTemplate { $this->name = $name; return $this; } - /** - * Get name - * - * @return string - */ - public function getName() + public function getName(): ?string { return $this->name; } - /** - * @return string - */ public function getTitle(): ?string { return $this->title; } - /** - * @param string $title - * @return InvoiceTemplate - */ - public function setTitle(string $title) + public function setTitle(string $title): InvoiceTemplate { $this->title = $title; return $this; } - /** - * @return string - */ public function getAddress(): ?string { return $this->address; } - /** - * @param string $address - * @return InvoiceTemplate - */ - public function setAddress($address) + public function setAddress(?string $address): InvoiceTemplate { $this->address = $address; return $this; } - /** - * @return string - */ - public function getNumberGenerator(): ?string + public function getNumberGenerator(): string { return $this->numberGenerator; } - /** - * @param string $numberGenerator - * @return InvoiceTemplate - */ - public function setNumberGenerator(string $numberGenerator) + public function setNumberGenerator(string $numberGenerator): InvoiceTemplate { $this->numberGenerator = $numberGenerator; return $this; } - /** - * @return int - */ - public function getDueDays(): ?int + public function getDueDays(): int { return $this->dueDays; } - /** - * @param int $dueDays - * @return InvoiceTemplate - */ - public function setDueDays(int $dueDays) + public function setDueDays(int $dueDays): InvoiceTemplate { $this->dueDays = $dueDays; return $this; } - /** - * @return float - */ - public function getVat(): ?float + public function getVat(): float { return $this->vat; } - /** - * @param float $vat - * @return InvoiceTemplate - */ - public function setVat(float $vat) + public function setVat(float $vat): InvoiceTemplate { $this->vat = $vat; return $this; } - /** - * @return string - */ public function getCompany(): ?string { return $this->company; } - /** - * @param string $company - * @return InvoiceTemplate - */ - public function setCompany(string $company) + public function setCompany(string $company): InvoiceTemplate { $this->company = $company; return $this; } - /** - * @return string - */ public function getRenderer(): string { return $this->renderer; } - /** - * @param string $renderer - * @return InvoiceTemplate - */ - public function setRenderer(string $renderer) + public function setRenderer(string $renderer): InvoiceTemplate { $this->renderer = $renderer; return $this; } - /** - * @return string - */ public function getCalculator(): string { return $this->calculator; } - /** - * @param string $calculator - * @return InvoiceTemplate - */ - public function setCalculator(string $calculator) + public function setCalculator(string $calculator): InvoiceTemplate { $this->calculator = $calculator; return $this; } - /** - * @return string - */ public function getPaymentTerms(): ?string { return $this->paymentTerms; } - /** - * @param string $paymentTerms - * @return InvoiceTemplate - */ - public function setPaymentTerms(?string $paymentTerms) + public function setPaymentTerms(?string $paymentTerms): InvoiceTemplate { $this->paymentTerms = $paymentTerms; diff --git a/src/Entity/Project.php b/src/Entity/Project.php index 0c20d2fa..ee9aa139 100644 --- a/src/Entity/Project.php +++ b/src/Entity/Project.php @@ -9,6 +9,8 @@ namespace App\Entity; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Validator\Constraints as Assert; @@ -77,7 +79,7 @@ class Project private $budget = 0.00; /** - * @var Activity[] + * @var Activity[]|ArrayCollection * * @ORM\OneToMany(targetEntity="App\Entity\Activity", mappedBy="project") */ @@ -88,54 +90,43 @@ class Project use ColorTrait; /** - * @var Timesheet[] + * @var Timesheet[]|ArrayCollection * * @ORM\OneToMany(targetEntity="App\Entity\Timesheet", mappedBy="project") */ private $timesheets; - /** - * @return int - */ - public function getId() + public function __construct() + { + $this->activities = new ArrayCollection(); + $this->timesheets = new ArrayCollection(); + } + + public function getId(): ?int { return $this->id; } - /** - * @return Customer - */ - public function getCustomer() + public function getCustomer(): ?Customer { return $this->customer; } - /** - * @param Customer $customer - * @return Project - */ - public function setCustomer($customer) + public function setCustomer(Customer $customer): Project { $this->customer = $customer; return $this; } - /** - * @param string $name - * @return Project - */ - public function setName($name) + public function setName(string $name): Project { $this->name = $name; return $this; } - /** - * @return string - */ - public function getName() + public function getName(): ?string { return $this->name; } @@ -144,26 +135,19 @@ class Project * @param string $comment * @return Project */ - public function setComment($comment) + public function setComment($comment): Project { $this->comment = $comment; return $this; } - /** - * @return string - */ - public function getComment() + public function getComment(): ?string { return $this->comment; } - /** - * @param bool $visible - * @return Project - */ - public function setVisible($visible) + public function setVisible(bool $visible): Project { $this->visible = $visible; @@ -173,7 +157,7 @@ class Project /** * @return bool */ - public function getVisible() + public function getVisible(): bool { return $this->visible; } @@ -182,7 +166,7 @@ class Project * @param float $budget * @return Project */ - public function setBudget($budget) + public function setBudget($budget): Project { $this->budget = $budget; @@ -198,39 +182,17 @@ class Project } /** - * @param Timesheet[] $timesheets - * @return Project + * @return Collection */ - public function setTimesheets($timesheets) - { - $this->timesheets = $timesheets; - - return $this; - } - - /** - * @return Timesheet[] - */ - public function getTimesheets() + public function getTimesheets(): Collection { return $this->timesheets; } /** - * @param Activity[] $activities - * @return Project + * @return Collection */ - public function setActivities($activities) - { - $this->activities = $activities; - - return $this; - } - - /** - * @return Activity[] - */ - public function getActivities() + public function getActivities(): Collection { return $this->activities; } @@ -247,7 +209,7 @@ class Project * @param string $orderNumber * @return Project */ - public function setOrderNumber($orderNumber) + public function setOrderNumber($orderNumber): Project { $this->orderNumber = $orderNumber; diff --git a/src/Entity/Tag.php b/src/Entity/Tag.php index 2cf3e31c..30753764 100644 --- a/src/Entity/Tag.php +++ b/src/Entity/Tag.php @@ -55,36 +55,23 @@ class Tag $this->timesheets = new ArrayCollection(); } - /** - * @return int - */ - public function getId() + public function getId(): ?int { return $this->id; } - /** - * @param string $tagName - * @return Tag - */ - public function setName($tagName) + public function setName(string $tagName): Tag { $this->name = $tagName; return $this; } - /** - * @return string - */ - public function getName() + public function getName(): ?string { return $this->name; } - /** - * @param Timesheet $timesheet - */ public function addTimesheet(Timesheet $timesheet) { if ($this->timesheets->contains($timesheet)) { @@ -95,9 +82,6 @@ class Tag $timesheet->addTag($this); } - /** - * @param Timesheet $timesheet - */ public function removeTimesheet(Timesheet $timesheet) { if (!$this->timesheets->contains($timesheet)) { diff --git a/src/Entity/Timesheet.php b/src/Entity/Timesheet.php index 4fd7d827..cc02ba56 100644 --- a/src/Entity/Timesheet.php +++ b/src/Entity/Timesheet.php @@ -10,6 +10,7 @@ namespace App\Entity; use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Validator\Constraints as Assert; @@ -148,11 +149,11 @@ class Timesheet } /** - * Get entry id + * Get entry id, returns null for new entities which were not persisted. * - * @return int + * @return int|null */ - public function getId() + public function getId(): ?int { return $this->id; } @@ -178,10 +179,7 @@ class Timesheet $this->localized = true; } - /** - * @return \DateTime - */ - public function getBegin() + public function getBegin(): ?\DateTime { $this->localizeDates(); @@ -192,7 +190,7 @@ class Timesheet * @param \DateTime $begin * @return Timesheet */ - public function setBegin(\DateTime $begin) + public function setBegin(\DateTime $begin): Timesheet { $this->begin = $begin; $this->timezone = $begin->getTimezone()->getName(); @@ -200,10 +198,7 @@ class Timesheet return $this; } - /** - * @return \DateTime|null - */ - public function getEnd() + public function getEnd(): ?\DateTime { $this->localizeDates(); @@ -214,7 +209,7 @@ class Timesheet * @param \DateTime $end * @return Timesheet */ - public function setEnd(?\DateTime $end) + public function setEnd(?\DateTime $end): Timesheet { $this->end = $end; @@ -232,7 +227,7 @@ class Timesheet * @param int $duration * @return Timesheet */ - public function setDuration($duration) + public function setDuration($duration): Timesheet { $this->duration = $duration; @@ -253,17 +248,14 @@ class Timesheet * @param User $user * @return Timesheet */ - public function setUser(User $user) + public function setUser(User $user): Timesheet { $this->user = $user; return $this; } - /** - * @return User - */ - public function getUser() + public function getUser(): ?User { return $this->user; } @@ -272,25 +264,19 @@ class Timesheet * @param Activity $activity * @return Timesheet */ - public function setActivity($activity) + public function setActivity($activity): Timesheet { $this->activity = $activity; return $this; } - /** - * @return Activity - */ - public function getActivity() + public function getActivity(): ?Activity { return $this->activity; } - /** - * @return Project - */ - public function getProject() + public function getProject(): ?Project { return $this->project; } @@ -299,7 +285,7 @@ class Timesheet * @param Project $project * @return Timesheet */ - public function setProject(Project $project) + public function setProject(Project $project): Timesheet { $this->project = $project; @@ -310,17 +296,14 @@ class Timesheet * @param string $description * @return Timesheet */ - public function setDescription($description) + public function setDescription($description): Timesheet { $this->description = $description; return $this; } - /** - * @return string - */ - public function getDescription() + public function getDescription(): ?string { return $this->description; } @@ -329,7 +312,7 @@ class Timesheet * @param float $rate * @return Timesheet */ - public function setRate($rate) + public function setRate($rate): Timesheet { $this->rate = $rate; @@ -348,7 +331,7 @@ class Timesheet * @param Tag $tag * @return Timesheet */ - public function addTag(Tag $tag) + public function addTag(Tag $tag): Timesheet { if ($this->tags->contains($tag)) { return $this; @@ -372,9 +355,9 @@ class Timesheet } /** - * @return Tag[]|ArrayCollection + * @return Collection */ - public function getTags() + public function getTags(): Collection { return $this->tags; } @@ -404,7 +387,7 @@ class Timesheet * @param bool $exported * @return Timesheet */ - public function setExported(bool $exported) + public function setExported(bool $exported): Timesheet { $this->exported = $exported; @@ -426,7 +409,7 @@ class Timesheet * @param string $timezone * @return Timesheet */ - public function setTimezone(string $timezone) + public function setTimezone(string $timezone): Timesheet { $this->timezone = $timezone; diff --git a/src/Entity/User.php b/src/Entity/User.php index fa007b79..7cbffc54 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -105,122 +105,77 @@ class User extends BaseUser implements UserInterface $this->preferences = new ArrayCollection(); } - /** - * @return int - */ - public function getId() + public function getId(): ?int { return $this->id; } - /** - * @return \DateTime - */ - public function getRegisteredAt() + public function getRegisteredAt(): ?\DateTime { return $this->registeredAt; } - /** - * @param \DateTime $registeredAt - * @return $this - */ - public function setRegisteredAt(\DateTime $registeredAt) + public function setRegisteredAt(\DateTime $registeredAt): User { $this->registeredAt = $registeredAt; return $this; } - /** - * @param string $alias - * @return $this - */ - public function setAlias($alias) + public function setAlias(?string $alias): User { $this->alias = $alias; return $this; } - /** - * @return string - */ - public function getAlias() + public function getAlias(): ?string { return $this->alias; } - /** - * @return string - */ - public function getTitle() + public function getTitle(): ?string { return $this->title; } - /** - * @param string $title - * @return $this - */ - public function setTitle($title) + public function setTitle(?string $title): User { $this->title = $title; return $this; } - /** - * @return string - */ - public function getAvatar() + public function getAvatar(): ?string { return $this->avatar; } - /** - * @param string $avatar - * @return $this - */ - public function setAvatar($avatar) + public function setAvatar(?string $avatar): User { $this->avatar = $avatar; return $this; } - /** - * @return string - */ - public function getApiToken() + public function getApiToken(): ?string { return $this->apiToken; } - /** - * @param string $apiToken - * @return User - */ - public function setApiToken($apiToken) + public function setApiToken(?string $apiToken): User { $this->apiToken = $apiToken; return $this; } - /** - * @return string - */ public function getPlainApiToken(): ?string { return $this->plainApiToken; } - /** - * @param string $plainApiToken - * @return User - */ - public function setPlainApiToken(string $plainApiToken) + public function setPlainApiToken(?string $plainApiToken): User { $this->plainApiToken = $plainApiToken; @@ -228,7 +183,7 @@ class User extends BaseUser implements UserInterface } /** - * @return UserPreference[]|Collection + * @return Collection */ public function getPreferences(): Collection { @@ -236,10 +191,10 @@ class User extends BaseUser implements UserInterface } /** - * @param UserPreference[]|Collection $preferences + * @param iterable $preferences * @return User */ - public function setPreferences($preferences) + public function setPreferences(iterable $preferences): User { $this->preferences = new ArrayCollection(); @@ -252,10 +207,29 @@ class User extends BaseUser implements UserInterface /** * @param string $name - * @return UserPreference|null + * @param bool|int|string|null $value */ - public function getPreference(string $name) + public function setPreferenceValue(string $name, $value = null) { + $pref = $this->getPreference($name); + + if (null === $pref) { + $pref = new UserPreference(); + $pref->setName($name); + $this->addPreference($pref); + } + + $pref->setValue($value); + } + + public function getPreference(string $name): ?UserPreference + { + // this code will be triggered, if a currently logged-in user will be deleted and the refreshed from the session + // via one of the UserProvider - e.g. see LdapUserProvider::refreshUser() which calls $user->getPreferenceValue() + if (empty($this->preferences)) { + return null; + } + foreach ($this->preferences as $preference) { if ($preference->getName() == $name) { return $preference; @@ -268,7 +242,7 @@ class User extends BaseUser implements UserInterface /** * @return string */ - public function getLocale() + public function getLocale(): string { return $this->getPreferenceValue(UserPreference::LOCALE, User::DEFAULT_LANGUAGE); } @@ -292,7 +266,7 @@ class User extends BaseUser implements UserInterface * @param UserPreference $preference * @return User */ - public function addPreference(UserPreference $preference) + public function addPreference(UserPreference $preference): User { $this->preferences->add($preference); $preference->setUser($this); diff --git a/src/Form/Type/UserRoleType.php b/src/Form/Type/UserRoleType.php index 1bd3d44f..5eaa4f70 100644 --- a/src/Form/Type/UserRoleType.php +++ b/src/Form/Type/UserRoleType.php @@ -9,6 +9,7 @@ namespace App\Form\Type; +use App\Security\RoleService; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\ChoiceType; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -19,14 +20,11 @@ use Symfony\Component\OptionsResolver\OptionsResolver; class UserRoleType extends AbstractType { /** - * @var string[] + * @var RoleService */ - protected $roles = []; + protected $roles; - /** - * @param string[] $roles - */ - public function __construct(array $roles = []) + public function __construct(RoleService $roles) { $this->roles = $roles; } @@ -37,13 +35,8 @@ class UserRoleType extends AbstractType public function configureOptions(OptionsResolver $resolver) { $roles = []; - foreach ($this->roles as $key => $value) { - $roles[$key] = $key; - if (is_array($value)) { - foreach ($value as $value2) { - $roles[$value2] = $value2; - } - } + foreach ($this->roles->getAvailableNames() as $name) { + $roles[$name] = $name; } $resolver->setDefaults([ diff --git a/src/Kernel.php b/src/Kernel.php index bcc48c9a..7c73e9e5 100644 --- a/src/Kernel.php +++ b/src/Kernel.php @@ -17,9 +17,11 @@ use App\Export\RendererInterface as ExportRendererInterface; use App\Invoice\CalculatorInterface as InvoiceCalculator; use App\Invoice\NumberGeneratorInterface; use App\Invoice\RendererInterface as InvoiceRendererInterface; +use App\Ldap\FormLoginLdapFactory; use App\Plugin\PluginInterface; use App\Timesheet\CalculatorInterface as TimesheetCalculator; use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait; +use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -58,6 +60,10 @@ class Kernel extends BaseKernel $container->registerForAutoconfiguration(NumberGeneratorInterface::class)->addTag(self::TAG_INVOICE_NUMBER_GENERATOR); $container->registerForAutoconfiguration(InvoiceCalculator::class)->addTag(self::TAG_INVOICE_CALCULATOR); $container->registerForAutoconfiguration(PluginInterface::class)->addTag(self::TAG_PLUGIN); + + /** @var SecurityExtension $extension */ + $extension = $container->getExtension('security'); + $extension->addSecurityListenerFactory(new FormLoginLdapFactory()); } public function registerBundles() diff --git a/src/Ldap/FormLoginLdapFactory.php b/src/Ldap/FormLoginLdapFactory.php new file mode 100644 index 00000000..9eab591d --- /dev/null +++ b/src/Ldap/FormLoginLdapFactory.php @@ -0,0 +1,72 @@ +createAuthProvider($container, $id, $userProviderId); + $listenerId = $this->createListener($container, $id, $config); + + return [$authProviderId, $listenerId, $defaultEntryPointId]; + } + + public function getPosition() + { + return 'pre_auth'; + } + + public function getKey() + { + return 'kimai_ldap'; + } + + public function addConfiguration(NodeDefinition $node) + { + } + + protected function createAuthProvider(ContainerBuilder $container, $id, $userProviderId) + { + $provider = 'kimai_ldap.security.authentication.provider'; + $providerId = $provider . '.' . $id; + + $container + ->setDefinition($providerId, new ChildDefinition($provider)) + ->replaceArgument(1, $id) + ->replaceArgument(2, new Reference($userProviderId)) + ; + + return $providerId; + } + + protected function createListener(ContainerBuilder $container, $id, $config) + { + $listenerId = 'security.authentication.listener.form'; + + $listener = new ChildDefinition($listenerId); + $listener->replaceArgument(4, $id); + $listener->replaceArgument(5, $config); + + $listenerId .= '.' . $id; + $container->setDefinition($listenerId, $listener); + + return $listenerId; + } +} diff --git a/src/Ldap/LdapAuthenticationProvider.php b/src/Ldap/LdapAuthenticationProvider.php new file mode 100644 index 00000000..9449bd9c --- /dev/null +++ b/src/Ldap/LdapAuthenticationProvider.php @@ -0,0 +1,131 @@ +ldapManager = $ldapManager; + $this->config = $config; + $this->userProvider = $userProvider; + } + + public function supports(TokenInterface $token) + { + if (!$this->config->isActivated()) { + return false; + } + + return parent::supports($token); + } + + protected function retrieveUser($username, UsernamePasswordToken $token) + { + $user = $token->getUser(); + if ($user instanceof UserInterface) { + return $user; + } + + try { + // this will always query the FOSUserBundle first... + // only first-time logins from LDAP user (not yet existing in local user database) + // will actually hit the LdapUserProvider + $user = $this->userProvider->loadUserByUsername($username); + + // do not update the user here from LDAP, as we don't know if the user can be authenticated + } catch (UsernameNotFoundException $notFound) { + throw $notFound; + } catch (\Exception $repositoryProblem) { + $e = new AuthenticationServiceException($repositoryProblem->getMessage(), (int) $repositoryProblem->getCode(), $repositoryProblem); + $e->setToken($token); + + throw $e; + } + + return $user; + } + + /** + * The updateUser() call should theoretically happen in retrieveUser() but that would require an additional + * $this->ldapManager->bind($user, $token->getCredentials()) + * to check if the user is still valid. + * + * Symfony calls retrieveUser() before checkAuthentication() + * and we should not used ldap->search() before ldap->bind() + * + * @param UserInterface $user + * @param UsernamePasswordToken $token + * @throws LdapDriverException + */ + protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token) + { + $currentUser = $token->getUser(); + $presentedPassword = $token->getCredentials(); + if ($currentUser instanceof UserInterface) { + if ('' === $presentedPassword) { + throw new BadCredentialsException( + 'The password in the token is empty. Check `erase_credentials` in your `security.yaml`' + ); + } + + if (!$this->ldapManager->bind($currentUser, $presentedPassword)) { + throw new BadCredentialsException('The credentials were changed from another session.'); + } + } else { + if ('' === $presentedPassword) { + throw new BadCredentialsException('The presented password cannot be empty.'); + } + + if (!$this->ldapManager->bind($user, $presentedPassword)) { + throw new BadCredentialsException('The presented password is invalid.'); + } + } + + if ($user instanceof User && null !== $user->getPreferenceValue('ldap.dn')) { + try { + $this->ldapManager->updateUser($user); + } catch (LdapDriverException $ex) { + throw new BadCredentialsException('Fetching user data/roles failed, probably DN is expired.'); + } + } + } +} diff --git a/src/Ldap/LdapDriver.php b/src/Ldap/LdapDriver.php new file mode 100644 index 00000000..46e206fc --- /dev/null +++ b/src/Ldap/LdapDriver.php @@ -0,0 +1,129 @@ +driver = $driver; + $this->logger = $logger; + } + + /** + * @param string $baseDn + * @param string $filter + * @param array $attributes + * @return array + * @throws LdapDriverException + */ + public function search(string $baseDn, string $filter, array $attributes = []): array + { + $attributes = array_unique(array_merge($attributes, ['+', '*'])); + + $this->logDebug('{action}({base_dn}, {filter}, {attributes})', [ + 'action' => 'ldap_search', + 'base_dn' => $baseDn, + 'filter' => $filter, + 'attributes' => $attributes, + ]); + + try { + $this->driver->bind(); + $entries = $this->driver->searchEntries($filter, $baseDn, Ldap::SEARCH_SCOPE_SUB, $attributes); + + // searchEntries don't return 'count' key as specified by php native function ldap_get_entries() + $entries['count'] = count($entries); + } catch (LdapException $exception) { + $this->zendExceptionHandler($exception); + + throw new LdapDriverException('An error occurred with the search operation.'); + } + + return $entries; + } + + public function bind(UserInterface $user, string $password): bool + { + $bindDn = $user->getUsername(); + + try { + $this->logDebug('{action}({bindDn}, ****)', [ + 'action' => 'ldap_bind', + 'bindDn' => $bindDn, + ]); + $bind = $this->driver->bind($bindDn, $password); + + return $bind instanceof Ldap; + } catch (LdapException $exception) { + $this->zendExceptionHandler($exception, $password); + } + + return false; + } + + /** + * Treat a Zend Ldap Exception. + */ + protected function zendExceptionHandler(LdapException $exception, string $password = null): void + { + $sanitizedException = null !== $password ? new SanitizingException($exception, $password) : $exception; + + switch ($exception->getCode()) { + // Error level codes + case LdapException::LDAP_SERVER_DOWN: + if ($this->logger) { + $this->logger->error('{exception}', ['exception' => $sanitizedException]); + } + break; + + // Other level codes + default: + $this->logDebug('{exception}', ['exception' => $sanitizedException]); + break; + } + } + + /** + * Log debug messages if the logger is set. + * + * @param string $message + * @param array $context + */ + private function logDebug(string $message, array $context = []): void + { + if (null === $this->logger) { + return; + } + $this->logger->debug($message, $context); + } +} diff --git a/src/Ldap/LdapDriverException.php b/src/Ldap/LdapDriverException.php new file mode 100644 index 00000000..bfd13bcd --- /dev/null +++ b/src/Ldap/LdapDriverException.php @@ -0,0 +1,18 @@ +params = $config->getUserParameters(); + $this->config = $config; + $this->driver = $driver; + $this->hydrator = $hydrator; + } + + /** + * Only executed for unknown local users. + * + * @param string $username + * @return User|null + * @throws \Exception + */ + public function findUserByUsername(string $username): ?UserInterface + { + return $this->findUserBy([$this->params['usernameAttribute'] => $username]); + } + + /** + * @param array $criteria + * @return User|null + * @throws LdapDriverException + */ + public function findUserBy(array $criteria): ?UserInterface + { + $filter = $this->buildFilter($criteria); + $entries = $this->driver->search($this->params['baseDn'], $filter); + + if ($entries['count'] > 1) { + throw new LdapDriverException('This search must only return a single user'); + } + + if (0 === $entries['count']) { + return null; + } + + // do not updateUser() here, as this would happen before bind() + return $this->hydrator->hydrate($entries[0]); + } + + protected function buildFilter(array $criteria, string $condition = '&'): string + { + $filters = []; + $filters[] = $this->params['filter']; + foreach ($criteria as $key => $value) { + $value = ldap_escape($value, '', LDAP_ESCAPE_FILTER); + $filters[] = sprintf('(%s=%s)', $key, $value); + } + + return sprintf('(%s%s)', $condition, implode($filters)); + } + + public function bind(UserInterface $user, string $password): bool + { + return $this->driver->bind($user, $password); + } + + /** + * This method does all the heavy lifting: + * - searching for latest 'dn' + * - syncing user attributes + * - syncing roles + * + * @param User $user + * @throws LdapDriverException + */ + public function updateUser(User $user) + { + $baseDn = $user->getPreferenceValue('ldap.dn'); + $filter = '(objectClass=*)'; + + if (null === $baseDn) { + throw new LdapDriverException('This account is not a registered LDAP user'); + } + + // always look up the users current DN first, as the cached DN might have been renamed in LDAP + $userFresh = $this->findUserByUsername($user->getUsername()); + if (null === $userFresh || null === ($baseDn = $userFresh->getPreferenceValue('ldap.dn'))) { + throw new LdapDriverException(sprintf('Failed fetching user DN for %s', $user->getUsername())); + } + $user->setPreferenceValue('ldap.dn', $baseDn); + + $entries = $this->driver->search($baseDn, $filter); + + if ($entries['count'] > 1) { + throw new LdapDriverException('This search must only return a single user'); + } + + if (0 === $entries['count']) { + return; + } + + $this->hydrator->hydrateUser($user, $entries[0]); + + $roleParameter = $this->config->getRoleParameters(); + if (null === $roleParameter['baseDn']) { + return; + } + + $param = $roleParameter['usernameAttribute']; + if (!isset($entries[0][$param]) && $param !== 'dn') { + $param = 'dn'; + } + + $roleValue = $entries[0][$param]; + if (is_array($roleValue)) { + $roleValue = $roleValue[0]; + } + $roles = $this->getRoles($roleValue, $roleParameter); + + if (!empty($roles)) { + $this->hydrator->hydrateRoles($user, $roles); + } + } + + protected function getRoles(string $dn, array $roleParameter): array + { + $filter = $roleParameter['filter'] ?? ''; + + return $this->driver->search( + $roleParameter['baseDn'], + sprintf('(&%s(%s=%s))', $filter, $roleParameter['userDnAttribute'], $dn), + [$roleParameter['nameAttribute']] + ); + } +} diff --git a/src/Ldap/LdapUserHydrator.php b/src/Ldap/LdapUserHydrator.php new file mode 100644 index 00000000..af84ac63 --- /dev/null +++ b/src/Ldap/LdapUserHydrator.php @@ -0,0 +1,144 @@ +config = $config; + $this->roles = $roles; + } + + protected function createUser(): User + { + $user = new User(); + $user->setEnabled(true); + + return $user; + } + + public function hydrate(array $ldapEntry): User + { + $user = $this->createUser(); + $this->hydrateUser($user, $ldapEntry); + + return $user; + } + + public function hydrateUser(User $user, array $ldapEntry) + { + $userParams = $this->config->getUserParameters(); + $attributeMap = $userParams['attributes']; + $attributeMap = array_merge( + [ + ['ldap_attr' => $userParams['usernameAttribute'], 'user_method' => 'setUsername'], + ], + $attributeMap + ); + + $this->hydrateUserWithAttributesMap($user, $ldapEntry, $attributeMap); + + if (null === $user->getEmail()) { + $user->setEmail($user->getUsername()); + } + + // prevent that users will define a password for the internal account + $user->setPassword(''); + + $user->setPreferenceValue('ldap.dn', $ldapEntry['dn']); + } + + /** + * @param User $user + * @param array $entries + */ + public function hydrateRoles(User $user, array $entries) + { + $roleParams = $this->config->getRoleParameters(); + $allowedRoles = $this->roles->getAvailableNames(); + $groupNameMapping = $roleParams['groups']; + $roleNameAttr = $roleParams['nameAttribute']; + + $roles = []; + for ($i = 0; $i < $entries['count']; $i++) { + $roleName = $entries[$i][$roleNameAttr][0]; + $mapped = false; + foreach ($groupNameMapping as $attr) { + if ($roleName === $attr['ldap_value']) { + $roleName = $attr['role']; + $mapped = true; + } + } + + if (!$mapped) { + $roleName = sprintf('ROLE_%s', self::slugify($roleName)); + } + + if (!in_array($roleName, $allowedRoles)) { + continue; + } + + $roles[] = $roleName; + } + + $user->setRoles($roles); + } + + private static function slugify(string $role): string + { + $role = preg_replace('/\W+/', '_', $role); + $role = trim($role, '_'); + $role = strtoupper($role); + + return $role; + } + + protected function hydrateUserWithAttributesMap(UserInterface $user, array $ldapUserAttributes, array $attributeMap) + { + /** @var array $attr */ + foreach ($attributeMap as $attr) { + if (!array_key_exists($attr['ldap_attr'], $ldapUserAttributes)) { + continue; + } + + $ldapValue = $ldapUserAttributes[$attr['ldap_attr']]; + + if (array_key_exists('count', $ldapValue)) { + unset($ldapValue['count']); + } + + if (1 === count($ldapValue)) { + $value = array_shift($ldapValue); + } else { + $value = $ldapValue; + } + + $user->{$attr['user_method']}($value); + } + } +} diff --git a/src/Ldap/LdapUserProvider.php b/src/Ldap/LdapUserProvider.php new file mode 100644 index 00000000..7aaf94a2 --- /dev/null +++ b/src/Ldap/LdapUserProvider.php @@ -0,0 +1,120 @@ +ldapManager = $ldapManager; + $this->logger = $logger; + $this->activated = $config->isActivated(); + } + + public function loadUserByUsername($username) + { + // this method is called at least for unknown user, no matter what supportsClass() returns, + // so we have to check if LDAP is activated here as well + if (!$this->activated) { + $ex = new UsernameNotFoundException(sprintf('LDAP is deactivated, user "%s" not searched', $username)); + $ex->setUsername($username); + + throw $ex; + } + + $user = $this->ldapManager->findUserByUsername($username); + + if (empty($user)) { + $this->logInfo('User {username} {result} on LDAP', [ + 'action' => 'loadUserByUsername', + 'username' => $username, + 'result' => 'not found', + ]); + $ex = new UsernameNotFoundException(sprintf('User "%s" not found', $username)); + $ex->setUsername($username); + + throw $ex; + } + + $this->logInfo('User {username} {result} on LDAP', [ + 'action' => 'loadUserByUsername', + 'username' => $username, + 'result' => 'found', + ]); + + return $user; + } + + public function refreshUser(UserInterface $user) + { + if (!($user instanceof User) || !$this->supportsClass(get_class($user))) { + throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_class($user))); + } + + if (null === $user->getPreferenceValue('ldap.dn')) { + throw new UnsupportedUserException(sprintf('Account "%s" is not a registered LDAP user.', $user->getUsername())); + } + + try { + $this->ldapManager->updateUser($user); + } catch (LdapDriverException $ex) { + throw new UnsupportedUserException(sprintf('Failed to refresh user "%s", probably DN is expired.', $user->getUsername())); + } + + return $user; + } + + public function supportsClass($class) + { + if (!$this->activated) { + return false; + } + + return $class === User::class || $class === 'App\Entity\User'; + } + + /** + * Log a message into the logger if this exists. + */ + private function logInfo(string $message, array $context = []): void + { + if (!$this->logger) { + return; + } + + $this->logger->info($message, $context); + } +} diff --git a/src/Ldap/SanitizingException.php b/src/Ldap/SanitizingException.php new file mode 100644 index 00000000..148ba027 --- /dev/null +++ b/src/Ldap/SanitizingException.php @@ -0,0 +1,40 @@ +stripSecret($actualException->getMessage(), $secret), + $actualException->getCode() + ); + + $this->actualException = $actualException; + $this->secret = $secret; + } + + protected function stripSecret(string $message, string $secret) + { + return str_replace($secret, '****', $message); + } + + public function __toString() + { + return $this->stripSecret($this->actualException->__toString(), $this->secret); + } +} diff --git a/src/Ldap/ZendLdap.php b/src/Ldap/ZendLdap.php new file mode 100644 index 00000000..1652ec27 --- /dev/null +++ b/src/Ldap/ZendLdap.php @@ -0,0 +1,29 @@ +isActivated()) { + return; + } + + parent::__construct($config->getConnectionParameters()); + } +} diff --git a/src/Model/InvoiceModel.php b/src/Model/InvoiceModel.php index b72480a4..f189eb95 100644 --- a/src/Model/InvoiceModel.php +++ b/src/Model/InvoiceModel.php @@ -23,7 +23,7 @@ use App\Repository\Query\InvoiceQuery; class InvoiceModel { /** - * @var Customer + * @var Customer|null */ protected $customer; @@ -95,36 +95,26 @@ class InvoiceModel * @param Timesheet[] $entries * @return InvoiceModel */ - public function setEntries(array $entries) + public function setEntries(array $entries): InvoiceModel { $this->entries = $entries; return $this; } - /** - * @return InvoiceTemplate - */ public function getTemplate(): ?InvoiceTemplate { return $this->template; } - /** - * @param InvoiceTemplate $template - * @return InvoiceModel - */ - public function setTemplate(InvoiceTemplate $template) + public function setTemplate(InvoiceTemplate $template): InvoiceModel { $this->template = $template; return $this; } - /** - * @return Customer - */ - public function getCustomer() + public function getCustomer(): ?Customer { return $this->customer; } @@ -133,16 +123,13 @@ class InvoiceModel * @param Customer $customer * @return InvoiceModel */ - public function setCustomer($customer) + public function setCustomer($customer): InvoiceModel { $this->customer = $customer; return $this; } - /** - * @return \DateTime - */ public function getDueDate(): ?\DateTime { if (null === $this->getTemplate()) { @@ -160,11 +147,7 @@ class InvoiceModel return $this->invoiceDate; } - /** - * @param NumberGeneratorInterface $generator - * @return InvoiceModel - */ - public function setNumberGenerator(NumberGeneratorInterface $generator) + public function setNumberGenerator(NumberGeneratorInterface $generator): InvoiceModel { $this->generator = $generator; $this->generator->setModel($this); @@ -172,19 +155,12 @@ class InvoiceModel return $this; } - /** - * @return NumberGeneratorInterface - */ public function getNumberGenerator(): ?NumberGeneratorInterface { return $this->generator; } - /** - * @param CalculatorInterface $calculator - * @return InvoiceModel - */ - public function setCalculator(CalculatorInterface $calculator) + public function setCalculator(CalculatorInterface $calculator): InvoiceModel { $this->calculator = $calculator; $this->calculator->setModel($this); @@ -192,9 +168,6 @@ class InvoiceModel return $this; } - /** - * @return CalculatorInterface - */ public function getCalculator(): ?CalculatorInterface { return $this->calculator; diff --git a/src/Model/TimesheetStatistic.php b/src/Model/TimesheetStatistic.php index 62ebbc9f..e9b1c75b 100644 --- a/src/Model/TimesheetStatistic.php +++ b/src/Model/TimesheetStatistic.php @@ -33,7 +33,7 @@ class TimesheetStatistic */ protected $amountTotal = 0; /** - * @var \DateTime + * @var \DateTime|null */ protected $firstEntry; /** @@ -41,10 +41,7 @@ class TimesheetStatistic */ protected $recordsTotal = 0; - /** - * @return int - */ - public function getDurationThisMonth() + public function getDurationThisMonth(): int { return $this->durationThisMonth; } @@ -57,10 +54,7 @@ class TimesheetStatistic $this->durationThisMonth = (int) $durationThisMonth; } - /** - * @return int - */ - public function getAmountTotal() + public function getAmountTotal(): int { return $this->amountTotal; } @@ -73,10 +67,7 @@ class TimesheetStatistic $this->amountTotal = (int) $amountTotal; } - /** - * @return int - */ - public function getDurationTotal() + public function getDurationTotal(): int { return $this->durationTotal; } @@ -89,10 +80,7 @@ class TimesheetStatistic $this->durationTotal = (int) $durationTotal; } - /** - * @return int - */ - public function getAmountThisMonth() + public function getAmountThisMonth(): int { return $this->amountThisMonth; } @@ -105,10 +93,7 @@ class TimesheetStatistic $this->amountThisMonth = (int) $amountThisMonth; } - /** - * @return DateTime - */ - public function getFirstEntry() + public function getFirstEntry(): ?\DateTime { return $this->firstEntry; } diff --git a/src/Repository/Query/ActivityQuery.php b/src/Repository/Query/ActivityQuery.php index 9d50beb9..7bb56e45 100644 --- a/src/Repository/Query/ActivityQuery.php +++ b/src/Repository/Query/ActivityQuery.php @@ -17,7 +17,7 @@ use App\Entity\Project; class ActivityQuery extends ProjectQuery { /** - * @var Project|int + * @var Project|int|null */ protected $project; /** @@ -37,11 +37,7 @@ class ActivityQuery extends ProjectQuery return $this->orderGlobalsFirst; } - /** - * @param bool $orderGlobalsFirst - * @return ActivityQuery - */ - public function setOrderGlobalsFirst(bool $orderGlobalsFirst) + public function setOrderGlobalsFirst(bool $orderGlobalsFirst): ActivityQuery { $this->orderGlobalsFirst = $orderGlobalsFirst; @@ -60,15 +56,15 @@ class ActivityQuery extends ProjectQuery * @param bool $globalsOnly * @return ActivityQuery */ - public function setGlobalsOnly($globalsOnly) + public function setGlobalsOnly($globalsOnly): ActivityQuery { - $this->globalsOnly = $globalsOnly; + $this->globalsOnly = (bool) $globalsOnly; return $this; } /** - * @return Project|int + * @return Project|int|null */ public function getProject() { @@ -76,10 +72,10 @@ class ActivityQuery extends ProjectQuery } /** - * @param Project|int $project - * @return $this + * @param Project|int|null $project + * @return ActivityQuery */ - public function setProject($project = null) + public function setProject($project = null): ActivityQuery { $this->project = $project; diff --git a/src/Repository/Query/BaseQuery.php b/src/Repository/Query/BaseQuery.php index 3c21cf75..da7982bc 100644 --- a/src/Repository/Query/BaseQuery.php +++ b/src/Repository/Query/BaseQuery.php @@ -25,7 +25,7 @@ class BaseQuery public const RESULT_TYPE_QUERYBUILDER = 'QueryBuilder'; /** - * @var object + * @var object|null */ protected $hiddenEntity; /** @@ -158,7 +158,7 @@ class BaseQuery } /** - * @return object + * @return object|null */ public function getHiddenEntity() { @@ -166,7 +166,7 @@ class BaseQuery } /** - * @param object|string $hiddenEntity + * @param object|string|null $hiddenEntity * @return BaseQuery */ public function setHiddenEntity($hiddenEntity) diff --git a/src/Repository/Query/ProjectQuery.php b/src/Repository/Query/ProjectQuery.php index aec5e0cb..3c4f6758 100644 --- a/src/Repository/Query/ProjectQuery.php +++ b/src/Repository/Query/ProjectQuery.php @@ -17,7 +17,7 @@ use App\Entity\Customer; class ProjectQuery extends VisibilityQuery { /** - * @var Customer|int + * @var Customer|int|null */ protected $customer; @@ -46,7 +46,7 @@ class ProjectQuery extends VisibilityQuery } /** - * @return Customer|int + * @return Customer|int|null */ public function getCustomer() { @@ -54,7 +54,7 @@ class ProjectQuery extends VisibilityQuery } /** - * @param Customer|int $customer + * @param Customer|int|null $customer * @return $this */ public function setCustomer($customer = null) diff --git a/src/Repository/Query/TimesheetQuery.php b/src/Repository/Query/TimesheetQuery.php index f3e41529..259026f9 100644 --- a/src/Repository/Query/TimesheetQuery.php +++ b/src/Repository/Query/TimesheetQuery.php @@ -36,11 +36,11 @@ class TimesheetQuery extends ActivityQuery */ protected $orderBy = 'begin'; /** - * @var User + * @var User|null */ protected $user; /** - * @var Activity + * @var Activity|null */ protected $activity; /** @@ -66,7 +66,7 @@ class TimesheetQuery extends ActivityQuery } /** - * @return User + * @return User|null */ public function getUser() { @@ -74,7 +74,7 @@ class TimesheetQuery extends ActivityQuery } /** - * @param User|int $user + * @param User|int|null $user * @return TimesheetQuery */ public function setUser($user = null) @@ -87,7 +87,7 @@ class TimesheetQuery extends ActivityQuery /** * Activity overwrites: setProject() and setCustomer() * - * @return Activity + * @return Activity|null */ public function getActivity() { @@ -95,7 +95,7 @@ class TimesheetQuery extends ActivityQuery } /** - * @param Activity|int $activity + * @param Activity|int|null $activity * @return TimesheetQuery */ public function setActivity($activity = null) diff --git a/src/Repository/Query/UserQuery.php b/src/Repository/Query/UserQuery.php index 8395de3f..b566eac5 100644 --- a/src/Repository/Query/UserQuery.php +++ b/src/Repository/Query/UserQuery.php @@ -15,12 +15,12 @@ namespace App\Repository\Query; class UserQuery extends VisibilityQuery { /** - * @var string + * @var string|null */ protected $role; /** - * @return string + * @return string|null */ public function getRole() { diff --git a/src/Repository/TimesheetRepository.php b/src/Repository/TimesheetRepository.php index 8a1a0bf1..da656a79 100644 --- a/src/Repository/TimesheetRepository.php +++ b/src/Repository/TimesheetRepository.php @@ -274,6 +274,7 @@ class TimesheetRepository extends AbstractRepository ->join('t.activity', 'a') ->join('t.project', 'p') ->join('p.customer', 'c') + ->leftJoin('t.tags', 'tags') ->where($qb->expr()->isNotNull('t.begin')) ->andWhere($qb->expr()->isNull('t.end')) ->orderBy('t.begin', 'DESC'); @@ -438,11 +439,12 @@ class TimesheetRepository extends AbstractRepository $ids = array_column($results, 'maxid'); $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->select('t', 'a', 'p', 'c') + $qb->select('t', 'a', 'p', 'c', 'tags') ->from(Timesheet::class, 't') ->join('t.activity', 'a') ->join('t.project', 'p') ->join('p.customer', 'c') + ->leftJoin('t.tags', 'tags') ->andWhere($qb->expr()->in('t.id', $ids)) ->orderBy('t.end', 'DESC') ; diff --git a/src/Security/AclDecisionManager.php b/src/Security/AclDecisionManager.php index 2ee17015..355ca33f 100644 --- a/src/Security/AclDecisionManager.php +++ b/src/Security/AclDecisionManager.php @@ -20,7 +20,6 @@ class AclDecisionManager protected $decisionManager; /** - * AbstractVoter constructor. * @param AccessDecisionManagerInterface $decisionManager */ public function __construct(AccessDecisionManagerInterface $decisionManager) @@ -40,22 +39,4 @@ class AclDecisionManager return false; } - - /** - * @param TokenInterface $token - * @param string|array $roles - * @return bool - */ - public function hasRole(TokenInterface $token, $roles) - { - if (!is_array($roles)) { - $roles = [$roles]; - } - - if ($this->decisionManager->decide($token, $roles)) { - return true; - } - - return false; - } } diff --git a/src/Security/RolePermissionManager.php b/src/Security/RolePermissionManager.php index 96875d59..3c6364bb 100644 --- a/src/Security/RolePermissionManager.php +++ b/src/Security/RolePermissionManager.php @@ -49,11 +49,6 @@ class RolePermissionManager return array_keys($this->permissions); } - public function roleHasPermission(string $role): bool - { - return isset($this->permissions[$role]); - } - public function getPermissions(): array { return $this->knownPermissions; diff --git a/src/Security/RoleService.php b/src/Security/RoleService.php new file mode 100644 index 00000000..9402b492 --- /dev/null +++ b/src/Security/RoleService.php @@ -0,0 +1,38 @@ +roles = $roles; + } + + public function getAvailableNames(): array + { + $roles = []; + foreach ($this->roles as $key => $value) { + $roles[] = $key; + if (is_array($value)) { + foreach ($value as $name) { + $roles[] = $name; + } + } + } + + return array_values(array_unique($roles)); + } +} diff --git a/src/Security/UserChecker.php b/src/Security/UserChecker.php index ee4bfa83..f760f489 100644 --- a/src/Security/UserChecker.php +++ b/src/Security/UserChecker.php @@ -11,7 +11,7 @@ namespace App\Security; use App\Entity\User; use Symfony\Component\Security\Core\Exception\AccountStatusException; -use Symfony\Component\Security\Core\Exception\LockedException; +use Symfony\Component\Security\Core\Exception\DisabledException; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -26,6 +26,15 @@ class UserChecker implements UserCheckerInterface */ public function checkPreAuth(UserInterface $user) { + if (!($user instanceof User)) { + return; + } + + if (!$user->isEnabled()) { + $ex = new DisabledException('User account is disabled.'); + $ex->setUser($user); + throw $ex; + } } /** @@ -34,13 +43,14 @@ class UserChecker implements UserCheckerInterface */ public function checkPostAuth(UserInterface $user) { - if (!$user instanceof User) { + if (!($user instanceof User)) { return; } - // user account is not enabled, the user may be notified if (!$user->isEnabled()) { - throw new LockedException(); + $ex = new DisabledException('User account is disabled.'); + $ex->setUser($user); + throw $ex; } } } diff --git a/src/Voter/AbstractVoter.php b/src/Voter/AbstractVoter.php index 12039ba7..e0327736 100644 --- a/src/Voter/AbstractVoter.php +++ b/src/Voter/AbstractVoter.php @@ -48,16 +48,6 @@ abstract class AbstractVoter extends Voter return $this->decisionManager->isFullyAuthenticated($token); } - /** - * @param string $role - * @param TokenInterface $token - * @return bool - */ - protected function hasRole($role, TokenInterface $token) - { - return $this->decisionManager->hasRole($token, [$role]); - } - /** * @param string $role * @param string $permission diff --git a/src/Voter/UserVoter.php b/src/Voter/UserVoter.php index 3ea7a83c..a8a71907 100644 --- a/src/Voter/UserVoter.php +++ b/src/Voter/UserVoter.php @@ -83,7 +83,6 @@ class UserVoter extends AbstractVoter // used in templates and ProfileController case self::VIEW: case self::EDIT: - case self::PASSWORD: case self::PREFERENCES: // always allow the user to edit these own settings if ($subject->getId() === $user->getId()) { @@ -91,6 +90,7 @@ class UserVoter extends AbstractVoter } // no break on purpose + case self::PASSWORD: case self::API_TOKEN: case self::ROLES: case self::HOURLY_RATE: diff --git a/symfony.lock b/symfony.lock index 79e969b9..c3c49818 100644 --- a/symfony.lock +++ b/symfony.lock @@ -327,6 +327,9 @@ "phpstan/phpstan-doctrine": { "version": "0.11.4" }, + "phpstan/phpstan-phpunit": { + "version": "0.11.2" + }, "phpstan/phpstan-symfony": { "version": "0.11.6" }, @@ -513,6 +516,9 @@ "symfony/intl": { "version": "v4.0.3" }, + "symfony/ldap": { + "version": "v4.2.8" + }, "symfony/maker-bundle": { "version": "1.0", "recipe": { @@ -594,18 +600,9 @@ "ref": "85834af1496735f28d831489d12ab1921a875e0d" } }, - "symfony/security-core": { - "version": "v4.2.6" - }, "symfony/security-csrf": { "version": "v4.0.4" }, - "symfony/security-guard": { - "version": "v4.2.6" - }, - "symfony/security-http": { - "version": "v4.2.6" - }, "symfony/serializer": { "version": "v4.1.2" }, @@ -732,6 +729,9 @@ "zendframework/zend-eventmanager": { "version": "3.2.0" }, + "zendframework/zend-ldap": { + "version": "2.10.0" + }, "zircote/swagger-php": { "version": "2.0.13" } diff --git a/tests/API/APIControllerBaseTest.php b/tests/API/APIControllerBaseTest.php index 2808eb4c..bdccff6c 100644 --- a/tests/API/APIControllerBaseTest.php +++ b/tests/API/APIControllerBaseTest.php @@ -21,11 +21,7 @@ use Symfony\Component\HttpFoundation\Response; */ abstract class APIControllerBaseTest extends ControllerBaseTest { - /** - * @param string $role - * @return Client - */ - protected function getClientForAuthenticatedUser(string $role = User::ROLE_USER) + protected function getClientForAuthenticatedUser(string $role = User::ROLE_USER): Client { switch ($role) { case User::ROLE_SUPER_ADMIN: @@ -57,7 +53,7 @@ abstract class APIControllerBaseTest extends ControllerBaseTest break; default: - $client = null; + throw new \Exception(sprintf('Unknown role "%s"', $role)); break; } @@ -74,11 +70,6 @@ abstract class APIControllerBaseTest extends ControllerBaseTest return '/' . ltrim($url, '/') . ($json ? '.json' : ''); } - /** - * @param Client $client - * @param string $url - * @param string $method - */ protected function assertRequestIsSecured(Client $client, string $url, $method = 'GET') { $this->request($client, $url, $method); @@ -134,26 +125,13 @@ abstract class APIControllerBaseTest extends ControllerBaseTest ); } - /** - * @param Client $client - * @param string $url - * @param string $method - * @param array $parameters - * @param string $content - * @return Crawler - */ - protected function request(Client $client, string $url, $method = 'GET', array $parameters = [], string $content = null) + protected function request(Client $client, string $url, $method = 'GET', array $parameters = [], string $content = null): Crawler { $server = ['HTTP_CONTENT_TYPE' => 'application/json', 'CONTENT_TYPE' => 'application/json']; return $client->request($method, $this->createUrl($url), $parameters, [], $server, $content); } - /** - * @param string $role - * @param string $url - * @param string $method - */ protected function assertEntityNotFound(string $role, string $url, string $method = 'GET') { $client = $this->getClientForAuthenticatedUser($role); @@ -172,11 +150,6 @@ abstract class APIControllerBaseTest extends ControllerBaseTest ); } - /** - * @param string $role - * @param string $url - * @param array $data - */ protected function assertEntityNotFoundForPatch(string $role, string $url, array $data) { $client = $this->getClientForAuthenticatedUser($role); @@ -198,11 +171,6 @@ abstract class APIControllerBaseTest extends ControllerBaseTest ); } - /** - * @param string $role - * @param string $url - * @param array $data - */ protected function assertEntityNotFoundForDelete(string $role, string $url, array $data) { $client = $this->getClientForAuthenticatedUser($role); @@ -224,10 +192,6 @@ abstract class APIControllerBaseTest extends ControllerBaseTest ); } - /** - * @param Response $response - * @param string $message - */ protected function assertApiException(Response $response, string $message) { $this->assertFalse($response->isSuccessful()); @@ -235,22 +199,12 @@ abstract class APIControllerBaseTest extends ControllerBaseTest $this->assertEquals(['code' => 500, 'message' => $message], json_decode($response->getContent(), true)); } - /** - * @param Client $client - * @param string $url - * @param string $message - */ protected function assertApiAccessDenied(Client $client, string $url, string $message) { $this->request($client, $url); $this->assertApiResponseAccessDenied($client->getResponse(), $message); } - /** - * @param Client $client - * @param string $url - * @param string $message - */ protected function assertApiResponseAccessDenied(Response $response, string $message) { $this->assertFalse($response->isSuccessful()); diff --git a/tests/API/Model/I18nTest.php b/tests/API/Model/I18nTest.php index 6cd7e1cc..5ff52e58 100644 --- a/tests/API/Model/I18nTest.php +++ b/tests/API/Model/I18nTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\API; +namespace App\Tests\API\Model; use App\API\Model\I18n; use PHPUnit\Framework\TestCase; diff --git a/tests/Command/InstallCommandTest.php b/tests/Command/InstallCommandTest.php index 6eb3eb08..e3819045 100644 --- a/tests/Command/InstallCommandTest.php +++ b/tests/Command/InstallCommandTest.php @@ -13,6 +13,7 @@ use App\Command\InstallCommand; use App\Utils\File; use Symfony\Bundle\FrameworkBundle\Console\Application; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; /** @@ -26,7 +27,7 @@ class InstallCommandTest extends KernelTestCase */ protected $application; - protected function getCommand($permission = 0777): InstallCommand + protected function getCommand($permission = 0777): Command { $fileMock = $this->getMockBuilder(File::class)->setMethods(['getPermissions'])->getMock(); $fileMock->expects($this->exactly(5))->method('getPermissions')->willReturn($permission); diff --git a/tests/Configuration/LdapConfigurationTest.php b/tests/Configuration/LdapConfigurationTest.php new file mode 100644 index 00000000..f127b4a7 --- /dev/null +++ b/tests/Configuration/LdapConfigurationTest.php @@ -0,0 +1,52 @@ + true, + 'connection' => [ + 'host' => '1.2.3.4', + ], + 'user' => [ + 'foo' => 'bar', + ], + 'role' => [ + 'bar' => 'foo', + ], + ]; + } + + public function testMapping() + { + $sut = $this->getSut($this->getDefaultSettings()); + $this->assertTrue($sut->isActivated()); + $this->assertEquals(['foo' => 'bar'], $sut->getUserParameters()); + $this->assertEquals(['bar' => 'foo'], $sut->getRoleParameters()); + $this->assertEquals(['host' => '1.2.3.4'], $sut->getConnectionParameters()); + + $sut = $this->getSut(['active' => false]); + $this->assertFalse($sut->isActivated()); + } +} diff --git a/tests/Controller/ActivityControllerTest.php b/tests/Controller/ActivityControllerTest.php index 06367858..0bd8e87b 100644 --- a/tests/Controller/ActivityControllerTest.php +++ b/tests/Controller/ActivityControllerTest.php @@ -39,7 +39,7 @@ class ActivityControllerTest extends ControllerBaseTest $this->assertAccessIsGranted($client, '/admin/activity/create'); $form = $client->getCrawler()->filter('form[name=activity_edit_form]')->form(); $this->assertTrue($form->has('activity_edit_form[create_more]')); - $this->assertNull($form->get('activity_edit_form[create_more]')->getValue()); + $this->assertFalse($form->get('activity_edit_form[create_more]')->hasValue()); $client->submit($form, [ 'activity_edit_form' => [ 'name' => 'An AcTiVitY Name', @@ -87,6 +87,7 @@ class ActivityControllerTest extends ControllerBaseTest $this->assertTrue($client->getResponse()->isSuccessful()); $form = $client->getCrawler()->filter('form[name=activity_edit_form]')->form(); $this->assertTrue($form->has('activity_edit_form[create_more]')); + $this->assertTrue($form->get('activity_edit_form[create_more]')->hasValue()); $this->assertEquals(1, $form->get('activity_edit_form[create_more]')->getValue()); $this->assertEquals($selectedProject, $form->get('activity_edit_form[project]')->getValue()); } diff --git a/tests/Controller/ControllerBaseTest.php b/tests/Controller/ControllerBaseTest.php index cf900596..d2e0983f 100644 --- a/tests/Controller/ControllerBaseTest.php +++ b/tests/Controller/ControllerBaseTest.php @@ -95,12 +95,13 @@ abstract class ControllerBaseTest extends WebTestCase * @param string $url * @param string $method */ - protected function assertRequestIsSecured(Client $client, string $url, $method = 'GET') + protected function assertRequestIsSecured(Client $client, string $url, ?string $method = 'GET') { $this->request($client, $url, $method); - /* @var RedirectResponse $response */ + /** @var RedirectResponse $response */ $response = $client->getResponse(); + self::assertInstanceOf(RedirectResponse::class, $response); $this->assertTrue( $response->isRedirect(), @@ -153,32 +154,19 @@ abstract class ControllerBaseTest extends WebTestCase ); } - /** - * @param Client $client - * @param $url - * @param string $method - * @param array $parameters - */ - protected function assertAccessIsGranted(Client $client, $url, $method = 'GET', array $parameters = []) + protected function assertAccessIsGranted(Client $client, string $url, string $method = 'GET', array $parameters = []) { $this->request($client, $url, $method, $parameters); $this->assertTrue($client->getResponse()->isSuccessful()); } - /** - * @param Client $client - */ protected function assertRouteNotFound(Client $client) { $this->assertFalse($client->getResponse()->isSuccessful()); $this->assertEquals(404, $client->getResponse()->getStatusCode()); } - /** - * @param Client $client - * @param string $classname - */ - protected function assertMainContentClass(Client $client, $classname) + protected function assertMainContentClass(Client $client, string $classname) { $this->assertContains('
', $client->getResponse()->getContent()); } @@ -279,19 +267,11 @@ abstract class ControllerBaseTest extends WebTestCase $this->assertContains($message, $node->text()); } - /** - * @param Client $client - * @param string|null $message - */ protected function assertHasFlashDeleteSuccess(Client $client) { $this->assertHasFlashSuccess($client, 'Entry was deleted'); } - /** - * @param Client $client - * @param string|null $message - */ protected function assertHasFlashSaveSuccess(Client $client) { $this->assertHasFlashSuccess($client, 'Saved changes'); diff --git a/tests/Controller/ProjectControllerTest.php b/tests/Controller/ProjectControllerTest.php index 700d6288..0c50bca3 100644 --- a/tests/Controller/ProjectControllerTest.php +++ b/tests/Controller/ProjectControllerTest.php @@ -39,7 +39,7 @@ class ProjectControllerTest extends ControllerBaseTest $this->assertAccessIsGranted($client, '/admin/project/create'); $form = $client->getCrawler()->filter('form[name=project_edit_form]')->form(); $this->assertTrue($form->has('project_edit_form[create_more]')); - $this->assertNull($form->get('project_edit_form[create_more]')->getValue()); + $this->assertFalse($form->get('project_edit_form[create_more]')->hasValue()); $client->submit($form, [ 'project_edit_form' => [ 'name' => 'Test 2', @@ -80,6 +80,7 @@ class ProjectControllerTest extends ControllerBaseTest $this->assertTrue($client->getResponse()->isSuccessful()); $form = $client->getCrawler()->filter('form[name=project_edit_form]')->form(); $this->assertTrue($form->has('project_edit_form[create_more]')); + $this->assertTrue($form->get('project_edit_form[create_more]')->hasValue()); $this->assertEquals(1, $form->get('project_edit_form[create_more]')->getValue()); $this->assertEquals($selectedCustomer, $form->get('project_edit_form[customer]')->getValue()); } diff --git a/tests/Controller/TimesheetControllerTest.php b/tests/Controller/TimesheetControllerTest.php index 68027e3a..3bbe3764 100644 --- a/tests/Controller/TimesheetControllerTest.php +++ b/tests/Controller/TimesheetControllerTest.php @@ -38,7 +38,9 @@ class TimesheetControllerTest extends ControllerBaseTest foreach ($result as $item) { $this->assertContains('btn btn-default', $item->getAttribute('class')); - $this->assertEquals('i', $item->firstChild->tagName); + /** @var \DOMElement $domElement */ + $domElement = $item->firstChild; + $this->assertEquals('i', $domElement->tagName); } } diff --git a/tests/Controller/TimesheetTeamControllerTest.php b/tests/Controller/TimesheetTeamControllerTest.php index 88666542..b311ea95 100644 --- a/tests/Controller/TimesheetTeamControllerTest.php +++ b/tests/Controller/TimesheetTeamControllerTest.php @@ -39,7 +39,9 @@ class TimesheetTeamControllerTest extends ControllerBaseTest foreach ($result as $item) { $this->assertContains('btn btn-default', $item->getAttribute('class')); - $this->assertEquals('i', $item->firstChild->tagName); + /** @var \DOMElement $domElement */ + $domElement = $item->firstChild; + $this->assertEquals('i', $domElement->tagName); } } diff --git a/tests/Controller/UserControllerTest.php b/tests/Controller/UserControllerTest.php index 70a3d100..ef5080f2 100644 --- a/tests/Controller/UserControllerTest.php +++ b/tests/Controller/UserControllerTest.php @@ -40,7 +40,6 @@ class UserControllerTest extends ControllerBaseTest $form = $client->getCrawler()->filter('form[name=user_create]')->form(); $this->assertTrue($form->has('user_create[create_more]')); $this->assertFalse($form->get('user_create[create_more]')->hasValue()); - $this->assertNull($form->get('user_create[create_more]')->getValue()); $client->submit($form, [ 'user_create' => [ 'username' => $username, @@ -86,6 +85,7 @@ class UserControllerTest extends ControllerBaseTest $this->assertTrue($client->getResponse()->isSuccessful()); $form = $client->getCrawler()->filter('form[name=user_create]')->form(); $this->assertTrue($form->has('user_create[create_more]')); + $this->assertTrue($form->get('user_create[create_more]')->hasValue()); $this->assertEquals(1, $form->get('user_create[create_more]')->getValue()); } diff --git a/tests/DataFixtures/TagFixtures.php b/tests/DataFixtures/TagFixtures.php index 2ba62379..b5fb05e5 100644 --- a/tests/DataFixtures/TagFixtures.php +++ b/tests/DataFixtures/TagFixtures.php @@ -54,11 +54,7 @@ class TagFixtures extends Fixture $manager->flush(); } - /** - * @param $tagName - * @return Tag - */ - protected function createTagEntry($tagName) + protected function createTagEntry(string $tagName): Tag { $tagObject = new Tag(); $tagObject->setName($tagName); diff --git a/tests/DataFixtures/TimesheetFixtures.php b/tests/DataFixtures/TimesheetFixtures.php index 7b15e958..28e6096b 100644 --- a/tests/DataFixtures/TimesheetFixtures.php +++ b/tests/DataFixtures/TimesheetFixtures.php @@ -58,7 +58,7 @@ class TimesheetFixtures extends Fixture */ protected $allowEmptyDescriptions = true; /** - * @var int + * @var bool */ protected $exported = false; /** @@ -265,11 +265,7 @@ class TimesheetFixtures extends Fixture $manager->flush(); } - /** - * @param $cnt - * @return array - */ - protected function getTagObjectList($cnt) + protected function getTagObjectList(int $cnt): array { if (true === $this->useTags) { $tagObject = new Tag(); @@ -281,11 +277,7 @@ class TimesheetFixtures extends Fixture return []; } - /** - * @param $i - * @return bool|\DateTime - */ - protected function getDateTime($i) + protected function getDateTime(int $i): \DateTime { $start = \DateTime::createFromFormat('Y-m-d', $this->startDate); $start->modify("+ $i days"); diff --git a/tests/DependencyInjection/AppExtensionTest.php b/tests/DependencyInjection/AppExtensionTest.php new file mode 100644 index 00000000..a825f978 --- /dev/null +++ b/tests/DependencyInjection/AppExtensionTest.php @@ -0,0 +1,316 @@ +extension = new AppExtension(); + } + + /** + * @return ContainerBuilder + */ + private function getContainer() + { + $container = new ContainerBuilder(); + + return $container; + } + + protected function getMinConfig() + { + return [ + 'kimai' => [ + 'data_dir' => '/tmp/', + 'plugin_dir' => '/tmp/', + 'timesheet' => [], + ] + ]; + } + + public function testDefaultValues() + { + $minConfig = $this->getMinConfig(); + + $this->extension->load($minConfig, $container = $this->getContainer()); + + $expected = [ + 'kimai.data_dir' => '/tmp/', + 'kimai.plugin_dir' => '/tmp/', + 'kimai.languages' => [], + 'kimai.calendar' => [ + 'week_numbers' => true, + 'day_limit' => 4, + 'businessHours' => [ + 'days' => [1, 2, 3, 4, 5], + 'begin' => '08:00', + 'end' => '20:00', + ], + 'visibleHours' => [ + 'begin' => '00:00', + 'end' => '24:00', + ], + 'google' => [ + 'api_key' => null, + 'sources' => [], + ], + 'weekends' => true + ], + 'kimai.dashboard' => [], + 'kimai.widgets' => [], + 'kimai.invoice.documents' => [ + 'var/invoices/', + 'templates/invoice/renderer/', + ], + 'kimai.defaults' => [ + 'customer' => [ + 'timezone' => 'Europe/Berlin', + 'country' => 'DE', + 'currency' => 'EUR', + ] + ], + + 'kimai.theme' => [ + 'active_warning' => 3, + 'box_color' => 'green', + 'select_type' => null, + 'show_about' => true, + ], + 'kimai.theme.select_type' => null, + 'kimai.theme.show_about' => true, + + 'kimai.fosuser' => [ + 'registration' => true, + 'password_reset' => true, + ], + + 'kimai.timesheet' => [ + 'mode' => 'default', + 'markdown_content' => false, + 'rounding' => [], + 'rates' => [], + 'active_entries' => [ + 'soft_limit' => 1, + 'hard_limit' => 1, + ], + 'rules' => [ + 'allow_future_times' => true, + ], + ], + 'kimai.timesheet.rates' => [], + 'kimai.timesheet.rounding' => [], + 'kimai.ldap' => [ + 'active' => false, + 'user' => [ + 'baseDn' => null, + 'filter' => '', + 'usernameAttribute' => 'uid', + 'attributes' => [], + ], + 'role' => [ + 'baseDn' => null, + 'nameAttribute' => 'cn', + 'userDnAttribute' => 'member', + 'groups' => [], + 'usernameAttribute' => 'dn', + ], + 'connection' => [ + 'baseDn' => null, + 'host' => null, + 'port' => 389, + 'useStartTls' => false, + 'useSsl' => false, + 'bindRequiresDn' => true, + 'accountFilterFormat' => '(&(uid=%s))', + ], + ], + 'kimai.permissions' => [ + 'ROLE_USER' => [], + 'ROLE_TEAMLEAD' => [], + 'ROLE_ADMIN' => [], + 'ROLE_SUPER_ADMIN' => [], + ], + ]; + + // nasty parameter, should be removed!!! + $this->assertTrue($container->hasParameter('kimai.config')); + + foreach ($expected as $key => $value) { + $this->assertTrue($container->hasParameter($key), 'Could not find config: ' . $key); + $this->assertEquals($value, $container->getParameter($key), 'Invalid config: ' . $key); + } + } + + public function testAdditionalAuthenticationRoutes() + { + $minConfig = $this->getMinConfig(); + $adminLte = [ + 'adminlte_registration' => 'foo', + 'adminlte_password_reset' => 'bar', + ]; + + $container = $this->getContainer(); + $container->setParameter('admin_lte_theme.routes', $adminLte); + + $this->extension->load($minConfig, $container); + + $this->assertEquals( + [ + 'adminlte_registration' => 'foo', + 'adminlte_password_reset' => 'bar', + ], + $container->getParameter('admin_lte_theme.routes') + ); + } + + public function testDeactivateAdditionalAuthenticationRoutes() + { + $minConfig = $this->getMinConfig(); + $minConfig['kimai']['user'] = [ + 'registration' => false, + 'password_reset' => false, + ]; + $adminLte = [ + 'adminlte_registration' => 'foo', + 'adminlte_password_reset' => 'bar', + ]; + + $container = $this->getContainer(); + $container->setParameter('admin_lte_theme.routes', $adminLte); + + $this->extension->load($minConfig, $container); + + $this->assertEquals( + [ + 'adminlte_registration' => null, + 'adminlte_password_reset' => null, + ], + $container->getParameter('admin_lte_theme.routes') + ); + } + + /** + * @expectedException \PHPUnit\Framework\Error\Notice + * @expectedExceptionMessage Found ambiguous configuration. Please remove "kimai.timesheet.duration_only" and set "kimai.timesheet.mode" instead. + * @expectedDeprecation Configuration "kimai.timesheet.duration_only" is deprecated, please remove it + * @group legacy + */ + public function testDurationOnlyDeprecationIsTriggered() + { + $minConfig = $this->getMinConfig(); + $minConfig['kimai']['timesheet']['duration_only'] = true; + $minConfig['kimai']['timesheet']['mode'] = 'punch'; + + $this->extension->load($minConfig, $container = $this->getContainer()); + } + + public function testLdapDefaultValues() + { + $minConfig = $this->getMinConfig(); + $minConfig['kimai']['ldap'] = [ + 'active' => true, + 'connection' => [ + 'host' => '9.9.9.9', + 'baseDn' => 'lkhiuzhkj', + 'accountFilterFormat' => '(uid=%s)' + ], + 'user' => [ + 'baseDn' => '123123123', + 'usernameAttribute' => 'xxx', + 'filter' => '(..........)' + ], + ]; + + $this->extension->load($minConfig, $container = $this->getContainer()); + + $ldapConfig = $container->getParameter('kimai.ldap'); + $this->assertEquals('123123123', $ldapConfig['user']['baseDn']); + $this->assertEquals('(..........)', $ldapConfig['user']['filter']); + $this->assertEquals('xxx', $ldapConfig['user']['usernameAttribute']); + $this->assertEquals('lkhiuzhkj', $ldapConfig['connection']['baseDn']); + $this->assertEquals('(uid=%s)', $ldapConfig['connection']['accountFilterFormat']); + } + + public function testLdapFallbackValue() + { + $minConfig = $this->getMinConfig(); + $minConfig['kimai']['ldap'] = [ + 'active' => true, + 'connection' => [ + 'host' => '9.9.9.9', + ], + 'user' => [ + 'baseDn' => '123123123', + 'usernameAttribute' => 'xxx', + ], + ]; + + $this->extension->load($minConfig, $container = $this->getContainer()); + + $ldapConfig = $container->getParameter('kimai.ldap'); + $this->assertEquals('123123123', $ldapConfig['user']['baseDn']); + $this->assertEquals('xxx', $ldapConfig['user']['usernameAttribute']); + $this->assertEquals('123123123', $ldapConfig['connection']['baseDn']); + $this->assertEquals('(&(xxx=%s))', $ldapConfig['connection']['accountFilterFormat']); + $this->assertEquals('', $ldapConfig['user']['filter']); + } + + public function testLdapMoreFallbackValue() + { + $minConfig = $this->getMinConfig(); + $minConfig['kimai']['ldap'] = [ + 'active' => true, + 'connection' => [ + 'host' => '9.9.9.9', + 'baseDn' => '7658765', + ], + 'user' => [ + 'baseDn' => '123123123', + 'usernameAttribute' => 'zzzz', + 'filter' => '(&(objectClass=inetOrgPerson))', + ], + ]; + + $this->extension->load($minConfig, $container = $this->getContainer()); + + $ldapConfig = $container->getParameter('kimai.ldap'); + $this->assertEquals('123123123', $ldapConfig['user']['baseDn']); + $this->assertEquals('zzzz', $ldapConfig['user']['usernameAttribute']); + $this->assertEquals('7658765', $ldapConfig['connection']['baseDn']); + $this->assertEquals('(&(&(objectClass=inetOrgPerson))(zzzz=%s))', $ldapConfig['connection']['accountFilterFormat']); + $this->assertEquals('(&(objectClass=inetOrgPerson))', $ldapConfig['user']['filter']); + } + + /** + * @expectedException \PHPUnit\Framework\Error\Notice + * @expectedExceptionMessage Found invalid "kimai" configuration: The child node "data_dir" at path "kimai" must be configured. + */ + public function testInvalidConfiguration() + { + $this->extension->load([], $container = $this->getContainer()); + } + + // TODO test permissions +} diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php new file mode 100644 index 00000000..b764f63c --- /dev/null +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -0,0 +1,246 @@ + $dataDir, + 'plugin_dir' => $pluginDir, + 'timesheet' => [], + ]; + } + + protected function assertConfig($inputConfig, $expectedConfig) + { + $finalizedConfig = $this->getCompiledConfig($inputConfig); + + self::assertEquals($expectedConfig, $finalizedConfig); + } + + protected function getCompiledConfig($inputConfig) + { + $configuration = new Configuration(); + + $node = $configuration->getConfigTreeBuilder()->buildTree(); + $normalizedConfig = $node->normalize($inputConfig); + + return $node->finalize($normalizedConfig); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.data_dir": Data directory does not exist + */ + public function testValidateDataDir() + { + $this->assertConfig($this->getMinConfig('sdfsdfsdfds'), []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.plugin_dir": Plugin directory does not exist + */ + public function testValidatePluginDir() + { + $this->assertConfig($this->getMinConfig('/tmp/', 'sdfsdfs'), []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap": The "ldap.user.baseDn" config must be set if LDAP is activated. + */ + public function testValidateLdapConfigUserBaseDn() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => true, + 'connection' => [ + 'host' => 'foo' + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap": The "ldap.connection.host" config must be set if LDAP is activated. + */ + public function testValidateLdapConfigConnectionHost() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => true, + 'connection' => [ + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap.connection": The ldap.connection.useSsl and ldap.connection.useStartTls options are mutually exclusive. + */ + public function testValidateLdapConfig() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => false, + 'connection' => [ + 'useSsl' => true, + 'useStartTls' => true, + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap.user.filter": The ldap.user.filter must be enclosed by a matching number of parentheses "()" and must NOT contain a "%s" replacer + */ + public function testValidateLdapFilterIncludingReplacer() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => true, + 'user' => [ + 'filter' => '(sdfsdfsdf)(uid=%s)', + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap.user.filter": The ldap.user.filter must be enclosed by a matching number of parentheses "()" and must NOT contain a "%s" replacer + */ + public function testValidateLdapFilterMissingStartingParenthesis() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => true, + 'user' => [ + 'filter' => 's(dfsdfsdf)', + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap.user.filter": The ldap.user.filter must be enclosed by a matching number of parentheses "()" and must NOT contain a "%s" replacer + */ + public function testValidateLdapFilterInvalidParenthesisCounter() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => true, + 'user' => [ + 'filter' => '(dfsdfsdf))', + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap.connection.accountFilterFormat": The accountFilterFormat must be enclosed by a matching number of parentheses "()" and contain one "%s" replacer for the username + */ + public function testValidateLdapAccountFilterFormatMissingUserAttributeReplacer() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => false, + 'connection' => [ + 'accountFilterFormat' => '(sdfsdfsdf)(uid=xx)', + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap.connection.accountFilterFormat": The accountFilterFormat must be enclosed by a matching number of parentheses "()" and contain one "%s" replacer for the username + */ + public function testValidateLdapAccountFilterFormatMissingStartingParenthesis() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => true, + 'connection' => [ + 'accountFilterFormat' => 's(dfsdfsdf)', + ], + ]; + + $this->assertConfig($config, []); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "kimai.ldap.connection.accountFilterFormat": The accountFilterFormat must be enclosed by a matching number of parentheses "()" and contain one "%s" replacer for the username + */ + public function testValidateLdapAccountFilterFormatInvalidParenthesisCounter() + { + $config = $this->getMinConfig(); + $config['ldap'] = [ + 'active' => true, + 'connection' => [ + 'accountFilterFormat' => '(dfsdfsdf))', + ], + ]; + + $this->assertConfig($config, []); + } + + public function testDefaultLdapSettings() + { + $finalizedConfig = $this->getCompiledConfig($this->getMinConfig()); + $expected = [ + 'active' => false, + 'user' => [ + 'baseDn' => '', + 'filter' => '', + 'usernameAttribute' => 'uid', + 'attributes' => [], + ], + 'role' => [ + 'baseDn' => null, + 'usernameAttribute' => 'dn', + 'nameAttribute' => 'cn', + 'userDnAttribute' => 'member', + 'groups' => [], + ], + 'connection' => [ + 'host' => null, + 'port' => 389, + 'useStartTls' => false, + 'useSsl' => false, + 'bindRequiresDn' => true, + 'accountFilterFormat' => '', + ] + ]; + self::assertEquals($expected, $finalizedConfig['ldap']); + } +} diff --git a/tests/Doctrine/SqliteSessionInitSubscriberTest.php b/tests/Doctrine/SqliteSessionInitSubscriberTest.php index ce52a6c4..1acc9d6a 100644 --- a/tests/Doctrine/SqliteSessionInitSubscriberTest.php +++ b/tests/Doctrine/SqliteSessionInitSubscriberTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\EventSubscriber; +namespace App\Tests\Doctrine; use App\Doctrine\SqliteSessionInitSubscriber; use Doctrine\DBAL\Connection; diff --git a/tests/Entity/AbstractEntityTest.php b/tests/Entity/AbstractEntityTest.php index 337aed91..084092e3 100644 --- a/tests/Entity/AbstractEntityTest.php +++ b/tests/Entity/AbstractEntityTest.php @@ -18,10 +18,10 @@ use Symfony\Component\Validator\ConstraintViolationInterface; abstract class AbstractEntityTest extends KernelTestCase { /** - * @param $entity + * @param object $entity * @param array|string $fieldNames */ - protected function assertHasViolationForField($entity, $fieldNames) + protected function assertHasViolationForField(object $entity, $fieldNames) { self::bootKernel(); $validator = static::$kernel->getContainer()->get('validator'); diff --git a/tests/Entity/ActivityTest.php b/tests/Entity/ActivityTest.php index 5f78bf60..309cb996 100644 --- a/tests/Entity/ActivityTest.php +++ b/tests/Entity/ActivityTest.php @@ -24,7 +24,7 @@ class ActivityTest extends AbstractEntityTest $this->assertNull($sut->getName()); $this->assertNull($sut->getComment()); $this->assertTrue($sut->getVisible()); - // timesheets + self::assertIsIterable($sut->getTimesheets()); $this->assertNull($sut->getFixedRate()); $this->assertNull($sut->getHourlyRate()); $this->assertNull($sut->getColor()); diff --git a/tests/Entity/CustomerTest.php b/tests/Entity/CustomerTest.php index 5cad6288..5939e9a3 100644 --- a/tests/Entity/CustomerTest.php +++ b/tests/Entity/CustomerTest.php @@ -10,7 +10,6 @@ namespace App\Tests\Entity; use App\Entity\Customer; -use App\Entity\Project; /** * @covers \App\Entity\Customer @@ -24,7 +23,8 @@ class CustomerTest extends AbstractEntityTest $this->assertNull($sut->getName()); $this->assertNull($sut->getNumber()); $this->assertNull($sut->getComment()); - // projects + self::assertIsIterable($sut->getProjects()); + self::assertEmpty($sut->getProjects()); $this->assertTrue($sut->getVisible()); $this->assertNull($sut->getCompany()); @@ -61,10 +61,6 @@ class CustomerTest extends AbstractEntityTest $this->assertInstanceOf(Customer::class, $sut->setColor('#fffccc')); $this->assertEquals('#fffccc', $sut->getColor()); - $projects = [(new Project())->setName('Test')]; - $this->assertInstanceOf(Customer::class, $sut->setProjects($projects)); - $this->assertSame($projects, $sut->getProjects()); - $this->assertInstanceOf(Customer::class, $sut->setCompany('test company')); $this->assertEquals('test company', $sut->getCompany()); diff --git a/tests/Entity/ProjectTest.php b/tests/Entity/ProjectTest.php index 9628df86..41f69e55 100644 --- a/tests/Entity/ProjectTest.php +++ b/tests/Entity/ProjectTest.php @@ -9,10 +9,8 @@ namespace App\Tests\Entity; -use App\Entity\Activity; use App\Entity\Customer; use App\Entity\Project; -use App\Entity\Timesheet; /** * @covers \App\Entity\Project @@ -29,10 +27,12 @@ class ProjectTest extends AbstractEntityTest $this->assertNull($sut->getComment()); $this->assertTrue($sut->getVisible()); $this->assertEquals(0.0, $sut->getBudget()); - // activities $this->assertNull($sut->getFixedRate()); $this->assertNull($sut->getHourlyRate()); - $this->assertNull($sut->getTimesheets()); + self::assertIsIterable($sut->getTimesheets()); + self::assertEmpty($sut->getTimesheets()); + self::assertIsIterable($sut->getActivities()); + self::assertEmpty($sut->getActivities()); $this->assertNull($sut->getColor()); } @@ -62,17 +62,9 @@ class ProjectTest extends AbstractEntityTest $this->assertInstanceOf(Project::class, $sut->setBudget(12345.67)); $this->assertEquals(12345.67, $sut->getBudget()); - $activities = [(new Activity())->setName('foo')]; - $this->assertInstanceOf(Project::class, $sut->setActivities($activities)); - $this->assertSame($activities, $sut->getActivities()); - $this->assertInstanceOf(Project::class, $sut->setFixedRate(13.47)); $this->assertEquals(13.47, $sut->getFixedRate()); $this->assertInstanceOf(Project::class, $sut->setHourlyRate(99)); $this->assertEquals(99, $sut->getHourlyRate()); - - $timesheets = [(new Timesheet())->setDescription('foo'), (new Timesheet())->setDescription('bar')]; - $this->assertInstanceOf(Project::class, $sut->setTimesheets($timesheets)); - $this->assertSame($timesheets, $sut->getTimesheets()); } } diff --git a/tests/Entity/UserTest.php b/tests/Entity/UserTest.php index b47a5e39..f6910cd2 100644 --- a/tests/Entity/UserTest.php +++ b/tests/Entity/UserTest.php @@ -116,6 +116,13 @@ class UserTest extends AbstractEntityTest $user->addPreference($preference); $this->assertEquals('foobar', $user->getPreferenceValue('test', 'foo')); $this->assertEquals($preference, $user->getPreference('test')); + + $user->setPreferenceValue('test', 'Hello World'); + $this->assertEquals('Hello World', $user->getPreferenceValue('test', 'foo')); + + $this->assertNull($user->getPreferenceValue('test2')); + $user->setPreferenceValue('test2', 'I like rain'); + $this->assertEquals('I like rain', $user->getPreferenceValue('test2')); } public function testToString() diff --git a/tests/Event/ConfigureAdminMenuEventTest.php b/tests/Event/ConfigureAdminMenuEventTest.php index a76d07ac..6fd5fb7e 100644 --- a/tests/Event/ConfigureAdminMenuEventTest.php +++ b/tests/Event/ConfigureAdminMenuEventTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\EventSubscriber; +namespace App\Tests\Event; use App\Event\ConfigureAdminMenuEvent; use KevinPapst\AdminLTEBundle\Model\MenuItemModel; diff --git a/tests/Event/DashboardEventTest.php b/tests/Event/DashboardEventTest.php index 5929e0e0..5f04f5e1 100644 --- a/tests/Event/DashboardEventTest.php +++ b/tests/Event/DashboardEventTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\EventSubscriber; +namespace App\Tests\Event; use App\Entity\User; use App\Event\DashboardEvent; diff --git a/tests/Event/ThemeEventTest.php b/tests/Event/ThemeEventTest.php index 82713725..785673b7 100644 --- a/tests/Event/ThemeEventTest.php +++ b/tests/Event/ThemeEventTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\EventSubscriber; +namespace App\Tests\Event; use App\Entity\User; use App\Event\ThemeEvent; diff --git a/tests/Event/UserPreferenceEventTest.php b/tests/Event/UserPreferenceEventTest.php index 38d3198b..e006fe10 100644 --- a/tests/Event/UserPreferenceEventTest.php +++ b/tests/Event/UserPreferenceEventTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\EventSubscriber; +namespace App\Tests\Event; use App\Entity\User; use App\Entity\UserPreference; diff --git a/tests/Form/DataTransformer/TagArrayToStringTransformerTest.php b/tests/Form/DataTransformer/TagArrayToStringTransformerTest.php index 8ea26f27..65a42492 100644 --- a/tests/Form/DataTransformer/TagArrayToStringTransformerTest.php +++ b/tests/Form/DataTransformer/TagArrayToStringTransformerTest.php @@ -7,16 +7,17 @@ * file that was distributed with this source code. */ -namespace App\Tests\Export\Renderer; +namespace App\Tests\Form\DataTransformer; use App\Entity\Tag; use App\Form\DataTransformer\TagArrayToStringTransformer; use App\Repository\TagRepository; +use PHPUnit\Framework\TestCase; /** * @covers \App\Form\DataTransformer\TagArrayToStringTransformer */ -class TagArrayToStringTransformerTest extends AbstractRendererTest +class TagArrayToStringTransformerTest extends TestCase { public function testTransform() { diff --git a/tests/Invoice/Calculator/AbstractInvoiceCalculatorTest.php b/tests/Invoice/Calculator/AbstractCalculatorTest.php similarity index 100% rename from tests/Invoice/Calculator/AbstractInvoiceCalculatorTest.php rename to tests/Invoice/Calculator/AbstractCalculatorTest.php diff --git a/tests/Invoice/NumberGenerator/DateNumberGeneratorTest.php b/tests/Invoice/NumberGenerator/DateNumberGeneratorTest.php index 00e577d9..b8feb07c 100644 --- a/tests/Invoice/NumberGenerator/DateNumberGeneratorTest.php +++ b/tests/Invoice/NumberGenerator/DateNumberGeneratorTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\Invoice\Calculator; +namespace App\Tests\Invoice\NumberGenerator; use App\Invoice\NumberGenerator\DateNumberGenerator; use App\Model\InvoiceModel; diff --git a/tests/Invoice/Renderer/AbstractRendererTest.php b/tests/Invoice/Renderer/AbstractRendererTest.php index bcbf348b..bab7be0e 100644 --- a/tests/Invoice/Renderer/AbstractRendererTest.php +++ b/tests/Invoice/Renderer/AbstractRendererTest.php @@ -190,11 +190,7 @@ abstract class AbstractRendererTest extends KernelTestCase return $model; } - /** - * @param Timesheet[] $timesheets - * @return InvoiceModel - */ - protected function getInvoiceModelOneEntry() + protected function getInvoiceModelOneEntry(): InvoiceModel { $customer = new Customer(); $customer->setCurrency('USD'); diff --git a/tests/Invoice/Renderer/DebugRenderer.php b/tests/Invoice/Renderer/DebugRenderer.php index c5dde25e..380273d6 100644 --- a/tests/Invoice/Renderer/DebugRenderer.php +++ b/tests/Invoice/Renderer/DebugRenderer.php @@ -54,7 +54,7 @@ class DebugRenderer implements RendererInterface } /** - * @param $amount + * @param mixed $amount * @return mixed */ protected function getFormattedMoney($amount) @@ -72,7 +72,7 @@ class DebugRenderer implements RendererInterface } /** - * @param $seconds + * @param mixed $seconds * @return mixed */ protected function getFormattedDuration($seconds) diff --git a/tests/Ldap/FormLoginLdapFactoryTest.php b/tests/Ldap/FormLoginLdapFactoryTest.php new file mode 100644 index 00000000..f4d821c9 --- /dev/null +++ b/tests/Ldap/FormLoginLdapFactoryTest.php @@ -0,0 +1,50 @@ +getKey()); + self::assertEquals('pre_auth', $sut->getPosition()); + } + + public function testCreate() + { + $container = new ContainerBuilder(); + $sut = new FormLoginLdapFactory(); + $result = $sut->create($container, 'test', ['foo' => 'bar'], 'fosuserbundle', 'secured_area'); + + self::assertEquals([ + 'kimai_ldap.security.authentication.provider.test', + 'security.authentication.listener.form.test', + 'secured_area' + ], $result); + + $definition = $container->getDefinition('kimai_ldap.security.authentication.provider.test'); + self::assertInstanceOf(ChildDefinition::class, $definition); + self::assertEquals('test', $definition->getArguments()['index_1']); + + $definition = $container->getDefinition('security.authentication.listener.form.test'); + self::assertInstanceOf(ChildDefinition::class, $definition); + self::assertEquals('test', $definition->getArguments()['index_4']); + self::assertEquals(['foo' => 'bar'], $definition->getArguments()['index_5']); + } +} diff --git a/tests/Ldap/LdapAuthenticationProviderTest.php b/tests/Ldap/LdapAuthenticationProviderTest.php new file mode 100644 index 00000000..42b4eea7 --- /dev/null +++ b/tests/Ldap/LdapAuthenticationProviderTest.php @@ -0,0 +1,238 @@ +getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => false]); + $userProvider = new LdapUserProvider($manager, $config); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', 'bar', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $result = $sut->supports($token); + self::assertFalse($result); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationException + * @expectedExceptionMessage The token is not supported by this authentication provider. + */ + public function testDeactivatedAuthenticate() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => false]); + $userProvider = new LdapUserProvider($manager, $config); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', 'bar', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $sut->authenticate($token); + } + + public function testSupports() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => true]); + $userProvider = new LdapUserProvider($manager, $config); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', 'bar', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $result = $sut->supports($token); + self::assertTrue($result); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException + * @expectedExceptionMessage The password in the token is empty. Check `erase_credentials` in your `security.yaml` + */ + public function testAuthenticateWithTokenUserButEnptyPasswordThrowsException() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => true]); + $userProvider = new LdapUserProvider($manager, $config); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $user = (new User())->setUsername('foo')->setEnabled(true); + $token = new UsernamePasswordToken($user, '', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $actual = $sut->authenticate($token); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException + * @expectedExceptionMessage The presented password cannot be empty. + */ + public function testAuthenticateWithUsernameReturnsUser() + { + $user = (new User())->setUsername('foo')->setEnabled(true); + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => true]); + $userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->setMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', '', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $actual = $sut->authenticate($token); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException + * @expectedExceptionMessage The presented password is invalid. + */ + public function testAuthenticateWithUsernameThrowsExceptionOnFailedBind() + { + $user = (new User())->setUsername('foo')->setEnabled(true); + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->setMethods(['bind'])->getMock(); + $manager->expects($this->once())->method('bind')->willReturn(false); + $config = new LdapConfiguration(['active' => true]); + $userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->setMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', 'sdfsdf', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $actual = $sut->authenticate($token); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException + * @expectedExceptionMessage The credentials were changed from another session. + */ + public function testAuthenticateWithUserThrowsExceptionOnFailedBind() + { + $user = (new User())->setUsername('foo')->setEnabled(true); + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->setMethods(['bind'])->getMock(); + $manager->expects($this->once())->method('bind')->willReturn(false); + $config = new LdapConfiguration(['active' => true]); + $userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->setMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->never())->method('loadUserByUsername'); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken($user, 'sdfsdf', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $actual = $sut->authenticate($token); + } + + public function testAuthenticateWithUsernameReturnsUserAndBinds() + { + $user = (new User())->setUsername('foo')->setEnabled(true); + $user->setPreferenceValue('ldap.dn', 'blub'); + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->setMethods(['bind', 'updateUser'])->getMock(); + $manager->expects($this->once())->method('bind')->willReturn(true); + $manager->expects($this->once())->method('updateUser')->willReturnCallback(function ($updateUser) use ($user) { + self::assertSame($updateUser, $user); + }); + $config = new LdapConfiguration(['active' => true]); + $userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->setMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', 'test', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $token = $sut->authenticate($token); + self::assertSame($token->getUser(), $user); + } + + public function testAuthenticateWithUserReturnsUserAndBinds() + { + $user = (new User())->setUsername('foo')->setEnabled(true); + $user->setPreferenceValue('ldap.dn', 'blub'); + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->setMethods(['bind', 'updateUser'])->getMock(); + $manager->expects($this->once())->method('bind')->willReturn(true); + $manager->expects($this->once())->method('updateUser')->willReturnCallback(function ($updateUser) use ($user) { + self::assertSame($updateUser, $user); + }); + $config = new LdapConfiguration(['active' => true]); + $userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->setMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->never())->method('loadUserByUsername'); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken($user, 'test', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $token = $sut->authenticate($token); + self::assertSame($token->getUser(), $user); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\UsernameNotFoundException + * @expectedExceptionMessage blub foo bar + */ + public function testAuthenticateThrowsExceptionOnLdapNotFound() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => true]); + $userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->setMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new UsernameNotFoundException('blub foo bar')); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', 'test', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $sut->authenticate($token); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationServiceException + * @expectedExceptionMessage server away + * @expectedExceptionCode 1234 + */ + public function testAuthenticateThrowsExceptionOnLdapDown() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => true]); + $userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->setMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new \Exception('server away', 1234)); + $providerKey = 'secured_area'; + $userChecker = new UserChecker(); + + $token = new UsernamePasswordToken('foo', 'test', $providerKey); + + $sut = new LdapAuthenticationProvider($userChecker, $providerKey, $userProvider, $manager, $config, false); + $sut->authenticate($token); + } +} diff --git a/tests/Ldap/LdapDriverExceptionTest.php b/tests/Ldap/LdapDriverExceptionTest.php new file mode 100644 index 00000000..50e52367 --- /dev/null +++ b/tests/Ldap/LdapDriverExceptionTest.php @@ -0,0 +1,28 @@ +getMessage()); + } +} diff --git a/tests/Ldap/LdapDriverTest.php b/tests/Ldap/LdapDriverTest.php new file mode 100644 index 00000000..b7356e37 --- /dev/null +++ b/tests/Ldap/LdapDriverTest.php @@ -0,0 +1,71 @@ +getMockBuilder(Ldap::class)->disableOriginalConstructor()->setMethods(['bind'])->getMock(); + $zendLdap->expects($this->once())->method('bind')->willReturnSelf(); + + $user = new User(); + $sut = new LdapDriver($zendLdap); + $result = $sut->bind($user, 'test123'); + self::assertTrue($result); + } + + public function testBindException() + { + $zendLdap = $this->getMockBuilder(Ldap::class)->disableOriginalConstructor()->setMethods(['bind'])->getMock(); + $zendLdap->expects($this->once())->method('bind')->willThrowException(new LdapException()); + + $user = new User(); + $sut = new LdapDriver($zendLdap); + $result = $sut->bind($user, 'test123'); + self::assertFalse($result); + } + + public function testSearchSuccess() + { + $zendLdap = $this->getMockBuilder(Ldap::class)->disableOriginalConstructor()->setMethods(['bind', 'searchEntries'])->getMock(); + $zendLdap->expects($this->once())->method('bind'); + $zendLdap->expects($this->once())->method('searchEntries')->willReturn([1, 2, 3]); + + $sut = new LdapDriver($zendLdap); + $result = $sut->search('', '', []); + self::assertEquals(['count' => 3, 1, 2, 3], $result); + } + + /** + * @expectedException \App\Ldap\LdapDriverException + * @expectedExceptionMessage An error occurred with the search operation. + */ + public function testSearchException() + { + $zendLdap = $this->getMockBuilder(Ldap::class)->disableOriginalConstructor()->setMethods(['bind', 'searchEntries'])->getMock(); + $zendLdap->expects($this->once())->method('bind'); + $zendLdap->expects($this->once())->method('searchEntries')->willThrowException( + new LdapException($zendLdap, '', LdapException::LDAP_SERVER_DOWN) + ); + + $sut = new LdapDriver($zendLdap); + $sut->search('', '', []); + } +} diff --git a/tests/Ldap/LdapManagerTest.php b/tests/Ldap/LdapManagerTest.php new file mode 100644 index 00000000..7864b8ae --- /dev/null +++ b/tests/Ldap/LdapManagerTest.php @@ -0,0 +1,454 @@ + 'ou=groups, dc=kimai, dc=org', + 'nameAttribute' => 'cn', + 'userDnAttribute' => 'member', + 'groups' => [ + ['ldap_value' => 'group1', 'role' => 'ROLE_TEAMLEAD'], + ['ldap_value' => 'group2', 'role' => 'ROLE_ADMIN'], + ['ldap_value' => 'group3', 'role' => 'ROLE_CUSTOMER'], // not existing! + ['ldap_value' => 'group4', 'role' => 'ROLE_SUPER_ADMIN'], + ], + ]; + } + + $config = new LdapConfiguration([ + 'user' => [ + 'attributes' => [], + 'filter' => '(&(objectClass=inetOrgPerson))', + 'usernameAttribute' => 'uid', + 'baseDn' => 'ou=users, dc=kimai, dc=org', + ], + 'role' => $roleConfig, + ]); + + $hydrator = new LdapUserHydrator($config, new RoleService([ + 'ROLE_TEAMLEAD' => ['ROLE_USER'], + 'ROLE_ADMIN' => ['ROLE_TEAMLEAD'], + 'ROLE_SUPER_ADMIN' => ['ROLE_ADMIN'] + ])); + + return new LdapManager($driver, $hydrator, $config); + } + + public function testFindUserByUsernameOnZeroResults() + { + $expected = [ + 'count' => 0 + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->once())->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + self::assertEquals('ou=users, dc=kimai, dc=org', $baseDn); + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foo))', $filter); + + return $expected; + }); + + $sut = $this->getLdapManager($driver); + $actual = $sut->findUserByUsername('foo'); + self::assertNull($actual); + } + + /** + * @expectedException \App\Ldap\LdapDriverException + * @expectedExceptionMessage This search must only return a single user + */ + public function testFindUserByUsernameOnMultiResults() + { + $expected = [ + 'count' => 3 + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->once())->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + self::assertEquals('ou=users, dc=kimai, dc=org', $baseDn); + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foo))', $filter); + + return $expected; + }); + + $sut = $this->getLdapManager($driver); + $sut->findUserByUsername('foo'); + } + + public function testFindUserByUsernameOnValidResult() + { + $expected = [ + 0 => ['dn' => 'foo'], + 'count' => 1, + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->once())->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + self::assertEquals('ou=users, dc=kimai, dc=org', $baseDn); + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foo))', $filter); + + return $expected; + }); + + $sut = $this->getLdapManager($driver); + $actual = $sut->findUserByUsername('foo'); + self::assertInstanceOf(User::class, $actual); + } + + public function testFindUserByOnZeroResults() + { + $expected = [ + 'count' => 0 + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->once())->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + self::assertEquals('ou=users, dc=kimai, dc=org', $baseDn); + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foo))', $filter); + + return $expected; + }); + + $sut = $this->getLdapManager($driver); + $actual = $sut->findUserBy(['uid' => 'foo']); + self::assertNull($actual); + } + + /** + * @expectedException \App\Ldap\LdapDriverException + * @expectedExceptionMessage This search must only return a single user + */ + public function testFindUserByOnMultiResults() + { + $expected = [ + 'count' => 3 + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->once())->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + self::assertEquals('ou=users, dc=kimai, dc=org', $baseDn); + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foo))', $filter); + + return $expected; + }); + + $sut = $this->getLdapManager($driver); + $sut->findUserBy(['uid' => 'foo']); + } + + public function testFindUserByOnValidResult() + { + $expected = [ + 0 => ['dn' => 'foo'], + 'count' => 1, + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->once())->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + self::assertEquals('ou=users, dc=kimai, dc=org', $baseDn); + self::assertEquals('(&(&(objectClass=inetOrgPerson))(träl=alß#\\\aa=XY\5cZ0)(test=fu=n))', $filter); + + return $expected; + }); + + $sut = $this->getLdapManager($driver); + $actual = $sut->findUserBy(['träl=alß#\\\aa' => 'XY\Z0', 'test' => 'fu=n']); + self::assertInstanceOf(User::class, $actual); + } + + public function testBind() + { + $user = (new User())->setUsername('foobar'); + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['bind'])->getMock(); + $driver->expects($this->once())->method('bind')->willReturnCallback(function ($bindUser, $password) use ($user) { + self::assertSame($user, $bindUser); + self::assertEquals('a-very-secret-secret', $password); + + return true; + }); + + $sut = $this->getLdapManager($driver); + $actual = $sut->bind($user, 'a-very-secret-secret'); + self::assertTrue($actual); + } + + public function testUpdateUserOnZeroResults() + { + $user = (new User())->setUsername('foobar'); + $user->setPreferenceValue('ldap.dn', 'fooooooooooo'); + $expected = [ + [ + 0 => ['dn' => 'blub'], + 'count' => 1, + ], + [ + 'count' => 0, + ], + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->exactly(2))->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + if ($baseDn === 'ou=users, dc=kimai, dc=org') { + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foobar))', $filter); + + return $expected[0]; + } elseif ($baseDn === 'blub') { + self::assertEquals('(objectClass=*)', $filter); + + return $expected[1]; + } + $this->fail(sprintf('Unexpected search with baseDn %s', $baseDn)); + }); + + $sut = $this->getLdapManager($driver); + + $userOrig = clone $user; + $sut->updateUser($user); + self::assertEquals($userOrig, $user); + } + + /** + * @expectedException \App\Ldap\LdapDriverException + * @expectedExceptionMessage This search must only return a single user + */ + public function testUpdateUserOnMultiResults() + { + $user = (new User())->setUsername('foobar'); + $user->setPreferenceValue('ldap.dn', 'xxxxxxx'); + + $expected = [ + [ + 0 => ['dn' => 'blub'], + 'count' => 1, + ], + [ + 'count' => 3, + ], + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->exactly(2))->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + if ($baseDn === 'ou=users, dc=kimai, dc=org') { + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foobar))', $filter); + + return $expected[0]; + } elseif ($baseDn === 'blub') { + self::assertEquals('(objectClass=*)', $filter); + + return $expected[1]; + } + + $this->fail(sprintf('Unexpected search with baseDn %s', $baseDn)); + }); + + $sut = $this->getLdapManager($driver); + $sut->updateUser($user); + } + + public function testUpdateUserOnValidResultWithEmptyRoleBaseDn() + { + $user = (new User())->setUsername('foobar'); + $user->setPreferenceValue('ldap.dn', 'sssssss'); + + $expected = [ + [ + 0 => ['dn' => 'blub'], + 'count' => 1, + ], + [ + 0 => ['dn' => 'blub-updated'], + 'count' => 1, + ], + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->exactly(2))->method('search')->willReturnCallback(function ($baseDn, $filter) use ($expected) { + if ($baseDn === 'ou=users, dc=kimai, dc=org') { + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=foobar))', $filter); + + return $expected[0]; + } elseif ($baseDn === 'blub') { + self::assertEquals('(objectClass=*)', $filter); + + return $expected[1]; + } + + $this->fail(sprintf('Unexpected search with baseDn %s', $baseDn)); + }); + + $sut = $this->getLdapManager($driver, [ + 'baseDn' => null, + 'nameAttribute' => 'cn', + 'userDnAttribute' => 'member', + 'groups' => [ + ['ldap_value' => 'group1', 'role' => 'ROLE_TEAMLEAD'], + ['ldap_value' => 'group2', 'role' => 'ROLE_ADMIN'], + ['ldap_value' => 'group3', 'role' => 'ROLE_CUSTOMER'], // not existing! + ['ldap_value' => 'group4', 'role' => 'ROLE_SUPER_ADMIN'], + ], + ]); + + $userOrig = clone $user; + $sut->updateUser($user); + self::assertEquals($userOrig->setEmail('foobar'), $user); + self::assertEquals($user->getPreferenceValue('ldap.dn'), 'blub-updated'); + } + + public function getValidConfigsTestData() + { + return [ + [ + [ + 0 => [ + 'dn' => 'blub', + 'uid' => ['Karl-Heinz'], + // just some rubbish data + 'blub' => ['dfsdfsdf'], + 'foo' => ['count' => 1, 'bar'], + 'bar' => ['count' => 1, 'foo', 'xxx'], + 'xxxxxxxx' => ['https://www.example.com'], + 'blub1' => ['dfsdfsdf'], + ], + 'count' => 1, + ], + [ + 'baseDn' => 'ou=groups, dc=kimai, dc=org', + 'nameAttribute' => 'cn', + 'usernameAttribute' => 'cn', // test that "cn" is not set and fallback to "dn" happens + 'userDnAttribute' => 'member', + 'groups' => [ + ['ldap_value' => 'group1', 'role' => 'ROLE_TEAMLEAD'], + ['ldap_value' => 'group2', 'role' => 'ROLE_ADMIN'], + ['ldap_value' => 'group3', 'role' => 'ROLE_CUSTOMER'], // not existing! + ['ldap_value' => 'group4', 'role' => 'ROLE_SUPER_ADMIN'], + ], + ], + '(&(member=blub))' + ], + [ + [ + 0 => [ + 'dn' => 'blub', + 'uid' => ['Karl-Heinz'], + // just some rubbish data + 'blub' => ['dfsdfsdf'], + 'foo' => ['count' => 1, 'bar'], + 'bar' => ['count' => 1, 'foo', 'xxx'], + 'xxxxxxxx' => ['https://www.example.com'], + 'blub1' => ['dfsdfsdf'], + ], + 'count' => 1, + ], + [ + 'baseDn' => 'ou=groups, dc=kimai, dc=org', + 'nameAttribute' => 'cn', + 'usernameAttribute' => 'blub1', + 'userDnAttribute' => 'memberuid', + 'groups' => [ + ['ldap_value' => 'group1', 'role' => 'ROLE_TEAMLEAD'], + ['ldap_value' => 'group2', 'role' => 'ROLE_ADMIN'], + ['ldap_value' => 'group3', 'role' => 'ROLE_CUSTOMER'], // not existing! + ['ldap_value' => 'group4', 'role' => 'ROLE_SUPER_ADMIN'], + ], + ], + '(&(memberuid=dfsdfsdf))' + ], + ]; + } + + /** + * @dataProvider getValidConfigsTestData + */ + public function testUpdateUserOnValidResultWithRolesResult(array $expectedUsers, array $groupConfig, string $expectedGroupQuery) + { + $expected = [ + 0 => ['dn' => 'blub'], + 'count' => 1, + ]; + + $expectedGroups = [ + // ROLE_TEAMLEAD + 0 => [ + 'cn' => [0 => 'group1'], + 'member' => [0 => 'uid=user1,ou=users,dc=kimai,dc=org', 1 => 'uid=user2,ou=users,dc=kimai,dc=org'], + ], + // ROLE_ADMIN + 1 => [ + 'cn' => [0 => 'admin'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + // will be ignored: unknown group + 2 => [ + 'cn' => [0 => 'kimai_admin'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + // will be ignored: unknown group + 3 => [ + 'cn' => [0 => 'group3'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + // will be ignored: the counter below does not announce this group! + 4 => [ + 'cn' => [0 => 'group4'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + 'count' => 4 + ]; + + $driver = $this->getMockBuilder(LdapDriver::class)->disableOriginalConstructor()->setMethods(['search'])->getMock(); + $driver->expects($this->exactly(3))->method('search')->willReturnCallback(function ($baseDn, $filter, $attributes) use ($expectedUsers, $expectedGroups, $expectedGroupQuery, $expected) { + if ($baseDn === 'ou=users, dc=kimai, dc=org') { + self::assertEquals('(&(&(objectClass=inetOrgPerson))(uid=Karl-Heinz))', $filter); + + return $expected; + } elseif ($baseDn === 'blub') { + // user attributes search + self::assertEquals('(objectClass=*)', $filter); + + return $expectedUsers; + } elseif ($baseDn === 'ou=groups, dc=kimai, dc=org') { + // roles search + self::assertEquals($expectedGroupQuery, $filter); + self::assertEquals([0 => 'cn'], $attributes); + + return $expectedGroups; + } + $this->fail(sprintf('Unexpected search with baseDn %s', $baseDn)); + }); + + $sut = $this->getLdapManager($driver, $groupConfig); + + $user = (new User())->setUsername('Karl-Heinz'); + $user->setPreferenceValue('ldap.dn', 'blub'); + $userOrig = clone $user; + $userOrig->setEmail('Karl-Heinz')->setRoles(['ROLE_TEAMLEAD', 'ROLE_ADMIN']); + + $sut->updateUser($user); + self::assertEquals($userOrig, $user); + self::assertEquals(['ROLE_TEAMLEAD', 'ROLE_ADMIN', 'ROLE_USER'], $user->getRoles()); + } +} diff --git a/tests/Ldap/LdapUserHydratorTest.php b/tests/Ldap/LdapUserHydratorTest.php new file mode 100644 index 00000000..aae7a280 --- /dev/null +++ b/tests/Ldap/LdapUserHydratorTest.php @@ -0,0 +1,189 @@ + false, + 'connection' => [ + 'host' => '1.1.1.1' + ], + 'user' => [ + 'usernameAttribute' => 'foo', + 'attributes' => [] + ], + 'role' => [], + ]); + + $sut = new LdapUserHydrator($config, new RoleService([])); + $user = $sut->hydrate(['dn' => 'blub']); + self::assertInstanceOf(User::class, $user); + self::assertEmpty($user->getUsername()); + self::assertEmpty($user->getEmail()); + } + + public function testHydrate() + { + $config = new LdapConfiguration([ + 'active' => false, + 'connection' => [ + 'host' => '1.1.1.1' + ], + 'user' => [ + 'usernameAttribute' => 'foo', + 'attributes' => [ + ['ldap_attr' => 'uid', 'user_method' => 'setUsername'], + ['ldap_attr' => 'foo', 'user_method' => 'setAlias'], + ['ldap_attr' => 'bar', 'user_method' => 'setTitle'], + ['ldap_attr' => 'roles', 'user_method' => 'setRoles'], + ['ldap_attr' => 'xxxxxxxx', 'user_method' => 'setAvatar'], + ['ldap_attr' => 'blubXX', 'user_method' => 'setAvatar'], + ] + ], + 'role' => [], + ]); + + $ldapEntry = [ + 'uid' => ['Karl-Heinz'], + 'blub' => ['dfsdfsdf'], + 'foo' => ['count' => 1, 0 => 'bar'], + 'bar' => ['foo'], + 'roles' => ['count' => 2, 0 => 'ROLE_TEAMLEAD', 1 => 'ROLE_ADMIN'], + 'xxxxxxxx' => ['https://www.example.com'], + 'blub1' => ['dfsdfsdf'], + 'dn' => 'blub', + ]; + + $sut = new LdapUserHydrator($config, new RoleService([])); + $user = $sut->hydrate($ldapEntry); + + self::assertInstanceOf(User::class, $user); + self::assertEquals('Karl-Heinz', $user->getUsername()); + self::assertEquals('bar', $user->getAlias()); + self::assertEquals('foo', $user->getTitle()); + self::assertEquals(['ROLE_TEAMLEAD', 'ROLE_ADMIN', 'ROLE_USER'], $user->getRoles()); + self::assertEquals('https://www.example.com', $user->getAvatar()); + self::assertEquals('Karl-Heinz', $user->getEmail()); + } + + public function testHydrateUser() + { + $config = new LdapConfiguration([ + 'active' => false, + 'connection' => [ + 'host' => '1.1.1.1' + ], + 'user' => [ + 'usernameAttribute' => 'foo', + 'attributes' => [ + ['ldap_attr' => 'uid', 'user_method' => 'setUsername'], + ['ldap_attr' => 'foo', 'user_method' => 'setAlias'], + ['ldap_attr' => 'bar', 'user_method' => 'setTitle'], + ['ldap_attr' => 'xxxxxxxx', 'user_method' => 'setAvatar'], + ] + ], + 'role' => [], + ]); + + $ldapEntry = [ + 'uid' => ['Karl-Heinz'], + 'blub' => ['dfsdfsdf'], + 'foo' => ['bar'], + 'bar' => ['foo'], + 'xxxxxxxx' => ['https://www.example.com'], + 'blub1' => ['dfsdfsdf'], + 'dn' => 'blub', + ]; + + $sut = new LdapUserHydrator($config, new RoleService([])); + $user = new User(); + $user->setPassword('foobar'); + $sut->hydrateUser($user, $ldapEntry); + self::assertEquals('Karl-Heinz', $user->getUsername()); + self::assertEquals('bar', $user->getAlias()); + self::assertEquals('foo', $user->getTitle()); + self::assertEquals('https://www.example.com', $user->getAvatar()); + self::assertEquals('Karl-Heinz', $user->getEmail()); + + // make sure that the password was resetted in hydrate + $pwdCheck = clone $user; + $pwdCheck->setPassword(''); + self::assertEquals($pwdCheck, $user); + } + + public function testHydrateRoles() + { + $config = new LdapConfiguration([ + 'user' => [ + 'attributes' => [] + ], + 'role' => [ + 'nameAttribute' => 'cn', + 'userDnAttribute' => 'member', + 'groups' => [ + ['ldap_value' => 'group1', 'role' => 'ROLE_TEAMLEAD'], + ['ldap_value' => 'group2', 'role' => 'ROLE_ADMIN'], + ['ldap_value' => 'group3', 'role' => 'ROLE_CUSTOMER'], // not existing! + ['ldap_value' => 'group4', 'role' => 'ROLE_SUPER_ADMIN'], + ], + ], + ]); + + $ldapGroups = [ + // ROLE_TEAMLEAD + 0 => [ + 'cn' => [0 => 'group1'], + 'member' => [0 => 'uid=user1,ou=users,dc=kimai,dc=org', 1 => 'uid=user2,ou=users,dc=kimai,dc=org'], + ], + // ROLE_ADMIN + 1 => [ + 'cn' => [0 => 'admin'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + // will be ignored: unknown group + 2 => [ + 'cn' => [0 => 'kimai_admin'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + // will be ignored: unknown group + 3 => [ + 'cn' => [0 => 'group3'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + // will be ignored: the counter below does not announce this group! + 4 => [ + 'cn' => [0 => 'group4'], + 'member' => [0 => 'uid=user2,ou=users,dc=kimai,dc=org', 1 => 'uid=user3,ou=users,dc=kimai,dc=org'], + ], + 'count' => 4 + ]; + + $sut = new LdapUserHydrator($config, new RoleService([ + 'ROLE_TEAMLEAD' => ['ROLE_USER'], + 'ROLE_ADMIN' => ['ROLE_TEAMLEAD'], + 'ROLE_SUPER_ADMIN' => ['ROLE_ADMIN'] + ])); + $user = new User(); + $sut->hydrateRoles($user, $ldapGroups); + self::assertEquals(['ROLE_TEAMLEAD', 'ROLE_ADMIN', 'ROLE_USER'], $user->getRoles()); + } +} diff --git a/tests/Ldap/LdapUserProviderTest.php b/tests/Ldap/LdapUserProviderTest.php new file mode 100644 index 00000000..fcf310bb --- /dev/null +++ b/tests/Ldap/LdapUserProviderTest.php @@ -0,0 +1,102 @@ +getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => false]); + + $sut = new LdapUserProvider($manager, $config); + self::assertFalse($sut->supportsClass(User::class)); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\UsernameNotFoundException + * @expectedExceptionMessage LDAP is deactivated, user "test" not searched + */ + public function testDeactivatedLoadUserByUsername() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => false]); + + $sut = new LdapUserProvider($manager, $config); + $sut->loadUserByUsername('test'); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\UnsupportedUserException + * @expectedExceptionMessage Instances of "App\Entity\User" are not supported. + */ + public function testDeactivatedRefreshUser() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); + $config = new LdapConfiguration(['active' => false]); + + $sut = new LdapUserProvider($manager, $config); + $sut->refreshUser(new User()); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\UsernameNotFoundException + * @expectedExceptionMessage User "test" not found + */ + public function testLoadUserByUsernameReturnsNull() + { + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->setMethods(['findUserByUsername'])->getMock(); + $manager->expects($this->once())->method('findUserByUsername')->willReturn(null); + $config = new LdapConfiguration(['active' => true]); + + $sut = new LdapUserProvider($manager, $config); + $sut->loadUserByUsername('test'); + } + + public function testLoadUserByUsernameReturnsUser() + { + $user = new User(); + $user->setUsername('foobar'); + + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->setMethods(['findUserByUsername'])->getMock(); + $manager->expects($this->once())->method('findUserByUsername')->willReturn($user); + $config = new LdapConfiguration(['active' => true]); + + $sut = new LdapUserProvider($manager, $config); + $actual = $sut->loadUserByUsername('test'); + self::assertInstanceOf(User::class, $actual); + self::assertSame($user, $actual); + } + + public function testRefreshUserReturnsUser() + { + $user = new User(); + $user->setUsername('foobar'); + $user->setPreferenceValue('ldap.dn', 'sdfdsf'); + + $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->setMethods(['updateUser'])->getMock(); + $config = new LdapConfiguration(['active' => true]); + + $sut = new LdapUserProvider($manager, $config); + $actual = $sut->refreshUser($user); + + self::assertInstanceOf(User::class, $actual); + self::assertSame($user, $actual); + } +} diff --git a/tests/Ldap/SanitizingExceptionTest.php b/tests/Ldap/SanitizingExceptionTest.php new file mode 100644 index 00000000..6ef58703 --- /dev/null +++ b/tests/Ldap/SanitizingExceptionTest.php @@ -0,0 +1,31 @@ +getMessage()); + self::assertNotContains('bar', (string) $sut); + self::assertEquals('Could not find user foo with password **** in your LDAP', $sut->getMessage()); + } +} diff --git a/tests/Ldap/ZendLdapTest.php b/tests/Ldap/ZendLdapTest.php new file mode 100644 index 00000000..b4b31ea4 --- /dev/null +++ b/tests/Ldap/ZendLdapTest.php @@ -0,0 +1,48 @@ + false, + 'connection' => [ + 'host' => '1.1.1.1' + ] + ]); + + $sut = new ZendLdap($config); + $options = $sut->getOptions(); + self::assertNull($options['host']); + } + + public function testConstructActivatedPassesOptions() + { + $config = new LdapConfiguration([ + 'active' => true, + 'connection' => [ + 'host' => '1.1.1.1' + ] + ]); + + $sut = new ZendLdap($config); + $options = $sut->getOptions(); + self::assertEquals('1.1.1.1', $options['host']); + } +} diff --git a/tests/Repository/AbstractRepositoryTest.php b/tests/Repository/AbstractRepositoryTest.php index 9582d057..0617409a 100644 --- a/tests/Repository/AbstractRepositoryTest.php +++ b/tests/Repository/AbstractRepositoryTest.php @@ -21,7 +21,7 @@ abstract class AbstractRepositoryTest extends KernelTestCase use KernelTestTrait; /** - * @var EntityManager + * @var EntityManager|null */ private $entityManager; diff --git a/tests/Repository/TagRepositoryTest.php b/tests/Repository/TagRepositoryTest.php index 90a40405..30d5fc91 100644 --- a/tests/Repository/TagRepositoryTest.php +++ b/tests/Repository/TagRepositoryTest.php @@ -10,6 +10,7 @@ namespace App\Tests\Repository; use App\Entity\Tag; +use App\Repository\TagRepository; use App\Tests\DataFixtures\TagFixtures; /** @@ -29,6 +30,7 @@ class TagRepositoryTest extends AbstractRepositoryTest public function testFindIds() { $em = $this->getEntityManager(); + /** @var TagRepository $repository */ $repository = $em->getRepository(Tag::class); $result = $repository->findIdsByTagNameList('2018,Test'); @@ -47,6 +49,7 @@ class TagRepositoryTest extends AbstractRepositoryTest public function testFindNoIds() { $em = $this->getEntityManager(); + /** @var TagRepository $repository */ $repository = $em->getRepository(Tag::class); $result = $repository->findIdsByTagNameList('Simply'); @@ -58,6 +61,7 @@ class TagRepositoryTest extends AbstractRepositoryTest public function testFindAllTagNames() { $em = $this->getEntityManager(); + /** @var TagRepository $repository */ $repository = $em->getRepository(Tag::class); $result = $repository->findAllTagNames('2018'); @@ -75,6 +79,7 @@ class TagRepositoryTest extends AbstractRepositoryTest public function testFindNoTagNames() { $em = $this->getEntityManager(); + /** @var TagRepository $repository */ $repository = $em->getRepository(Tag::class); $result = $repository->findAllTagNames('Nothing'); diff --git a/tests/Repository/TimesheetRepositoryTest.php b/tests/Repository/TimesheetRepositoryTest.php index e9f4008c..6d2604bc 100644 --- a/tests/Repository/TimesheetRepositoryTest.php +++ b/tests/Repository/TimesheetRepositoryTest.php @@ -14,9 +14,12 @@ use App\Entity\Project; use App\Entity\Tag; use App\Entity\Timesheet; use App\Entity\User; +use App\Repository\ActivityRepository; +use App\Repository\ProjectRepository; use App\Repository\Query\BaseQuery; use App\Repository\Query\TimesheetQuery; use App\Repository\RepositoryException; +use App\Repository\TimesheetRepository; use App\Tests\DataFixtures\TimesheetFixtures; use Doctrine\ORM\QueryBuilder; use Pagerfanta\Pagerfanta; @@ -29,6 +32,7 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest public function testResultTypeForQueryState() { $em = $this->getEntityManager(); + /** @var TimesheetRepository $repository */ $repository = $em->getRepository(Timesheet::class); $query = new TimesheetQuery(); @@ -53,6 +57,7 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest { $em = $this->getEntityManager(); $user = $this->getUserByRole($em, User::ROLE_USER); + /** @var TimesheetRepository $repository */ $repository = $em->getRepository(Timesheet::class); $fixtures = new TimesheetFixtures(); @@ -66,6 +71,7 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest $query->setUser($user); $query->setState(TimesheetQuery::STATE_STOPPED); + /** @var array $entities */ $entities = $repository->findByQuery($query); $this->assertCount(1, $entities); @@ -81,6 +87,7 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest { $em = $this->getEntityManager(); $user = $this->getUserByRole($em, User::ROLE_USER); + /** @var TimesheetRepository $repository */ $repository = $em->getRepository(Timesheet::class); $fixtures = new TimesheetFixtures(); @@ -100,12 +107,15 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest public function testSave() { $em = $this->getEntityManager(); + /** @var ActivityRepository $activityRepository */ $activityRepository = $em->getRepository(Activity::class); $activity = $activityRepository->find(1); + /** @var ProjectRepository $projectRepository */ $projectRepository = $em->getRepository(Project::class); $project = $projectRepository->find(1); $user = $this->getUserByRole($em, User::ROLE_USER); + /** @var TimesheetRepository $repository */ $repository = $em->getRepository(Timesheet::class); $timesheet = new Timesheet(); $timesheet @@ -124,12 +134,15 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest public function testSaveWithTags() { $em = $this->getEntityManager(); + /** @var ActivityRepository $activityRepository */ $activityRepository = $em->getRepository(Activity::class); $activity = $activityRepository->find(1); + /** @var ProjectRepository $projectRepository */ $projectRepository = $em->getRepository(Project::class); $project = $projectRepository->find(1); $user = $this->getUserByRole($em, User::ROLE_USER); + /** @var TimesheetRepository $repository */ $repository = $em->getRepository(Timesheet::class); $tagOne = new Tag(); $tagOne->setName('Travel'); diff --git a/tests/Security/RoleServiceTest.php b/tests/Security/RoleServiceTest.php new file mode 100644 index 00000000..0793319c --- /dev/null +++ b/tests/Security/RoleServiceTest.php @@ -0,0 +1,34 @@ + [0 => 'ROLE_USER'], + 'ROLE_ADMIN' => [0 => 'ROLE_TEAMLEAD'], + 'ROLE_SUPER_ADMIN' => [0 => 'ROLE_ADMIN'] + ]; + + $sut = new RoleService($real); + + $expected = ['ROLE_TEAMLEAD', 'ROLE_USER', 'ROLE_ADMIN', 'ROLE_SUPER_ADMIN']; + + self::assertEquals($expected, $sut->getAvailableNames()); + } +} diff --git a/tests/Security/UserCheckerTest.php b/tests/Security/UserCheckerTest.php index 1af1c88b..808fa347 100644 --- a/tests/Security/UserCheckerTest.php +++ b/tests/Security/UserCheckerTest.php @@ -7,51 +7,57 @@ * file that was distributed with this source code. */ -namespace App\Tests\Model; +namespace App\Tests\Security; use App\Entity\User; use App\Security\UserChecker; use PHPUnit\Framework\TestCase; -use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Core\User\User as SymfonyUser; /** * @covers \App\Security\UserChecker */ class UserCheckerTest extends TestCase { - public function testCheckPreAuth() + public function testCheckPreAuthReturnsOnUnknownUserClass() { $sut = new UserChecker(); - $user = new User(); try { - $sut->checkPreAuth($user); + $sut->checkPreAuth(new SymfonyUser('sdf', null)); } catch (\Exception $ex) { - $this->fail('UserChecker should not throw exception in checkPreAuth()'); + $this->fail('UserChecker should not throw exception in checkPreAuth(), ' . $ex->getMessage()); + } + $this->assertTrue(true); + } + + public function testCheckPostAuthReturnsOnUnknownUserClass() + { + $sut = new UserChecker(); + + try { + $sut->checkPostAuth(new SymfonyUser('sdf', null)); + } catch (\Exception $ex) { + $this->fail('UserChecker should not throw exception in checkPostAuth(), ' . $ex->getMessage()); } $this->assertTrue(true); } /** - * @expectedException \Symfony\Component\Security\Core\Exception\LockedException + * @expectedException \Symfony\Component\Security\Core\Exception\DisabledException + * @expectedExceptionMessage User account is disabled. */ - public function testDisabledCannotLogin() + public function testDisabledCannotLoginInCheckPreAuth() { - $sut = new UserChecker(); - $user = new User(); - $user->setEnabled(false); - - $sut->checkPostAuth($user); + (new UserChecker())->checkPreAuth((new User())->setEnabled(false)); } - public function testCheckPostAuth() + /** + * @expectedException \Symfony\Component\Security\Core\Exception\DisabledException + * @expectedExceptionMessage User account is disabled. + */ + public function testDisabledCannotLoginInCheckPostAuth() { - $sut = new UserChecker(); - - $mock = $this->getMockBuilder(UserInterface::class)->setMethods(['isEnabled'])->getMockForAbstractClass(); - $mock->expects($this->never())->method('isEnabled')->willReturn(false); - - $sut->checkPostAuth($mock); - $this->assertTrue(true); + (new UserChecker())->checkPostAuth((new User())->setEnabled(false)); } } diff --git a/tests/Timesheet/Rounding/CeilRoundingTest.php b/tests/Timesheet/Rounding/CeilRoundingTest.php index aaaf773b..1a1d1f53 100644 --- a/tests/Timesheet/Rounding/CeilRoundingTest.php +++ b/tests/Timesheet/Rounding/CeilRoundingTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\Timesheet\Calculator; +namespace App\Tests\Timesheet\Rounding; use App\Entity\Timesheet; use App\Timesheet\Rounding\CeilRounding; diff --git a/tests/Timesheet/Rounding/ClosestRoundingTest.php b/tests/Timesheet/Rounding/ClosestRoundingTest.php index 4beeca83..822035f8 100644 --- a/tests/Timesheet/Rounding/ClosestRoundingTest.php +++ b/tests/Timesheet/Rounding/ClosestRoundingTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\Timesheet\Calculator; +namespace App\Tests\Timesheet\Rounding; use App\Entity\Timesheet; use App\Timesheet\Rounding\ClosestRounding; diff --git a/tests/Timesheet/Rounding/DefaultRoundingTest.php b/tests/Timesheet/Rounding/DefaultRoundingTest.php index 18164e21..a2236255 100644 --- a/tests/Timesheet/Rounding/DefaultRoundingTest.php +++ b/tests/Timesheet/Rounding/DefaultRoundingTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\Timesheet\Calculator; +namespace App\Tests\Timesheet\Rounding; use App\Entity\Timesheet; use App\Timesheet\Rounding\DefaultRounding; diff --git a/tests/Timesheet/Rounding/FloorRoundingTest.php b/tests/Timesheet/Rounding/FloorRoundingTest.php index 20201940..0f662160 100644 --- a/tests/Timesheet/Rounding/FloorRoundingTest.php +++ b/tests/Timesheet/Rounding/FloorRoundingTest.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\Timesheet\Calculator; +namespace App\Tests\Timesheet\Rounding; use App\Entity\Timesheet; use App\Timesheet\Rounding\FloorRounding; diff --git a/tests/Twig/ExtensionsTest.php b/tests/Twig/ExtensionsTest.php index 917cda1a..25ec4b9a 100644 --- a/tests/Twig/ExtensionsTest.php +++ b/tests/Twig/ExtensionsTest.php @@ -242,7 +242,6 @@ class ExtensionsTest extends TestCase 'timesheet.html#duration-format' => 'https://www.kimai.org/documentation/timesheet.html#duration-format', 'invoice.html' => 'https://www.kimai.org/documentation/invoice.html', '' => 'https://www.kimai.org/documentation/', - null => 'https://www.kimai.org/documentation/', ]; $sut = $this->getSut($this->localeEn); diff --git a/tests/Voter/AbstractVoterTest.php b/tests/Voter/AbstractVoterTest.php index 00c46da7..b2a8f554 100644 --- a/tests/Voter/AbstractVoterTest.php +++ b/tests/Voter/AbstractVoterTest.php @@ -28,18 +28,18 @@ abstract class AbstractVoterTest extends TestCase $isAuthenticated = empty($user->getRoles()); $accessManager = $this->getMockBuilder(AclDecisionManager::class)->disableOriginalConstructor()->getMock(); $accessManager->method('isFullyAuthenticated')->willReturn($isAuthenticated); - $accessManager->method('hasRole')->willReturnCallback(function ($role) use ($user) { - return in_array($role, $user->getRoles()); - }); $class = new \ReflectionClass($voterClass); + /** @var AbstractVoter $voter */ + $voter = $class->newInstance($accessManager, $this->getRolePermissionManager()); + self::assertInstanceOf(AbstractVoter::class, $voter); - return $class->newInstance($accessManager, $this->getRolePermissionManager()); + return $voter; } /** - * @param $id - * @param $role + * @param int $id + * @param string $role * @return User */ protected function getUser($id, $role) diff --git a/tests/Voter/TimesheetVoterTest.php b/tests/Voter/TimesheetVoterTest.php index af0fbd09..ad047d78 100644 --- a/tests/Voter/TimesheetVoterTest.php +++ b/tests/Voter/TimesheetVoterTest.php @@ -148,8 +148,8 @@ class TimesheetVoterTest extends AbstractVoterTest } /** - * @param $id - * @param $role + * @param int $id + * @param string $role * @return User */ protected function getUser($id, $role) diff --git a/tests/phpstan.neon b/tests/phpstan.neon new file mode 100644 index 00000000..3a16544b --- /dev/null +++ b/tests/phpstan.neon @@ -0,0 +1,10 @@ +includes: + - %rootDir%/../phpstan-symfony/extension.neon + - %rootDir%/../phpstan-doctrine/extension.neon + - %rootDir%/../phpstan-phpunit/extension.neon + +parameters: + ignoreErrors: + - '#Access to an undefined property Faker\\Generator::\$stateAbbr.#' + - '#Access to an undefined property Faker\\Generator::\$catchPhrase.#' + - '#Access to an undefined property Faker\\Generator::\$bs.#'