From 0c8e7bb3ef89c48c9dfac4f9f9bd0e252849740a Mon Sep 17 00:00:00 2001 From: Patrick Radtke Date: Tue, 12 Mar 2024 16:09:58 -0700 Subject: [PATCH 1/3] Initial SSP2 update --- .gitignore | 3 ++- composer.json | 11 +++++++++-- {lib => src}/Auth/Process/ScopeMap.php | 2 +- {lib => src}/Auth/Process/ScopeRewrite.php | 2 +- tests/bootstrap.php | 1 + tests/config/config.php | 20 +++++++------------- tests/lib/Auth/Process/ScopeMapTest.php | 17 ++++++++--------- tests/lib/Auth/Process/ScopeRewriteTest.php | 21 ++++++++++----------- 8 files changed, 39 insertions(+), 38 deletions(-) rename {lib => src}/Auth/Process/ScopeMap.php (97%) rename {lib => src}/Auth/Process/ScopeRewrite.php (98%) diff --git a/.gitignore b/.gitignore index 85cf442..4860570 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .idea *~ vendor -composer.lock \ No newline at end of file +composer.lock +.phpunit.result.cache \ No newline at end of file diff --git a/composer.json b/composer.json index 5f1d6c4..9d90360 100644 --- a/composer.json +++ b/composer.json @@ -3,11 +3,18 @@ "description": "Filter to rewrite the scope on attributes", "type": "simplesamlphp-module", "require": { + "php" : ">= 8.1", + "simplesamlphp/simplesamlphp": "~2.0", "simplesamlphp/composer-module-installer": "^1" }, "require-dev": { - "phpunit/phpunit": "5.7.*", - "simplesamlphp/simplesamlphp": "^1.18", + "vimeo/psalm": "^5.0", + "phpunit/phpunit": "^9.5", "squizlabs/php_codesniffer": "^3.5" + }, + "config": { + "allow-plugins": { + "simplesamlphp/composer-module-installer": true + } } } diff --git a/lib/Auth/Process/ScopeMap.php b/src/Auth/Process/ScopeMap.php similarity index 97% rename from lib/Auth/Process/ScopeMap.php rename to src/Auth/Process/ScopeMap.php index 3769160..d832c26 100644 --- a/lib/Auth/Process/ScopeMap.php +++ b/src/Auth/Process/ScopeMap.php @@ -31,7 +31,7 @@ public function __construct($config, $reserved) * * @param array &$request the current request */ - public function process(&$request) + public function process(array &$request): void { $newValues = []; $pattern = '/^(.*)@(.*)$/'; diff --git a/lib/Auth/Process/ScopeRewrite.php b/src/Auth/Process/ScopeRewrite.php similarity index 98% rename from lib/Auth/Process/ScopeRewrite.php rename to src/Auth/Process/ScopeRewrite.php index 123c68a..fe62b28 100644 --- a/lib/Auth/Process/ScopeRewrite.php +++ b/src/Auth/Process/ScopeRewrite.php @@ -48,7 +48,7 @@ public function __construct($config, $reserved) * * @param array &$request the current request */ - public function process(&$request) + public function process(array &$request): void { foreach ($this->attributesOldScopeToUsername as $attributeName) { diff --git a/tests/bootstrap.php b/tests/bootstrap.php index ef318f8..8328487 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -2,6 +2,7 @@ $projectRoot = dirname(__DIR__); require_once($projectRoot . '/vendor/autoload.php'); +putenv('SIMPLESAMLPHP_CONFIG_DIR=' . __DIR__ . '/config'); // Symlink module into ssp vendor lib so that templates and urls can resolve correctly $linkPath = $projectRoot . '/vendor/simplesamlphp/simplesamlphp/modules/scoperewrite'; diff --git a/tests/config/config.php b/tests/config/config.php index 1c5fc43..7ba29f9 100644 --- a/tests/config/config.php +++ b/tests/config/config.php @@ -1,14 +1,8 @@ '/', - 'debug' => true, - 'logging.level' => SimpleSAML\Logger::DEBUG, - 'logging.handler' => 'stderr', - - 'auth.adminpassword' => 'integrationtest', - - // currently: ignored 'templatedir' => dirname(dirname(dirname(__DIR__))) - - 'trusted.url.domains' => array(), - -); +$config = []; +// require a vanilla SSP config +require dirname(__DIR__, 2) . "/vendor/simplesamlphp/simplesamlphp/config/config.php.dist"; +$config['module.enable']['scoperewrite'] = true; +$config['baseurlpath'] = '/'; +$config['logging.handler'] = 'stderr'; +$config['logging.level'] = \SimpleSAML\Logger::DEBUG; diff --git a/tests/lib/Auth/Process/ScopeMapTest.php b/tests/lib/Auth/Process/ScopeMapTest.php index e513b47..bed5388 100644 --- a/tests/lib/Auth/Process/ScopeMapTest.php +++ b/tests/lib/Auth/Process/ScopeMapTest.php @@ -1,15 +1,13 @@ 'scoperewrite:ScopeMap', 'scopeMap' => [ 'student.example.edu' => 'example.edu', @@ -19,6 +17,7 @@ public static function setUpBeforeClass() 'srcAttribute' => 'scopedAttr', 'destAttribute' => 'rescopedAttr', ]; + /** * Helper function to run the filter with a given configuration. * @@ -26,7 +25,7 @@ public static function setUpBeforeClass() * @param array $request The request state. * @return array The state array after processing. */ - private static function processFilter(array $config, array $request) + private static function processFilter(array $config, array $request): array { $filter = new ScopeMap($config, null); $filter->process($request); @@ -36,7 +35,7 @@ private static function processFilter(array $config, array $request) /** * Test with no attributes */ - public function testNoAttributes() + public function testNoAttributes(): void { $request = array( 'Attributes' => array(), @@ -46,7 +45,7 @@ public function testNoAttributes() $this->assertEmpty($attributes, var_export($attributes, true)); } - public function testMapping() + public function testMapping(): void { $request = array( 'Attributes' => [ @@ -75,4 +74,4 @@ public function testMapping() $attributes = $result['Attributes']; $this->assertEquals($expectedAttributes, $attributes); } -} \ No newline at end of file +} diff --git a/tests/lib/Auth/Process/ScopeRewriteTest.php b/tests/lib/Auth/Process/ScopeRewriteTest.php index 547e628..22d49f1 100644 --- a/tests/lib/Auth/Process/ScopeRewriteTest.php +++ b/tests/lib/Auth/Process/ScopeRewriteTest.php @@ -1,14 +1,13 @@ process($request); @@ -27,7 +26,7 @@ private static function processFilter(array $config, array $request) /** * Test with no attributes */ - public function testNoAttributes() + public function testNoAttributes(): void { $config = array('newScope' => 'tester.com'); $request = array( @@ -41,7 +40,7 @@ public function testNoAttributes() /** * Test the most basic functionality. */ - public function testScopeRewriteDefaultConfig() + public function testScopeRewriteDefaultConfig(): void { $config = array('newScope' => 'tester.com'); $request = array( @@ -66,7 +65,7 @@ public function testScopeRewriteDefaultConfig() /** * Test optional enablement */ - public function testIgnoreScope() + public function testIgnoreScope(): void { $config = array( 'newScope' => 'tester.com', @@ -96,7 +95,7 @@ public function testIgnoreScope() /** * Test all config options */ - public function testScopeRewriteCustomConfig() + public function testScopeRewriteCustomConfig(): void { $config = array( 'newScope' => 'tester.com', @@ -128,7 +127,7 @@ public function testScopeRewriteCustomConfig() /** * Test picking a separator for the old scope and username */ - public function testOldScopeSeparator() + public function testOldScopeSeparator(): void { $config = array( 'newScope' => 'tester.com', From 513cf1cc59c15cd795109d0818ad5710eb5b5dab Mon Sep 17 00:00:00 2001 From: Patrick Radtke Date: Tue, 12 Mar 2024 16:31:51 -0700 Subject: [PATCH 2/3] psalm + phpcs --- README.md | 4 +- phpcs.xml | 19 +++++ psalm.xml | 25 ++++++ src/Auth/Process/ScopeMap.php | 23 ++++-- src/Auth/Process/ScopeRewrite.php | 88 ++++++++++----------- tests/lib/Auth/Process/ScopeMapTest.php | 1 - tests/lib/Auth/Process/ScopeRewriteTest.php | 2 - 7 files changed, 105 insertions(+), 57 deletions(-) create mode 100644 phpcs.xml create mode 100644 psalm.xml diff --git a/README.md b/README.md index 6635f4a..e05cb95 100644 --- a/README.md +++ b/README.md @@ -63,4 +63,6 @@ composer require cirrusidentity/simplesamlphp-module-scoperewrite:dev-master # Development * Write tests. Run tests with `phpunit`. The `phpunit.xml` file already defines the location for the tests -* Check PSR-2 style. `phpcs --standard=PSR2 lib` +* Check PSR-12 style. `phpcs --standard=PSR2 lib` +* Psalm. `./vendor/bin/psalm ` +* PHP unit `./vendor/bin/phpunit` diff --git a/phpcs.xml b/phpcs.xml new file mode 100644 index 0000000..bf78b8f --- /dev/null +++ b/phpcs.xml @@ -0,0 +1,19 @@ + + + + + + By default it is less stringent about long lines than other coding standards + + + src + tests + + + tests/config/config.php + tests/bootstrap.php + + + + + diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 0000000..ebcdea2 --- /dev/null +++ b/psalm.xml @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + diff --git a/src/Auth/Process/ScopeMap.php b/src/Auth/Process/ScopeMap.php index d832c26..3608a2d 100644 --- a/src/Auth/Process/ScopeMap.php +++ b/src/Auth/Process/ScopeMap.php @@ -8,8 +8,14 @@ class ScopeMap extends ProcessingFilter { + /** + * @var array + */ + private array $scopeMap; + private string $srcAttribute; + private string $destAttribute; - public function __construct($config, $reserved) + public function __construct(array &$config, mixed $reserved) { parent::__construct($config, $reserved); $conf = Configuration::loadFromArray($config); @@ -29,17 +35,20 @@ public function __construct($config, $reserved) /** * Apply filter. * - * @param array &$request the current request + * @param array &$state the current request */ - public function process(array &$request): void + public function process(array &$state): void { $newValues = []; $pattern = '/^(.*)@(.*)$/'; - foreach ($request['Attributes'][$this->srcAttribute] ?? [] as $value) { + /** @var string $value */ + foreach ($state['Attributes'][$this->srcAttribute] ?? [] as $value) { // pull off scope $matches = []; if (preg_match($pattern, $value, $matches) !== 1) { - Logger::warning('Unable to get user + scope from value ' . $value . '. Passing it through to dstAttribute'); + Logger::warning( + 'Unable to get user + scope from value ' . $value . '. Passing it through to dstAttribute' + ); $newValues[] = $value; continue; } @@ -53,7 +62,7 @@ public function process(array &$request): void } } if (!empty($newValues)) { - $request['Attributes'][$this->destAttribute] = $newValues; + $state['Attributes'][$this->destAttribute] = $newValues; } } -} \ No newline at end of file +} diff --git a/src/Auth/Process/ScopeRewrite.php b/src/Auth/Process/ScopeRewrite.php index fe62b28..c7b3453 100644 --- a/src/Auth/Process/ScopeRewrite.php +++ b/src/Auth/Process/ScopeRewrite.php @@ -3,61 +3,56 @@ namespace SimpleSAML\Module\scoperewrite\Auth\Process; use SimpleSAML\Auth\ProcessingFilter; +use SimpleSAML\Configuration; class ScopeRewrite extends ProcessingFilter { - private $newScope; + private string $newScope; - private $attributesOldScopeToUsername = array( - 'eduPersonPrincipalName', - ); + /** + * @var string[] + */ + private array $attributesOldScopeToUsername; - private $attributesReplaceScope = array( - 'eduPersonScopedAffiliation', - ); + /** + * @var string[] + */ + private array $attributesReplaceScope; - private $ignoreForScopes = array(); + /** + * @var string[] + */ + private array $ignoreForScopes; - private $oldScopeSeparator = '+'; + private string $oldScopeSeparator; - public function __construct($config, $reserved) + public function __construct(array &$config, mixed $reserved) { parent::__construct($config, $reserved); - if (empty($config['newScope'])) { - throw new \SimpleSAML\Error\Exception('ScopeRewrite: "newScope" value must be provided'); - } - - $this->newScope = $config['newScope']; - - if (array_key_exists('attributesOldScopeToUsername', $config)) { - $this->attributesOldScopeToUsername = $config['attributesOldScopeToUsername']; - } - if (array_key_exists('attributesReplaceScope', $config)) { - $this->attributesReplaceScope = $config['attributesReplaceScope']; - } - if (array_key_exists('ignoreForScopes', $config)) { - $this->ignoreForScopes = $config['ignoreForScopes']; - } - if (array_key_exists('oldScopeSeparator', $config)) { - $this->oldScopeSeparator = $config['oldScopeSeparator']; - } + $conf = Configuration::loadFromArray($config); + $this->newScope = $conf->getString('newScope'); + $this->attributesOldScopeToUsername = $conf->getOptionalArray('attributesOldScopeToUsername', [ + 'eduPersonPrincipalName', + ]); + + $this->attributesReplaceScope = $conf->getOptionalArray('attributesReplaceScope', [ + 'eduPersonScopedAffiliation', + ]); + $this->ignoreForScopes = $conf->getOptionalArray('ignoreForScopes', []); + $this->oldScopeSeparator = $conf->getOptionalString('oldScopeSeparator', '+'); } - /** - * Apply filter. - * - * @param array &$request the current request - */ - public function process(array &$request): void + public function process(array &$state): void { foreach ($this->attributesOldScopeToUsername as $attributeName) { - if (!isset($request['Attributes'][$attributeName])) { + if (!isset($state['Attributes'][$attributeName])) { continue; } - $values = $request['Attributes'][$attributeName]; + $values = $state['Attributes'][$attributeName]; $newValues = array(); + /** @var string $value */ foreach ($values as $value) { $scope = ''; if (($pos = strpos($value, '@')) !== false) { @@ -70,16 +65,17 @@ public function process(array &$request): void $newValues[] = str_replace('@', $this->oldScopeSeparator, $value) . '@' . $this->newScope; } } - $request['Attributes'][$attributeName] = $newValues; + $state['Attributes'][$attributeName] = $newValues; } foreach ($this->attributesReplaceScope as $attributeName) { - if (!isset($request['Attributes'][$attributeName])) { + if (!isset($state['Attributes'][$attributeName])) { continue; } - $values = $request['Attributes'][$attributeName]; + $values = $state['Attributes'][$attributeName]; $newValues = array(); + /** @var string $value */ foreach ($values as $value) { $scope = ''; if (($pos = strpos($value, '@')) !== false) { @@ -92,23 +88,23 @@ public function process(array &$request): void $newValues[] = $this->unscope($value) . '@' . $this->newScope; } } - $request['Attributes'][$attributeName] = $newValues; + $state['Attributes'][$attributeName] = $newValues; } } /** * Remove any scoping from the string * - * @param $string string to check - * @return string unscope version of string. If param has no scope then it is returned as is + * @param $value string to check + * @return string unscoped version of string. If param has no scope then it is returned as is */ - private function unscope($string) + private function unscope(string $value): string { - $pos = strpos($string, '@'); + $pos = strpos($value, '@'); if ($pos === false) { - return $string; + return $value; } else { - return (substr($string, 0, $pos)); + return substr($value, 0, $pos); } } } diff --git a/tests/lib/Auth/Process/ScopeMapTest.php b/tests/lib/Auth/Process/ScopeMapTest.php index bed5388..f8934b4 100644 --- a/tests/lib/Auth/Process/ScopeMapTest.php +++ b/tests/lib/Auth/Process/ScopeMapTest.php @@ -6,7 +6,6 @@ class ScopeMapTest extends \PHPUnit\Framework\TestCase { - private array $testConfig = [ 'class' => 'scoperewrite:ScopeMap', 'scopeMap' => [ diff --git a/tests/lib/Auth/Process/ScopeRewriteTest.php b/tests/lib/Auth/Process/ScopeRewriteTest.php index 22d49f1..641eb26 100644 --- a/tests/lib/Auth/Process/ScopeRewriteTest.php +++ b/tests/lib/Auth/Process/ScopeRewriteTest.php @@ -7,8 +7,6 @@ class ScopeRewriteTest extends TestCase { - - /** * Helper function to run the filter with a given configuration. * From 5dece57e6b35b9942b220694deb682be21f7f4cf Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sun, 1 Jun 2025 10:39:18 +0300 Subject: [PATCH 3/3] Enable github actions. Fix psalm issues. --- .github/workflows/php.yml | 187 ++++++++++++++++++++ composer.json | 34 +++- phpunit.xml | 26 ++- psalm.xml | 23 ++- src/Auth/Process/ScopeMap.php | 15 +- src/Auth/Process/ScopeRewrite.php | 12 +- tests/lib/Auth/Process/ScopeMapTest.php | 21 ++- tests/lib/Auth/Process/ScopeRewriteTest.php | 98 +++++----- 8 files changed, 337 insertions(+), 79 deletions(-) create mode 100644 .github/workflows/php.yml diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml new file mode 100644 index 0000000..58c86e8 --- /dev/null +++ b/.github/workflows/php.yml @@ -0,0 +1,187 @@ +name: CI + +on: [push, pull_request] + +permissions: + pull-requests: write + contents: read + +jobs: + basic-tests: + name: Syntax and unit tests, PHP ${{ matrix.php-versions }}, ${{ matrix.operating-system }} + runs-on: ${{ matrix.operating-system }} + strategy: + fail-fast: false + matrix: + operating-system: [ubuntu-latest] + php-versions: ['8.1', '8.2', '8.3'] + + steps: + - name: Setup PHP, with composer and extensions + uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php + with: + php-version: ${{ matrix.php-versions }} + extensions: intl, mbstring, mysql, pdo, pdo_sqlite, xml + tools: composer:v2 + 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@v3 + + - name: Get composer cache directory + id: composer-cache + run: echo "::set-output name=dir::$(composer config cache-files-dir)" + + - name: Cache composer dependencies + uses: actions/cache@v3 + with: + path: ${{ steps.composer-cache.outputs.dir }} + 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: Decide whether to run code coverage or not + if: ${{ matrix.php-versions != '8.2' || matrix.operating-system != 'ubuntu-latest' }} + run: | + echo "NO_COVERAGE=--no-coverage" >> $GITHUB_ENV + + - name: Run unit tests + run: | + echo $NO_COVERAGE + ./vendor/bin/phpunit $NO_COVERAGE + + - name: Save coverage data + if: ${{ matrix.php-versions == '8.2' && matrix.operating-system == 'ubuntu-latest' }} + uses: actions/upload-artifact@v4 + with: + name: build-data + path: ${{ github.workspace }}/build + + - name: List files in the workspace + if: ${{ matrix.php-versions == '8.2' && matrix.operating-system == 'ubuntu-latest' }} + run: | + ls -la ${{ github.workspace }}/build + ls -la ${{ github.workspace }}/build/logs + + - name: Code Coverage Report + if: ${{ matrix.php-versions == '8.2' && matrix.operating-system == 'ubuntu-latest' }} + uses: irongut/CodeCoverageSummary@v1.3.0 + with: + filename: build/logs/cobertura.xml + format: markdown + badge: true + fail_below_min: true + hide_branch_rate: false + hide_complexity: true + indicators: true + output: both + thresholds: '60 80' + + security: + name: Security checks + runs-on: [ ubuntu-latest ] + steps: + - name: Setup PHP, with composer and extensions + uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php + with: + php-version: '8.2' + extensions: mbstring, xml + tools: composer:v2 + coverage: none + + - name: Setup problem matchers for PHP + run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" + + - uses: actions/checkout@v3 + + - name: Get composer cache directory + id: composer-cache + run: echo "::set-output name=dir::$(composer config cache-files-dir)" + + - name: Cache composer dependencies + uses: actions/cache@v3 + with: + path: ${{ steps.composer-cache.outputs.dir }} + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-composer- + + - name: Install Composer dependencies + run: composer install --no-progress --prefer-dist --optimize-autoloader + + - name: Security check for locked dependencies + uses: symfonycorp/security-checker-action@v3 + + - name: Update Composer dependencies + run: composer update --no-progress --prefer-dist --optimize-autoloader + + - name: Security check for updated dependencies + uses: symfonycorp/security-checker-action@v3 + + quality: + name: Quality control + runs-on: [ ubuntu-latest ] + needs: [ basic-tests ] + + steps: + - name: Setup PHP, with composer and extensions + id: setup-php + uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php + with: + php-version: '8.2' + tools: composer:v2 + extensions: mbstring, xml + + - name: Setup problem matchers for PHP + run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" + + - uses: actions/checkout@v3 + + - name: Get composer cache directory + id: composer-cache + run: echo "::set-output name=dir::$(composer config cache-files-dir)" + + - name: Cache composer dependencies + uses: actions/cache@v3 + with: + path: ${{ steps.composer-cache.outputs.dir }} + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-composer- + + - name: Install Composer dependencies + run: composer install --no-progress --prefer-dist --optimize-autoloader + + - uses: actions/download-artifact@v4 + with: + name: build-data + path: ${{ github.workspace }}/build + + - name: Codecov + uses: codecov/codecov-action@v3 + + - name: PHP Code Sniffer + if: always() + run: php vendor/bin/phpcs + + - name: Psalm + if: always() + run: php vendor/bin/psalm --no-cache --show-info=true --shepherd --php-version=${{ steps.setup-php.outputs.php-version }} + + - name: Psalter + if: always() + run: php vendor/bin/psalter --issues=UnnecessaryVarAnnotation --dry-run --php-version=${{ steps.setup-php.outputs.php-version }} diff --git a/composer.json b/composer.json index 9d90360..89cdad2 100644 --- a/composer.json +++ b/composer.json @@ -2,19 +2,45 @@ "name": "cirrusidentity/simplesamlphp-module-scoperewrite", "description": "Filter to rewrite the scope on attributes", "type": "simplesamlphp-module", + "keywords": [ + "simplesamlphp", + "module", + "scope" + ], "require": { "php" : ">= 8.1", "simplesamlphp/simplesamlphp": "~2.0", "simplesamlphp/composer-module-installer": "^1" }, "require-dev": { - "vimeo/psalm": "^5.0", - "phpunit/phpunit": "^9.5", - "squizlabs/php_codesniffer": "^3.5" + "vimeo/psalm": "^6", + "phpunit/phpunit": "^10.5", + "squizlabs/php_codesniffer": "^3.7" + }, + "autoload": { + "psr-4": { + "SimpleSAML\\Module\\scoperewrite\\": "src/" + } + }, + "autoload-dev": { + "psr-4": { + "Test\\SimpleSAML\\": "tests/lib/" + } }, "config": { "allow-plugins": { - "simplesamlphp/composer-module-installer": true + "simplesamlphp/composer-module-installer": true, + "simplesamlphp/composer-xmlprovider-installer": true } + }, + "scripts": { + "pre-commit": [ + "vendor/bin/phpunit --no-coverage --testdox", + "vendor/bin/phpcs -p", + "vendor/bin/psalm --no-cache" + ], + "tests": [ + "vendor/bin/phpunit --no-coverage" + ] } } diff --git a/phpunit.xml b/phpunit.xml index ff74e76..09e3bce 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,8 +1,32 @@ - + ./tests + + + + ./src + + + + + + + + + + diff --git a/psalm.xml b/psalm.xml index ebcdea2..a5d4d83 100644 --- a/psalm.xml +++ b/psalm.xml @@ -1,8 +1,7 @@ - - - - + + + + + + + + + + + + + + + + diff --git a/src/Auth/Process/ScopeMap.php b/src/Auth/Process/ScopeMap.php index 3608a2d..8a343db 100644 --- a/src/Auth/Process/ScopeMap.php +++ b/src/Auth/Process/ScopeMap.php @@ -6,7 +6,7 @@ use SimpleSAML\Configuration; use SimpleSAML\Logger; -class ScopeMap extends ProcessingFilter +final class ScopeMap extends ProcessingFilter { /** * @var array @@ -19,17 +19,20 @@ public function __construct(array &$config, mixed $reserved) { parent::__construct($config, $reserved); $conf = Configuration::loadFromArray($config); - $this->scopeMap = $conf->getArray('scopeMap'); - $this->srcAttribute = $conf->getString('srcAttribute'); - $this->destAttribute = $conf->getString('destAttribute'); - foreach ($this->scopeMap as $oldScope => $newScope) { + $loadedMap = $conf->getArray('scopeMap'); + $this->scopeMap = []; + foreach ($loadedMap as $oldScope => $newScope) { if (!is_string($oldScope)) { throw new \Exception('scopeMap contains non-string key'); } if (!is_string($newScope)) { throw new \Exception('scopeMap contains non-string value for key ' . $oldScope); } + $this->scopeMap[$oldScope] = $newScope; } + + $this->srcAttribute = $conf->getString('srcAttribute'); + $this->destAttribute = $conf->getString('destAttribute'); } /** @@ -37,6 +40,7 @@ public function __construct(array &$config, mixed $reserved) * * @param array &$state the current request */ + #[\Override] public function process(array &$state): void { $newValues = []; @@ -62,6 +66,7 @@ public function process(array &$state): void } } if (!empty($newValues)) { + /** @psalm-suppress MixedArrayAssignment */ $state['Attributes'][$this->destAttribute] = $newValues; } } diff --git a/src/Auth/Process/ScopeRewrite.php b/src/Auth/Process/ScopeRewrite.php index c7b3453..e817fa1 100644 --- a/src/Auth/Process/ScopeRewrite.php +++ b/src/Auth/Process/ScopeRewrite.php @@ -5,7 +5,7 @@ use SimpleSAML\Auth\ProcessingFilter; use SimpleSAML\Configuration; -class ScopeRewrite extends ProcessingFilter +final class ScopeRewrite extends ProcessingFilter { private string $newScope; @@ -42,16 +42,18 @@ public function __construct(array &$config, mixed $reserved) $this->oldScopeSeparator = $conf->getOptionalString('oldScopeSeparator', '+'); } + #[\Override] public function process(array &$state): void { + /** @var array{Attributes?: array} $state */ foreach ($this->attributesOldScopeToUsername as $attributeName) { if (!isset($state['Attributes'][$attributeName])) { continue; } - $values = $state['Attributes'][$attributeName]; - $newValues = array(); + $values = (array)$state['Attributes'][$attributeName]; + $newValues = []; /** @var string $value */ foreach ($values as $value) { $scope = ''; @@ -73,8 +75,8 @@ public function process(array &$state): void continue; } - $values = $state['Attributes'][$attributeName]; - $newValues = array(); + $values = (array)$state['Attributes'][$attributeName]; + $newValues = []; /** @var string $value */ foreach ($values as $value) { $scope = ''; diff --git a/tests/lib/Auth/Process/ScopeMapTest.php b/tests/lib/Auth/Process/ScopeMapTest.php index f8934b4..4191d61 100644 --- a/tests/lib/Auth/Process/ScopeMapTest.php +++ b/tests/lib/Auth/Process/ScopeMapTest.php @@ -2,9 +2,10 @@ namespace Auth\Process; +use PHPUnit\Framework\TestCase; use SimpleSAML\Module\scoperewrite\Auth\Process\ScopeMap; -class ScopeMapTest extends \PHPUnit\Framework\TestCase +final class ScopeMapTest extends TestCase { private array $testConfig = [ 'class' => 'scoperewrite:ScopeMap', @@ -20,9 +21,10 @@ class ScopeMapTest extends \PHPUnit\Framework\TestCase /** * Helper function to run the filter with a given configuration. * - * @param array $config The filter configuration. - * @param array $request The request state. + * @param array $config The filter configuration. + * @param array $request The request state. * @return array The state array after processing. + * @throws \Exception */ private static function processFilter(array $config, array $request): array { @@ -33,12 +35,13 @@ private static function processFilter(array $config, array $request): array /** * Test with no attributes + * @throws \Exception */ public function testNoAttributes(): void { - $request = array( - 'Attributes' => array(), - ); + $request = [ + 'Attributes' => [], + ]; $result = self::processFilter($this->testConfig, $request); $attributes = $result['Attributes']; $this->assertEmpty($attributes, var_export($attributes, true)); @@ -46,7 +49,7 @@ public function testNoAttributes(): void public function testMapping(): void { - $request = array( + $request = [ 'Attributes' => [ 'gn' => ['name'], 'scopedAttr' => [ @@ -58,7 +61,7 @@ public function testMapping(): void 'mult@ple@s' ] ], - ); + ]; $expectedAttributes = $request['Attributes'] + [ 'rescopedAttr' => [ 'user@nochange.com', @@ -70,7 +73,7 @@ public function testMapping(): void ] ]; $result = self::processFilter($this->testConfig, $request); - $attributes = $result['Attributes']; + $attributes = (array)$result['Attributes']; $this->assertEquals($expectedAttributes, $attributes); } } diff --git a/tests/lib/Auth/Process/ScopeRewriteTest.php b/tests/lib/Auth/Process/ScopeRewriteTest.php index 641eb26..bb171c2 100644 --- a/tests/lib/Auth/Process/ScopeRewriteTest.php +++ b/tests/lib/Auth/Process/ScopeRewriteTest.php @@ -5,7 +5,7 @@ use PHPUnit\Framework\TestCase; use SimpleSAML\Module\scoperewrite\Auth\Process\ScopeRewrite; -class ScopeRewriteTest extends TestCase +final class ScopeRewriteTest extends TestCase { /** * Helper function to run the filter with a given configuration. @@ -40,21 +40,21 @@ public function testNoAttributes(): void */ public function testScopeRewriteDefaultConfig(): void { - $config = array('newScope' => 'tester.com'); - $request = array( - 'Attributes' => array( - 'eduPersonPrincipalName' => array('joe@home.com'), - 'eduPersonScopedAffiliation' => array('student@home.com', 'staff@home.com')), - ); + $config = ['newScope' => 'tester.com']; + $request = [ + 'Attributes' => [ + 'eduPersonPrincipalName' => ['joe@home.com'], + 'eduPersonScopedAffiliation' => ['student@home.com', 'staff@home.com']], + ]; $result = self::processFilter($config, $request); - $attributes = $result['Attributes']; + $attributes = (array)$result['Attributes']; $this->assertEquals( - array('joe+home.com@tester.com'), + ['joe+home.com@tester.com'], $attributes['eduPersonPrincipalName'], 'Eppn should have old scope as part of value.' ); $this->assertEquals( - array('student@tester.com', 'staff@tester.com'), + ['student@tester.com', 'staff@tester.com'], $attributes['eduPersonScopedAffiliation'], 'Scoped affilation should have scope changed' ); @@ -65,26 +65,26 @@ public function testScopeRewriteDefaultConfig(): void */ public function testIgnoreScope(): void { - $config = array( + $config = [ 'newScope' => 'tester.com', 'ignoreForScopes' => [ 'home.com' ] - ); - $request = array( - 'Attributes' => array( - 'eduPersonPrincipalName' => array('joe@home.com'), - 'eduPersonScopedAffiliation' => array('student@home.com', 'staff@not-ignored.com')), - ); + ]; + $request = [ + 'Attributes' => [ + 'eduPersonPrincipalName' => ['joe@home.com'], + 'eduPersonScopedAffiliation' => ['student@home.com', 'staff@not-ignored.com']], + ]; $result = self::processFilter($config, $request); - $attributes = $result['Attributes']; + $attributes = (array)$result['Attributes']; $this->assertEquals( - array('joe@home.com'), + ['joe@home.com'], $attributes['eduPersonPrincipalName'], 'Eppn has scope that should not be changed' ); $this->assertEquals( - array('student@home.com', 'staff@tester.com'), + ['student@home.com', 'staff@tester.com'], $attributes['eduPersonScopedAffiliation'], 'Scoped affiliation should have 1 scope changed' ); @@ -95,29 +95,29 @@ public function testIgnoreScope(): void */ public function testScopeRewriteCustomConfig(): void { - $config = array( + $config = [ 'newScope' => 'tester.com', - 'attributesOldScopeToUsername' => array('username1', 'username2'), - 'attributesReplaceScope' => array('rewrite1', 'rewrite2'), - ); - $request = array( - 'Attributes' => array( - 'username1' => array('joe@home.com'), - 'username2' => array('jeff'), // not pre-scoped test. - 'rewrite1' => array('student@home.com'), - 'rewrite2' => array("staff"), // not pre-scoped test - ), - ); + 'attributesOldScopeToUsername' => ['username1', 'username2'], + 'attributesReplaceScope' => ['rewrite1', 'rewrite2'], + ]; + $request = [ + 'Attributes' => [ + 'username1' => ['joe@home.com'], + 'username2' => ['jeff'], // not pre-scoped test. + 'rewrite1' => ['student@home.com'], + 'rewrite2' => ["staff"], // not pre-scoped test + ], + ]; $result = self::processFilter($config, $request); - $attributes = $result['Attributes']; + $attributes = (array)$result['Attributes']; $this->assertEquals( - array('joe+home.com@tester.com'), + ['joe+home.com@tester.com'], $attributes['username1'], 'username1 should have old scope as part of value.' ); - $this->assertEquals(array('jeff@tester.com'), $attributes['username2']); - $this->assertEquals(array('student@tester.com'), $attributes['rewrite1']); - $this->assertEquals(array('staff@tester.com'), $attributes['rewrite2']); + $this->assertEquals(['jeff@tester.com'], $attributes['username2']); + $this->assertEquals(['student@tester.com'], $attributes['rewrite1']); + $this->assertEquals(['staff@tester.com'], $attributes['rewrite2']); } @@ -127,25 +127,25 @@ public function testScopeRewriteCustomConfig(): void */ public function testOldScopeSeparator(): void { - $config = array( + $config = [ 'newScope' => 'tester.com', - 'attributesOldScopeToUsername' => array('username1', 'username2'), + 'attributesOldScopeToUsername' => ['username1', 'username2'], 'oldScopeSeparator' => '(at)', - ); - $request = array( - 'Attributes' => array( - 'username1' => array('joe@home.com', 'joe+something@example.com'), - 'username2' => array('jeff'), // not pre-scoped test. + ]; + $request = [ + 'Attributes' => [ + 'username1' => ['joe@home.com', 'joe+something@example.com'], + 'username2' => ['jeff'], // not pre-scoped test. - ), - ); + ], + ]; $result = self::processFilter($config, $request); - $attributes = $result['Attributes']; + $attributes = (array)$result['Attributes']; $this->assertEquals( - array('joe(at)home.com@tester.com', 'joe+something(at)example.com@tester.com'), + ['joe(at)home.com@tester.com', 'joe+something(at)example.com@tester.com'], $attributes['username1'], 'username1 should have old scope as part of value.' ); - $this->assertEquals(array('jeff@tester.com'), $attributes['username2']); + $this->assertEquals(['jeff@tester.com'], $attributes['username2']); } }