diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index d8b309b..abf591c 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -28,6 +28,7 @@ jobs: - name: Lint Code Base uses: super-linter/super-linter/slim@v6 env: + SAVE_SUPER_LINTER_OUTPUT: false # To report GitHub Actions status checks GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} LINTER_RULES_PATH: 'tools/linters' diff --git a/.gitignore b/.gitignore index 9e12716..0fa63d4 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ composer.lock composer.phar phpunit.xml.bak +/super-linter-output/ /vendor/ # Commit your application's lock file https://getcomposer.org/doc/01-basic-usage.md#commit-your-composer-lock-file-to-version-control diff --git a/composer.json b/composer.json index 1d6e5d4..1b30c3c 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,8 @@ "composer/package-versions-deprecated": true, "dealerdirect/phpcodesniffer-composer-installer": true, "phpstan/extension-installer": true, - "simplesamlphp/composer-module-installer": true + "simplesamlphp/composer-module-installer": true, + "simplesamlphp/composer-xmlprovider-installer": true } }, "autoload": { diff --git a/docs/authorize.md b/docs/authorize.md index 43679a5..9a7bb67 100644 --- a/docs/authorize.md +++ b/docs/authorize.md @@ -55,6 +55,24 @@ REFEDS SAML2 Metadata Deployment Profile for errorURL. Defaults to TRUE. **Note**: This option needs to be boolean (TRUE/FALSE) else it will be considered an attribute matching rule. +### `allow_reauthentication` + +This option can be used to allow users to re-authenticate if they are +unauthorized. If set to TRUE, the user will be shown a button to re-authenticate. +If set to FALSE, the user will not be shown a button to re-authenticate. + +**Note**: This option needs to be boolean (TRUE/FALSE) else it will be + considered FALSE. + +### `show_user_attribute` + +This option can be used to show the user attribute, to inform the with which +account they are logged in. If set to a valid attribute, the user will see +the first value of that attribute. + +**Note**: This option needs to be a string else it will be considered disabled. + Default value is NULL. + ## Attribute Rules Each additional filter configuration option is considered an attribute matching diff --git a/locales/en/LC_MESSAGES/authorize.po b/locales/en/LC_MESSAGES/authorize.po index 8d47ce6..d9f2d8e 100644 --- a/locales/en/LC_MESSAGES/authorize.po +++ b/locales/en/LC_MESSAGES/authorize.po @@ -25,3 +25,8 @@ msgstr "" "You don't have the needed privileges to access this application. Please " "contact the administrator if you find this to be incorrect." +msgid "Change account?" +msgstr "Change account?" + +msgid "You are currently logged in as" +msgstr "You are currently logged in as" diff --git a/locales/nl/LC_MESSAGES/authorize.po b/locales/nl/LC_MESSAGES/authorize.po index 2570f88..22ab2b3 100644 --- a/locales/nl/LC_MESSAGES/authorize.po +++ b/locales/nl/LC_MESSAGES/authorize.po @@ -25,3 +25,8 @@ msgstr "" "U heeft niet genoeg rechten om deze applicatie te benaderen. Benader de " "beheerder als u denkt dat dit ten onrechte is." +msgid "Change account?" +msgstr "Verander account?" + +msgid "You are currently logged in as" +msgstr "Je bent ingelogd als" diff --git a/routing/routes/routes.yml b/routing/routes/routes.yml index e01ca02..931152c 100644 --- a/routing/routes/routes.yml +++ b/routing/routes/routes.yml @@ -6,3 +6,9 @@ authorize-error-forbidden: _controller: 'SimpleSAML\Module\authorize\Controller\Authorize::forbidden' } methods: [GET] +authorize-reauthenticate: + path: /error/reauthenticate + defaults: { + _controller: 'SimpleSAML\Module\authorize\Controller\Authorize::reauthenticate' + } + methods: [GET] diff --git a/src/Auth/Process/Authorize.php b/src/Auth/Process/Authorize.php index d44eae7..181564a 100644 --- a/src/Auth/Process/Authorize.php +++ b/src/Auth/Process/Authorize.php @@ -28,7 +28,7 @@ * @package SimpleSAMLphp */ -class Authorize extends Auth\ProcessingFilter +final class Authorize extends Auth\ProcessingFilter { /** * Flag to deny/unauthorize the user a attribute filter IS found @@ -66,6 +66,18 @@ class Authorize extends Auth\ProcessingFilter */ protected array $valid_attribute_values = []; + /** + * Flag to allow re-authentication when user is not authorized + * @var bool + */ + protected bool $allow_reauthentication = false; + + /** + * The attribute to show in the error page + * @var string|null + */ + protected ?string $show_user_attribute = null; + /** * Initialize this filter. * Validate configuration parameters. @@ -104,6 +116,16 @@ public function __construct(array $config, $reserved) unset($config['errorURL']); } + if (isset($config['allow_reauthentication']) && is_bool($config['allow_reauthentication'])) { + $this->allow_reauthentication = $config['allow_reauthentication']; + unset($config['allow_reauthentication']); + } + + if (isset($config['show_user_attribute']) && is_string($config['show_user_attribute'])) { + $this->show_user_attribute = $config['show_user_attribute']; + unset($config['show_user_attribute']); + } + foreach ($config as $attribute => $values) { if (is_string($values)) { $arrayUtils = new Utils\Arrays(); @@ -148,7 +170,7 @@ public function process(array &$state): void $state['authprocAuthorize_reject_msg'] = $this->reject_msg; } $state['authprocAuthorize_errorURL'] = $this->errorURL; - + $state['authprocAuthorize_allow_reauthentication'] = $this->allow_reauthentication; $arrayUtils = new Utils\Arrays(); foreach ($this->valid_attribute_values as $name => $patterns) { if (array_key_exists($name, $attributes)) { @@ -172,6 +194,13 @@ public function process(array &$state): void } if (!$authorize) { + if ($this->show_user_attribute !== null && array_key_exists($this->show_user_attribute, $attributes)) { + $userAttribute = $attributes[$this->show_user_attribute][0] ?? null; + if ($userAttribute !== null) { + $state['authprocAuthorize_user_attribute'] = $userAttribute; + } + } + // Try to hint at which attributes may have failed as context for errorURL processing if ($this->deny) { $state['authprocAuthorize_ctx'] = implode(' ', $ctx); diff --git a/src/Controller/Authorize.php b/src/Controller/Authorize.php index 0686a02..428197f 100644 --- a/src/Controller/Authorize.php +++ b/src/Controller/Authorize.php @@ -19,8 +19,7 @@ * * @package SimpleSAML\Module\authorize */ - -class Authorize +final class Authorize { /** * Controller constructor. @@ -68,6 +67,14 @@ public function forbidden(Request $request): Template 'core/logout/' . urlencode($state['Source']['auth']), ); } + if (isset($state['authprocAuthorize_user_attribute'])) { + $t->data['user_attribute'] = $state['authprocAuthorize_user_attribute']; + } + + $t->data['allow_reauthentication'] = $state['authprocAuthorize_allow_re_authenticate_on_unauthorized'] ?? false; + $stateId = Auth\State::saveState($state, 'authorize:Authorize'); + $t->data['url_reauthentication'] = + Module::getModuleURL('authorize/error/reauthenticate', ['StateId' => $stateId]); if ( isset($state['authprocAuthorize_errorURL']) @@ -94,4 +101,28 @@ public function forbidden(Request $request): Template $t->setStatusCode(403); return $t; } + + public function reauthenticate(Request $request): void + { + $stateId = $request->query->get('StateId', false); + if (!is_string($stateId)) { + throw new Error\BadRequest('Missing required StateId query parameter.'); + } + /** @var array $state */ + $state = Auth\State::loadState($stateId, 'authorize:Authorize'); + + $authSource = $state['Source']['auth']; + if (empty($authSource)) { + throw new Error\BadRequest('Missing required auth source.'); + } + $parameters = ['ForceAuthn' => true]; + + if (isset($state['\\SimpleSAML\\Auth\\State.restartURL'])) { + $returnToUrl = $state['\\SimpleSAML\\Auth\\State.restartURL'] ; + $parameters['ReturnTo'] = $returnToUrl; + } + + $auth = new Auth\Simple($authSource); + $auth->login($parameters); + } } diff --git a/templates/authorize_403.twig b/templates/authorize_403.twig index 7a9ead3..534bc53 100644 --- a/templates/authorize_403.twig +++ b/templates/authorize_403.twig @@ -7,6 +7,16 @@ {% else %}

