diff --git a/.docker/data/.gitignore b/.docker/data/.gitignore new file mode 100644 index 0000000..4ce1020 --- /dev/null +++ b/.docker/data/.gitignore @@ -0,0 +1,5 @@ +# Ignore everything in this directory +* +# Except +!.gitignore +!README.md diff --git a/.docker/data/README.md b/.docker/data/README.md new file mode 100644 index 0000000..8895d7b --- /dev/null +++ b/.docker/data/README.md @@ -0,0 +1,26 @@ +# .docker/data + +Please map persistent volumes to this directory on the servers. + +If a container needs to persist data between restarts you can map the relevant files in the container to ``docker/data/`. + +## RabbitMQ example +If you are using RabbitMQ running in a container as a message broker you need to configure a persistent volume for RabbitMQs data directory to avoid losing message on container restarts. + +```yaml +# docker-compose.server.override.yml + +services: + rabbit: + image: rabbitmq:3.9-management-alpine + hostname: "${COMPOSE_PROJECT_NAME}" + networks: + - app + - frontend + environment: + - "RABBITMQ_DEFAULT_USER=${RABBITMQ_USER}" + - "RABBITMQ_DEFAULT_PASS=${RABBITMQ_PASSWORD}" + - "RABBITMQ_ERLANG_COOKIE=${RABBITMQ_ERLANG_COOKIE}" + volumes: + - ".docker/data/rabbitmq:/var/lib/rabbitmq/mnesia/" +``` diff --git a/.github/workflows/changelog.yaml b/.github/workflows/changelog.yaml new file mode 100644 index 0000000..fead572 --- /dev/null +++ b/.github/workflows/changelog.yaml @@ -0,0 +1,29 @@ +# Do not edit this file! Make a pull request on changing +# github/workflows/changelog.yaml in +# https://github.com/itk-dev/devops_itkdev-docker if need be. + +### ### Changelog +### +### Checks that changelog has been updated + +name: Changelog + +on: + pull_request: + +jobs: + changelog: + runs-on: ubuntu-latest + strategy: + fail-fast: false + steps: + - name: Checkout + uses: actions/checkout@v5 + with: + fetch-depth: 2 + + - name: Git fetch + run: git fetch + + - name: Check that changelog has been updated. + run: git diff --exit-code origin/${{ github.base_ref }} -- CHANGELOG.md && exit 1 || exit 0 diff --git a/.github/workflows/composer.yaml b/.github/workflows/composer.yaml new file mode 100644 index 0000000..eebafe7 --- /dev/null +++ b/.github/workflows/composer.yaml @@ -0,0 +1,83 @@ +# Do not edit this file! Make a pull request on changing +# github/workflows/composer.yaml in +# https://github.com/itk-dev/devops_itkdev-docker if need be. + +### ### Composer +### +### Validates composer.json and checks that it's normalized. +### +### #### Assumptions +### +### 1. A docker compose service named `phpfpm` can be run and `composer` can be +### run inside the `phpfpm` service. +### 2. [ergebnis/composer-normalize](https://github.com/ergebnis/composer-normalize) +### is a dev requirement in `composer.json`: +### +### ``` shell +### docker compose run --rm phpfpm composer require --dev ergebnis/composer-normalize +### ``` +### +### Normalize `composer.json` by running +### +### ``` shell +### docker compose run --rm phpfpm composer normalize +### ``` + +name: Composer + +env: + COMPOSE_USER: runner + +on: + pull_request: + push: + branches: + - main + - develop + +jobs: + composer-validate: + name: Validate composer (${{ matrix.prefer }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + prefer: [prefer-lowest, prefer-stable] + steps: + - uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm phpfpm composer validate --strict + + composer-normalized: + runs-on: ubuntu-latest + strategy: + fail-fast: false + steps: + - uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm phpfpm composer install + docker compose run --rm phpfpm composer normalize --dry-run + + composer-audit: + runs-on: ubuntu-latest + strategy: + fail-fast: false + steps: + - uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm phpfpm composer audit diff --git a/.github/workflows/github_build_release.yml b/.github/workflows/github_build_release.yml index d80d889..0751011 100644 --- a/.github/workflows/github_build_release.yml +++ b/.github/workflows/github_build_release.yml @@ -1,7 +1,7 @@ on: push: tags: - - '*.*.*' + - "*.*.*" name: Create Github Release @@ -16,7 +16,7 @@ jobs: APP_ENV: prod steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Create a release in GitHub run: gh release create ${{ github.ref_name }} --verify-tag --generate-notes diff --git a/.github/workflows/markdown.yaml b/.github/workflows/markdown.yaml new file mode 100644 index 0000000..ae83163 --- /dev/null +++ b/.github/workflows/markdown.yaml @@ -0,0 +1,44 @@ +# Do not edit this file! Make a pull request on changing +# github/workflows/markdown.yaml in +# https://github.com/itk-dev/devops_itkdev-docker if need be. + +### ### Markdown +### +### Lints Markdown files (`**/*.md`) in the project. +### +### [markdownlint-cli configuration +### files](https://github.com/igorshubovych/markdownlint-cli?tab=readme-ov-file#configuration), +### `.markdownlint.jsonc` and `.markdownlintignore`, control what is actually +### linted and how. +### +### #### Assumptions +### +### 1. A docker compose service named `markdownlint` for running `markdownlint` +### (from +### [markdownlint-cli](https://github.com/igorshubovych/markdownlint-cli)) +### exists. + +name: Markdown + +on: + pull_request: + push: + branches: + - main + - develop + +jobs: + markdown-lint: + runs-on: ubuntu-latest + strategy: + fail-fast: false + steps: + - name: Checkout + uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm markdownlint markdownlint '**/*.md' diff --git a/.github/workflows/php.yaml b/.github/workflows/php.yaml new file mode 100644 index 0000000..39a0643 --- /dev/null +++ b/.github/workflows/php.yaml @@ -0,0 +1,85 @@ +name: PHP + +env: + COMPOSE_USER: runner + +on: + pull_request: + push: + branches: + - main + - develop + +jobs: + coding-standards: + name: PHP - Check Coding Standards + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm phpfpm composer install + docker compose run --rm phpfpm vendor/bin/php-cs-fixer fix --dry-run --diff + + phpstan: + name: PHPStan + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm phpfpm composer install + docker compose run --rm phpfpm vendor/bin/phpstan + + unit-tests: + name: Unit tests (${{ matrix.php }}, ${{ matrix.prefer }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - service: phpfpm + php: "8.3" + prefer: prefer-lowest + - service: phpfpm + php: "8.3" + prefer: prefer-stable + - service: phpfpm84 + php: "8.4" + prefer: prefer-lowest + - service: phpfpm84 + php: "8.4" + prefer: prefer-stable + - service: phpfpm85 + php: "8.5" + prefer: prefer-lowest + - service: phpfpm85 + php: "8.5" + prefer: prefer-stable + steps: + - uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm -e XDEBUG_MODE=coverage ${{ matrix.service }} composer update --${{ matrix.prefer }} + docker compose run --rm -e XDEBUG_MODE=coverage ${{ matrix.service }} vendor/bin/phpunit --coverage-clover=coverage/unit.xml + + - name: Upload coverage to Codecov + if: matrix.service == 'phpfpm' && matrix.prefer == 'prefer-stable' + uses: codecov/codecov-action@v5 + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: ./coverage/unit.xml + fail_ci_if_error: true + flags: unittests diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml deleted file mode 100644 index ac92345..0000000 --- a/.github/workflows/pr.yaml +++ /dev/null @@ -1,166 +0,0 @@ -on: pull_request -name: Test & Code Style Review -jobs: - - test-composer-install: - name: Validate composer (${{ matrix.php}}) / (${{ matrix.prefer}}) - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - php: [ '8.3', '8.4' ] - prefer: [ prefer-lowest, prefer-stable ] - steps: - - uses: actions/checkout@v4 - - - name: Setup PHP, with composer and extensions - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php}} - coverage: none - - - name: Get composer cache directory - id: composer-cache - run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - - - name: Cache dependencies - uses: actions/cache@v4 - with: - path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }} - restore-keys: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.prefer }}- - - - name: Validate composer files - run: composer validate composer.json --strict - - - name: Install dependencies - run: | - composer update --${{ matrix.prefer }} --prefer-dist --no-interaction - - unit-tests: - name: Unit tests (${{ matrix.php}}) / (${{ matrix.prefer}}) - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - php: [ '8.3', '8.4' ] - prefer: [ prefer-lowest, prefer-stable ] - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 2 - - - name: Setup PHP, with composer and extensions - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php}} - coverage: xdebug - - - name: Get composer cache directory - id: composer-cache - run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - - - name: Cache dependencies - uses: actions/cache@v4 - with: - path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }} - restore-keys: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.prefer }}- - - - name: Install dependencies - run: | - composer update --${{ matrix.prefer }} --prefer-dist --no-interaction - - - name: Unit tests - run: ./vendor/bin/phpunit --coverage-clover=coverage/unit.xml - - - name: Upload coverage to Codecov - uses: codecov/codecov-action@v5 - with: - token: ${{ secrets.CODECOV_TOKEN }} - files: ./coverage/unit.xml - env_vars: ${{ matrix.php}},${{ matrix.dependency-version}} - fail_ci_if_error: true - flags: unittests - - phpcsfixer: - name: Coding style (${{ matrix.php }}) - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - php: [ '8.3' ] - prefer: [ prefer-stable ] - steps: - - uses: actions/checkout@v4 - - - name: Setup PHP, with composer and extensions - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php}} - coverage: none - tools: cs2pr, phpcs - - - name: Get composer cache directory - id: composer-cache - run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - - - name: Cache dependencies - uses: actions/cache@v4 - with: - path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }} - restore-keys: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.prefer }}- - - - name: Install dependencies - run: composer update --prefer-stable --prefer-dist --no-interaction - - phpstan: - name: PHPStan (${{ matrix.php }}) - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - php: [ '8.3' ] - prefer: [ prefer-stable ] - steps: - - uses: actions/checkout@v4 - - - name: Setup PHP, with composer and extensions - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php}} - coverage: none - tools: cs2pr, phpcs - - - name: Get composer cache directory - id: composer-cache - run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - - - name: Cache dependencies - uses: actions/cache@v4 - with: - path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }} - restore-keys: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.prefer }}- - - - name: Install dependencies - run: composer update --prefer-stable --prefer-dist --no-interaction - - - name: Run PHPStan - run: ./vendor/bin/phpstan - - changelog: - runs-on: ubuntu-latest - name: Changelog should be updated - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 2 - - - name: Git fetch - run: git fetch - - - name: Check that changelog has been updated. - run: git diff --exit-code origin/${{ github.base_ref }} -- CHANGELOG.md && exit 1 || exit 0 diff --git a/.github/workflows/yaml.yaml b/.github/workflows/yaml.yaml new file mode 100644 index 0000000..631e525 --- /dev/null +++ b/.github/workflows/yaml.yaml @@ -0,0 +1,41 @@ +# Do not edit this file! Make a pull request on changing +# github/workflows/yaml.yaml in +# https://github.com/itk-dev/devops_itkdev-docker if need be. + +### ### YAML +### +### Validates YAML files. +### +### #### Assumptions +### +### 1. A docker compose service named `prettier` for running +### [Prettier](https://prettier.io/) exists. +### +### #### Symfony YAML +### +### Symfony's YAML config files use 4 spaces for indentation and single quotes. +### Therefore we use a [Prettier configuration +### file](https://prettier.io/docs/configuration), `.prettierrc.yaml`, to make +### Prettier format YAML files in the `config/` folder like Symfony expects. + +name: YAML + +on: + pull_request: + push: + branches: + - main + - develop + +jobs: + yaml-lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm prettier '**/*.{yml,yaml}' --check diff --git a/.gitignore b/.gitignore index 38adadd..493bbe2 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ var .phpcs-cache .php-cs-fixer.cache .phpunit.cache +unit.xml diff --git a/.markdownlint.json b/.markdownlint.json deleted file mode 100644 index cf2915d..0000000 --- a/.markdownlint.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "default": true, - "line-length": { "code_blocks": false } -} \ No newline at end of file diff --git a/.markdownlint.jsonc b/.markdownlint.jsonc new file mode 100644 index 0000000..0253096 --- /dev/null +++ b/.markdownlint.jsonc @@ -0,0 +1,22 @@ +// This file is copied from config/markdown/.markdownlint.jsonc in https://github.com/itk-dev/devops_itkdev-docker. +// Feel free to edit the file, but consider making a pull request if you find a general issue with the file. + +// markdownlint-cli configuration file (cf. https://github.com/igorshubovych/markdownlint-cli?tab=readme-ov-file#configuration) +{ + "default": true, + // https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md + "line-length": { + "line_length": 120, + "code_blocks": false, + "tables": false + }, + // https://github.com/DavidAnson/markdownlint/blob/main/doc/md024.md + "no-duplicate-heading": { + "siblings_only": true + }, + // https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/organizing-information-with-collapsed-sections#creating-a-collapsed-section + // https://github.com/DavidAnson/markdownlint/blob/main/doc/md033.md + "no-inline-html": { + "allowed_elements": ["details", "summary"] + } +} diff --git a/.markdownlintignore b/.markdownlintignore new file mode 100644 index 0000000..d143ace --- /dev/null +++ b/.markdownlintignore @@ -0,0 +1,12 @@ +# This file is copied from config/markdown/.markdownlintignore in https://github.com/itk-dev/devops_itkdev-docker. +# Feel free to edit the file, but consider making a pull request if you find a general issue with the file. + +# https://github.com/igorshubovych/markdownlint-cli?tab=readme-ov-file#ignoring-files +vendor/ +node_modules/ +LICENSE.md +# Drupal +web/*.md +web/core/ +web/libraries/ +web/*/contrib/ diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 55d9971..6543763 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -1,21 +1,20 @@ in(__DIR__) - ->exclude('var') -; +$finder = new PhpCsFixer\Finder(); +// Check all files … +$finder->in(__DIR__); +// … that are not ignored by VCS +$finder->ignoreVCSIgnored(true); -return (new PhpCsFixer\Config()) - ->registerCustomFixers(new PhpCsFixerCustomFixers\Fixers()) - ->setRules([ - '@Symfony' => true, - '@Symfony:risky' => false, - 'phpdoc_align' => false, - 'no_superfluous_phpdoc_tags' => false, - 'array_syntax' => ['syntax' => 'short'], - MultilinePromotedPropertiesFixer::name() => true, - ]) - ->setFinder($finder) - ; \ No newline at end of file +$config = new PhpCsFixer\Config(); +$config->setFinder($finder); + +$config->setRules([ + '@Symfony' => true, +]); + +return $config; diff --git a/.prettierrc.yaml b/.prettierrc.yaml new file mode 100644 index 0000000..12e0898 --- /dev/null +++ b/.prettierrc.yaml @@ -0,0 +1,11 @@ +# This file is copied from config/symfony/yaml/.prettierrc.yaml in https://github.com/itk-dev/devops_itkdev-docker. +# Feel free to edit the file, but consider making a pull request if you find a general issue with the file. + +# https://prettier.io/docs/configuration +overrides: + # Symfony config + - files: + - "config/**/*.{yml,yaml}" + options: + tabWidth: 4 + singleQuote: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 03cd580..b43bf5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,32 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) +## [Unreleased] + +## [4.1.0] - 2026-03-20 + +### Added + +- Added Symfony 8 support + +### Changed + +- Upgraded to PHPUnit 12 +- Set phpstan to max level +- Set default for ADMIN_OIDC_ALLOW_HTTP to false in README to prevent unsafe settings in production + +### Fixed + +- Fixed type safety issues identified by phpstan max level +- Applied code style fixes +- Increased test coverage to 100% + ## [4.0.1] - 2025-01-16 -- Fix doctrine/orm require + +- Fix doctrine/orm require ## [4.0.0] - 2025-01-13 + - Remove support for PHP 8.1 and 8.2 (BC) - Remove support for Symfony versions lower than 6.4 (BC) - Bump dependency requirements @@ -28,46 +50,65 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed return annotation. ## [3.0.2] - 2022-09-14 + ### Fixed + - State passed instead of nonce when validating id token ## [3.0.1] - 2022-09-13 + ### Fixed + - Auto wiring when `itkdev_openid_connect.user_provider` was configured ## [3.0.0] - 2022-09-13 + ### Added + - Support for multiple user providers - Symfony 6.x support - Rector tooling - php-cs-fixer tooling ### Removed + - PHP 7.4 and 8.0 support - phpcodesniffer ## [2.0.0] - 2021-12-08 + ### Added + - Migrated to Symfony's new (5.1+) security system + ### Changed + - Require Symfony 5.4 - Moved `leeway` config to provider config - ITK OpenID Connect: Upgraded from `itk-dev/openid-connect` 2.1.0 to 3.0.0 + ### Removed + - Remove support for PHP 7.3 ## [1.1.0] - 2021-12-08 + ### Added + - Support for multiple open id connect configuration providers ## [1.0.1] - 2021-09-20 + ### Fixed + - Updated README - Avoided duplicate cache configuration ## [1.0.0] - 2021-09-16 + ### Added + - README - LICENSE - OpenID Connect Bundle: Added bundle files, a login controller and an abstract authenticator. @@ -82,7 +123,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `itk-dev/openid-connect` 1.0.0 to 2.1.0 - OpenId Connect Bundle: Added CLI login feature. -[unreleased]: https://github.com/itk-dev/openid-connect-bundle/compare/3.1.0...HEAD +[unreleased]: https://github.com/itk-dev/openid-connect-bundle/compare/4.1.0...HEAD +[4.1.0]: https://github.com/itk-dev/openid-connect-bundle/compare/4.0.1...4.1.0 +[4.0.1]: https://github.com/itk-dev/openid-connect-bundle/compare/4.0.0...4.0.1 +[4.0.0]: https://github.com/itk-dev/openid-connect-bundle/compare/3.1.0...4.0.0 [3.1.0]: https://github.com/itk-dev/openid-connect-bundle/compare/3.0.3...3.1.0 [3.0.3]: https://github.com/itk-dev/openid-connect-bundle/compare/3.0.2...3.0.3 [3.0.2]: https://github.com/itk-dev/openid-connect-bundle/compare/3.0.1...3.0.2 diff --git a/README.md b/README.md index 4ba9598..d7e7dd6 100644 --- a/README.md +++ b/README.md @@ -10,17 +10,23 @@ Symfony bundle for authorization via OpenID Connect. -## Note: Symfony Native OIDC Support - -Since theis bundle was created Symfony has added [support for OpenID Connect](https://symfony.com/blog/new-in-symfony-6-3-openid-connect-token-handler) -as documented in ["Using OpenID Connect (OIDC)"](https://symfony.com/doc/current/security/access_token.html#using-openid-connect-oidc) - -As of Symfony 7.2 (jan. 2025) it seems this still a work in progress: -* [OIDC discovery](https://github.com/symfony/symfony/pull/54932) is not yet implemented making config a bit cumbersome. -* It's not obvious how to implement support for multiple providers, although it may be possible using [Multiple Authenticators](https://symfony.com/doc/current/security/entry_point.html#multiple-authenticators-with-separate-entry-points) - -Until these issues are resolved this bundle cannot be fully replaced by the native features. - +> [!NOTE] +> +> ## Symfony Native OIDC Support +> +> Since this bundle was created Symfony has added [support for OpenID Connect](https://symfony.com/blog/new-in-symfony-6-3-openid-connect-token-handler) +> as documented in ["Using OpenID Connect (OIDC)"](https://symfony.com/doc/current/security/access_token.html#using-openid-connect-oidc). +> +> As of Symfony 7.4 (March 2026), Symfony's native OIDC support has matured: +> +> * [OIDC discovery](https://github.com/symfony/symfony/pull/54932) was added in Symfony 7.3, removing the need +> for manual keyset configuration. +> * Multiple providers are supported via multiple `base_uri` and `issuers` entries in the discovery config. +> +> However, Symfony's native OIDC support is designed for **bearer token validation** (API authentication) only. +> It does not implement the **authorization code flow** (browser-based login with redirect to the IdP and callback +> handling), which is the primary use case of this bundle. If your application needs browser-based OIDC login, +> this bundle is still required. ## Installation @@ -70,7 +76,7 @@ itkdev_openid_connect: # Optional: Specify leeway (seconds) to account for clock skew between provider and hosting # Defaults to 10 leeway: '%env(int:ADMIN_OIDC_LEEWAY)%' - # Optional: Allow http requests (used for mocking a IdP) + # Optional: Allow (non-secure) http requests (used for mocking a IdP). NOT RECOMMENDED FOR PRODUCTION. # Defaults to false allow_http: '%env(bool:ADMIN_OIDC_ALLOW_HTTP)%' user: @@ -95,7 +101,7 @@ ADMIN_OIDC_CLIENT_ID=ADMIN_APP_CLIENT_ID ADMIN_OIDC_CLIENT_SECRET=ADMIN_APP_CLIENT_SECRET ADMIN_OIDC_REDIRECT_URI=ADMIN_APP_REDIRECT_URI ADMIN_OIDC_LEEWAY=30 -ADMIN_OIDC_ALLOW_HTTP=true +ADMIN_OIDC_ALLOW_HTTP=false # "user" open id connect configuration variables USER_OIDC_METADATA_URL=USER_APP_METADATA_URL @@ -356,78 +362,83 @@ argument. ## Development Setup -A [`docker-compose.yml`](docker-compose.yml) file with a PHP 8.1 image is -included in this project. To install the dependencies you can run +A `docker-compose.yml` file with a PHP 8.3+ image is included in this project. +A [Taskfile](https://taskfile.dev/) is used to run common development tasks. + +To set up the project: ```shell -docker compose up -d -docker compose exec phpfpm composer install +task setup ``` -### Unit Testing +This starts the Docker containers and installs Composer dependencies. + +### Running All CI Checks -A PhpUnit setup is included in this library. To run the unit tests: +To run all checks locally (coding standards, static analysis, tests): ```shell -docker compose exec phpfpm composer install -docker compose exec phpfpm ./vendor/bin/phpunit +task pr:actions ``` -### Psalm static analysis - -We’re using [Psalm](https://psalm.dev/) for static analysis. To run psalm do +### Unit Testing ```shell -docker compose exec phpfpm composer install -docker compose exec phpfpm ./vendor/bin/psalm +task test ``` -### Check Coding Standard +### Test Matrix -The following command let you test that the code follows the coding standard for -the project. +Run the test suite across all supported PHP versions (8.3, 8.4, 8.5) with both +lowest and stable dependencies, mirroring the CI matrix: -* PHP files (PHP-CS-Fixer) - - ```shell - docker compose exec phpfpm composer coding-standards-check - ``` +```shell +task test:matrix +``` -* Markdown files (markdownlint standard rules) +This runs PHPUnit with coverage for each combination and prints a summary of +pass/fail results. - ```shell - docker run --rm -v "$PWD":/usr/src/app -w /usr/src/app node:18 yarn install - docker run --rm -v "$PWD":/usr/src/app -w /usr/src/app node:18 yarn coding-standards-check - ``` +### PHPStan Static Analysis -### Apply Coding Standards +```shell +task analyze +``` -To attempt to automatically fix coding style +### Coding Standards -* PHP files (PHP-CS-Fixer) +Check all coding standards: - ```sh - docker compose exec phpfpm composer coding-standards-apply - ``` +```shell +task lint +``` -* Markdown files (markdownlint standard rules) +Fix PHP coding standards (php-cs-fixer): - ```shell - docker run --rm -v "$PWD":/usr/src/app -w /usr/src/app node:18 yarn install - docker run --rm -v "$PWD":/usr/src/app -w /usr/src/app node:18 yarn coding-standards-apply - ``` +```shell +task lint:php:fix +``` -## CI +Fix Markdown files: -GitHub Actions are used to run the test suite and code style checks on all PRs. +```shell +task lint:markdown:fix +``` -If you wish to test against the jobs locally you can install -[act](https://github.com/nektos/act). Then do: +Fix YAML files: ```shell -act -P ubuntu-latest=shivammathur/node:latest pull_request +task lint:yaml:fix ``` +### Available Tasks + +Run `task --list` to see all available tasks. + +## CI + +GitHub Actions are used to run the test suite and code style checks on all PRs. + ## Versioning We use [SemVer](http://semver.org/) for versioning. For the versions available, diff --git a/Taskfile.yml b/Taskfile.yml new file mode 100644 index 0000000..ba09624 --- /dev/null +++ b/Taskfile.yml @@ -0,0 +1,243 @@ +# yaml-language-server: $schema=https://taskfile.dev/schema.json +--- +version: "3" + +vars: + DOCKER_COMPOSE: "docker compose" + PHP_EXEC: "{{.DOCKER_COMPOSE}} exec phpfpm" + COMPOSER: "{{.PHP_EXEC}} composer" + +tasks: + default: + silent: true + cmds: + - task --list + + install: + desc: Install dependencies + cmds: + - task: composer:install + + setup: + desc: Set up the project + cmds: + - task: up + - task: install + + # Container management + + up: + desc: Start docker containers + cmds: + - task: network:frontend + - "{{.DOCKER_COMPOSE}} up -d" + + down: + desc: Stop docker containers + cmds: + - "{{.DOCKER_COMPOSE}} down" + + restart: + desc: Restart docker containers + cmds: + - task: down + - task: up + + network:frontend: + internal: true + desc: Create docker frontend network + cmds: + - docker network create frontend 2>/dev/null || true + + # Composer + + composer: + desc: Run arbitrary composer command + cmds: + - "{{.COMPOSER}} {{.CLI_ARGS}}" + + composer:install: + desc: Install composer dependencies + cmds: + - "{{.COMPOSER}} install" + + composer:update: + desc: Update composer dependencies + cmds: + - "{{.COMPOSER}} update" + + composer:normalize: + desc: Normalize composer.json + cmds: + - "{{.COMPOSER}} normalize" + + composer:check: + desc: Validate and audit composer + cmds: + - "{{.PHP_EXEC}} composer validate --strict" + - "{{.COMPOSER}} normalize --dry-run" + - "{{.COMPOSER}} audit" + + # Code quality + + lint: + desc: Run all linters + cmds: + - task: lint:php + - task: lint:markdown + - task: lint:yaml + + lint:php: + desc: Check PHP coding standards + cmds: + - "{{.PHP_EXEC}} vendor/bin/php-cs-fixer fix --dry-run --diff" + + lint:php:fix: + desc: Fix PHP coding standards + cmds: + - "{{.PHP_EXEC}} vendor/bin/php-cs-fixer fix" + + lint:markdown: + desc: Lint markdown files + cmds: + - "{{.DOCKER_COMPOSE}} run --rm markdownlint markdownlint '**/*.md'" + + lint:markdown:fix: + desc: Fix markdown files + cmds: + - "{{.DOCKER_COMPOSE}} run --rm markdownlint markdownlint '**/*.md' --fix" + + lint:yaml: + desc: Lint YAML files + cmds: + - "{{.DOCKER_COMPOSE}} run --rm prettier '**/*.{yml,yaml}' --check" + + lint:yaml:fix: + desc: Fix YAML files + cmds: + - "{{.DOCKER_COMPOSE}} run --rm prettier '**/*.{yml,yaml}' --write" + + # Analysis + + analyze:php: + desc: Run PHPStan static analysis + cmds: + - "{{.PHP_EXEC}} vendor/bin/phpstan" + + # Testing + + test: + desc: Run tests + cmds: + - "{{.PHP_EXEC}} vendor/bin/phpunit" + + test:coverage: + desc: Run tests with coverage + cmds: + - "{{.PHP_EXEC}} vendor/bin/phpunit --coverage-clover=coverage/unit.xml" + + test:run: + desc: "Run tests for a PHP version and dependency set (e.g. task test:run PHP=8.4 DEPS=lowest)" + vars: + PHP: '{{.PHP | default "8.3"}}' + DEPS: '{{.DEPS | default "stable"}}' + PREFER: "prefer-{{.DEPS}}" + SERVICE: 'phpfpm{{.PHP | replace "." ""}}-{{.DEPS}}' + cmds: + - cmd: | + trap 'stty sane 2>/dev/null || true' EXIT + set -e + echo "Testing PHP {{.PHP}} ({{.PREFER}})..." + {{.DOCKER_COMPOSE}} run --rm -T --user root {{.SERVICE}} chown -R deploy:deploy /app/vendor /home/deploy/.composer + {{.DOCKER_COMPOSE}} run --rm -T -e XDEBUG_MODE=coverage {{.SERVICE}} sh -c ' + if [ -f /app/vendor/.composer.lock ]; then + cp /app/vendor/.composer.lock /app/composer.lock + composer install -q + else + composer update -q --{{.PREFER}} + cp /app/composer.lock /app/vendor/.composer.lock + fi' + {{.DOCKER_COMPOSE}} run --rm -T -e XDEBUG_MODE=coverage {{.SERVICE}} vendor/bin/phpunit --coverage-clover=coverage/unit.xml + + test:matrix:reset: + desc: Remove cached vendor volumes to force a fresh dependency resolve + cmds: + - "{{.DOCKER_COMPOSE}} down --volumes" + + test:matrix: + desc: Run tests across all PHP versions (mirrors CI matrix) + vars: + RESULTS_FILE: + sh: mktemp + cmds: + - task: test:matrix:run + vars: { PHP: "8.3", DEPS: lowest, RESULTS_FILE: "{{.RESULTS_FILE}}" } + - task: test:matrix:run + vars: { PHP: "8.3", DEPS: stable, RESULTS_FILE: "{{.RESULTS_FILE}}" } + - task: test:matrix:run + vars: { PHP: "8.4", DEPS: lowest, RESULTS_FILE: "{{.RESULTS_FILE}}" } + - task: test:matrix:run + vars: { PHP: "8.4", DEPS: stable, RESULTS_FILE: "{{.RESULTS_FILE}}" } + - task: test:matrix:run + vars: { PHP: "8.5", DEPS: lowest, RESULTS_FILE: "{{.RESULTS_FILE}}" } + - task: test:matrix:run + vars: { PHP: "8.5", DEPS: stable, RESULTS_FILE: "{{.RESULTS_FILE}}" } + - task: test:summary + vars: { RESULTS_FILE: "{{.RESULTS_FILE}}" } + + test:matrix:run: + internal: true + desc: Run a single matrix combination and record the result + vars: + PREFER: "prefer-{{.DEPS}}" + cmds: + - cmd: | + if task test:run PHP={{.PHP}} DEPS={{.DEPS}}; then + echo "PASS PHP {{.PHP}} ({{.PREFER}})" >> "{{.RESULTS_FILE}}" + else + echo "FAIL PHP {{.PHP}} ({{.PREFER}})" >> "{{.RESULTS_FILE}}" + fi + ignore_error: true + + test:summary: + internal: true + silent: true + desc: Print test matrix summary + cmds: + - cmd: | + echo "" + echo "==============================" + echo " Test Matrix Summary" + echo "==============================" + while IFS= read -r line; do + if [[ "$line" == PASS* ]]; then + echo " OK ${line#PASS }" + elif [[ "$line" == FAIL* ]]; then + echo " FAIL ${line#FAIL }" + fi + done < "{{.RESULTS_FILE}}" + echo "==============================" + if grep -q "^FAIL" "{{.RESULTS_FILE}}"; then + echo "" + echo " Failed combinations:" + grep "^FAIL" "{{.RESULTS_FILE}}" | while IFS= read -r line; do + echo " - ${line#FAIL }" + done + echo "" + rm -f "{{.RESULTS_FILE}}" + exit 1 + else + echo " All tests PASSED" + echo "==============================" + rm -f "{{.RESULTS_FILE}}" + fi + + # CI + + pr:actions: + desc: Run all CI checks locally + cmds: + - task: composer:check + - task: lint + - task: analyze:php + - task: test:matrix diff --git a/composer.json b/composer.json index 6730d46..ac1ba58 100644 --- a/composer.json +++ b/composer.json @@ -17,23 +17,21 @@ "php": "^8.3", "ext-json": "*", "ext-openssl": "*", - "doctrine/orm": "^2.8|^3.0", + "doctrine/orm": "^2.8 || ^3.0", "itk-dev/openid-connect": "^4.0", - "symfony/cache": "^6.4|^7.0", - "symfony/framework-bundle": "^6.4.13|^7.0", - "symfony/security-bundle": "^6.4.13|^7.0", - "symfony/uid": "^6.4|^7.0", - "symfony/yaml": "^6.4|^7.0" + "symfony/cache": "^6.4 || ^7.0 || ^8.0", + "symfony/framework-bundle": "^6.4.13 || ^7.0 || ^8.0", + "symfony/security-bundle": "^6.4.13 || ^7.0 || ^8.0", + "symfony/uid": "^6.4 || ^7.0 || ^8.0", + "symfony/yaml": "^6.4 || ^7.0 || ^8.0" }, "require-dev": { "ergebnis/composer-normalize": "^2.28", - "escapestudios/symfony2-coding-standard": "^3.12", "friendsofphp/php-cs-fixer": "^3.11", - "kubawerlos/php-cs-fixer-custom-fixers": "^3.11", "phpstan/phpstan": "^2.1", - "phpunit/phpunit": "^11.0", + "phpunit/phpunit": "^12.0", "rector/rector": "^2.0", - "symfony/runtime": "^6.4.13|^7.0" + "symfony/runtime": "^6.4.13 || ^7.0 || ^8.0" }, "autoload": { "psr-4": { @@ -53,11 +51,11 @@ "sort-packages": true }, "scripts": { - "coding-standards-apply": [ - "./vendor/bin/php-cs-fixer fix" + "apply-coding-standards": [ + "vendor/bin/php-cs-fixer fix" ], - "coding-standards-check": [ - "./vendor/bin/php-cs-fixer fix --dry-run --format=checkstyle" + "check-coding-standards": [ + "vendor/bin/php-cs-fixer fix --dry-run --diff" ], "test": "XDEBUG_MODE=coverage ./vendor/bin/phpunit" } diff --git a/docker-compose.yml b/docker-compose.yml index b55a7cf..f130e8f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,16 +1,130 @@ -# itk-version: 3.2.1 +# itk-version: 3.2.4 +networks: + frontend: + external: true + app: + driver: bridge + internal: false services: phpfpm: image: itkdev/php8.3-fpm:latest + user: ${COMPOSE_USER:-deploy} + networks: + - app + extra_hosts: + - "host.docker.internal:host-gateway" environment: - PHP_XDEBUG_MODE=${PHP_XDEBUG_MODE:-off} - PHP_MAX_EXECUTION_TIME=30 - PHP_MEMORY_LIMIT=256M # Depending on the setup, you may have to remove --read-envelope-from from msmtp (cf. https://marlam.de/msmtp/msmtp.html) or use SMTP to send mail - PHP_SENDMAIL_PATH=/usr/bin/msmtp --host=mail --port=1025 --read-recipients --read-envelope-from - - DOCKER_HOST_DOMAIN=${COMPOSE_DOMAIN} - - COMPOSER_VERSION=2 + - DOCKER_HOST_DOMAIN=${COMPOSE_DOMAIN:?} - PHP_IDE_CONFIG=serverName=localhost volumes: - - .:/app \ No newline at end of file + - .:/app + - composer-cache:/home/deploy/.composer/cache + + phpfpm84: + extends: + service: phpfpm + image: itkdev/php8.4-fpm:latest + profiles: + - ci + + phpfpm85: + extends: + service: phpfpm + image: itkdev/php8.5-fpm:latest + profiles: + - ci + + # Test matrix services (PHP version × dependency set) + phpfpm83-stable: + extends: + service: phpfpm + profiles: + - ci + volumes: + - .:/app + - phpfpm83-stable-vendor:/app/vendor + - composer-cache:/home/deploy/.composer/cache + + phpfpm83-lowest: + extends: + service: phpfpm + profiles: + - ci + volumes: + - .:/app + - phpfpm83-lowest-vendor:/app/vendor + - composer-cache:/home/deploy/.composer/cache + + phpfpm84-stable: + extends: + service: phpfpm84 + profiles: + - ci + volumes: + - .:/app + - phpfpm84-stable-vendor:/app/vendor + - composer-cache:/home/deploy/.composer/cache + + phpfpm84-lowest: + extends: + service: phpfpm84 + profiles: + - ci + volumes: + - .:/app + - phpfpm84-lowest-vendor:/app/vendor + - composer-cache:/home/deploy/.composer/cache + + phpfpm85-stable: + extends: + service: phpfpm85 + profiles: + - ci + volumes: + - .:/app + - phpfpm85-stable-vendor:/app/vendor + - composer-cache:/home/deploy/.composer/cache + + phpfpm85-lowest: + extends: + service: phpfpm85 + profiles: + - ci + volumes: + - .:/app + - phpfpm85-lowest-vendor:/app/vendor + - composer-cache:/home/deploy/.composer/cache + + # Code checks tools + markdownlint: + image: itkdev/markdownlint + profiles: + - dev + volumes: + - ./:/md + + prettier: + # Prettier does not (yet, fcf. + # https://github.com/prettier/prettier/issues/15206) have an official + # docker image. + # https://hub.docker.com/r/jauderho/prettier is good candidate (cf. https://hub.docker.com/search?q=prettier&sort=updated_at&order=desc) + image: jauderho/prettier + profiles: + - dev + volumes: + - ./:/work + +volumes: + phpfpm83-stable-vendor: + phpfpm83-lowest-vendor: + phpfpm84-stable-vendor: + phpfpm84-lowest-vendor: + phpfpm85-stable-vendor: + phpfpm85-lowest-vendor: + composer-cache: diff --git a/package.json b/package.json deleted file mode 100644 index 545a441..0000000 --- a/package.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "devDependencies": { - "markdownlint-cli": "^0.27.1" - }, - "scripts": { - "coding-standards-check/markdownlint": "markdownlint README.md", - "coding-standards-check": "yarn coding-standards-check/markdownlint", - "coding-standards-apply/markdownlint": "markdownlint --fix README.md", - "coding-standards-apply": "yarn coding-standards-apply/markdownlint" - } -} diff --git a/phpstan.neon b/phpstan.neon index 3bcdeac..309bd4f 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,21 +1,4 @@ parameters: - level: 8 + level: max paths: - src - ignoreErrors: - - - message: '#^Call to an undefined method Symfony\\Component\\Config\\Definition\\Builder\\NodeDefinition\:\:children\(\)\.$#' - identifier: method.notFound - path: src/DependencyInjection/Configuration.php - - - message: '#^Method ItkDev\\OpenIdConnectBundle\\Security\\OpenIdConfigurationProviderManager\:\:__construct\(\) has parameter \$config with no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - path: src/Security/OpenIdConfigurationProviderManager.php - - - message: '#^Method ItkDev\\OpenIdConnectBundle\\Security\\OpenIdConfigurationProviderManager\:\:getProviderKeys\(\) should return array\ but returns list\\.$#' - identifier: return.type - path: src/Security/OpenIdConfigurationProviderManager.php - - - message: '#^Method ItkDev\\OpenIdConnectBundle\\Command\\UserLoginCommand\:\:__construct\(\) has parameter \$userProvider with generic interface Symfony\\Component\\Security\\Core\\User\\UserProviderInterface but does not specify its types\: TUser$#' - identifier: missingType.generics - path: src/Command/UserLoginCommand.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 7fbb2ba..1642b0e 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,20 +1,10 @@ - - - - - - - - + tests - - - src/ diff --git a/src/Command/UserLoginCommand.php b/src/Command/UserLoginCommand.php index 398a62f..d65e5e7 100644 --- a/src/Command/UserLoginCommand.php +++ b/src/Command/UserLoginCommand.php @@ -12,6 +12,7 @@ use Symfony\Component\Console\Style\SymfonyStyle; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Security\Core\Exception\UserNotFoundException; +use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; #[AsCommand( @@ -20,19 +21,12 @@ )] class UserLoginCommand extends Command { - /** - * UserLoginCommand constructor. - * - * @param CliLoginHelper $cliLoginHelper - * @param string $cliLoginRoute - * @param UrlGeneratorInterface $urlGenerator - * @param UserProviderInterface $userProvider - */ + /** @param UserProviderInterface $userProvider */ public function __construct( private readonly CliLoginHelper $cliLoginHelper, private readonly string $cliLoginRoute, private readonly UrlGeneratorInterface $urlGenerator, - private readonly UserProviderInterface $userProvider + private readonly UserProviderInterface $userProvider, ) { parent::__construct(); } @@ -51,9 +45,11 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); + + /** @var string $username Symfony's getArgument() always returns a string for a REQUIRED argument */ $username = $input->getArgument('username'); - // Check if username is registered in User database + // Check if the username is registered in the user database try { $this->userProvider->loadUserByIdentifier($username); } catch (UserNotFoundException) { diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 75a2787..576cb05 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -16,7 +16,7 @@ class LoginController extends AbstractController { public function __construct( - private readonly OpenIdConfigurationProviderManager $providerManager + private readonly OpenIdConfigurationProviderManager $providerManager, ) { } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3890cc0..a099b77 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -7,9 +7,6 @@ class Configuration implements ConfigurationInterface { - /** - * {@inheritdoc} - */ public function getConfigTreeBuilder(): TreeBuilder { $treeBuilder = new TreeBuilder('itkdev_openid_connect'); @@ -26,7 +23,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->scalarNode('cache_pool') ->info('Method for caching') ->defaultValue('cache.app') - ->isRequired()->cannotBeEmpty() + ->cannotBeEmpty() ->end() // cache_pool ->end() ->end() // cache_options @@ -85,17 +82,8 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->validate() - ->always() - ->then( - static function (array $value) { - // Complain if both redirect_uri and redirect_route are set. - if (isset($value['redirect_uri'], $value['redirect_route'])) { - throw new \InvalidArgumentException('Only one of redirect_uri or redirect_route must be set.'); - } - - return $value; - } - ) + ->ifTrue(static fn (array $v) => isset($v['redirect_uri'], $v['redirect_route'])) + ->thenInvalid('Only one of redirect_uri or redirect_route must be set.') ->end() ->end() ->end() diff --git a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php index bd5eddc..8d71a88 100644 --- a/src/DependencyInjection/ItkDevOpenIdConnectExtension.php +++ b/src/DependencyInjection/ItkDevOpenIdConnectExtension.php @@ -15,8 +15,6 @@ class ItkDevOpenIdConnectExtension extends Extension { /** - * {@inheritdoc} - * * @throws \Exception */ public function load(array $configs, ContainerBuilder $container): void @@ -25,6 +23,13 @@ public function load(array $configs, ContainerBuilder $container): void $loader->load('services.yaml'); $configuration = new Configuration(); + /** @var array{ + * cache_options: array{cache_pool: string}, + * cli_login_options: array{route: string}, + * user_provider: string|null, + * openid_providers: array}> + * } $config + */ $config = $this->processConfiguration($configuration, $configs); $definition = $container->getDefinition(OpenIdConfigurationProviderManager::class); @@ -33,7 +38,7 @@ public function load(array $configs, ContainerBuilder $container): void 'default_providers_options' => [ 'cacheItemPool' => new Reference($config['cache_options']['cache_pool']), ], - 'providers' => array_map(static fn (array $options) => $options['options'], $config['openid_providers']), + 'providers' => array_map(static fn (array $options): array => $options['options'], $config['openid_providers']), ]; $definition->replaceArgument('$config', $providersConfig); @@ -50,9 +55,6 @@ public function load(array $configs, ContainerBuilder $container): void $definition->replaceArgument('$cliLoginRoute', $config['cli_login_options']['route']); } - /** - * {@inheritdoc} - */ #[\Override] public function getAlias(): string { diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index de563a2..e08a56e 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -2,13 +2,13 @@ services: # default configuration for services in *this* file _defaults: - autowire: true # Automatically injects dependencies in your services. + autowire: true # Automatically injects dependencies in your services. autoconfigure: true # Automatically registers your services as commands, event subscribers, etc. ItkDev\OpenIdConnectBundle\Security\OpenIdConfigurationProviderManager: - public: true - arguments: - $config: ~ + public: true + arguments: + $config: ~ ItkDev\OpenIdConnectBundle\Controller\LoginController: public: true diff --git a/src/Security/CliLoginTokenAuthenticator.php b/src/Security/CliLoginTokenAuthenticator.php index 963d6c5..c2d6ccc 100644 --- a/src/Security/CliLoginTokenAuthenticator.php +++ b/src/Security/CliLoginTokenAuthenticator.php @@ -26,7 +26,7 @@ class CliLoginTokenAuthenticator extends AbstractAuthenticator public function __construct( private readonly CliLoginHelper $cliLoginHelper, private readonly string $cliLoginRoute, - private readonly UrlGeneratorInterface $router + private readonly UrlGeneratorInterface $router, ) { } diff --git a/src/Security/OpenIdConfigurationProviderManager.php b/src/Security/OpenIdConfigurationProviderManager.php index 1504809..589d950 100644 --- a/src/Security/OpenIdConfigurationProviderManager.php +++ b/src/Security/OpenIdConfigurationProviderManager.php @@ -13,9 +13,24 @@ class OpenIdConfigurationProviderManager /** @var array */ private array $providers = []; + /** + * @param array{ + * default_providers_options: array, + * providers: array, + * leeway?: int, + * allow_http?: bool, + * }>, + * } $config + */ public function __construct( private readonly RouterInterface $router, - private readonly array $config + private readonly array $config, ) { } @@ -40,10 +55,10 @@ public function getProvider(string $key): OpenIdConfigurationProvider if (!isset($this->providers[$key]) && isset($this->config['providers'][$key])) { $options = $this->config['providers'][$key]; $providerOptions = [ - 'openIDConnectMetadataUrl' => $options['metadata_url'], - 'clientId' => $options['client_id'], - 'clientSecret' => $options['client_secret'], - ] + $this->config['default_providers_options']; + 'openIDConnectMetadataUrl' => $options['metadata_url'], + 'clientId' => $options['client_id'], + 'clientSecret' => $options['client_secret'], + ] + $this->config['default_providers_options']; if (isset($options['redirect_uri'])) { $providerOptions['redirectUri'] = $options['redirect_uri']; diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php index 3410e9b..2379900 100644 --- a/src/Security/OpenIdLoginAuthenticator.php +++ b/src/Security/OpenIdLoginAuthenticator.php @@ -19,15 +19,12 @@ abstract class OpenIdLoginAuthenticator extends AbstractAuthenticator implements { /** * OpenIdLoginAuthenticator constructor. - * - * @param OpenIdConfigurationProviderManager $providerManager */ public function __construct( private readonly OpenIdConfigurationProviderManager $providerManager, ) { } - /** {@inheritDoc} */ public function supports(Request $request): ?bool { // Check if request has state and code @@ -37,9 +34,7 @@ public function supports(Request $request): ?bool /** * Validate oidc claims. * - * @param Request $request - * - * @return array|string[] Array of claims + * @return array Array of claims * * @throws InvalidProviderException * @throws ItkOpenIdConnectException @@ -49,7 +44,8 @@ public function supports(Request $request): ?bool protected function validateClaims(Request $request): array { $session = $request->getSession(); - $providerKey = (string) $session->remove('oauth2provider'); + $providerKey = $session->remove('oauth2provider'); + $providerKey = is_string($providerKey) ? $providerKey : ''; $provider = $this->providerManager->getProvider($providerKey); // Make sure state and oauth2state are the same @@ -60,19 +56,15 @@ protected function validateClaims(Request $request): array } $oauth2nonce = $session->remove('oauth2nonce'); - if (empty($oauth2nonce)) { + if (!is_string($oauth2nonce) || '' === $oauth2nonce) { throw new ValidationException('Nonce empty or not found'); } try { $code = $request->query->get('code'); - if (null === $code) { - throw new ValidationException('Missing code'); - } - if (!is_string($code)) { - throw new ValidationException('Code not type string'); + throw new ValidationException('Missing or invalid code'); } $idToken = $provider->getIdToken($code); @@ -83,10 +75,12 @@ protected function validateClaims(Request $request): array throw new ValidationException($exception->getMessage()); } - return (array) $claims + ['open_id_connect_provider' => $providerKey]; + /** @var array $claimsArray */ + $claimsArray = (array) $claims; + + return $claimsArray + ['open_id_connect_provider' => $providerKey]; } - /** {@inheritDoc} */ public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response { throw new AuthenticationException('Error occurred validating openid login'); diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php index f40b321..333dbfd 100644 --- a/src/Util/CliLoginHelper.php +++ b/src/Util/CliLoginHelper.php @@ -16,7 +16,7 @@ class CliLoginHelper private const string ITK_NAMESPACE = 'itk-dev-cli-login'; public function __construct( - private readonly CacheItemPoolInterface $cache + private readonly CacheItemPoolInterface $cache, ) { } @@ -38,7 +38,13 @@ public function createToken(string $username): string } if ($revCacheItem->isHit()) { - return $revCacheItem->get(); + $cachedToken = $revCacheItem->get(); + + if (!is_string($cachedToken)) { + throw new CacheException('Cached token is not a string'); + } + + return $cachedToken; } $revCacheItem->set($token); $this->cache->save($revCacheItem); @@ -76,6 +82,10 @@ public function getUsername(string $token): ?string $username = $usernameItem->get(); + if (!is_string($username)) { + throw new CacheException('Cached username is not a string'); + } + // Delete both entries from cache try { $this->cache->deleteItem($token); diff --git a/tests/Command/UserLoginCommandTest.php b/tests/Command/UserLoginCommandTest.php new file mode 100644 index 0000000..f8e2dcb --- /dev/null +++ b/tests/Command/UserLoginCommandTest.php @@ -0,0 +1,64 @@ +stubCliLoginHelper = $this->createStub(CliLoginHelper::class); + $this->stubUrlGenerator = $this->createStub(UrlGeneratorInterface::class); + $this->stubUserProvider = $this->createStub(UserProviderInterface::class); + + $this->command = new UserLoginCommand( + $this->stubCliLoginHelper, + 'cli_login_route', + $this->stubUrlGenerator, + $this->stubUserProvider + ); + } + + public function testExecuteSuccess(): void + { + $this->stubCliLoginHelper + ->method('createToken') + ->willReturn('generated-token'); + + $this->stubUrlGenerator + ->method('generate') + ->willReturn('https://app.com/login?loginToken=generated-token'); + + $tester = new CommandTester($this->command); + $result = $tester->execute(['username' => 'testuser']); + + $this->assertSame(Command::SUCCESS, $result); + $this->assertStringContainsString('https://app.com/login?loginToken=generated-token', $tester->getDisplay()); + } + + public function testExecuteUserNotFound(): void + { + $this->stubUserProvider + ->method('loadUserByIdentifier') + ->willThrowException(new UserNotFoundException()); + + $tester = new CommandTester($this->command); + $result = $tester->execute(['username' => 'nonexistent']); + + $this->assertSame(Command::FAILURE, $result); + $this->assertStringContainsString('User does not exist', $tester->getDisplay()); + } +} diff --git a/tests/Controller/LoginControllerTest.php b/tests/Controller/LoginControllerTest.php index 4fc2919..7305e74 100644 --- a/tests/Controller/LoginControllerTest.php +++ b/tests/Controller/LoginControllerTest.php @@ -13,7 +13,7 @@ class LoginControllerTest extends TestCase { - private $loginController; + private LoginController $loginController; public function setUp(): void { @@ -46,8 +46,8 @@ public function setUp(): void public function testLogin(): void { - $mockRequest = $this->createMock(Request::class); - $mockRequest->query = new InputBag(['provider' => 'test']); + $stubRequest = $this->createStub(Request::class); + $stubRequest->query = new InputBag(['provider' => 'test']); $mockSession = $this->createMock(SessionInterface::class); $matcher = $this->exactly(3); $mockSession @@ -67,7 +67,7 @@ public function testLogin(): void } }); - $response = $this->loginController->login($mockRequest, $mockSession, 'test'); + $response = $this->loginController->login($stubRequest, $mockSession, 'test'); $this->assertInstanceOf(RedirectResponse::class, $response); $this->assertSame('https://test.com', $response->getTargetUrl()); } diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php new file mode 100644 index 0000000..385ee00 --- /dev/null +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -0,0 +1,132 @@ +processor = new Processor(); + $this->configuration = new Configuration(); + } + + private function getMinimalConfig(): array + { + return [ + 'cache_options' => [ + 'cache_pool' => 'cache.app', + ], + 'cli_login_options' => [ + 'route' => 'my_route', + ], + 'openid_providers' => [ + 'provider1' => [ + 'options' => [ + 'metadata_url' => 'https://example.com/.well-known/openid-configuration', + 'client_id' => 'my_id', + 'client_secret' => 'my_secret', + ], + ], + ], + ]; + } + + public function testMinimalConfig(): void + { + $config = $this->processor->processConfiguration( + $this->configuration, + [$this->getMinimalConfig()] + ); + + $this->assertSame('cache.app', $config['cache_options']['cache_pool']); + $this->assertSame('my_route', $config['cli_login_options']['route']); + $this->assertNull($config['user_provider']); + $this->assertArrayHasKey('provider1', $config['openid_providers']); + + $provider = $config['openid_providers']['provider1']['options']; + $this->assertSame('https://example.com/.well-known/openid-configuration', $provider['metadata_url']); + $this->assertSame('my_id', $provider['client_id']); + $this->assertSame('my_secret', $provider['client_secret']); + $this->assertSame(10, $provider['leeway']); + $this->assertFalse($provider['allow_http']); + } + + public function testFullConfig(): void + { + $input = $this->getMinimalConfig(); + $input['user_provider'] = 'my_user_provider'; + $input['openid_providers']['provider1']['options']['leeway'] = 30; + $input['openid_providers']['provider1']['options']['redirect_uri'] = 'https://app.com/callback'; + $input['openid_providers']['provider1']['options']['allow_http'] = true; + + $config = $this->processor->processConfiguration( + $this->configuration, + [$input] + ); + + $this->assertSame('my_user_provider', $config['user_provider']); + + $provider = $config['openid_providers']['provider1']['options']; + $this->assertSame(30, $provider['leeway']); + $this->assertSame('https://app.com/callback', $provider['redirect_uri']); + $this->assertTrue($provider['allow_http']); + } + + public function testRedirectRouteConfig(): void + { + $input = $this->getMinimalConfig(); + $input['openid_providers']['provider1']['options']['redirect_route'] = 'my_redirect_route'; + + $config = $this->processor->processConfiguration( + $this->configuration, + [$input] + ); + + $provider = $config['openid_providers']['provider1']['options']; + $this->assertSame('my_redirect_route', $provider['redirect_route']); + } + + public function testBothRedirectUriAndRouteThrows(): void + { + $input = $this->getMinimalConfig(); + $input['openid_providers']['provider1']['options']['redirect_uri'] = 'https://app.com/callback'; + $input['openid_providers']['provider1']['options']['redirect_route'] = 'my_route'; + + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Only one of redirect_uri or redirect_route must be set.'); + + $this->processor->processConfiguration( + $this->configuration, + [$input] + ); + } + + public function testMultipleProviders(): void + { + $input = $this->getMinimalConfig(); + $input['openid_providers']['provider2'] = [ + 'options' => [ + 'metadata_url' => 'https://other.com/.well-known/openid-configuration', + 'client_id' => 'other_id', + 'client_secret' => 'other_secret', + ], + ]; + + $config = $this->processor->processConfiguration( + $this->configuration, + [$input] + ); + + $this->assertCount(2, $config['openid_providers']); + $this->assertArrayHasKey('provider1', $config['openid_providers']); + $this->assertArrayHasKey('provider2', $config['openid_providers']); + } +} diff --git a/tests/DependencyInjection/ItkDevOpenIdConnectExtensionTest.php b/tests/DependencyInjection/ItkDevOpenIdConnectExtensionTest.php new file mode 100644 index 0000000..3e2b14a --- /dev/null +++ b/tests/DependencyInjection/ItkDevOpenIdConnectExtensionTest.php @@ -0,0 +1,69 @@ + [ + 'cache_pool' => 'cache.app', + ], + 'cli_login_options' => [ + 'route' => 'test_route', + ], + 'user_provider' => $userProvider, + 'openid_providers' => [ + 'test_provider' => [ + 'options' => [ + 'metadata_url' => 'https://example.com/.well-known/openid-configuration', + 'client_id' => 'test_id', + 'client_secret' => 'test_secret', + ], + ], + ], + ]; + } + + public function testLoad(): void + { + $extension = new ItkDevOpenIdConnectExtension(); + $container = new ContainerBuilder(); + + $extension->load([$this->getBaseConfig()], $container); + + $this->assertTrue($container->hasDefinition(OpenIdConfigurationProviderManager::class)); + $this->assertTrue($container->hasDefinition(CliLoginHelper::class)); + $this->assertTrue($container->hasDefinition(UserLoginCommand::class)); + $this->assertTrue($container->hasDefinition(CliLoginTokenAuthenticator::class)); + } + + public function testLoadWithUserProvider(): void + { + $extension = new ItkDevOpenIdConnectExtension(); + $container = new ContainerBuilder(); + + $extension->load([$this->getBaseConfig('my_custom_user_provider')], $container); + + $definition = $container->getDefinition(UserLoginCommand::class); + $userProviderArg = $definition->getArgument('$userProvider'); + $this->assertInstanceOf(Reference::class, $userProviderArg); + $this->assertSame('my_custom_user_provider', (string) $userProviderArg); + } + + public function testGetAlias(): void + { + $extension = new ItkDevOpenIdConnectExtension(); + $this->assertSame('itkdev_openid_connect', $extension->getAlias()); + } +} diff --git a/tests/ItkDevOpenIdConnectBundleTest.php b/tests/ItkDevOpenIdConnectBundleTest.php index 92253a9..6948230 100644 --- a/tests/ItkDevOpenIdConnectBundleTest.php +++ b/tests/ItkDevOpenIdConnectBundleTest.php @@ -18,7 +18,7 @@ class ItkDevOpenIdConnectBundleTest extends TestCase /** * Test service wiring. */ - public function testServiceWiring() + public function testServiceWiring(): void { $kernel = new ItkDevOpenIdConnectBundleTestingKernel([ __DIR__.'/config/framework.yml', diff --git a/tests/ItkDevOpenIdConnectBundleTestingKernel.php b/tests/ItkDevOpenIdConnectBundleTestingKernel.php index 56e8a04..b8f81b1 100644 --- a/tests/ItkDevOpenIdConnectBundleTestingKernel.php +++ b/tests/ItkDevOpenIdConnectBundleTestingKernel.php @@ -21,14 +21,11 @@ class ItkDevOpenIdConnectBundleTestingKernel extends Kernel { public function __construct( - private readonly array $pathToConfigs + private readonly array $pathToConfigs, ) { parent::__construct('test', true); } - /** - * {@inheritdoc} - */ public function registerBundles(): iterable { return [ @@ -39,11 +36,9 @@ public function registerBundles(): iterable } /** - * {@inheritdoc} - * * @throws \Exception */ - public function registerContainerConfiguration(LoaderInterface $loader) + public function registerContainerConfiguration(LoaderInterface $loader): void { $loader->load(function (ContainerBuilder $builder) { $builder->register(TestAuthenticator::class, TestAuthenticator::class); diff --git a/tests/Security/CliLoginTokenAuthenticatorTest.php b/tests/Security/CliLoginTokenAuthenticatorTest.php new file mode 100644 index 0000000..eb332ed --- /dev/null +++ b/tests/Security/CliLoginTokenAuthenticatorTest.php @@ -0,0 +1,149 @@ +stubCliLoginHelper = $this->createStub(CliLoginHelper::class); + $this->stubRouter = $this->createStub(UrlGeneratorInterface::class); + + $this->authenticator = new CliLoginTokenAuthenticator( + $this->stubCliLoginHelper, + 'cli_login_route', + $this->stubRouter + ); + } + + public function testSupportsWithLoginToken(): void + { + $request = $this->createStub(Request::class); + $request->query = new InputBag(['loginToken' => 'some-token']); + + $this->assertTrue($this->authenticator->supports($request)); + } + + public function testSupportsWithoutLoginToken(): void + { + $request = $this->createStub(Request::class); + $request->query = new InputBag(); + + $this->assertFalse($this->authenticator->supports($request)); + } + + public function testAuthenticateWithEmptyToken(): void + { + $request = $this->createStub(Request::class); + $request->query = new InputBag(['loginToken' => '']); + + $this->expectException(CustomUserMessageAuthenticationException::class); + $this->expectExceptionMessage('No login token provided'); + + $this->authenticator->authenticate($request); + } + + public function testAuthenticateWithInvalidToken(): void + { + $this->stubCliLoginHelper + ->method('getUsername') + ->willThrowException(new TokenNotFoundException('Token does not exist')); + + $request = $this->createStub(Request::class); + $request->query = new InputBag(['loginToken' => 'invalid-token']); + + $this->expectException(CustomUserMessageAuthenticationException::class); + $this->expectExceptionMessage('Cannot get username'); + + $this->authenticator->authenticate($request); + } + + public function testAuthenticateWithCacheException(): void + { + $this->stubCliLoginHelper + ->method('getUsername') + ->willThrowException(new CacheException('Cache error')); + + $request = $this->createStub(Request::class); + $request->query = new InputBag(['loginToken' => 'some-token']); + + $this->expectException(CustomUserMessageAuthenticationException::class); + $this->expectExceptionMessage('Cannot get username'); + + $this->authenticator->authenticate($request); + } + + public function testAuthenticateWithNullUsername(): void + { + $this->stubCliLoginHelper + ->method('getUsername') + ->willReturn(null); + + $request = $this->createStub(Request::class); + $request->query = new InputBag(['loginToken' => 'some-token']); + + $this->expectException(UsernameDoesNotExistException::class); + + $this->authenticator->authenticate($request); + } + + public function testAuthenticateSuccess(): void + { + $this->stubCliLoginHelper + ->method('getUsername') + ->willReturn('test@example.com'); + + $request = $this->createStub(Request::class); + $request->query = new InputBag(['loginToken' => 'valid-token']); + + $passport = $this->authenticator->authenticate($request); + + $userBadge = $passport->getBadge(UserBadge::class); + $this->assertSame('test@example.com', $userBadge->getUserIdentifier()); + } + + public function testOnAuthenticationSuccess(): void + { + $this->stubRouter + ->method('generate') + ->willReturn('/login'); + + $request = $this->createStub(Request::class); + $token = $this->createStub(TokenInterface::class); + + $response = $this->authenticator->onAuthenticationSuccess($request, $token, 'main'); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('/login', $response->getTargetUrl()); + } + + public function testOnAuthenticationFailure(): void + { + $request = $this->createStub(Request::class); + $exception = new AuthenticationException(); + + $this->expectException(AuthenticationException::class); + $this->expectExceptionMessage('Error occurred validating login token'); + + $this->authenticator->onAuthenticationFailure($request, $exception); + } +} diff --git a/tests/Security/OpenIdConfigurationProviderManagerTest.php b/tests/Security/OpenIdConfigurationProviderManagerTest.php new file mode 100644 index 0000000..94c3837 --- /dev/null +++ b/tests/Security/OpenIdConfigurationProviderManagerTest.php @@ -0,0 +1,135 @@ +stubRouter = $this->createStub(RouterInterface::class); + } + + private function getBaseProviderConfig(): array + { + return [ + 'metadata_url' => 'https://example.com/.well-known/openid-configuration', + 'client_id' => 'test_id', + 'client_secret' => 'test_secret', + ]; + } + + private function createManager(array $providers, array $defaultOptions = []): OpenIdConfigurationProviderManager + { + $config = [ + 'default_providers_options' => array_merge( + ['cacheItemPool' => new ArrayAdapter()], + $defaultOptions + ), + 'providers' => $providers, + ]; + + return new OpenIdConfigurationProviderManager($this->stubRouter, $config); + } + + public function testGetProviderKeys(): void + { + $manager = $this->createManager([ + 'provider_a' => $this->getBaseProviderConfig(), + 'provider_b' => $this->getBaseProviderConfig(), + ]); + + $this->assertSame(['provider_a', 'provider_b'], $manager->getProviderKeys()); + } + + public function testGetProviderThrowsOnInvalidKey(): void + { + $manager = $this->createManager([]); + + $this->expectException(InvalidProviderException::class); + $this->expectExceptionMessage('Invalid provider: nonexistent'); + + $manager->getProvider('nonexistent'); + } + + public function testGetProviderWithRedirectRoute(): void + { + $this->stubRouter + ->method('generate') + ->willReturn('https://app.com/callback'); + + $manager = $this->createManager([ + 'test' => $this->getBaseProviderConfig() + [ + 'redirect_route' => 'my_route', + 'redirect_route_parameters' => ['param' => 'value'], + ], + ]); + + $provider = $manager->getProvider('test'); + $this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider); + } + + public function testGetProviderWithRedirectRouteNoParameters(): void + { + $this->stubRouter + ->method('generate') + ->willReturn('https://app.com/callback'); + + $manager = $this->createManager([ + 'test' => $this->getBaseProviderConfig() + [ + 'redirect_route' => 'my_route', + ], + ]); + + $provider = $manager->getProvider('test'); + $this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider); + } + + public function testGetProviderWithLeeway(): void + { + $manager = $this->createManager([ + 'test' => $this->getBaseProviderConfig() + [ + 'redirect_uri' => 'https://app.com/callback', + 'leeway' => 30, + ], + ]); + + $provider = $manager->getProvider('test'); + $this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider); + } + + public function testGetProviderWithAllowHttp(): void + { + $manager = $this->createManager([ + 'test' => $this->getBaseProviderConfig() + [ + 'redirect_uri' => 'https://app.com/callback', + 'allow_http' => true, + ], + ]); + + $provider = $manager->getProvider('test'); + $this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider); + } + + public function testGetProviderCachesInstance(): void + { + $manager = $this->createManager([ + 'test' => $this->getBaseProviderConfig() + [ + 'redirect_uri' => 'https://app.com/callback', + ], + ]); + + $provider1 = $manager->getProvider('test'); + $provider2 = $manager->getProvider('test'); + + $this->assertSame($provider1, $provider2); + } +} diff --git a/tests/Security/OpenIdLoginAuthenticatorTest.php b/tests/Security/OpenIdLoginAuthenticatorTest.php index fbb9f96..238b468 100644 --- a/tests/Security/OpenIdLoginAuthenticatorTest.php +++ b/tests/Security/OpenIdLoginAuthenticatorTest.php @@ -7,7 +7,6 @@ use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider; use ItkDev\OpenIdConnectBundle\Security\OpenIdConfigurationProviderManager; use ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\InputBag; use Symfony\Component\HttpFoundation\Request; @@ -17,27 +16,27 @@ class OpenIdLoginAuthenticatorTest extends TestCase { private OpenIdLoginAuthenticator $authenticator; - private $mockProviderManager; + private OpenIdConfigurationProviderManager $stubProviderManager; - public function setup(): void + protected function setUp(): void { - $this->mockProviderManager = $this->createMock(OpenIdConfigurationProviderManager::class); + $this->stubProviderManager = $this->createStub(OpenIdConfigurationProviderManager::class); - $this->authenticator = new TestAuthenticator($this->mockProviderManager); + $this->authenticator = new TestAuthenticator($this->stubProviderManager); } public function testSupports(): void { - $mockRequest = $this->createMock(Request::class); - $mockRequest->query = new InputBag(); + $request = $this->createStub(Request::class); + $request->query = new InputBag(); - $this->assertFalse($this->authenticator->supports($mockRequest)); + $this->assertFalse($this->authenticator->supports($request)); - $mockRequest->query->set('state', 'abcd'); - $this->assertFalse($this->authenticator->supports($mockRequest)); + $request->query->set('state', 'abcd'); + $this->assertFalse($this->authenticator->supports($request)); - $mockRequest->query->set('code', 'xyz'); - $this->assertTrue($this->authenticator->supports($mockRequest)); + $request->query->set('code', 'xyz'); + $this->assertTrue($this->authenticator->supports($request)); } public function testOnAuthenticationFailure(): void @@ -52,90 +51,87 @@ public function testOnAuthenticationFailure(): void public function testValidateClaimsWrongState(): void { - $mockRequest = $this->createMock(Request::class); + $request = $this->createStub(Request::class); + $request->query = new InputBag(['state' => 'wrong_test_state']); - $mockRequest->query = new InputBag(['state' => 'wrong_test_state']); - - $this->setupMockSessionOnMockRequest($mockRequest); + $this->setupStubSessionOnRequest($request); $this->expectException(ValidationException::class); $this->expectExceptionMessage('Invalid state'); - $this->authenticator->authenticate($mockRequest); + $this->authenticator->authenticate($request); } - public function testValidateClaimsNoCode(): void + public function testValidateClaimsEmptyNonce(): void { - $mockRequest = $this->createMock(Request::class); - - $mockRequest->query = new InputBag(['state' => 'test_state']); + $request = $this->createStub(Request::class); + $request->query = new InputBag(['state' => 'test_state']); - $this->setupMockSessionOnMockRequest($mockRequest); + $this->setupStubSessionOnRequest($request, nonce: null); $this->expectException(ValidationException::class); - $this->expectExceptionMessage('Missing code'); - $this->authenticator->authenticate($mockRequest); + $this->expectExceptionMessage('Nonce empty or not found'); + $this->authenticator->authenticate($request); } - public function testValidateClaimsCodeNotString(): void + public function testValidateClaimsMissingCode(): void { - $mockRequest = $this->createMock(Request::class); - - $mockRequest->query = new InputBag(['state' => 'test_state', 'code' => 42]); + $request = $this->createStub(Request::class); + $request->query = new InputBag(['state' => 'test_state']); - $this->setupMockSessionOnMockRequest($mockRequest); + $this->setupStubSessionOnRequest($request); $this->expectException(ValidationException::class); - $this->expectExceptionMessage('Code not type string'); - $this->authenticator->authenticate($mockRequest); + $this->expectExceptionMessage('Missing or invalid code'); + $this->authenticator->authenticate($request); } public function testValidateClaimsCodeDoesNotValidate(): void { - $mockProvider = $this->createMock(OpenIdConfigurationProvider::class); - $mockProvider->method('validateIdToken')->willThrowException(new ClaimsException('test message')); - $this->mockProviderManager->method('getProvider')->willReturn($mockProvider); + $stubProvider = $this->createStub(OpenIdConfigurationProvider::class); + $stubProvider->method('validateIdToken')->willThrowException(new ClaimsException('test message')); + $this->stubProviderManager->method('getProvider')->willReturn($stubProvider); - $mockRequest = $this->createMock(Request::class); - $mockRequest->query = new InputBag(['state' => 'test_state', 'code' => 'test_code']); + $request = $this->createStub(Request::class); + $request->query = new InputBag(['state' => 'test_state', 'code' => 'test_code']); - $this->setupMockSessionOnMockRequest($mockRequest); + $this->setupStubSessionOnRequest($request); $this->expectException(ValidationException::class); $this->expectExceptionMessage('test message'); - $this->authenticator->authenticate($mockRequest); + $this->authenticator->authenticate($request); } public function testValidateClaimsSuccess(): void { - $mockProvider = $this->createMock(OpenIdConfigurationProvider::class); + $stubProvider = $this->createStub(OpenIdConfigurationProvider::class); $claims = new \stdClass(); $claims->email = 'test@test.com'; $claims->name = 'Test Tester'; - $mockProvider->method('validateIdToken')->willReturn($claims); + $stubProvider->method('validateIdToken')->willReturn($claims); - $this->mockProviderManager->method('getProvider')->willReturn($mockProvider); + $this->stubProviderManager->method('getProvider')->willReturn($stubProvider); - $mockRequest = $this->createMock(Request::class); - $mockRequest->query = new InputBag(['state' => 'test_state', 'code' => 'test_code']); + $request = $this->createStub(Request::class); + $request->query = new InputBag(['state' => 'test_state', 'code' => 'test_code']); - $this->setupMockSessionOnMockRequest($mockRequest); + $this->setupStubSessionOnRequest($request); - $passport = $this->authenticator->authenticate($mockRequest); + $passport = $this->authenticator->authenticate($request); $this->assertSame('test@test.com', $passport->getUser()->getUserIdentifier()); } - private function setupMockSessionOnMockRequest(MockObject $mockRequest) + private function setupStubSessionOnRequest(Request $request, ?string $nonce = 'test_nonce'): void { - $mockSession = $this->createMock(SessionInterface::class); + $stubSession = $this->createStub(SessionInterface::class); $map = [ ['oauth2provider', 'test_provider_1'], ['oauth2state', 'test_state'], - ['oauth2nonce', 'test_nonce'], + ['oauth2nonce', $nonce], ]; - $mockSession->method('remove')->willReturnMap($map); + $stubSession->method('remove')->willReturnMap($map); - $mockRequest->method('getSession')->willReturn($mockSession); + $request->method('getSession')->willReturn($stubSession); } } diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php index 791dfb4..8c05943 100644 --- a/tests/Util/CliLoginHelperTest.php +++ b/tests/Util/CliLoginHelperTest.php @@ -2,15 +2,18 @@ namespace ItkDev\OpenIdConnectBundle\Tests\Util; +use ItkDev\OpenIdConnectBundle\Exception\CacheException; use ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException; use ItkDev\OpenIdConnectBundle\Util\CliLoginHelper; use PHPUnit\Framework\TestCase; +use Psr\Cache\CacheItemInterface; +use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Uid\Uuid; class CliLoginHelperTest extends TestCase { - public function testEncodeAndDecode() + public function testEncodeAndDecode(): void { $cache = new ArrayAdapter(); $cliHelper = new CliLoginHelper($cache); @@ -23,7 +26,7 @@ public function testEncodeAndDecode() $this->assertEquals($randomUsername, $decodedUsername); } - public function testThrowExceptionIfTokenDoesNotExist() + public function testThrowExceptionIfTokenDoesNotExist(): void { $this->expectException(ItkOpenIdConnectBundleException::class); @@ -31,10 +34,10 @@ public function testThrowExceptionIfTokenDoesNotExist() $cliHelper = new CliLoginHelper($cache); - $username = $cliHelper->getUsername('random_gibberish_token'); + $cliHelper->getUsername('random_gibberish_token'); } - public function testReuseSetTokenRatherThanRemake() + public function testReuseSetTokenRatherThanRemake(): void { $cache = new ArrayAdapter(); @@ -47,7 +50,7 @@ public function testReuseSetTokenRatherThanRemake() $this->assertEquals($token, $token2); } - public function testTokenIsRemovedAfterUse() + public function testTokenIsRemovedAfterUse(): void { $cache = new ArrayAdapter(); @@ -57,13 +60,14 @@ public function testTokenIsRemovedAfterUse() $token = $cliHelper->createToken($testUser); $username = $cliHelper->getUsername($token); + $this->assertEquals($testUser, $username); $this->expectException(ItkOpenIdConnectBundleException::class); - $username = $cliHelper->getUsername($token); + $cliHelper->getUsername($token); } - public function testCreateTokenAndGetUsername() + public function testCreateTokenAndGetUsername(): void { $cache = new ArrayAdapter(); @@ -76,4 +80,111 @@ public function testCreateTokenAndGetUsername() $this->assertEquals($testUser, $username); } + + public function testCreateTokenThrowsCacheExceptionOnGetItem(): void + { + $stubCache = $this->createStub(CacheItemPoolInterface::class); + $stubCache->method('getItem') + ->willThrowException(new TestInvalidArgumentException('Cache error')); + + $cliHelper = new CliLoginHelper($stubCache); + + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Cache error'); + + $cliHelper->createToken('test_user'); + } + + public function testCreateTokenThrowsCacheExceptionOnSecondGetItem(): void + { + $stubCacheItem = $this->createStub(CacheItemInterface::class); + $stubCacheItem->method('isHit')->willReturn(false); + $stubCacheItem->method('get')->willReturn(null); + + $stubCache = $this->createStub(CacheItemPoolInterface::class); + $callCount = 0; + $stubCache->method('getItem') + ->willReturnCallback(function () use ($stubCacheItem, &$callCount) { + ++$callCount; + if (1 === $callCount) { + return $stubCacheItem; + } + throw new TestInvalidArgumentException('Second cache error'); + }); + $stubCache->method('save')->willReturn(true); + + $cliHelper = new CliLoginHelper($stubCache); + + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Second cache error'); + + $cliHelper->createToken('another_user'); + } + + public function testGetUsernameThrowsCacheExceptionOnGetItem(): void + { + $stubCache = $this->createStub(CacheItemPoolInterface::class); + $stubCache->method('getItem') + ->willThrowException(new TestInvalidArgumentException('Cache error')); + + $cliHelper = new CliLoginHelper($stubCache); + + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Cache error'); + + $cliHelper->getUsername('some-token'); + } + + public function testCreateTokenThrowsCacheExceptionOnNonStringCachedToken(): void + { + $stubCacheItem = $this->createStub(CacheItemInterface::class); + $stubCacheItem->method('isHit')->willReturn(true); + $stubCacheItem->method('get')->willReturn(42); + + $stubCache = $this->createStub(CacheItemPoolInterface::class); + $stubCache->method('getItem')->willReturn($stubCacheItem); + + $cliHelper = new CliLoginHelper($stubCache); + + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Cached token is not a string'); + + $cliHelper->createToken('test_user'); + } + + public function testGetUsernameThrowsCacheExceptionOnNonStringCachedUsername(): void + { + $stubCacheItem = $this->createStub(CacheItemInterface::class); + $stubCacheItem->method('isHit')->willReturn(true); + $stubCacheItem->method('get')->willReturn(42); + + $stubCache = $this->createStub(CacheItemPoolInterface::class); + $stubCache->method('getItem')->willReturn($stubCacheItem); + + $cliHelper = new CliLoginHelper($stubCache); + + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Cached username is not a string'); + + $cliHelper->getUsername('some-token'); + } + + public function testGetUsernameThrowsCacheExceptionOnDeleteItem(): void + { + $stubCacheItem = $this->createStub(CacheItemInterface::class); + $stubCacheItem->method('isHit')->willReturn(true); + $stubCacheItem->method('get')->willReturn('encoded_username'); + + $stubCache = $this->createStub(CacheItemPoolInterface::class); + $stubCache->method('getItem')->willReturn($stubCacheItem); + $stubCache->method('deleteItem') + ->willThrowException(new TestInvalidArgumentException('Delete error')); + + $cliHelper = new CliLoginHelper($stubCache); + + $this->expectException(CacheException::class); + $this->expectExceptionMessage('Delete error'); + + $cliHelper->getUsername('some-token'); + } } diff --git a/tests/Util/TestInvalidArgumentException.php b/tests/Util/TestInvalidArgumentException.php new file mode 100644 index 0000000..c6a2f78 --- /dev/null +++ b/tests/Util/TestInvalidArgumentException.php @@ -0,0 +1,9 @@ +