diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index e310c5d..bd355e3 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -14,50 +14,62 @@ on: # yamllint disable-line rule:truthy workflow_dispatch: jobs: + phplinter: + name: 'PHP-Linter' + strategy: + fail-fast: false + matrix: + php-version: ['8.1', '8.2', '8.3', '8.4'] + + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_phplinter.yml@v1.9.2 + with: + php-version: ${{ matrix.php-version }} + linter: - name: Linter - runs-on: ['ubuntu-latest'] + name: 'Linter' + strategy: + fail-fast: false - steps: - - uses: actions/checkout@v5 - with: - fetch-depth: 0 - - - name: Lint Code Base - uses: super-linter/super-linter/slim@v8 - env: - SAVE_SUPER_LINTER_OUTPUT: false - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - LINTER_RULES_PATH: 'tools/linters' - LOG_LEVEL: NOTICE - VALIDATE_ALL_CODEBASE: true - VALIDATE_CSS: true - VALIDATE_JAVASCRIPT_ES: true - VALIDATE_JSON: true - VALIDATE_PHP_BUILTIN: true - VALIDATE_YAML: true - VALIDATE_XML: true - VALIDATE_GITHUB_ACTIONS: true + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_linter.yml@v1.9.2 + with: + enable_eslinter: false + enable_jsonlinter: true + enable_stylelinter: true + enable_yamllinter: true - quality: - name: Quality control - runs-on: [ubuntu-latest] + unit-tests-linux: + name: "Unit tests, PHP ${{ matrix.php-versions }}, ${{ matrix.operating-system }}" + runs-on: ${{ matrix.operating-system }} + needs: [phplinter, linter] + + strategy: + fail-fast: false + matrix: + operating-system: [ubuntu-latest] + php-versions: ['8.1', '8.2', '8.3', '8.4'] steps: - name: Setup PHP, with composer and extensions - id: setup-php # https://github.com/shivammathur/setup-php uses: shivammathur/setup-php@v2 with: - # Should be the higest supported version, so we can use the newest tools - php-version: '8.3' - tools: composer, composer-require-checker, composer-unused, phpcs, psalm - # optional performance gain for psalm: opcache - extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, opcache, openssl, pcre, posix, spl, xml + php-version: ${{ matrix.php-versions }} + extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, posix, spl, xml + tools: composer + ini-values: error_reporting=E_ALL + coverage: pcov - name: Setup problem matchers for PHP run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" + - name: Setup problem matchers for PHPUnit + run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" + + - name: Set git to use LF + run: | + git config --global core.autocrlf false + git config --global core.eol lf + - uses: actions/checkout@v5 - name: Get composer cache directory @@ -70,64 +82,61 @@ jobs: key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} restore-keys: ${{ runner.os }}-composer- - - name: Validate composer.json and composer.lock - run: composer validate - - name: Install Composer dependencies run: composer install --no-progress --prefer-dist --optimize-autoloader - - name: Check code for hard dependencies missing in composer.json - run: composer-require-checker check --config-file=tools/composer-require-checker.json composer.json - - - name: Check code for unused dependencies in composer.json - run: composer-unused + - name: Run unit tests with coverage + if: ${{ matrix.php-versions == '8.4' }} + run: vendor/bin/phpunit - - name: PHP Code Sniffer - run: phpcs + - name: Run unit tests (no coverage) + if: ${{ matrix.php-versions != '8.4' }} + run: vendor/bin/phpunit --no-coverage - - name: Psalm - continue-on-error: true - run: | - psalm -c psalm.xml \ - --show-info=true \ - --shepherd \ - --php-version=${{ steps.setup-php.outputs.php-version }} + - name: Save coverage data + if: ${{ matrix.php-versions == '8.4' }} + uses: actions/upload-artifact@v4 + with: + name: coverage-data + path: ${{ github.workspace }}/build - - name: Psalm (testsuite) - run: | - psalm -c psalm-dev.xml \ - --show-info=true \ - --shepherd \ - --php-version=${{ steps.setup-php.outputs.php-version }} + unit-tests-windows: + name: "Unit tests, PHP ${{ matrix.php-versions }}, ${{ matrix.operating-system }}" + runs-on: ${{ matrix.operating-system }} + needs: [phplinter, linter] - - name: Psalter - run: | - psalm --alter \ - --issues=UnnecessaryVarAnnotation \ - --dry-run \ - --php-version=${{ steps.setup-php.outputs.php-version }} + strategy: + fail-fast: true + matrix: + operating-system: [windows-latest] + php-versions: ['8.1', '8.2', '8.3', '8.4'] - security: - name: Security checks - runs-on: [ubuntu-latest] steps: - name: Setup PHP, with composer and extensions # https://github.com/shivammathur/setup-php uses: shivammathur/setup-php@v2 with: - # Should be the lowest supported version - php-version: '8.1' - extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, posix, spl, xml + php-version: ${{ matrix.php-versions }} + extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, posix, spl, xml, zip tools: composer + ini-values: error_reporting=E_ALL coverage: none - name: Setup problem matchers for PHP run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" + - name: Setup problem matchers for PHPUnit + run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" + + - name: Set git to use LF + run: | + git config --global core.autocrlf false + git config --global core.eol lf + - uses: actions/checkout@v5 - name: Get composer cache directory - run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$GITHUB_ENV" + run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$env:GITHUB_ENV" - name: Cache composer dependencies uses: actions/cache@v4 @@ -137,37 +146,26 @@ jobs: restore-keys: ${{ runner.os }}-composer- - name: Install Composer dependencies - run: composer install --no-progress --prefer-dist --optimize-autoloader - - - name: Security check for locked dependencies - run: composer audit - - - name: Update Composer dependencies - run: composer update --no-progress --prefer-dist --optimize-autoloader + run: composer install --no-progress --prefer-dist --optimize-autoloader --ignore-platform-req=ext-posix - - name: Security check for updated dependencies - run: composer audit + - name: Run unit tests + run: vendor/bin/phpunit --no-coverage - unit-tests-linux: - name: "Unit tests, PHP ${{ matrix.php-versions }}, ${{ matrix.operating-system }}" - runs-on: ${{ matrix.operating-system }} - needs: [linter, quality, security] - strategy: - fail-fast: false - matrix: - operating-system: [ubuntu-latest] - php-versions: ['8.1', '8.2', '8.3'] + quality: + name: Quality control + runs-on: [ubuntu-latest] + needs: [unit-tests-linux] steps: - name: Setup PHP, with composer and extensions + id: setup-php # https://github.com/shivammathur/setup-php uses: shivammathur/setup-php@v2 with: - php-version: ${{ matrix.php-versions }} + # Should be the higest supported version, so we can use the newest tools + php-version: '8.4' + tools: composer, composer-require-checker, composer-unused, phpcs, phpstan extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, posix, spl, xml - tools: composer - ini-values: error_reporting=E_ALL - coverage: pcov - name: Setup problem matchers for PHP run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" @@ -192,43 +190,43 @@ jobs: key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} restore-keys: ${{ runner.os }}-composer- + - name: Validate composer.json and composer.lock + run: composer validate + - name: Install Composer dependencies run: composer install --no-progress --prefer-dist --optimize-autoloader - - name: Run unit tests with coverage - if: ${{ matrix.php-versions == '8.3' }} - run: vendor/bin/phpunit + - name: Check code for hard dependencies missing in composer.json + run: composer-require-checker check --config-file=tools/composer-require-checker.json composer.json - - name: Run unit tests (no coverage) - if: ${{ matrix.php-versions != '8.3' }} - run: vendor/bin/phpunit --no-coverage + - name: Check code for unused dependencies in composer.json + run: composer-unused - - name: Save coverage data - if: ${{ matrix.php-versions == '8.3' }} - uses: actions/upload-artifact@v4 - with: - name: coverage-data - path: ${{ github.workspace }}/build + - name: PHP Code Sniffer + run: phpcs - unit-tests-windows: - name: "Unit tests, PHP ${{ matrix.php-versions }}, ${{ matrix.operating-system }}" - runs-on: ${{ matrix.operating-system }} - needs: [linter, quality, security] - strategy: - fail-fast: true - matrix: - operating-system: [windows-latest] - php-versions: ['8.1', '8.2', '8.3'] + - name: PHPStan + run: | + vendor/bin/phpstan analyze -c phpstan.neon + + - name: PHPStan (testsuite) + run: | + vendor/bin/phpstan analyze -c phpstan-dev.neon + + security: + name: Security checks + runs-on: [ubuntu-latest] + needs: [unit-tests-linux] steps: - name: Setup PHP, with composer and extensions # https://github.com/shivammathur/setup-php uses: shivammathur/setup-php@v2 with: - php-version: ${{ matrix.php-versions }} + # Should be the lowest supported version + php-version: '8.1' extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, posix, spl, xml tools: composer - ini-values: error_reporting=E_ALL coverage: none - name: Setup problem matchers for PHP @@ -245,7 +243,7 @@ jobs: - uses: actions/checkout@v5 - name: Get composer cache directory - run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$env:GITHUB_ENV" + run: echo COMPOSER_CACHE="$(composer config cache-files-dir)" >> "$GITHUB_ENV" - name: Cache composer dependencies uses: actions/cache@v4 @@ -255,15 +253,22 @@ jobs: restore-keys: ${{ runner.os }}-composer- - name: Install Composer dependencies - run: composer install --no-progress --prefer-dist --optimize-autoloader --ignore-platform-req=ext-posix + run: composer install --no-progress --prefer-dist --optimize-autoloader - - name: Run unit tests - run: vendor/bin/phpunit --no-coverage + - name: Security check for locked dependencies + run: composer audit + + - name: Update Composer dependencies + run: composer update --no-progress --prefer-dist --optimize-autoloader + + - name: Security check for updated dependencies + run: composer audit coverage: name: Code coverage runs-on: [ubuntu-latest] needs: [unit-tests-linux] + steps: - uses: actions/checkout@v5 diff --git a/codecov.yml b/codecov.yml index f9d26c6..019c39c 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,3 +1,5 @@ +--- + coverage: status: project: diff --git a/phpstan-dev.neon b/phpstan-dev.neon new file mode 100644 index 0000000..09d9773 --- /dev/null +++ b/phpstan-dev.neon @@ -0,0 +1,4 @@ +parameters: + level: 5 + paths: + - tests diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..db37782 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,4 @@ +parameters: + level: 6 + paths: + - src diff --git a/psalm-dev.xml b/psalm-dev.xml deleted file mode 100644 index 6116331..0000000 --- a/psalm-dev.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - - - - - - - - - - - - diff --git a/psalm.xml b/psalm.xml deleted file mode 100644 index 4270d1c..0000000 --- a/psalm.xml +++ /dev/null @@ -1,31 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/routing/routes/routes.yml b/routing/routes/routes.yml index 8df54c1..83ac0f1 100644 --- a/routing/routes/routes.yml +++ b/routing/routes/routes.yml @@ -1,16 +1,29 @@ +--- + consent-getconsent: - path: /getconsent - defaults: { _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::getconsent' } - methods: [GET] + path: /getconsent + defaults: { + _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::getconsent' + } + methods: [GET] + consent-noconsent: - path: /noconsent - defaults: { _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::noconsent' } - methods: [GET] + path: /noconsent + defaults: { + _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::noconsent' + } + methods: [GET] + consent-logout: - path: /logout - defaults: { _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::logout' } - methods: [GET] + path: /logout + defaults: { + _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::logout' + } + methods: [GET] + consent-logoutcompleted: - path: /logoutcompleted - defaults: { _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::logoutcompleted' } - methods: [GET] + path: /logoutcompleted + defaults: { + _controller: 'SimpleSAML\Module\consent\Controller\ConsentController::logoutcompleted' + } + methods: [GET] diff --git a/src/Auth/Process/Consent.php b/src/Auth/Process/Consent.php index f0f5575..4223934 100644 --- a/src/Auth/Process/Consent.php +++ b/src/Auth/Process/Consent.php @@ -57,14 +57,14 @@ class Consent extends Auth\ProcessingFilter /** * Attributes where the value should be hidden * - * @var array + * @var array */ private array $hiddenAttributes = []; /** * Attributes which should not require consent * - * @var array + * @var array */ private array $noconsentattributes = []; @@ -88,12 +88,12 @@ class Consent extends Auth\ProcessingFilter * * Validates and parses the configuration. * - * @param array $config Configuration information. + * @param array $config Configuration information. * @param mixed $reserved For future use. * * @throws \SimpleSAML\Error\Exception if the configuration is not valid. */ - public function __construct(array $config, $reserved) + public function __construct($config, $reserved) { parent::__construct($config, $reserved); @@ -181,7 +181,7 @@ public function __construct(array $config, $reserved) /** * Helper function to check whether consent is disabled. * - * @param mixed $option The consent.disable option. Either an array of array, an array or a boolean. + * @param mixed $option The consent.disable option. Either an array of array, an array or a boolean. * @param string $entityId The entityID of the SP/IdP. * * @return boolean True if disabled, false if not. @@ -241,8 +241,7 @@ private static function checkDisable($option, string $entityId): bool * This function saves the state, and redirects the user to the page where the user can authorize the release of * the attributes. If storage is used and the consent has already been given the user is passed on. * - * @param array &$state The state of the response. - * + * @param array &$state The state of the response. * * @throws \SimpleSAML\Module\saml\Error\NoPassive if the request was passive and consent is needed. */ @@ -282,6 +281,7 @@ public function process(array &$state): void Stats::log('consent:disabled', $statsData); return; } + if ( isset($state['Destination']['consent.disable']) && self::checkDisable($state['Destination']['consent.disable'], $idpEntityId) @@ -416,7 +416,7 @@ public static function getTargetedID(string $userid, string $source, string $des * Create a hash value for the attributes that changes when attributes are added or removed. If the attribute * values are included in the hash, the hash will change if the values change. * - * @param array $attributes The attributes. + * @param array $attributes The attributes. * @param bool $includeValues Whether or not to include the attribute value in the generation of the hash. * * @return string SHA1 of the user id, source id, destination id and salt. diff --git a/src/Consent/Store/Cookie.php b/src/Consent/Store/Cookie.php index 924b56c..5e3b0b2 100644 --- a/src/Consent/Store/Cookie.php +++ b/src/Consent/Store/Cookie.php @@ -34,39 +34,40 @@ class Cookie extends \SimpleSAML\Module\consent\Store /** * @var string Cookie name prefix */ - private $name; + private string $name; /** * @var int Cookie lifetime */ - private $lifetime; + private int $lifetime; /** * @var string Cookie path */ - private $path; + private string $path; /** * @var string Cookie domain */ - private $domain = ''; + private string $domain = ''; /** * @var bool Cookie secure flag */ - private $secure; + private bool $secure; /** * @var string|null Cookie samesite flag */ - private $samesite = null; + private ?string $samesite = null; + /** * Parse configuration. * * This constructor parses the configuration. * - * @param array $config Configuration for database consent store. + * @param array $config Configuration for database consent store. * * @throws \Exception in case of a configuration error. */ @@ -109,6 +110,7 @@ public function __construct(array $config) } } + /** * Check for consent. * @@ -224,7 +226,7 @@ public function deleteAllConsents(string $userId): void * * @param string $userId The hash identifying the user at an IdP. * - * @return array Array of all destination ids the user has given consent for. + * @return array Array of all destination ids the user has given consent for. */ public function getConsents(string $userId): array { @@ -343,7 +345,7 @@ private function setConsentCookie(string $name, ?string $value): bool 'path' => $this->path, 'domain' => $this->domain, 'httponly' => true, - 'secure' => $httpUtils->isHTTPS(), + 'secure' => $this->secure, 'samesite' => $this->samesite, ]; diff --git a/src/Consent/Store/Database.php b/src/Consent/Store/Database.php index c488f2a..0fb0f48 100644 --- a/src/Consent/Store/Database.php +++ b/src/Consent/Store/Database.php @@ -49,6 +49,8 @@ class Database extends \SimpleSAML\Module\consent\Store /** * Options for the database; + * + * @var array */ private array $options = []; @@ -77,7 +79,7 @@ class Database extends \SimpleSAML\Module\consent\Store * * This constructor parses the configuration. * - * @param array $config Configuration for database consent store. + * @param array $config Configuration for database consent store. * * @throws \Exception in case of a configuration error. */ @@ -139,7 +141,7 @@ public function __construct(array $config) /** * Called before serialization. * - * @return array The variables which should be serialized. + * @return string[] The variables which should be serialized. */ public function __sleep(): array { @@ -303,7 +305,7 @@ public function deleteAllConsents(string $userId): int * * @param string $userId The hash identifying the user at an IdP. * - * @return array Array of all destination ids the user has given consent for. + * @return string[] Array of all destination ids the user has given consent for. */ public function getConsents(string $userId): array { @@ -334,16 +336,13 @@ public function getConsents(string $userId): array * returned. * * @param string $statement The statement which should be executed. - * @param array $parameters Parameters for the statement. + * @param array $parameters Parameters for the statement. * * @return \PDOStatement|false The statement, or false if execution failed. */ private function execute(string $statement, array $parameters) { $db = $this->getDB(); - if ($db === false) { - return false; - } $st = $db->prepare($statement); if ($st === false) { @@ -374,7 +373,7 @@ private function execute(string $statement, array $parameters) * - users: Total number of uses that have given consent * ' services: Total number of services that has been given consent to * - * @return array Array containing the statistics + * @return array Array containing the statistics */ public function getStatistics(): array { @@ -427,9 +426,9 @@ public function getStatistics(): array /** * Get database handle. * - * @return \PDO|false Database handle, or false if we fail to connect. + * @return \PDO Database handle, or false if we fail to connect. */ - private function getDB() + private function getDB(): PDO { if ($this->db !== null) { return $this->db; @@ -439,6 +438,7 @@ private function getDB() if (isset($this->timeout)) { $driver_options[PDO::ATTR_TIMEOUT] = $this->timeout; } + if (!empty($this->options)) { $this->options = array_merge($driver_options, $this->options); } else { @@ -456,7 +456,7 @@ private function getDB() * * This function formats a PDO error, as returned from errorInfo. * - * @param array $error The error information. + * @param string[] $error The error information. * * @return string Error text. */ diff --git a/src/Logout.php b/src/Logout.php index 94bf702..dd4ae10 100644 --- a/src/Logout.php +++ b/src/Logout.php @@ -18,7 +18,7 @@ class Logout { /** * @param \SimpleSAML\IdP $idp - * @param array $state + * @param array $state */ public static function postLogout(IdP $idp, array $state): void { diff --git a/src/Store.php b/src/Store.php index 581474a..0156081 100644 --- a/src/Store.php +++ b/src/Store.php @@ -20,10 +20,11 @@ abstract class Store * * This constructor should always be called first in any class which implements this class. * - * @param array &$config The configuration for this storage handler. + * @param array &$config The configuration for this storage handler. */ - protected function __construct(array &$config) - { + protected function __construct( + protected array &$config, + ) { } @@ -108,7 +109,7 @@ public function getStatistics() * * @param string $userId The hash identifying the user at an IdP. * - * @return array Array of all destination ids the user has given consent for. + * @return array Array of all destination ids the user has given consent for. */ abstract public function getConsents(string $userId): array; diff --git a/tests/src/Auth/Process/ConsentTest.php b/tests/src/Auth/Process/ConsentTest.php index 47d3b25..e2717e1 100644 --- a/tests/src/Auth/Process/ConsentTest.php +++ b/tests/src/Auth/Process/ConsentTest.php @@ -13,7 +13,7 @@ * * @package SimpleSAMLphp */ -class ConsentTest extends TestCase +final class ConsentTest extends TestCase { /** @var \SimpleSAML\Configuration */ protected $config; diff --git a/tests/src/Controller/ConsentControllerTest.php b/tests/src/Controller/ConsentControllerTest.php index 3452ff7..1d2c94d 100644 --- a/tests/src/Controller/ConsentControllerTest.php +++ b/tests/src/Controller/ConsentControllerTest.php @@ -22,7 +22,7 @@ * * @package SimpleSAML\Test */ -class ConsentControllerTest extends TestCase +final class ConsentControllerTest extends TestCase { /** @var \SimpleSAML\Configuration */ protected static Configuration $config; @@ -91,7 +91,7 @@ public static function loadState(string $id, string $stage, bool $allowMissing = 'consent:store.destination' => 'urn:some:sp', 'consent:store.attributeSet' => 'some hash', 'consent:store' => new class () extends Cookie { - public function __construct(array &$config = []) + public function __construct() { } diff --git a/tools/linters/.stylelintrc.json b/tools/linters/.stylelintrc.json new file mode 100644 index 0000000..531712e --- /dev/null +++ b/tools/linters/.stylelintrc.json @@ -0,0 +1,4 @@ +{ + "rules": { + } +} diff --git a/tools/linters/.yaml-lint.yml b/tools/linters/.yaml-lint.yml new file mode 100644 index 0000000..630095a --- /dev/null +++ b/tools/linters/.yaml-lint.yml @@ -0,0 +1,7 @@ +--- + +extends: default + +rules: + line-length: + max: 120