{% trans %}You don't have the needed privileges to access this application. Please contact the administrator if you find this to be incorrect.{% endtrans %}

{% endif %} +{% if user_attribute is defined %} +

{% trans %}You are currently logged in as{% endtrans %} {{ user_attribute }}

+{% endif %} +{% if allow_reauthentication is defined and allow_reauthentication %} +

+ + + +

+{% endif %} {% if errorURL is defined %}

{% trans %}Your identity provider provides additional information that may help you solve this.{% endtrans %} diff --git a/tests/src/Auth/Process/AuthorizeTest.php b/tests/src/Auth/Process/AuthorizeTest.php index 284a8f3..a5e4755 100644 --- a/tests/src/Auth/Process/AuthorizeTest.php +++ b/tests/src/Auth/Process/AuthorizeTest.php @@ -181,4 +181,63 @@ public static function noregexScenarioProvider(): array [['group' => 'CN=wrongCN=SimpleSAML Students,CN=Users,DC=example,DC=edu'], false], ]; } + + /** + * Test that having a matching attribute prevents access + * + * @param array $userAttributes The attributes to test + * @param bool $isAuthorized Should the user be authorized + * @param string|null $shownUserAttribute The attribute to show + */ + #[DataProvider('showUserAttributeScenarioProvider')] + public function testShowUserAttribute( + array $userAttributes, + bool $isAuthorized, + bool $isShowUserAttributeSet, + ?string $shownUserAttribute, + ): void { + $attributeUtils = new Utils\Attributes(); + $userAttributes = $attributeUtils->normalizeAttributesArray($userAttributes); + $config = [ + 'regex' => false, + 'uid' => [ + 'test', + ], + 'show_user_attribute' => 'mail', + ]; + + $resultState = $this->processFilter($config, ['Attributes' => $userAttributes]); + $resultAuthorized = isset($resultState['NOT_AUTHORIZED']) ? false : true; + + $this->assertEquals($isAuthorized, $resultAuthorized, 'Authorization behaviour does not match'); + $isShownUserAttributeInState = isset($resultState['authprocAuthorize_user_attribute']); + $this->assertEquals( + $isShowUserAttributeSet, + $isShownUserAttributeInState, + 'Attribute shown behaviour does not match', + ); + if ($isShownUserAttributeInState) { + $isShownUserAttributeInState = $resultState['authprocAuthorize_user_attribute']; + $this->assertEquals($shownUserAttribute, $isShownUserAttributeInState); + } + } + + /** + * @return array + */ + public static function showUserAttributeScenarioProvider(): array + { + return [ + // Should be allowed, and not shown + [['uid' => 'test'], true, false, null], + [['uid' => 'test', 'mail' => 'user@example.edu'], true, false, null], + + // Should be denied, and not shown + [['uid' => 'anything@students.example.edu'], false, false, null], + [['uid' => 'anything@students.example.edu', 'mail' => []], false, false, null], + + // Should be denied, and shown + [['uid' => 'stu3@example.edu', 'mail' => 'user@example.edu'], false, true, 'user@example.edu'], + ]; + } } diff --git a/tests/src/Controller/AuthorizeAllowReAuthenticateOnUnauthorizedTest.php b/tests/src/Controller/AuthorizeAllowReAuthenticateOnUnauthorizedTest.php new file mode 100644 index 0000000..ac96e64 --- /dev/null +++ b/tests/src/Controller/AuthorizeAllowReAuthenticateOnUnauthorizedTest.php @@ -0,0 +1,83 @@ +config = Configuration::loadFromArray( + [ + 'baseurlpath' => 'https://example.org/simplesaml', + 'module.enable' => ['authorize' => true], + ], + '[ARRAY]', + 'simplesaml', + ); + + $state = [ + 'StateId' => 'SomeState', + 'Source' => ['auth' => 'test'], + 'authprocAuthorize_reject_msg' => 'Test Rejected', + 'authprocAuthorize_error_url' => true, + 'authprocAuthorize_ctx' => 'example', + 'authprocAuthorize_allow_re_authenticate_on_unauthorized' => true, + + ]; + $this->stateId = Auth\State::saveState($state, 'authorize:Authorize'); + + Configuration::setPreLoadedConfig($this->config, 'config.php'); + } + + + /** + * Test that a valid requests results in a HTTP/403 Forbidden page with translated messages + * @return void + * @throws \Exception + */ + public function testValidRequest() + { + $request = Request::create( + '/', + 'GET', + ['StateId' => $this->stateId], + ); + $session = Session::getSessionFromRequest(); + + $c = new Controller\Authorize($this->config, $session); + + /** @var \SimpleSAML\XHTML\Template $response */ + $response = $c->forbidden($request); + + $this->assertInstanceOf(Template::class, $response); + $this->assertTrue($response->isForbidden()); + } +} diff --git a/tests/src/Controller/AuthorizeShowUserAttributeTest.php b/tests/src/Controller/AuthorizeShowUserAttributeTest.php new file mode 100644 index 0000000..24c28b0 --- /dev/null +++ b/tests/src/Controller/AuthorizeShowUserAttributeTest.php @@ -0,0 +1,81 @@ +config = Configuration::loadFromArray( + [ + 'baseurlpath' => 'https://example.org/simplesaml', + 'module.enable' => ['authorize' => true], + ], + '[ARRAY]', + 'simplesaml', + ); + + $state = [ + 'StateId' => 'SomeState', + 'Source' => ['auth' => 'test'], + 'authprocAuthorize_ctx' => 'example', + 'authprocAuthorize_user_attribute' => 'shown_user_attribute', + ]; + $this->stateId = Auth\State::saveState($state, 'authorize:Authorize'); + + Configuration::setPreLoadedConfig($this->config, 'config.php'); + } + + + /** + * Test that a valid requests results in a HTTP/403 Forbidden page with translated messages + * @return void + * @throws \Exception + */ + public function testValidRequest() + { + $request = Request::create( + '/', + 'GET', + ['StateId' => $this->stateId], + ); + $session = Session::getSessionFromRequest(); + + $c = new Controller\Authorize($this->config, $session); + + /** @var \SimpleSAML\XHTML\Template $response */ + $response = $c->forbidden($request); + + $this->assertInstanceOf(Template::class, $response); + $this->assertTrue($response->isForbidden()); + $this->assertEquals('shown_user_attribute', $response->data['user_attribute']); + } +}