diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..5599209 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,17 @@ +# Editor configuration normalization +# @see http://editorconfig.org/ + +# This is the top-most .editorconfig file; do not search in parent directories. +root = true + +# All files. +[*] +end_of_line = LF +indent_style = space +indent_size = 4 +charset = utf-8 +trim_trailing_whitespace = true +insert_final_newline = true + +[{*.yml,*yaml}] +indent_size = 2 diff --git a/.gitignore b/.gitignore index e26f45e..f8d51f0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ composer.phar /vendor/ composer.lock +.bash_history +/.phpunit.result.cache diff --git a/.lando.env b/.lando.env new file mode 100644 index 0000000..36dddbc --- /dev/null +++ b/.lando.env @@ -0,0 +1,10 @@ +# Lando specific - uses user home directory to store Composer cache and other stuff +# Helps to re-use Composer cache between projects. +COMPOSER_HOME=/user/.composer +# https://xdebug.org/docs/step_debug +# https://xdebug.org/docs/step_debug#client_host +# https://docs.lando.dev/config/php.html#configuration +PHP_IDE_CONFIG=serverName=drupalauth.lndo.site +# Ignore commands starting with space and duplicates. +HISTCONTROL=ignoreboth +HOME=/app diff --git a/.lando.yml b/.lando.yml new file mode 100644 index 0000000..655c9e0 --- /dev/null +++ b/.lando.yml @@ -0,0 +1,53 @@ +name: drupalauth + +services: + php80: + type: php:8.0 + via: cli + xdebug: "off" + composer_version: 2 + php81: + type: php:8.1 + via: cli + xdebug: "off" + composer_version: 2 + php82: + type: php:8.2 + via: cli + xdebug: "off" + composer_version: 2 + + + + +env_file: + - .lando.env + +tooling: + style-lint: + cmd: ./scripts/style-lint.sh + service: :service + options: + service: + default: php80 + describe: Run phpcs in different service + alias: + - s + style-fix: + cmd: ./scripts/style-fix.sh + service: :service + options: + service: + default: php80 + describe: Run phpcs in different service + alias: + - s + phpunit: + cmd: ./scripts/phpunit.sh + service: :service + options: + service: + default: php80 + describe: Run phpunit in different service + alias: + - s diff --git a/.travis.yml b/.travis.yml index 3549c0b..e89dadf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,13 @@ +os: linux language: php -php: - - '7.1' - - '7.2' - - '7.3' - - '7.4' - - '8.0' +jobs: + include: + - php: '8.0' + dist: focal + - php: '8.1' + dist: jammy + - php: '8.2' + dist: jammy before_script: composer install script: diff --git a/composer.json b/composer.json index 6d4d588..95b3153 100644 --- a/composer.json +++ b/composer.json @@ -21,16 +21,16 @@ } ], "require": { - "php": "^5.6.0|^7.0|^8.0", - "simplesamlphp/simplesamlphp": "~2.0", + "php": "^8.0", + "simplesamlphp/simplesamlphp": "^2.1", "simplesamlphp/composer-module-installer": "~1.0" }, "require-dev": { - "phpunit/phpunit": "^5|^6|^7|^8|^9", - "squizlabs/php_codesniffer": "^2.0.0|^3.0.0" + "phpunit/phpunit": "^9.0 | ^10.0", + "squizlabs/php_codesniffer": "^3.0" }, "autoload-dev": { - "classmap": ["lib/", "tests/lib/"] + "classmap": ["src/", "tests/"] }, "extra": { "branch-alias": { diff --git a/default-enable b/default-enable deleted file mode 100644 index 25615cb..0000000 --- a/default-enable +++ /dev/null @@ -1,3 +0,0 @@ -This file indicates that the default state of this module -is enabled. To disable, create a file named disable in the -same directory as this file. diff --git a/lib/ConfigHelper.php b/lib/ConfigHelper.php deleted file mode 100644 index d67650f..0000000 --- a/lib/ConfigHelper.php +++ /dev/null @@ -1,126 +0,0 @@ -location = $location; - - /* Get authsource configuration. */ - $config = Configuration::loadFromArray($config, $location); - - $this->drupalroot = $config->getString('drupalroot'); - $this->debug = $config->getBoolean('debug', FALSE); - $this->attributes = $config->getArray('attributes', []); - $this->drupal_logout_url = $config->getString('drupal_logout_url', NULL); - $this->drupal_login_url = $config->getString('drupal_login_url', NULL); - } - - /** - * Returns debug mode. - * - * @return boolean - */ - public function getDebug() - { - return $this->debug; - } - - /** - * Returns Drupal root directory. - * - * @return string - */ - public function getDrupalroot() - { - return $this->drupalroot; - } - - /** - * Return the attributes - * - * @return array - */ - public function getAttributes() - { - return $this->attributes; - } - - - /** - * Returns Drupal logout URL. - * - * @return string - */ - public function getDrupalLogoutURL() - { - return $this->drupal_logout_url; - } - - /** - * Returns Drupal login URL. - * - * @return string - */ - public function getDrupalLoginURL() - { - return $this->drupal_login_url; - } -} diff --git a/phpcs.xml b/phpcs.xml index b0ede41..bd16e4e 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -2,7 +2,7 @@ The coding standard for drupalauth. - - ./lib - ./www + + ./src + ./public diff --git a/www/resume.php b/public/resume.php similarity index 100% rename from www/resume.php rename to public/resume.php diff --git a/scripts/phpunit.sh b/scripts/phpunit.sh new file mode 100755 index 0000000..4d300d5 --- /dev/null +++ b/scripts/phpunit.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +php -v +rm -rf ./vendor composer.lock +composer install + +./vendor/bin/phpunit diff --git a/scripts/style-fix.sh b/scripts/style-fix.sh new file mode 100755 index 0000000..133e82c --- /dev/null +++ b/scripts/style-fix.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + +rm -rf ./vendor composer.lock +composer install + +./vendor/bin/phpcbf diff --git a/scripts/style-lint.sh b/scripts/style-lint.sh new file mode 100755 index 0000000..af2acab --- /dev/null +++ b/scripts/style-lint.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash + +rm -rf ./vendor composer.lock +composer install + +./vendor/bin/phpcs + +#lando phpunit -s php80 +#lando style-lint -s php80 +#lando phpunit -s php81 +#lando style-lint -s php81 +#lando phpunit -s php82 +#lando style-lint -s php82 diff --git a/lib/Auth/Source/External.php b/src/Auth/Source/External.php similarity index 83% rename from lib/Auth/Source/External.php rename to src/Auth/Source/External.php index b335105..bdaf802 100755 --- a/lib/Auth/Source/External.php +++ b/src/Auth/Source/External.php @@ -74,28 +74,27 @@ */ class External extends Source { + /** + * The string used to identify Drupal user ID. + */ + public const DRUPALAUTH_EXTERNAL_USER_ID = 'drupalauth:External:UserID'; - /** - * The string used to identify Drupal user ID. - */ - const DRUPALAUTH_EXTERNAL_USER_ID = 'drupalauth:External:UserID'; - - /** - * The string used to identify authentication source. - */ - const DRUPALAUTH_AUTH_ID = 'drupalauth:AuthID'; + /** + * The string used to identify authentication source. + */ + public const DRUPALAUTH_AUTH_ID = 'drupalauth:AuthID'; - /** - * The string used to identify our states. - */ - const DRUPALAUTH_EXTERNAL = 'drupalauth:External'; + /** + * The string used to identify our states. + */ + public const DRUPALAUTH_EXTERNAL = 'drupalauth:External'; - /** + /** * Configuration object. * * @var \SimpleSAML\Module\drupalauth\ConfigHelper */ - private $config; + private ConfigHelper $config; /** * Constructor for this authentication source. @@ -103,7 +102,7 @@ class External extends Source * @param array $info Information about this authentication source. * @param array $config Configuration. */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { assert(is_array($info)); assert(is_array($config)); @@ -114,54 +113,56 @@ public function __construct($info, $config) /* Get the configuration for this module */ $drupalAuthConfig = new ConfigHelper( $config, - 'Authentication source ' . var_export($this->getAuthId(), true) + 'Authentication source ' . $this->getAuthId() ); $this->config = $drupalAuthConfig; } - /** * Retrieve attributes for the user. * - * @return array|NULL The user's attributes, or NULL if the user isn't authenticated. + * @return array|NULL The user's attributes, or NULL if the user isn't + * authenticated. */ - private function getUser($drupaluid) + private function getUser($drupalUid): ?array { - if (!empty($drupaluid)) { + if (!empty($drupalUid)) { $drupalHelper = new DrupalHelper(); - $drupalHelper->bootDrupal($this->config->getDrupalroot()); + $drupalHelper->bootDrupal($this->config->getDrupalRoot()); - // Load the user object from Drupal. - $drupaluser = User::load($drupaluid); - if ($drupaluser->isBlocked()) { + // Load the user object from Drupal. + $drupalUser = User::load($drupalUid); + if ($drupalUser->isBlocked()) { throw new Error('NOACCESS'); } - $requested_attributes = $this->config->getAttributes(); + $requestedAttributes = $this->config->getAttributes(); - return $drupalHelper->getAttributes($drupaluser, $requested_attributes); + return $drupalHelper->getAttributes($drupalUser, $requestedAttributes); } + + return null; } /** * Log in using an external authentication helper. * - * @param array &$state Information about the current authentication. + * @param array &$state Information about the current authentication. */ - public function authenticate(&$state) + public function authenticate(array &$state): void { assert(is_array($state)); - /* + /* * The user is already authenticated. * * Add the users attributes to the $state-array, and return control * to the authentication process. */ - if (!empty($state[self::DRUPALAUTH_EXTERNAL_USER_ID])) { - $state['Attributes'] = $this->getUser($state[self::DRUPALAUTH_EXTERNAL_USER_ID]); - return; + if (!empty($state[self::DRUPALAUTH_EXTERNAL_USER_ID])) { + $state['Attributes'] = $this->getUser($state[self::DRUPALAUTH_EXTERNAL_USER_ID]); + return; } /* @@ -206,7 +207,7 @@ public function authenticate(&$state) * is also part of this module, but in a real example, this would likely be * the absolute URL of the login page for the site. */ - $authPage = $this->config->getDrupalLoginURL(); + $authPage = $this->config->getDrupalLoginUrl(); /* * The redirect to the authentication page. @@ -214,7 +215,8 @@ public function authenticate(&$state) * Note the 'ReturnTo' parameter. This must most likely be replaced with * the real name of the parameter for the login page. */ - HTTP::redirectTrustedURL($authPage, [ + $http = new HTTP(); + $http->redirectTrustedURL($authPage, [ 'ReturnTo' => $returnTo, ]); @@ -230,7 +232,7 @@ public function authenticate(&$state) * This function resumes the authentication process after the user has * entered his or her credentials. * - * @param array &$state The authentication state. + * @param array &$state The authentication state. */ public static function resume($stateID) { @@ -274,7 +276,7 @@ public static function resume($stateID) * First we check that the user is acutally logged in, and didn't simply skip the login page. */ if (empty($state[self::DRUPALAUTH_EXTERNAL_USER_ID])) { - throw new Exception('User ID is missing.'); + throw new Exception('User ID is missing.'); } /* @@ -306,12 +308,12 @@ public static function resume($stateID) } /** - * This function is called when the user start a logout operation, for example - * by logging out of a SP that supports single logout. + * This function is called when the user start a logout operation, for + * example by logging out of a SP that supports single logout. * - * @param array &$state The logout state array. + * @param array &$state The logout state array. */ - public function logout(&$state) + public function logout(array &$state): void { assert(is_array($state)); @@ -320,12 +322,13 @@ public function logout(&$state) session_start(); } - $logout_url = $this->config->getDrupalLogoutURL(); + $logoutUrl = $this->config->getDrupalLogoutUrl(); $parameters = []; if (!empty($state['ReturnTo'])) { $parameters['ReturnTo'] = $state['ReturnTo']; } - HTTP::redirectTrustedURL($logout_url, $parameters); + $http = new HTTP(); + $http->redirectTrustedURL($logoutUrl, $parameters); } } diff --git a/lib/Auth/Source/UserPass.php b/src/Auth/Source/UserPass.php similarity index 83% rename from lib/Auth/Source/UserPass.php rename to src/Auth/Source/UserPass.php index fbadab3..89f5573 100644 --- a/lib/Auth/Source/UserPass.php +++ b/src/Auth/Source/UserPass.php @@ -3,6 +3,7 @@ namespace SimpleSAML\Module\drupalauth\Auth\Source; use Drupal\user\Entity\User; +use SimpleSAML\Assert\Assert; use SimpleSAML\Error\Error; use SimpleSAML\Module\core\Auth\UserPassBase; use SimpleSAML\Module\drupalauth\ConfigHelper; @@ -56,13 +57,12 @@ */ class UserPass extends UserPassBase { - /** * Configuration object. * * @var \SimpleSAML\Module\drupalauth\ConfigHelper */ - private $config; + private ConfigHelper $config; /** * Constructor for this authentication source. @@ -70,7 +70,7 @@ class UserPass extends UserPassBase * @param array $info Information about this authentication source. * @param array $config Configuration. */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { assert(is_array($info)); assert(is_array($config)); @@ -81,7 +81,7 @@ public function __construct($info, $config) /* Get the configuration for this module */ $drupalAuthConfig = new ConfigHelper( $config, - 'Authentication source ' . var_export($this->getAuthId(), true) + 'Authentication source ' . $this->getAuthId() ); $this->config = $drupalAuthConfig; @@ -91,23 +91,25 @@ public function __construct($info, $config) /** * Attempt to log in using the given username and password. * - * On a successful login, this function should return the users attributes. On failure, - * it should throw an exception. If the error was caused by the user entering the wrong - * username or password, a SimpleSAML_Error_Error('WRONGUSERPASS') should be thrown. + * On a successful login, this function should return the users attributes. + * On failure, it should throw an exception. If the error was caused by the + * user entering the wrong username or password, a + * SimpleSAML_Error_Error('WRONGUSERPASS') should be thrown. * * Note that both the username and the password are UTF-8 encoded. * * @param string $username The username the user wrote. * @param string $password The password the user wrote. + * * @return array Associative array with the users attributes. */ - protected function login($username, $password) + protected function login(string $username, string $password): array { assert(is_string($username)); assert(is_string($password)); $drupalHelper = new DrupalHelper(); - $drupalHelper->bootDrupal($this->config->getDrupalroot()); + $drupalHelper->bootDrupal($this->config->getDrupalRoot()); /* @value \Drupal\user\UserAuth $userAuth */ $userAuth = \Drupal::service('user.auth'); @@ -119,13 +121,13 @@ protected function login($username, $password) } // Load the user object from Drupal. - $drupaluser = User::load($uid); - if ($drupaluser->isBlocked()) { + $drupalUser = User::load($uid); + if ($drupalUser->isBlocked()) { throw new Error('NOACCESS'); } - $requested_attributes = $this->config->getAttributes(); + $requestedAttributes = $this->config->getAttributes(); - return $drupalHelper->getAttributes($drupaluser, $requested_attributes); + return $drupalHelper->getAttributes($drupalUser, $requestedAttributes); } } diff --git a/src/ConfigHelper.php b/src/ConfigHelper.php new file mode 100644 index 0000000..4a9457a --- /dev/null +++ b/src/ConfigHelper.php @@ -0,0 +1,78 @@ +config = Configuration::loadFromArray($config, $location); + } + + /** + * Returns debug mode. + * + * @return bool + */ + public function getDebug(): bool + { + return $this->config->getOptionalBoolean('debug', false); + } + + /** + * Returns Drupal root directory. + * + * @return string + */ + public function getDrupalRoot(): string + { + return $this->config->getString('drupalroot'); + } + + /** + * Return the attributes + * + * @return array + */ + public function getAttributes(): ?array + { + return $this->config->getOptionalArray('attributes', null); + } + + + /** + * Returns Drupal logout URL. + * + * @return string + */ + public function getDrupalLogoutUrl(): string + { + return $this->config->getString('drupal_logout_url'); + } + + /** + * Returns Drupal login URL. + * + * @return string + */ + public function getDrupalLoginUrl(): string + { + return $this->config->getString('drupal_login_url'); + } +} diff --git a/lib/DrupalHelper.php b/src/DrupalHelper.php similarity index 97% rename from lib/DrupalHelper.php rename to src/DrupalHelper.php index 40f6bbe..8488c55 100644 --- a/lib/DrupalHelper.php +++ b/src/DrupalHelper.php @@ -1,6 +1,5 @@ forbiddenAttributes; @@ -89,7 +87,7 @@ public function getAttributes($drupaluser, $requested_attributes) * @param $forbiddenAttributes * @return array */ - protected function getAllAttributes($drupaluser, $forbiddenAttributes) + protected function getAllAttributes($drupaluser, $forbiddenAttributes): array { $attributes = []; foreach ($drupaluser as $field_name => $field) { diff --git a/tests/lib/DrupalHelperTest.php b/tests/DrupalHelperTest.php similarity index 97% rename from tests/lib/DrupalHelperTest.php rename to tests/DrupalHelperTest.php index 26c9d97..3fa3c84 100644 --- a/tests/lib/DrupalHelperTest.php +++ b/tests/DrupalHelperTest.php @@ -14,7 +14,7 @@ class DrupalHelperTest extends TestCase /** * @var ReflectionClass */ - protected $class; + protected ReflectionClass $class; protected function setUp(): void { @@ -24,7 +24,7 @@ protected function setUp(): void } - public function getPropertyNameProvider() + public static function getPropertyNameProvider(): array { return [ [[], 'value'], @@ -46,7 +46,7 @@ public function testGetPropertyName($attribute_definition, $expected_property_na } - public function getAttributeNameProvider() + public static function getAttributeNameProvider(): array { return [ [['field_name' => 'some_field'], 'some_field:value'], @@ -73,7 +73,7 @@ public function testGetAttributeName($attribute_definition, $expected_attribute_ $this->assertEquals($expected_attribute_name, $attribute_name, 'Expected attribute name returned'); } - public function getAllAttributesDataProvider() + public static function getAllAttributesDataProvider(): array { return [ // Set #0. @@ -180,7 +180,7 @@ public function testGetAllAttributes($values, $forbidden_attributes, $expected_a $this->assertEquals($expected_attributes, $attributes, 'Expected attributes returned'); } - public function getAttributesDataProvider() + public static function getAttributesDataProvider(): array { $field_values = [ 'field_name' => [ diff --git a/tests/lib/Field.php b/tests/Field.php similarity index 67% rename from tests/lib/Field.php rename to tests/Field.php index 85a4697..a7afa9f 100644 --- a/tests/lib/Field.php +++ b/tests/Field.php @@ -9,7 +9,7 @@ class Field { - protected $properties = []; + protected array $properties = []; public function __set($name, $value) { @@ -18,11 +18,11 @@ public function __set($name, $value) public function __get($name) { - return isset($this->properties[$name]) ? $this->properties[$name] : null; + return $this->properties[$name] ?? null; } - public function getProperties() + public function getProperties(): array { return array_keys($this->properties); } diff --git a/tests/lib/FieldList.php b/tests/FieldList.php similarity index 94% rename from tests/lib/FieldList.php rename to tests/FieldList.php index 2cc13fa..7409bfe 100644 --- a/tests/lib/FieldList.php +++ b/tests/FieldList.php @@ -8,7 +8,7 @@ class FieldList { - protected $list = []; + protected array $list = []; public function get($index) { @@ -16,7 +16,7 @@ public function get($index) throw new \InvalidArgumentException('Unable to get a value with a non-numeric delta in a list.'); } - return isset($this->list[$index]) ? $this->list[$index] : null; + return $this->list[$index] ?? null; } public function set($index, $properties) diff --git a/tests/lib/FieldListTest.php b/tests/FieldListTest.php similarity index 100% rename from tests/lib/FieldListTest.php rename to tests/FieldListTest.php diff --git a/tests/lib/FieldTest.php b/tests/FieldTest.php similarity index 100% rename from tests/lib/FieldTest.php rename to tests/FieldTest.php diff --git a/tests/lib/PropertyDefinition.php b/tests/PropertyDefinition.php similarity index 100% rename from tests/lib/PropertyDefinition.php rename to tests/PropertyDefinition.php diff --git a/tests/lib/PropertyDefinitionTest.php b/tests/PropertyDefinitionTest.php similarity index 100% rename from tests/lib/PropertyDefinitionTest.php rename to tests/PropertyDefinitionTest.php diff --git a/tests/lib/User.php b/tests/User.php similarity index 87% rename from tests/lib/User.php rename to tests/User.php index 0c8e7f5..caa4ab1 100644 --- a/tests/lib/User.php +++ b/tests/User.php @@ -2,7 +2,7 @@ class User implements IteratorAggregate { - protected $fields = []; + protected array $fields = []; /** * User constructor. @@ -11,7 +11,7 @@ class User implements IteratorAggregate * Each key corresponds to field name. Each value either scalar or array. Scalar would would be treated as index 0 * value of 'value' property. In case of array */ - public function __construct($values) + public function __construct(array $values) { foreach ($values as $field_name => $field_value) { $this->fields[$field_name] = new FieldList(); @@ -37,6 +37,6 @@ public function hasField($field_name) public function __get($field_name) { - return isset($this->fields[$field_name]) ? $this->fields[$field_name] : null; + return $this->fields[$field_name] ?? null; } } diff --git a/tests/lib/UserTest.php b/tests/UserTest.php similarity index 97% rename from tests/lib/UserTest.php rename to tests/UserTest.php index 60b4f27..dabaa48 100644 --- a/tests/lib/UserTest.php +++ b/tests/UserTest.php @@ -11,7 +11,7 @@ class UserTest extends TestCase { - public function userDataProvider() + public static function userDataProvider() { return [ [ @@ -58,7 +58,7 @@ public function testUser($values, $count) $this->assertEquals($count, count($user->getIterator()), 'Returned expected quantity of fields'); } - public function userHasFieldDataProvider() + public static function userHasFieldDataProvider() { return [ [