diff --git a/.github/workflows/documentation.yml b/.github/workflows/documentation.yml index a4b63e10..9e21e570 100644 --- a/.github/workflows/documentation.yml +++ b/.github/workflows/documentation.yml @@ -19,7 +19,7 @@ jobs: runs-on: [ubuntu-latest] steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Lint markdown files uses: nosborn/github-action-markdown-cli@v3 diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 4eb8c8a6..2fbb5fe6 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-latest continue-on-error: true steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: SonarQube Scan uses: SonarSource/sonarqube-scan-action@v2 env: @@ -17,4 +17,4 @@ jobs: with: args: > -Dsonar.projectKey=${{ github.event.repository.name }} - -Dsonar.projectName=${{ github.event.repository.name }} \ No newline at end of file + -Dsonar.projectName=${{ github.event.repository.name }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 7223fb26..c3e718aa 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -13,7 +13,7 @@ jobs: strategy: fail-fast: false matrix: - php-versions: ["8.2", "8.3"] + php-versions: ["8.3", "8.4", "8.5"] steps: - name: Setup PHP, with composer and extensions @@ -35,14 +35,14 @@ jobs: git config --global core.autocrlf false git config --global core.eol lf - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Get composer cache directory id: composer-cache run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$GITHUB_ENV" - name: Cache composer dependencies - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: $COMPOSER_CACHE key: "${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}" @@ -55,7 +55,7 @@ jobs: run: composer install --no-progress --prefer-dist --optimize-autoloader - name: Decide whether to run code coverage or not - if: ${{ matrix.php-versions != '8.2' }} + if: ${{ matrix.php-versions != '8.5' }} run: | echo "NO_COVERAGE=--no-coverage" >> $GITHUB_ENV @@ -70,13 +70,13 @@ jobs: ./vendor/bin/phpunit $NO_COVERAGE --no-configuration -c phpunit.integration.xml - name: Merge coverage data - if: ${{ matrix.php-versions == '8.2' }} + if: ${{ matrix.php-versions == '8.5' }} run: | ./vendor/bin/phpunit-merger log build/logs/partial_junit/ build/logs/junit.xml ./vendor/bin/phpunit-merger coverage build/logs/partial_clover/ build/logs/clover.xml - name: Save coverage data - if: ${{ matrix.php-versions == '8.2' }} + if: ${{ matrix.php-versions == '8.5' }} uses: actions/upload-artifact@v4 with: name: build-data @@ -89,7 +89,7 @@ jobs: - name: Setup PHP, with composer and extensions uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php with: - php-version: "8.2" + php-version: "8.3" extensions: mbstring, xml tools: composer:v2 coverage: none @@ -97,14 +97,14 @@ jobs: - name: Setup problem matchers for PHP run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Get composer cache directory id: composer-cache run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$GITHUB_ENV" - name: Cache composer dependencies - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: $COMPOSER_CACHE key: "${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}" @@ -130,7 +130,7 @@ jobs: - name: Setup PHP, with composer and extensions uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php with: - php-version: "8.2" + php-version: "8.3" extensions: mbstring, xml tools: composer:v2 coverage: none @@ -138,14 +138,14 @@ jobs: - name: Setup problem matchers for PHP run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Get composer cache directory id: composer-cache run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$GITHUB_ENV" - name: Cache composer dependencies - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: $COMPOSER_CACHE key: "${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}" @@ -163,21 +163,21 @@ jobs: - name: Setup PHP, with composer and extensions uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php with: - php-version: "8.2" + php-version: "8.5" tools: composer:v2 extensions: mbstring, xml - name: Setup problem matchers for PHP run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Get composer cache directory id: composer-cache run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$GITHUB_ENV" - name: Cache composer dependencies - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: $COMPOSER_CACHE key: "${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}" @@ -211,12 +211,12 @@ jobs: strategy: fail-fast: false matrix: - ssp-version: ["v2.3.7", "v2.4.4"] + ssp-version: ["v2.5.0"] env: SUITE_BASE_URL: https://localhost.emobix.co.uk:8443 VERSION: release-v5.1.35 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: path: main - name: Setup Python Dependencies diff --git a/composer.json b/composer.json index 9e529ef6..6bac59e6 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ } ], "require": { - "php": "^8.2", + "php": "^8.3", "ext-curl": "*", "ext-json": "*", "ext-openssl": "*", @@ -33,20 +33,18 @@ "simplesamlphp/composer-module-installer": "^1.3", "simplesamlphp/openid": "~v0.1.1", "spomky-labs/base64url": "^2.0", - "symfony/expression-language": "^6.3", - "symfony/psr-http-message-bridge": "^7.1", + "symfony/expression-language": "^7.4", + "symfony/psr-http-message-bridge": "^7.4", "web-token/jwt-framework": "^3", - "symfony/cache": "^6.4", + "symfony/cache": "^7.4", "psr/simple-cache": "^3" }, "require-dev": { "friendsofphp/php-cs-fixer": "^3", - "phpunit/phpunit": "^10", - "rector/rector": "^0.18.3", - "simplesamlphp/simplesamlphp": "2.3.*", - "simplesamlphp/simplesamlphp-test-framework": "^1.5", - "squizlabs/php_codesniffer": "^3", - "vimeo/psalm": "^5", + "rector/rector": "^1.2.10", + "simplesamlphp/simplesamlphp": "2.5.*", + "simplesamlphp/simplesamlphp-test-framework": "^1.9.3", + "vimeo/psalm": "^6.15.1", "testcontainers/testcontainers": "^0.2", "nimut/phpunit-merger": "^2.0" }, @@ -57,6 +55,7 @@ "sort-packages": true, "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true, + "php-http/discovery": true, "phpstan/extension-installer": true, "simplesamlphp/composer-module-installer": true, "simplesamlphp/composer-xmlprovider-installer": true diff --git a/docker/Dockerfile b/docker/Dockerfile index 10ce2bb1..e9fc90b9 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,6 +1,6 @@ -ARG SSP_VERSION="v2.4.4" -FROM cirrusid/simplesamlphp:${SSP_VERSION} -#FROM cicnavi/simplesamlphp:${SSP_VERSION} +ARG SSP_VERSION="v2.5.0" +#FROM cirrusid/simplesamlphp:${SSP_VERSION} +FROM cicnavi/simplesamlphp:${SSP_VERSION} RUN apt-get update && apt-get --no-install-recommends install -y sqlite3 # Prepopulate the DB with items needed for testing diff --git a/docs/1-oidc.md b/docs/1-oidc.md index 3bfd3c56..b4dcb14c 100644 --- a/docs/1-oidc.md +++ b/docs/1-oidc.md @@ -34,16 +34,16 @@ OIDFed is implemented using the Minor versions listed show which SimpleSAMLphp versions were used during module development. SimpleSAMLphp follows semantic versioning for its -API since v2.0. For example, v5.\* of the OIDC module should work with -any v2.\* of SimpleSAMLphp. PHP version requirements may differ. - -| OIDC module | Tested SimpleSAMLphp | PHP | Note | -|:------------|:---------------------|:------:|-------------| -| v6.\* | v2.3.\*, v2.4.\* | \>=8.2 | Recommended | -| v5.\* | v2.1.\* | \>=8.1 | | -| v4.\* | v2.0.\* | \>=8.0 | | -| v3.\* | v2.0.\* | \>=7.4 | | -| v2.\* | v1.19.\* | \>=7.4 | | +API since v2.0. PHP version requirements may differ. + +| OIDC module | Tested SimpleSAMLphp | PHP | +|:------------|:---------------------|:------:| +| v6.4.\* | v2.5.\* | \>=8.3 | +| v6.3.\* | v2.3.\*, v2.4.\* | \>=8.2 | +| v5.\* | v2.1.\* | \>=8.1 | +| v4.\* | v2.0.\* | \>=8.0 | +| v3.\* | v2.0.\* | \>=7.4 | +| v2.\* | v1.19.\* | \>=7.4 | Upgrading? See the [upgrade guide](6-oidc-upgrade.md). diff --git a/docs/6-oidc-upgrade.md b/docs/6-oidc-upgrade.md index eae2b3f5..8567c86a 100644 --- a/docs/6-oidc-upgrade.md +++ b/docs/6-oidc-upgrade.md @@ -3,6 +3,12 @@ This is an upgrade guide from versions 1 → 6. Review the changes and apply those relevant to your deployment. +## Version 6.3 to 6.4 + +This is a minor release in order to enable installation of the module with +SimpleSAMLphp v2.5.*, which now requires at least PHP v8.3 and bumps a bunch +of dependent Symfony packages to v7.4. + ## Version 5 to 6 New features: diff --git a/psalm.xml b/psalm.xml index 7ea003ea..ce0e8ea4 100644 --- a/psalm.xml +++ b/psalm.xml @@ -23,18 +23,20 @@ - - - - - - + + + + + + + + @@ -42,6 +44,12 @@ + + + + + + diff --git a/src/Controllers/Federation/EntityStatementController.php b/src/Controllers/Federation/EntityStatementController.php index b1b74f84..58fe43b1 100644 --- a/src/Controllers/Federation/EntityStatementController.php +++ b/src/Controllers/Federation/EntityStatementController.php @@ -27,8 +27,8 @@ class EntityStatementController { - protected const KEY_OP_ENTITY_CONFIGURATION_STATEMENT = 'op_entity_configuration_statement'; - protected const KEY_RP_SUBORDINATE_ENTITY_STATEMENT = 'rp_subordinate_entity_statement'; + protected const string KEY_OP_ENTITY_CONFIGURATION_STATEMENT = 'op_entity_configuration_statement'; + protected const string KEY_RP_SUBORDINATE_ENTITY_STATEMENT = 'rp_subordinate_entity_statement'; /** * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException diff --git a/src/Entities/ClientEntity.php b/src/Entities/ClientEntity.php index d834d41e..6543c355 100644 --- a/src/Entities/ClientEntity.php +++ b/src/Entities/ClientEntity.php @@ -28,29 +28,31 @@ class ClientEntity implements ClientEntityInterface use EntityTrait; use ClientTrait; - public const KEY_ID = 'id'; - public const KEY_SECRET = 'secret'; - public const KEY_NAME = 'name'; - public const KEY_DESCRIPTION = 'description'; - public const KEY_AUTH_SOURCE = 'auth_source'; - public const KEY_REDIRECT_URI = 'redirect_uri'; - public const KEY_SCOPES = 'scopes'; - public const KEY_IS_ENABLED = 'is_enabled'; - public const KEY_IS_CONFIDENTIAL = 'is_confidential'; - public const KEY_OWNER = 'owner'; - public const KEY_POST_LOGOUT_REDIRECT_URI = 'post_logout_redirect_uri'; - public const KEY_BACKCHANNEL_LOGOUT_URI = 'backchannel_logout_uri'; - public const KEY_ENTITY_IDENTIFIER = 'entity_identifier'; - public const KEY_CLIENT_REGISTRATION_TYPES = 'client_registration_types'; - public const KEY_FEDERATION_JWKS = 'federation_jwks'; - public const KEY_JWKS = 'jwks'; - public const KEY_JWKS_URI = 'jwks_uri'; - public const KEY_SIGNED_JWKS_URI = 'signed_jwks_uri'; - public const KEY_REGISTRATION_TYPE = 'registration_type'; - public const KEY_UPDATED_AT = 'updated_at'; - public const KEY_CREATED_AT = 'created_at'; - public const KEY_EXPIRES_AT = 'expires_at'; - public const KEY_IS_FEDERATED = 'is_federated'; + + public const string KEY_ID = 'id'; + public const string KEY_SECRET = 'secret'; + public const string KEY_NAME = 'name'; + public const string KEY_DESCRIPTION = 'description'; + public const string KEY_AUTH_SOURCE = 'auth_source'; + public const string KEY_REDIRECT_URI = 'redirect_uri'; + public const string KEY_SCOPES = 'scopes'; + public const string KEY_IS_ENABLED = 'is_enabled'; + public const string KEY_IS_CONFIDENTIAL = 'is_confidential'; + public const string KEY_OWNER = 'owner'; + public const string KEY_POST_LOGOUT_REDIRECT_URI = 'post_logout_redirect_uri'; + public const string KEY_BACKCHANNEL_LOGOUT_URI = 'backchannel_logout_uri'; + public const string KEY_ENTITY_IDENTIFIER = 'entity_identifier'; + public const string KEY_CLIENT_REGISTRATION_TYPES = 'client_registration_types'; + public const string KEY_FEDERATION_JWKS = 'federation_jwks'; + public const string KEY_JWKS = 'jwks'; + public const string KEY_JWKS_URI = 'jwks_uri'; + public const string KEY_SIGNED_JWKS_URI = 'signed_jwks_uri'; + public const string KEY_REGISTRATION_TYPE = 'registration_type'; + public const string KEY_UPDATED_AT = 'updated_at'; + public const string KEY_CREATED_AT = 'created_at'; + public const string KEY_EXPIRES_AT = 'expires_at'; + public const string KEY_IS_FEDERATED = 'is_federated'; + private string $secret; diff --git a/src/Factories/ClaimTranslatorExtractorFactory.php b/src/Factories/ClaimTranslatorExtractorFactory.php index 98c1a649..f8f06388 100644 --- a/src/Factories/ClaimTranslatorExtractorFactory.php +++ b/src/Factories/ClaimTranslatorExtractorFactory.php @@ -22,9 +22,10 @@ class ClaimTranslatorExtractorFactory { - protected const CONFIG_KEY_CLAIM_NAME_PREFIX = 'claim_name_prefix'; + protected const string CONFIG_KEY_CLAIM_NAME_PREFIX = 'claim_name_prefix'; + + protected const string CONFIG_KEY_MULTIPLE_CLAIM_VALUES_ALLOWED = 'are_multiple_claim_values_allowed'; - protected const CONFIG_KEY_MULTIPLE_CLAIM_VALUES_ALLOWED = 'are_multiple_claim_values_allowed'; public function __construct( private readonly ModuleConfig $moduleConfig, diff --git a/src/Forms/ClientForm.php b/src/Forms/ClientForm.php index 1b59c65d..10699b46 100644 --- a/src/Forms/ClientForm.php +++ b/src/Forms/ClientForm.php @@ -30,33 +30,34 @@ */ class ClientForm extends Form { - protected const TYPE_ARRAY = 'array'; + protected const string TYPE_ARRAY = 'array'; /** * RFC3986. AppendixB. Parsing a URI Reference with a Regular Expression. * From v6.*, the regex was modified to allow URI without host, to support adding entries like * `openid-credential-offer://` */ - final public const REGEX_URI = '/^[^:]+:\/\/?([^\s\/$.?#].[^\s]*)?$/'; + final public const string REGEX_URI = '/^[^:]+:\/\/?([^\s\/$.?#].[^\s]*)?$/'; /** * Must have http:// or https:// scheme, and at least one 'domain.top-level-domain' pair, or more subdomains. * Top-level-domain may end with '.'. * No reserved chars allowed, meaning no userinfo, path, query or fragment components. May end with port number. */ - final public const REGEX_ALLOWED_ORIGIN_URL = + final public const string REGEX_ALLOWED_ORIGIN_URL = "/^http(s?):\/\/([^\s\/!$&'()+,;=.?#@*:]+\.)" . "?[^\s\/!$&'()+,;=.?#@*:]+(\.[^\s\/!$&'()+,;=.?#@*:]+)*\.?(:\d{1,5})?$/i"; /** * URI which must contain https or http scheme, can contain path and query, and can't contain fragment. */ - final public const REGEX_HTTP_URI = '/^http(s?):\/\/[^\s\/$.?#][^\s#]*$/i'; + final public const string REGEX_HTTP_URI = '/^http(s?):\/\/[^\s\/$.?#][^\s#]*$/i'; /** * URI with https or http scheme and host / domain. It can contain path, but no query, or fragment component. */ - final public const REGEX_HTTP_URI_PATH = '/^http(s?):\/\/[^\s\/$.?#][^\s?#]*$/i'; + final public const string REGEX_HTTP_URI_PATH = '/^http(s?):\/\/[^\s\/$.?#][^\s?#]*$/i'; + /** * @throws \Exception diff --git a/src/Forms/Controls/CsrfProtection.php b/src/Forms/Controls/CsrfProtection.php index 0b93b51c..b659de67 100644 --- a/src/Forms/Controls/CsrfProtection.php +++ b/src/Forms/Controls/CsrfProtection.php @@ -24,7 +24,11 @@ class CsrfProtection extends BaseCsrfProtection { - final public const PROTECTION = [\SimpleSAML\Module\oidc\Forms\Controls\CsrfProtection::class, 'validateCsrf']; + final public const array PROTECTION = [ + \SimpleSAML\Module\oidc\Forms\Controls\CsrfProtection::class, + 'validateCsrf', + ]; + /** @noinspection PhpMissingParentConstructorInspection */ /** @@ -35,6 +39,7 @@ public function __construct(string|Stringable|null $errorMessage, protected Sess { // Instead of calling CsrfProtection parent class constructor, go to it's parent (HiddenField), and call // its constructor. This is to avoid setting a Nette session in CsrfProtection parent, and use the SSP one. + /** @psalm-suppress PossiblyFalseArgument */ $hiddentFieldParent = get_parent_class(get_parent_class($this)); if (!is_string($hiddentFieldParent)) { diff --git a/src/Helpers/Str.php b/src/Helpers/Str.php index 9218119d..5df6e4aa 100644 --- a/src/Helpers/Str.php +++ b/src/Helpers/Str.php @@ -23,8 +23,10 @@ public function convertScopesStringToArray(string $scopes, string $delimiter = ' */ public function convertTextToArray(string $text, string $pattern = "/[\t\r\n]+/"): array { + $split = is_array($split = preg_split($pattern, $text)) ? $split : [$text]; + return array_filter( - preg_split($pattern, $text), + $split, fn(string $line): bool => !empty(trim($line)), ); } diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index 3abe4bbe..2f42d085 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -29,78 +29,83 @@ class ModuleConfig { - final public const MODULE_NAME = 'oidc'; - protected const KEY_DESCRIPTION = 'description'; + final public const string MODULE_NAME = 'oidc'; + protected const string KEY_DESCRIPTION = 'description'; /** * Default file name for module configuration. Can be overridden in constructor, for example, for testing purposes. */ - final public const DEFAULT_FILE_NAME = 'module_oidc.php'; - - final public const OPTION_PKI_PRIVATE_KEY_PASSPHRASE = 'pass_phrase'; - final public const OPTION_PKI_PRIVATE_KEY_FILENAME = 'privatekey'; - final public const DEFAULT_PKI_PRIVATE_KEY_FILENAME = 'oidc_module.key'; - final public const OPTION_PKI_CERTIFICATE_FILENAME = 'certificate'; - final public const DEFAULT_PKI_CERTIFICATE_FILENAME = 'oidc_module.crt'; - final public const OPTION_TOKEN_AUTHORIZATION_CODE_TTL = 'authCodeDuration'; - final public const OPTION_TOKEN_REFRESH_TOKEN_TTL = 'refreshTokenDuration'; - final public const OPTION_TOKEN_ACCESS_TOKEN_TTL = 'accessTokenDuration'; - final public const OPTION_TOKEN_SIGNER = 'signer'; - final public const OPTION_AUTH_SOURCE = 'auth'; - final public const OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE = 'useridattr'; - final public const OPTION_AUTH_SAML_TO_OIDC_TRANSLATE_TABLE = 'translate'; - final public const OPTION_AUTH_CUSTOM_SCOPES = 'scopes'; - final public const OPTION_AUTH_ACR_VALUES_SUPPORTED = 'acrValuesSupported'; - final public const OPTION_AUTH_SOURCES_TO_ACR_VALUES_MAP = 'authSourcesToAcrValuesMap'; - final public const OPTION_AUTH_FORCED_ACR_VALUE_FOR_COOKIE_AUTHENTICATION = 'forcedAcrValueForCookieAuthentication'; - final public const OPTION_AUTH_PROCESSING_FILTERS = 'authproc.oidc'; - final public const OPTION_CRON_TAG = 'cron_tag'; - final public const OPTION_ADMIN_UI_PERMISSIONS = 'permissions'; - final public const OPTION_ADMIN_UI_PAGINATION_ITEMS_PER_PAGE = 'items_per_page'; - final public const OPTION_FEDERATION_TOKEN_SIGNER = 'federation_token_signer'; - final public const OPTION_PKI_FEDERATION_PRIVATE_KEY_PASSPHRASE = 'federation_private_key_passphrase'; - final public const OPTION_PKI_FEDERATION_PRIVATE_KEY_FILENAME = 'federation_private_key_filename'; - final public const DEFAULT_PKI_FEDERATION_PRIVATE_KEY_FILENAME = 'oidc_module_federation.key'; - final public const OPTION_PKI_FEDERATION_CERTIFICATE_FILENAME = 'federation_certificate_filename'; - final public const DEFAULT_PKI_FEDERATION_CERTIFICATE_FILENAME = 'oidc_module_federation.crt'; - final public const OPTION_ISSUER = 'issuer'; - final public const OPTION_FEDERATION_ENTITY_STATEMENT_DURATION = 'federation_entity_statement_duration'; - final public const OPTION_FEDERATION_AUTHORITY_HINTS = 'federation_authority_hints'; - final public const OPTION_ORGANIZATION_NAME = 'organization_name'; - final public const OPTION_DISPLAY_NAME = 'display_name'; - final public const OPTION_DESCRIPTION = 'description'; - final public const OPTION_KEYWORDS = 'keywords'; - final public const OPTION_CONTACTS = 'contacts'; - final public const OPTION_LOGO_URI = 'logo_uri'; - final public const OPTION_POLICY_URI = 'policy_uri'; - final public const OPTION_INFORMATION_URI = 'information_uri'; - final public const OPTION_HOMEPAGE_URI = 'homepage_uri'; - final public const OPTION_ORGANIZATION_URI = 'organization_uri'; - final public const OPTION_FEDERATION_ENABLED = 'federation_enabled'; - final public const OPTION_FEDERATION_CACHE_ADAPTER = 'federation_cache_adapter'; - final public const OPTION_FEDERATION_CACHE_ADAPTER_ARGUMENTS = 'federation_cache_adapter_arguments'; - final public const OPTION_FEDERATION_CACHE_MAX_DURATION_FOR_FETCHED = 'federation_cache_max_duration_for_fetched'; - final public const OPTION_FEDERATION_TRUST_ANCHORS = 'federation_trust_anchors'; - final public const OPTION_FEDERATION_TRUST_MARK_TOKENS = 'federation_trust_mark_tokens'; - final public const OPTION_FEDERATION_DYNAMIC_TRUST_MARKS = 'federation_dynamic_trust_mark_tokens'; - final public const OPTION_FEDERATION_PARTICIPATION_LIMIT_BY_TRUST_MARKS = + final public const string DEFAULT_FILE_NAME = 'module_oidc.php'; + + final public const string OPTION_PKI_PRIVATE_KEY_PASSPHRASE = 'pass_phrase'; + final public const string OPTION_PKI_PRIVATE_KEY_FILENAME = 'privatekey'; + final public const string DEFAULT_PKI_PRIVATE_KEY_FILENAME = 'oidc_module.key'; + final public const string OPTION_PKI_CERTIFICATE_FILENAME = 'certificate'; + final public const string DEFAULT_PKI_CERTIFICATE_FILENAME = 'oidc_module.crt'; + final public const string OPTION_TOKEN_AUTHORIZATION_CODE_TTL = 'authCodeDuration'; + final public const string OPTION_TOKEN_REFRESH_TOKEN_TTL = 'refreshTokenDuration'; + final public const string OPTION_TOKEN_ACCESS_TOKEN_TTL = 'accessTokenDuration'; + final public const string OPTION_TOKEN_SIGNER = 'signer'; + final public const string OPTION_AUTH_SOURCE = 'auth'; + final public const string OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE = 'useridattr'; + final public const string OPTION_AUTH_SAML_TO_OIDC_TRANSLATE_TABLE = 'translate'; + final public const string OPTION_AUTH_CUSTOM_SCOPES = 'scopes'; + final public const string OPTION_AUTH_ACR_VALUES_SUPPORTED = 'acrValuesSupported'; + final public const string OPTION_AUTH_SOURCES_TO_ACR_VALUES_MAP = 'authSourcesToAcrValuesMap'; + final public const string OPTION_AUTH_FORCED_ACR_VALUE_FOR_COOKIE_AUTHENTICATION = + 'forcedAcrValueForCookieAuthentication'; + final public const string OPTION_AUTH_PROCESSING_FILTERS = 'authproc.oidc'; + final public const string OPTION_CRON_TAG = 'cron_tag'; + final public const string OPTION_ADMIN_UI_PERMISSIONS = 'permissions'; + final public const string OPTION_ADMIN_UI_PAGINATION_ITEMS_PER_PAGE = 'items_per_page'; + final public const string OPTION_FEDERATION_TOKEN_SIGNER = 'federation_token_signer'; + final public const string OPTION_PKI_FEDERATION_PRIVATE_KEY_PASSPHRASE = 'federation_private_key_passphrase'; + final public const string OPTION_PKI_FEDERATION_PRIVATE_KEY_FILENAME = 'federation_private_key_filename'; + final public const string DEFAULT_PKI_FEDERATION_PRIVATE_KEY_FILENAME = 'oidc_module_federation.key'; + final public const string OPTION_PKI_FEDERATION_CERTIFICATE_FILENAME = 'federation_certificate_filename'; + final public const string DEFAULT_PKI_FEDERATION_CERTIFICATE_FILENAME = 'oidc_module_federation.crt'; + final public const string OPTION_ISSUER = 'issuer'; + final public const string OPTION_FEDERATION_ENTITY_STATEMENT_DURATION = 'federation_entity_statement_duration'; + final public const string OPTION_FEDERATION_AUTHORITY_HINTS = 'federation_authority_hints'; + final public const string OPTION_ORGANIZATION_NAME = 'organization_name'; + final public const string OPTION_DISPLAY_NAME = 'display_name'; + final public const string OPTION_DESCRIPTION = 'description'; + final public const string OPTION_KEYWORDS = 'keywords'; + final public const string OPTION_CONTACTS = 'contacts'; + final public const string OPTION_LOGO_URI = 'logo_uri'; + final public const string OPTION_POLICY_URI = 'policy_uri'; + final public const string OPTION_INFORMATION_URI = 'information_uri'; + final public const string OPTION_HOMEPAGE_URI = 'homepage_uri'; + final public const string OPTION_ORGANIZATION_URI = 'organization_uri'; + final public const string OPTION_FEDERATION_ENABLED = 'federation_enabled'; + final public const string OPTION_FEDERATION_CACHE_ADAPTER = 'federation_cache_adapter'; + final public const string OPTION_FEDERATION_CACHE_ADAPTER_ARGUMENTS = 'federation_cache_adapter_arguments'; + final public const string OPTION_FEDERATION_CACHE_MAX_DURATION_FOR_FETCHED = + 'federation_cache_max_duration_for_fetched'; + final public const string OPTION_FEDERATION_TRUST_ANCHORS = 'federation_trust_anchors'; + final public const string OPTION_FEDERATION_TRUST_MARK_TOKENS = 'federation_trust_mark_tokens'; + final public const string OPTION_FEDERATION_DYNAMIC_TRUST_MARKS = 'federation_dynamic_trust_mark_tokens'; + final public const string OPTION_FEDERATION_PARTICIPATION_LIMIT_BY_TRUST_MARKS = 'federation_participation_limit_by_trust_marks'; - final public const OPTION_FEDERATION_TRUST_MARK_STATUS_ENDPOINT_USAGE_POLICY = + final public const string OPTION_FEDERATION_TRUST_MARK_STATUS_ENDPOINT_USAGE_POLICY = 'federation_trust_mark_status_endpoint_usage_policy'; - final public const OPTION_FEDERATION_CACHE_DURATION_FOR_PRODUCED = 'federation_cache_duration_for_produced'; - final public const OPTION_PROTOCOL_CACHE_ADAPTER = 'protocol_cache_adapter'; - final public const OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS = 'protocol_cache_adapter_arguments'; - final public const OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION = 'protocol_user_entity_cache_duration'; - final public const OPTION_PROTOCOL_CLIENT_ENTITY_CACHE_DURATION = 'protocol_client_entity_cache_duration'; - final public const OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED = 'protocol_discover_show_claims_supported'; - - final public const OPTION_PKI_NEW_PRIVATE_KEY_PASSPHRASE = 'new_private_key_passphrase'; - final public const OPTION_PKI_NEW_PRIVATE_KEY_FILENAME = 'new_privatekey'; - final public const OPTION_PKI_NEW_CERTIFICATE_FILENAME = 'new_certificate'; - - final public const OPTION_PKI_FEDERATION_NEW_PRIVATE_KEY_PASSPHRASE = 'federation_new_private_key_passphrase'; - final public const OPTION_PKI_FEDERATION_NEW_PRIVATE_KEY_FILENAME = 'federation_new_private_key_filename'; - final public const OPTION_PKI_FEDERATION_NEW_CERTIFICATE_FILENAME = 'federation_new_certificate_filename'; + final public const string OPTION_FEDERATION_CACHE_DURATION_FOR_PRODUCED = 'federation_cache_duration_for_produced'; + final public const string OPTION_PROTOCOL_CACHE_ADAPTER = 'protocol_cache_adapter'; + final public const string OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS = 'protocol_cache_adapter_arguments'; + final public const string OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION = 'protocol_user_entity_cache_duration'; + final public const string OPTION_PROTOCOL_CLIENT_ENTITY_CACHE_DURATION = 'protocol_client_entity_cache_duration'; + final public const string OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED = + 'protocol_discover_show_claims_supported'; + + final public const string OPTION_PKI_NEW_PRIVATE_KEY_PASSPHRASE = 'new_private_key_passphrase'; + final public const string OPTION_PKI_NEW_PRIVATE_KEY_FILENAME = 'new_privatekey'; + final public const string OPTION_PKI_NEW_CERTIFICATE_FILENAME = 'new_certificate'; + + final public const string OPTION_PKI_FEDERATION_NEW_PRIVATE_KEY_PASSPHRASE = + 'federation_new_private_key_passphrase'; + final public const string OPTION_PKI_FEDERATION_NEW_PRIVATE_KEY_FILENAME = 'federation_new_private_key_filename'; + final public const string OPTION_PKI_FEDERATION_NEW_CERTIFICATE_FILENAME = 'federation_new_certificate_filename'; + protected static array $standardScopes = [ ScopesEnum::OpenId->value => [ diff --git a/src/Repositories/AccessTokenRepository.php b/src/Repositories/AccessTokenRepository.php index 1c3fef69..7be4ac56 100644 --- a/src/Repositories/AccessTokenRepository.php +++ b/src/Repositories/AccessTokenRepository.php @@ -35,7 +35,8 @@ class AccessTokenRepository extends AbstractDatabaseRepository implements AccessTokenRepositoryInterface { - final public const TABLE_NAME = 'oidc_access_token'; + final public const string TABLE_NAME = 'oidc_access_token'; + public function __construct( ModuleConfig $moduleConfig, diff --git a/src/Repositories/AllowedOriginRepository.php b/src/Repositories/AllowedOriginRepository.php index 30299dcb..9f76746d 100644 --- a/src/Repositories/AllowedOriginRepository.php +++ b/src/Repositories/AllowedOriginRepository.php @@ -8,7 +8,8 @@ class AllowedOriginRepository extends AbstractDatabaseRepository { - final public const TABLE_NAME = 'oidc_allowed_origin'; + final public const string TABLE_NAME = 'oidc_allowed_origin'; + public function getTableName(): string { diff --git a/src/Repositories/AuthCodeRepository.php b/src/Repositories/AuthCodeRepository.php index 46d46832..a4fe3301 100644 --- a/src/Repositories/AuthCodeRepository.php +++ b/src/Repositories/AuthCodeRepository.php @@ -32,7 +32,8 @@ class AuthCodeRepository extends AbstractDatabaseRepository implements AuthCodeRepositoryInterface { - final public const TABLE_NAME = 'oidc_auth_code'; + final public const string TABLE_NAME = 'oidc_auth_code'; + public function __construct( ModuleConfig $moduleConfig, diff --git a/src/Repositories/ClientRepository.php b/src/Repositories/ClientRepository.php index 27bc952a..0caf7f01 100644 --- a/src/Repositories/ClientRepository.php +++ b/src/Repositories/ClientRepository.php @@ -26,6 +26,9 @@ class ClientRepository extends AbstractDatabaseRepository implements ClientRepositoryInterface { + final public const string TABLE_NAME = 'oidc_client'; + + public function __construct( ModuleConfig $moduleConfig, Database $database, @@ -35,8 +38,6 @@ public function __construct( parent::__construct($moduleConfig, $database, $protocolCache); } - final public const TABLE_NAME = 'oidc_client'; - public function getTableName(): string { return $this->database->applyPrefix(self::TABLE_NAME); diff --git a/src/Repositories/RefreshTokenRepository.php b/src/Repositories/RefreshTokenRepository.php index 0d1ed120..63876843 100644 --- a/src/Repositories/RefreshTokenRepository.php +++ b/src/Repositories/RefreshTokenRepository.php @@ -32,7 +32,8 @@ class RefreshTokenRepository extends AbstractDatabaseRepository implements RefreshTokenRepositoryInterface { - final public const TABLE_NAME = 'oidc_refresh_token'; + final public const string TABLE_NAME = 'oidc_refresh_token'; + public function __construct( ModuleConfig $moduleConfig, diff --git a/src/Repositories/UserRepository.php b/src/Repositories/UserRepository.php index db644bdb..faf7da06 100644 --- a/src/Repositories/UserRepository.php +++ b/src/Repositories/UserRepository.php @@ -31,7 +31,8 @@ class UserRepository extends AbstractDatabaseRepository implements UserRepositoryInterface, IdentityProviderInterface { - final public const TABLE_NAME = 'oidc_user'; + final public const string TABLE_NAME = 'oidc_user'; + public function __construct( ModuleConfig $moduleConfig, diff --git a/src/Server/RequestRules/Rules/ClientAuthenticationRule.php b/src/Server/RequestRules/Rules/ClientAuthenticationRule.php index 62b522ca..edc4cecf 100644 --- a/src/Server/RequestRules/Rules/ClientAuthenticationRule.php +++ b/src/Server/RequestRules/Rules/ClientAuthenticationRule.php @@ -22,7 +22,8 @@ class ClientAuthenticationRule extends AbstractRule { - protected const KEY_CLIENT_ASSERTION_JTI = 'client_assertion_jti'; + protected const string KEY_CLIENT_ASSERTION_JTI = 'client_assertion_jti'; + public function __construct( RequestParamsResolver $requestParamsResolver, diff --git a/src/Server/RequestRules/Rules/ClientIdRule.php b/src/Server/RequestRules/Rules/ClientIdRule.php index b377377f..acb0a84e 100644 --- a/src/Server/RequestRules/Rules/ClientIdRule.php +++ b/src/Server/RequestRules/Rules/ClientIdRule.php @@ -30,7 +30,8 @@ class ClientIdRule extends AbstractRule { - protected const KEY_REQUEST_OBJECT_JTI = 'request_object_jti'; + protected const string KEY_REQUEST_OBJECT_JTI = 'request_object_jti'; + public function __construct( RequestParamsResolver $requestParamsResolver, diff --git a/src/Server/TokenIssuers/AbstractTokenIssuer.php b/src/Server/TokenIssuers/AbstractTokenIssuer.php index 5b13d703..8490bb6b 100644 --- a/src/Server/TokenIssuers/AbstractTokenIssuer.php +++ b/src/Server/TokenIssuers/AbstractTokenIssuer.php @@ -8,7 +8,8 @@ abstract class AbstractTokenIssuer { - public const MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS = 5; + public const int MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS = 5; + public function __construct( protected readonly Helpers $helpers, diff --git a/src/Server/Validators/BearerTokenValidator.php b/src/Server/Validators/BearerTokenValidator.php index 0a371aa4..3bcec36d 100644 --- a/src/Server/Validators/BearerTokenValidator.php +++ b/src/Server/Validators/BearerTokenValidator.php @@ -21,6 +21,7 @@ use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Services\LoggerService; +use function apache_request_headers; use function count; use function date_default_timezone_get; use function is_array; diff --git a/src/Services/AuthContextService.php b/src/Services/AuthContextService.php index 7783aed2..1ca2dfd2 100644 --- a/src/Services/AuthContextService.php +++ b/src/Services/AuthContextService.php @@ -19,7 +19,8 @@ class AuthContextService /** * Users with this permission can register,edit,etc. their own clients */ - final public const PERM_CLIENT = 'client'; + final public const string PERM_CLIENT = 'client'; + /** * AuthContextService constructor. diff --git a/src/Services/LoggerService.php b/src/Services/LoggerService.php index 9afcc3ef..58bffa9c 100644 --- a/src/Services/LoggerService.php +++ b/src/Services/LoggerService.php @@ -14,42 +14,42 @@ class LoggerService implements LoggerInterface { public function emergency(string|Stringable $message, array $context = []): void { - Logger::emergency($message . ($context ? " " . var_export($context, true) : "")); + Logger::emergency((string)$message . ($context ? " " . var_export($context, true) : "")); } public function alert(string|Stringable $message, array $context = []): void { - Logger::alert($message . ($context ? " " . var_export($context, true) : "")); + Logger::alert((string)$message . ($context ? " " . var_export($context, true) : "")); } public function critical(string|Stringable $message, array $context = []): void { - Logger::critical($message . ($context ? " " . var_export($context, true) : "")); + Logger::critical((string)$message . ($context ? " " . var_export($context, true) : "")); } public function error(string|Stringable $message, array $context = []): void { - Logger::error($message . ($context ? " " . var_export($context, true) : "")); + Logger::error((string)$message . ($context ? " " . var_export($context, true) : "")); } public function warning(string|Stringable $message, array $context = []): void { - Logger::warning($message . ($context ? " " . var_export($context, true) : "")); + Logger::warning((string)$message . ($context ? " " . var_export($context, true) : "")); } public function notice(string|Stringable $message, array $context = []): void { - Logger::notice($message . ($context ? " " . var_export($context, true) : "")); + Logger::notice((string)$message . ($context ? " " . var_export($context, true) : "")); } public function info(string|Stringable $message, array $context = []): void { - Logger::info($message . ($context ? " " . var_export($context, true) : "")); + Logger::info((string)$message . ($context ? " " . var_export($context, true) : "")); } public function debug(string|Stringable $message, array $context = []): void { - Logger::debug($message . ($context ? " " . var_export($context, true) : "")); + Logger::debug((string)$message . ($context ? " " . var_export($context, true) : "")); } public function log(mixed $level, string|Stringable $message, array $context = []): void diff --git a/src/Services/SessionService.php b/src/Services/SessionService.php index aaeccd3c..2349ae84 100644 --- a/src/Services/SessionService.php +++ b/src/Services/SessionService.php @@ -9,16 +9,18 @@ class SessionService { - final public const SESSION_DATA_TYPE = 'oidc'; + final public const string SESSION_DATA_TYPE = 'oidc'; - final public const SESSION_DATA_ID_IS_COOKIE_BASED_AUTHN = 'is-cookie-based-authn'; + final public const string SESSION_DATA_ID_IS_COOKIE_BASED_AUTHN = 'is-cookie-based-authn'; - final public const SESSION_DATA_ID_RP_ASSOCIATIONS = 'rp-associations'; + final public const string SESSION_DATA_ID_RP_ASSOCIATIONS = 'rp-associations'; - final public const SESSION_DATA_ID_IS_AUTHN_PERFORMED_IN_PREVIOUS_REQUEST = + final public const string SESSION_DATA_ID_IS_AUTHN_PERFORMED_IN_PREVIOUS_REQUEST = 'is-authn-performed-in-previous-request'; - final public const SESSION_DATA_ID_IS_OIDC_INITIATED_LOGOUT = 'is-logout-handler-disabled'; + final public const string SESSION_DATA_ID_IS_OIDC_INITIATED_LOGOUT = + 'is-logout-handler-disabled'; + public function __construct(protected Session $session) { diff --git a/src/Stores/Session/LogoutTicketStoreDb.php b/src/Stores/Session/LogoutTicketStoreDb.php index c7025cde..18bcf24c 100644 --- a/src/Stores/Session/LogoutTicketStoreDb.php +++ b/src/Stores/Session/LogoutTicketStoreDb.php @@ -12,7 +12,8 @@ class LogoutTicketStoreDb implements LogoutTicketStoreInterface { - final public const TABLE_NAME = 'oidc_session_logout_ticket'; + final public const string TABLE_NAME = 'oidc_session_logout_ticket'; + protected Database $database; diff --git a/src/Utils/ClaimTranslatorExtractor.php b/src/Utils/ClaimTranslatorExtractor.php index 9d536136..f04760cb 100644 --- a/src/Utils/ClaimTranslatorExtractor.php +++ b/src/Utils/ClaimTranslatorExtractor.php @@ -31,6 +31,49 @@ class ClaimTranslatorExtractor { + /** + * From JSON Web Token Claims registry: https://www.iana.org/assignments/jwt/jwt.xhtml + */ + final public const array REGISTERED_CLAIMS = [ + ...RegisteredClaims::ALL, + 'azp', + 'nonce', + 'auth_time', + 'at_hash', + 'c_hash', + 'acr', + 'amr', + 'sub_jwk', + ]; + + /** + * As per https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + */ + final public const array MANDATORY_SINGLE_VALUE_CLAIMS = [ + 'sub', + // TODO mivanci v7 Uncomment the rest of the claims, as this was a potential breaking change in v6. +// 'name', +// 'given_name', +// 'family_name', +// 'middle_name', +// 'nickname', +// 'preferred_username', +// 'profile', +// 'picture', +// 'website', +// 'email', +// 'email_verified', +// 'gender', +// 'birthdate', +// 'zoneinfo', +// 'locale', +// 'phone_number', +// 'phone_number_verified', +// 'address', +// 'updated_at', + ]; + + /** @var array */ protected array $claimSets = []; @@ -110,47 +153,6 @@ class ClaimTranslatorExtractor ], ]; - /** - * From JSON Web Token Claims registry: https://www.iana.org/assignments/jwt/jwt.xhtml - */ - final public const REGISTERED_CLAIMS = [ - ...RegisteredClaims::ALL, - 'azp', - 'nonce', - 'auth_time', - 'at_hash', - 'c_hash', - 'acr', - 'amr', - 'sub_jwk', - ]; - - /** - * As per https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - */ - final public const MANDATORY_SINGLE_VALUE_CLAIMS = [ - 'sub', - // TODO mivanci v7 Uncomment the rest of the claims, as this was a potential breaking change in v6. -// 'name', -// 'given_name', -// 'family_name', -// 'middle_name', -// 'nickname', -// 'preferred_username', -// 'profile', -// 'picture', -// 'website', -// 'email', -// 'email_verified', -// 'gender', -// 'birthdate', -// 'zoneinfo', -// 'locale', -// 'phone_number', -// 'phone_number_verified', -// 'address', -// 'updated_at', - ]; /** * ClaimTranslatorExtractor constructor. diff --git a/src/Utils/Debug/ArrayLogger.php b/src/Utils/Debug/ArrayLogger.php index d228693f..337ec8fc 100644 --- a/src/Utils/Debug/ArrayLogger.php +++ b/src/Utils/Debug/ArrayLogger.php @@ -12,20 +12,22 @@ class ArrayLogger implements LoggerInterface { - public const WEIGHT_EMERGENCY = 8; - public const WEIGH_ALERT = 7; - public const WEIGHT_CRITICAL = 6; - public const WEIGHT_ERROR = 5; - public const WEIGHT_WARNING = 4; - public const WEIGHT_NOTICE = 3; - public const WEIGHT_INFO = 2; - public const WEIGHT_DEBUG = 1; + public const int WEIGHT_EMERGENCY = 8; + public const int WEIGH_ALERT = 7; + public const int WEIGHT_CRITICAL = 6; + public const int WEIGHT_ERROR = 5; + public const int WEIGHT_WARNING = 4; + public const int WEIGHT_NOTICE = 3; + public const int WEIGHT_INFO = 2; + public const int WEIGHT_DEBUG = 1; + protected int $weight; /** @var string[] */ protected array $entries = []; + public function __construct( protected readonly Helpers $helpers, int $weight = self::WEIGHT_DEBUG, diff --git a/src/Utils/FingerprintGenerator.php b/src/Utils/FingerprintGenerator.php index a7cdc965..0733adac 100644 --- a/src/Utils/FingerprintGenerator.php +++ b/src/Utils/FingerprintGenerator.php @@ -21,7 +21,7 @@ public static function forFile(string $path, string $algo = 'md5'): string { $fingerprint = hash_file($algo, $path); - if (false === (bool) $fingerprint) { + if (false === $fingerprint) { throw new InvalidArgumentException( 'Could not create a fingerprint for provided file using provided algorithm.', ); @@ -41,14 +41,6 @@ public static function forFile(string $path, string $algo = 'md5'): string */ public static function forString(string $content, string $algo = 'md5'): string { - $fingerprint = hash($algo, $content); - - if (false === (bool) $fingerprint) { - throw new InvalidArgumentException( - 'Could not create a fingerprint for provided content using provided algorithm.', - ); - } - - return $fingerprint; + return hash($algo, $content); } } diff --git a/tests/unit/src/Controllers/Admin/ClientControllerTest.php b/tests/unit/src/Controllers/Admin/ClientControllerTest.php index f591f966..47e2ca36 100644 --- a/tests/unit/src/Controllers/Admin/ClientControllerTest.php +++ b/tests/unit/src/Controllers/Admin/ClientControllerTest.php @@ -23,7 +23,6 @@ use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Services\SessionMessagesService; use SimpleSAML\Module\oidc\Utils\Routes; -use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\Request; #[CoversClass(ClientController::class)] @@ -41,9 +40,6 @@ class ClientControllerTest extends TestCase protected MockObject $helpersMock; protected MockObject $loggerMock; protected MockObject $clientEntityMock; - protected MockObject $requestMock; - protected MockObject $queryInputBagMock; - protected MockObject $requestInputBagMock; protected MockObject $clientFormMock; protected array $sampleFormData = [ @@ -105,12 +101,6 @@ protected function setUp(): void $this->clientEntityMock = $this->createMock(ClientEntityInterface::class); - $this->requestMock = $this->createMock(Request::class); - $this->queryInputBagMock = $this->createMock(ParameterBag::class); - $this->requestMock->query = $this->queryInputBagMock; - $this->requestInputBagMock = $this->createMock(ParameterBag::class); - $this->requestMock->request = $this->requestInputBagMock; - $this->clientFormMock = $this->createMock(ClientForm::class); $this->formFactoryMock->method('build')->willReturn($this->clientFormMock); } @@ -163,10 +153,12 @@ public function testCanCreateInstance(): void public function testIndex(): void { - $this->queryInputBagMock->expects($this->once())->method('getInt')->with('page') - ->willReturn(1); - $this->queryInputBagMock->expects($this->once())->method('getString')->with('q') - ->willReturn('abc'); + $request = Request::create( + '/', + 'GET', + ['page' => '1', 'q' => 'abc'], + ); + $this->clientRepositoryMock->expects($this->once())->method('findPaginated') ->with(1, 'abc', null)->willReturn([ 'items' => [$this->clientEntityMock], @@ -177,90 +169,113 @@ public function testIndex(): void $this->templateFactoryMock->expects($this->once())->method('build') ->with('oidc:clients.twig'); - $this->sut()->index($this->requestMock); + $this->sut()->index($request); } public function testShow(): void { - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); + $request = Request::create( + '/', + 'GET', + ['client_id' => 'clientId'], + ); + $this->clientEntityMock->expects($this->once())->method('getIdentifier')->willReturn('clientId'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); $this->templateFactoryMock->expects($this->once())->method('build') ->with('oidc:clients/show.twig'); - $this->sut()->show($this->requestMock); + $this->sut()->show($request); } public function testShowThrowsIfClientIdNotProvided(): void { + $request = Request::create( + '/', + 'GET', + [], + ); + $this->expectException(OidcException::class); $this->expectExceptionMessage('Client ID'); - $this->sut()->show($this->requestMock); + $this->sut()->show($request); } public function testCanResetSecret(): void { - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); + $request = Request::create( + '/resetSecret?client_id=clientId', + 'POST', + ['client_id' => 'clientId', 'secret' => '123'], + ); + $this->clientEntityMock->expects($this->once())->method('getSecret')->willReturn('123'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); - $this->requestInputBagMock->expects($this->once())->method('getString') - ->with('secret')->willReturn('123'); $this->clientEntityMock->expects($this->once())->method('restoreSecret'); $this->clientRepositoryMock->expects($this->once())->method('update') ->with($this->clientEntityMock); $this->sessionMessagesServiceMock->expects($this->once())->method('addMessage') ->with($this->stringContains('secret')); - $this->sut()->resetSecret($this->requestMock); + $this->sut()->resetSecret($request); } public function testResetSecretThrowsIfCurrentSecretNotValid(): void { - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); + $request = Request::create( + '/resetSecret?client_id=clientId', + 'POST', + ['client_id' => 'clientId', 'secret' => '321'], + ); + $this->clientEntityMock->expects($this->once())->method('getSecret')->willReturn('123'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); - $this->requestInputBagMock->expects($this->once())->method('getString') - ->with('secret')->willReturn('321'); $this->expectException(OidcException::class); $this->expectExceptionMessage('Client secret'); - $this->sut()->resetSecret($this->requestMock); + $this->sut()->resetSecret($request); } public function testCanDelete(): void { - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); + $request = Request::create( + '/delete?client_id=clientId', + 'POST', + ['client_id' => 'clientId', 'secret' => '123'], + ); + $this->clientEntityMock->expects($this->once())->method('getSecret')->willReturn('123'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); - $this->requestInputBagMock->expects($this->once())->method('getString') - ->with('secret')->willReturn('123'); $this->sessionMessagesServiceMock->expects($this->once())->method('addMessage') ->with($this->stringContains('deleted')); $this->clientRepositoryMock->expects($this->once())->method('delete') ->with($this->clientEntityMock); - $this->sut()->delete($this->requestMock); + $this->sut()->delete($request); } public function testDeleteThrowsIfCurrentSecretNotValid(): void { - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); + $request = Request::create( + '/resetSecret?client_id=clientId', + 'POST', + ['client_id' => 'clientId', 'secret' => '321'], + ); + $this->clientEntityMock->expects($this->once())->method('getSecret')->willReturn('123'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); - $this->requestInputBagMock->expects($this->once())->method('getString') - ->with('secret')->willReturn('321'); $this->expectException(OidcException::class); $this->expectExceptionMessage('Client secret'); - $this->sut()->delete($this->requestMock); + $this->sut()->delete($request); } public function testCanAdd(): void @@ -348,10 +363,15 @@ public function testThrowsForInvalidClientData(): void public function testCanEdit(): void { + $request = Request::create( + '/edit?client_id=clientId', + 'GET', + ['client_id' => 'clientId', 'secret' => '123'], + ); + // Original client. // Enum can't be doubled :/. $this->clientEntityMock->method('getRegistrationType')->willReturn(RegistrationTypeEnum::Manual); - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); $this->clientEntityMock->method('getIdentifier')->willReturn('clientId'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); @@ -373,15 +393,20 @@ public function testCanEdit(): void $this->allowedOriginRepositoryMock->expects($this->once())->method('set') ->with('clientId'); - $this->sut()->edit($this->requestMock); + $this->sut()->edit($request); } public function testWontEditIfClientEntityIdentifierExists(): void { + $request = Request::create( + '/edit?client_id=clientId', + 'GET', + ['client_id' => 'clientId', 'secret' => '123'], + ); + // Original client. // Enum can't be doubled :/. $this->clientEntityMock->method('getRegistrationType')->willReturn(RegistrationTypeEnum::Manual); - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); $this->clientEntityMock->method('getIdentifier')->willReturn('clientId'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); @@ -408,12 +433,17 @@ public function testWontEditIfClientEntityIdentifierExists(): void $this->sessionMessagesServiceMock->expects($this->once())->method('addMessage') ->with($this->stringContains('exists')); - $this->sut()->edit($this->requestMock); + $this->sut()->edit($request); } public function testCanShowEditForm(): void { - $this->queryInputBagMock->expects($this->once())->method('getString')->willReturn('clientId'); + $request = Request::create( + '/edit?client_id=clientId', + 'GET', + ['client_id' => 'clientId', 'secret' => '123'], + ); + $this->clientEntityMock->method('getIdentifier')->willReturn('clientId'); $this->clientRepositoryMock->expects($this->once())->method('findById')->with('clientId') ->willReturn($this->clientEntityMock); @@ -423,6 +453,6 @@ public function testCanShowEditForm(): void $this->templateFactoryMock->expects($this->once())->method('build') ->with('oidc:clients/edit.twig'); - $this->sut()->edit($this->requestMock); + $this->sut()->edit($request); } } diff --git a/tests/unit/src/Controllers/Federation/SubordinateListingsControllerTest.php b/tests/unit/src/Controllers/Federation/SubordinateListingsControllerTest.php index d1396f1c..60591dee 100644 --- a/tests/unit/src/Controllers/Federation/SubordinateListingsControllerTest.php +++ b/tests/unit/src/Controllers/Federation/SubordinateListingsControllerTest.php @@ -14,8 +14,6 @@ use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Utils\Routes; use SimpleSAML\OpenID\Codebooks\ErrorsEnum; -use SimpleSAML\OpenID\Codebooks\ParamsEnum; -use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\Request; #[CoversClass(SubordinateListingsController::class)] @@ -26,8 +24,6 @@ final class SubordinateListingsControllerTest extends TestCase private MockObject $routesMock; private bool $isFederationEnabled; - private MockObject $requestMock; - private MockObject $requestQueryMock; protected function setUp(): void @@ -37,10 +33,6 @@ protected function setUp(): void $this->routesMock = $this->createMock(Routes::class); $this->isFederationEnabled = true; - - $this->requestMock = $this->createMock(Request::class); - $this->requestQueryMock = $this->createMock(ParameterBag::class); - $this->requestMock->query = $this->requestQueryMock; } public function sut( @@ -78,6 +70,12 @@ public function testThrowsIfFederationNotEnabled(): void public function testCanListFederatedEntities(): void { + $request = Request::create( + '/list', + 'GET', + [], + ); + $client = $this->createMock(ClientEntityInterface::class); $client->method('getEntityIdentifier')->willReturn('entity-id'); @@ -93,18 +91,20 @@ public function testCanListFederatedEntities(): void $client->getEntityIdentifier(), ]); - $this->sut()->list($this->requestMock); + $this->sut()->list($request); } public function testListReturnsErrorOnUnsuportedQueryParameter(): void { - $this->requestQueryMock->method('all')->willReturn([ - ParamsEnum::EntityType->value => 'something', - ]); + $request = Request::create( + '/list', + 'GET', + ['entity_type' => 'something'], + ); $this->routesMock->expects($this->once())->method('newJsonErrorResponse') ->with(ErrorsEnum::UnsupportedParameter->value); - $this->sut()->list($this->requestMock); + $this->sut()->list($request); } }