From 8a0a8e33b14a9f109f856547b673018ac557f2bc Mon Sep 17 00:00:00 2001 From: Tim van Dijen <tim.dijen@minbzk.nl> Date: Tue, 23 Jul 2019 20:33:12 +0200 Subject: [PATCH] Fix Psalm-issues batch #2 (#1159) Fix Psalm-issues in the modules-directory --- lib/SimpleSAML/Auth/State.php | 1 + modules/admin/lib/ConfigController.php | 5 +- modules/admin/lib/FederationController.php | 2 +- modules/admin/lib/TestController.php | 18 ++++-- .../core/lib/Auth/Process/AttributeAdd.php | 6 +- .../core/lib/Auth/Process/AttributeAlter.php | 9 +++ .../core/lib/Auth/Process/AttributeCopy.php | 2 + .../core/lib/Auth/Process/AttributeLimit.php | 4 ++ .../core/lib/Auth/Process/AttributeMap.php | 5 +- .../core/lib/Auth/Process/AttributeRealm.php | 2 + .../lib/Auth/Process/AttributeValueMap.php | 6 +- modules/core/lib/Auth/Process/Cardinality.php | 2 + .../lib/Auth/Process/CardinalitySingle.php | 1 + .../core/lib/Auth/Process/GenerateGroups.php | 3 + .../core/lib/Auth/Process/LanguageAdaptor.php | 1 + modules/core/lib/Auth/Process/PHP.php | 9 +-- .../Auth/Process/StatisticsWithAttribute.php | 2 +- modules/core/lib/Auth/Process/TargetedID.php | 2 + .../core/lib/Auth/Source/AdminPassword.php | 1 + modules/core/lib/Auth/UserPassBase.php | 11 ++++ modules/core/lib/Auth/UserPassOrgBase.php | 12 ++++ modules/core/lib/Controller.php | 2 + modules/core/lib/Stats/Output/File.php | 13 ++-- .../core/lib/Storage/SQLPermanentStorage.php | 4 +- .../core/templates/frontpage_config.tpl.php | 2 +- ...oginuserpass.php => loginuserpass.tpl.php} | 0 ...pper.php => logout-iframe-wrapper.tpl.php} | 0 ...ogout-iframe.php => logout-iframe.tpl.php} | 0 ...nterval.php => short_sso_interval.tpl.php} | 0 modules/core/www/authenticate.php | 2 + modules/core/www/cardinality_error.php | 1 + modules/core/www/frontpage_config.php | 5 +- modules/core/www/idp/logout-iframe-post.php | 1 + modules/core/www/idp/logout-iframe.php | 4 +- modules/core/www/loginuserpass.php | 7 ++- modules/core/www/loginuserpassorg.php | 5 +- modules/core/www/no_cookie.php | 13 +++- modules/core/www/short_sso_interval.php | 5 +- modules/cron/lib/Cron.php | 1 + ...nfo-result.php => croninfo-result.tpl.php} | 0 modules/cron/www/cron.php | 2 +- .../exampleauth/lib/Auth/Source/External.php | 3 +- .../lib/Auth/Source/StaticSource.php | 2 + .../exampleauth/lib/Auth/Source/UserPass.php | 4 ++ modules/exampleauth/www/redirecttest.php | 2 + .../multiauth/lib/Auth/Source/MultiAuth.php | 3 +- ...{selectsource.php => selectsource.tpl.php} | 0 modules/multiauth/www/selectsource.php | 4 +- modules/portal/hooks/hook_htmlinject.php | 2 + modules/portal/lib/Portal.php | 13 +++- .../Process/ExpectedAuthnContextClassRef.php | 4 +- .../saml/lib/Auth/Process/FilterScopes.php | 62 ++++++++++--------- modules/saml/lib/Auth/Source/SP.php | 51 ++++++++++----- modules/saml/lib/IdP/SAML1.php | 1 + modules/saml/lib/IdP/SAML2.php | 18 ++++++ modules/saml/lib/Message.php | 8 ++- .../{metadata.php => metadata.tpl.php} | 0 ...id_session.php => invalid_session.tpl.php} | 0 modules/saml/www/idp/certs.php | 7 ++- modules/saml/www/proxy/invalid_session.php | 9 ++- modules/saml/www/sp/discoresp.php | 5 ++ modules/saml/www/sp/metadata.php | 10 +-- modules/saml/www/sp/saml1-acs.php | 4 +- modules/saml/www/sp/saml2-acs.php | 33 +++++----- modules/saml/www/sp/saml2-logout.php | 16 +++-- psalm.xml | 31 +++++++++- 66 files changed, 337 insertions(+), 126 deletions(-) rename modules/core/templates/{loginuserpass.php => loginuserpass.tpl.php} (100%) rename modules/core/templates/{logout-iframe-wrapper.php => logout-iframe-wrapper.tpl.php} (100%) rename modules/core/templates/{logout-iframe.php => logout-iframe.tpl.php} (100%) rename modules/core/templates/{short_sso_interval.php => short_sso_interval.tpl.php} (100%) rename modules/cron/templates/{croninfo-result.php => croninfo-result.tpl.php} (100%) rename modules/multiauth/templates/{selectsource.php => selectsource.tpl.php} (100%) rename modules/saml/templates/{metadata.php => metadata.tpl.php} (100%) rename modules/saml/templates/proxy/{invalid_session.php => invalid_session.tpl.php} (100%) diff --git a/lib/SimpleSAML/Auth/State.php b/lib/SimpleSAML/Auth/State.php index 09daaa9a1..22310d180 100644 --- a/lib/SimpleSAML/Auth/State.php +++ b/lib/SimpleSAML/Auth/State.php @@ -378,6 +378,7 @@ class State */ throw $exception; } + throw new \Exception(); // This should never happen } diff --git a/modules/admin/lib/ConfigController.php b/modules/admin/lib/ConfigController.php index 97786c4ba..8d2c0758f 100644 --- a/modules/admin/lib/ConfigController.php +++ b/modules/admin/lib/ConfigController.php @@ -363,7 +363,7 @@ class ConfigController ); } } - + /* * Check for updates. Store the remote result in the session so we don't need to fetch it on every access to * this page. @@ -385,7 +385,8 @@ class ConfigController curl_setopt($ch, CURLOPT_PROXYUSERPWD, $this->config->getValue('proxy.auth', null)); $response = curl_exec($ch); - if (curl_getinfo($ch, CURLINFO_HTTP_CODE) === 200) { + if (curl_getinfo($ch, CURLINFO_RESPONSE_CODE) === 200) { + /** @psalm-suppress InvalidScalarArgument */ $latest = json_decode($response, true); $this->session->setData(self::LATEST_VERSION_STATE_KEY, 'version', $latest); } diff --git a/modules/admin/lib/FederationController.php b/modules/admin/lib/FederationController.php index 1929504c3..17d5744c0 100644 --- a/modules/admin/lib/FederationController.php +++ b/modules/admin/lib/FederationController.php @@ -227,7 +227,7 @@ class FederationController $this->mdHandler->getMetaDataCurrentEntityID('shib13-idp-hosted') ); } - + foreach ($shib13entities as $index => $entity) { $builder = new SAMLBuilder($entity['entityid']); $builder->addMetadataIdP11($entity['metadata_array']); diff --git a/modules/admin/lib/TestController.php b/modules/admin/lib/TestController.php index 957aca6c6..ee81541aa 100644 --- a/modules/admin/lib/TestController.php +++ b/modules/admin/lib/TestController.php @@ -69,6 +69,7 @@ class TestController $authsource->logout($this->config->getBasePath().'logout.php'); } elseif (!is_null($request->query->get(Auth\State::EXCEPTION_PARAM))) { // This is just a simple example of an error + /** @var array $state */ $state = Auth\State::loadExceptionState(); assert(array_key_exists(Auth\State::EXCEPTION_DATA, $state)); throw $state[Auth\State::EXCEPTION_DATA]; @@ -114,16 +115,21 @@ class TestController */ private function getNameIDHTML(Template $t, NameID $nameId) { + $translator = $t->getTranslator(); $result = ''; if ($nameId->getValue() === null) { - $list = ["NameID" => [$t->t('{status:subject_notset}')]]; - $result .= "<p>NameID: <span class=\"notset\">".$t->t('{status:subject_notset}')."</span></p>"; + $list = ["NameID" => [$translator->t('{status:subject_notset}')]]; + /** @var string $notset */ + $notset = $translator->t('{status:subject_notset}'); + $result .= "<p>NameID: <span class=\"notset\">".$notset."</span></p>"; } else { $list = [ "NameId" => [$nameId->getValue()], ]; if ($nameId->getFormat() !== null) { - $list[$t->t('{status:subject_format}')] = [$nameId->getFormat()]; + /** @var string $format */ + $format = $translator->t('{status:subject_format}'); + $list[$format] = [$nameId->getFormat()]; } if ($nameId->getNameQualifier() !== null) { $list['NameQualifier'] = [$nameId->getNameQualifier()]; @@ -226,7 +232,7 @@ class TestController if (is_array($attr) && count($attr) > 1) { $str = '<ul>'; foreach ($attr as $value) { - $str .= '<li>'.htmlspecialchars($attr).'</li>'; + $str .= '<li>'.htmlspecialchars(strval($attr)).'</li>'; } $str .= '</ul>'; return $str; @@ -266,7 +272,9 @@ class TestController 'NameID' => [$nameID->getValue()], ]; if ($nameID->getFormat() !== null) { - $eptid[$t->t('{status:subject_format}')] = [$nameID->getFormat()]; + /** @var string $format */ + $format = $t->t('{status:subject_format}'); + $eptid[$format] = [$nameID->getFormat()]; } if ($nameID->getNameQualifier() !== null) { $eptid['NameQualifier'] = [$nameID->getNameQualifier()]; diff --git a/modules/core/lib/Auth/Process/AttributeAdd.php b/modules/core/lib/Auth/Process/AttributeAdd.php index b56899886..06fdb41bd 100644 --- a/modules/core/lib/Auth/Process/AttributeAdd.php +++ b/modules/core/lib/Auth/Process/AttributeAdd.php @@ -14,16 +14,19 @@ class AttributeAdd extends \SimpleSAML\Auth\ProcessingFilter { /** * Flag which indicates wheter this filter should append new values or replace old values. + * @var bool */ private $replace = false; /** * Attributes which should be added/appended. * - * Assiciative array of arrays. + * Associative array of arrays. + * @var array */ private $attributes = []; + /** * Initialize this filter. * @@ -59,6 +62,7 @@ class AttributeAdd extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Apply filter to add or replace attributes. * diff --git a/modules/core/lib/Auth/Process/AttributeAlter.php b/modules/core/lib/Auth/Process/AttributeAlter.php index a522facfc..5212e6938 100644 --- a/modules/core/lib/Auth/Process/AttributeAlter.php +++ b/modules/core/lib/Auth/Process/AttributeAlter.php @@ -16,34 +16,41 @@ class AttributeAlter extends \SimpleSAML\Auth\ProcessingFilter { /** * Should the pattern found be replaced? + * @var bool */ private $replace = false; /** * Should the value found be removed? + * @var bool */ private $remove = false; /** * Pattern to search for. + * @var string */ private $pattern = ''; /** * String to replace the pattern found with. + * @var string|false */ private $replacement = false; /** * Attribute to search in + * @var string */ private $subject = ''; /** * Attribute to place the result in. + * @var string */ private $target = ''; + /** * Initialize this filter. * @@ -85,6 +92,7 @@ class AttributeAlter extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Apply the filter to modify attributes. * @@ -175,6 +183,7 @@ class AttributeAlter extends \SimpleSAML\Auth\ProcessingFilter $attributes[$this->subject] ); } else { + /** @psalm-suppress InvalidArgument */ $attributes[$this->target] = array_diff( preg_replace( $this->pattern, diff --git a/modules/core/lib/Auth/Process/AttributeCopy.php b/modules/core/lib/Auth/Process/AttributeCopy.php index e9fab68d7..0dd53c8fa 100644 --- a/modules/core/lib/Auth/Process/AttributeCopy.php +++ b/modules/core/lib/Auth/Process/AttributeCopy.php @@ -21,6 +21,7 @@ class AttributeCopy extends \SimpleSAML\Auth\ProcessingFilter { /** * Assosiative array with the mappings of attribute names. + * @var array */ private $map = []; @@ -50,6 +51,7 @@ class AttributeCopy extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Apply filter to rename attributes. * diff --git a/modules/core/lib/Auth/Process/AttributeLimit.php b/modules/core/lib/Auth/Process/AttributeLimit.php index 8559db4d6..864324b26 100644 --- a/modules/core/lib/Auth/Process/AttributeLimit.php +++ b/modules/core/lib/Auth/Process/AttributeLimit.php @@ -15,6 +15,7 @@ class AttributeLimit extends \SimpleSAML\Auth\ProcessingFilter { /** * List of attributes which this filter will allow through. + * @var array */ private $allowedAttributes = []; @@ -60,6 +61,7 @@ class AttributeLimit extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Get list of allowed from the SP/IdP config. * @@ -79,6 +81,7 @@ class AttributeLimit extends \SimpleSAML\Auth\ProcessingFilter return null; } + /** * Apply filter to remove attributes. * @@ -129,6 +132,7 @@ class AttributeLimit extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Perform the filtering of attributes * @param array $values The current values for a given attribute diff --git a/modules/core/lib/Auth/Process/AttributeMap.php b/modules/core/lib/Auth/Process/AttributeMap.php index db5bdbc4f..f4b79d249 100644 --- a/modules/core/lib/Auth/Process/AttributeMap.php +++ b/modules/core/lib/Auth/Process/AttributeMap.php @@ -15,11 +15,13 @@ class AttributeMap extends \SimpleSAML\Auth\ProcessingFilter { /** * Associative array with the mappings of attribute names. + * @var array */ private $map = []; /** * Should attributes be duplicated or renamed. + * @var bool */ private $duplicate = false; @@ -89,7 +91,8 @@ class AttributeMap extends \SimpleSAML\Auth\ProcessingFilter } $filePath = Module::getModuleDir($m[0]).'/attributemap/'.$m[1].'.php'; } else { - $filePath = $config->getPathValue('attributenamemapdir', 'attributemap/').$fileName.'.php'; + $attributenamemapdir = $config->getPathValue('attributenamemapdir', 'attributemap/') ?: 'attributemap/'; + $filePath = $attributenamemapdir.$fileName.'.php'; } if (!file_exists($filePath)) { diff --git a/modules/core/lib/Auth/Process/AttributeRealm.php b/modules/core/lib/Auth/Process/AttributeRealm.php index c708ece37..58533a7cd 100644 --- a/modules/core/lib/Auth/Process/AttributeRealm.php +++ b/modules/core/lib/Auth/Process/AttributeRealm.php @@ -16,6 +16,7 @@ class AttributeRealm extends \SimpleSAML\Auth\ProcessingFilter /** @var string */ private $attributename = 'realm'; + /** * Initialize this filter. * @@ -32,6 +33,7 @@ class AttributeRealm extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Apply filter to add or replace attributes. * diff --git a/modules/core/lib/Auth/Process/AttributeValueMap.php b/modules/core/lib/Auth/Process/AttributeValueMap.php index bf747b634..82ed34a3d 100644 --- a/modules/core/lib/Auth/Process/AttributeValueMap.php +++ b/modules/core/lib/Auth/Process/AttributeValueMap.php @@ -30,7 +30,7 @@ class AttributeValueMap extends \SimpleSAML\Auth\ProcessingFilter * @var array */ private $values = []; - + /** * Whether $sourceattribute should be kept or not. * @var bool @@ -42,7 +42,7 @@ class AttributeValueMap extends \SimpleSAML\Auth\ProcessingFilter * @var bool */ private $replace = false; - + /** * Initialize the filter. @@ -83,7 +83,7 @@ class AttributeValueMap extends \SimpleSAML\Auth\ProcessingFilter if ($name === 'sourceattribute') { $this->sourceattribute = $value; } - + // set the values if ($name === 'values') { $this->values = $value; diff --git a/modules/core/lib/Auth/Process/Cardinality.php b/modules/core/lib/Auth/Process/Cardinality.php index b60fc389a..023f4c57a 100644 --- a/modules/core/lib/Auth/Process/Cardinality.php +++ b/modules/core/lib/Auth/Process/Cardinality.php @@ -25,6 +25,7 @@ class Cardinality extends \SimpleSAML\Auth\ProcessingFilter /** @var \SimpleSAML\Utils\HttpAdapter */ private $http; + /** * Initialize this filter, parse configuration. * @@ -97,6 +98,7 @@ class Cardinality extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Process this filter * diff --git a/modules/core/lib/Auth/Process/CardinalitySingle.php b/modules/core/lib/Auth/Process/CardinalitySingle.php index 13dad416b..1d1e39bae 100644 --- a/modules/core/lib/Auth/Process/CardinalitySingle.php +++ b/modules/core/lib/Auth/Process/CardinalitySingle.php @@ -36,6 +36,7 @@ class CardinalitySingle extends \SimpleSAML\Auth\ProcessingFilter /** @var \SimpleSAML\Utils\HttpAdapter */ private $http; + /** * Initialize this filter, parse configuration. * diff --git a/modules/core/lib/Auth/Process/GenerateGroups.php b/modules/core/lib/Auth/Process/GenerateGroups.php index b5d738e1d..206433cd5 100644 --- a/modules/core/lib/Auth/Process/GenerateGroups.php +++ b/modules/core/lib/Auth/Process/GenerateGroups.php @@ -14,9 +14,11 @@ class GenerateGroups extends \SimpleSAML\Auth\ProcessingFilter { /** * The attributes we should generate groups from. + * @var array */ private $generateGroupsFrom; + /** * Initialize this filter. * @@ -48,6 +50,7 @@ class GenerateGroups extends \SimpleSAML\Auth\ProcessingFilter } } + /** * Apply filter to add groups attribute. * diff --git a/modules/core/lib/Auth/Process/LanguageAdaptor.php b/modules/core/lib/Auth/Process/LanguageAdaptor.php index 818b866a9..64cef5884 100644 --- a/modules/core/lib/Auth/Process/LanguageAdaptor.php +++ b/modules/core/lib/Auth/Process/LanguageAdaptor.php @@ -16,6 +16,7 @@ class LanguageAdaptor extends \SimpleSAML\Auth\ProcessingFilter /** @var string */ private $langattr = 'preferredLanguage'; + /** * Initialize this filter. * diff --git a/modules/core/lib/Auth/Process/PHP.php b/modules/core/lib/Auth/Process/PHP.php index 96eb1773b..77d8266af 100644 --- a/modules/core/lib/Auth/Process/PHP.php +++ b/modules/core/lib/Auth/Process/PHP.php @@ -55,11 +55,12 @@ class PHP extends \SimpleSAML\Auth\ProcessingFilter /** * @param array &$attributes * @param array &$state - * @return void */ - $function = function ( - /** @scrutinizer ignore-unused */ &$attributes, - /** @scrutinizer ignore-unused */ &$state + $function = /** @return void */ function ( + /** @scrutinizer ignore-unused */ + array &$attributes, + /** @scrutinizer ignore-unused */ + array &$state ) { eval($this->code); }; diff --git a/modules/core/lib/Auth/Process/StatisticsWithAttribute.php b/modules/core/lib/Auth/Process/StatisticsWithAttribute.php index bf920128d..862e0c0d0 100644 --- a/modules/core/lib/Auth/Process/StatisticsWithAttribute.php +++ b/modules/core/lib/Auth/Process/StatisticsWithAttribute.php @@ -83,7 +83,7 @@ class StatisticsWithAttribute extends \SimpleSAML\Auth\ProcessingFilter $isPassive = 'passive-'; } - if (array_key_exists($this->attribute, $state['Attributes'])) { + if (!is_null($this->attribute) && array_key_exists($this->attribute, $state['Attributes'])) { $logAttribute = $state['Attributes'][$this->attribute][0]; } diff --git a/modules/core/lib/Auth/Process/TargetedID.php b/modules/core/lib/Auth/Process/TargetedID.php index 6c720d6ba..1ad7d2028 100644 --- a/modules/core/lib/Auth/Process/TargetedID.php +++ b/modules/core/lib/Auth/Process/TargetedID.php @@ -39,6 +39,8 @@ class TargetedID extends \SimpleSAML\Auth\ProcessingFilter /** * The attribute we should generate the targeted id from, or NULL if we should use the * UserID. + * + * @var string|null */ private $attribute = null; diff --git a/modules/core/lib/Auth/Source/AdminPassword.php b/modules/core/lib/Auth/Source/AdminPassword.php index 931707dbb..ea03893e7 100644 --- a/modules/core/lib/Auth/Source/AdminPassword.php +++ b/modules/core/lib/Auth/Source/AdminPassword.php @@ -31,6 +31,7 @@ class AdminPassword extends \SimpleSAML\Module\core\Auth\UserPassBase $this->setForcedUsername("admin"); } + /** * Attempt to log in using the given username and password. * diff --git a/modules/core/lib/Auth/UserPassBase.php b/modules/core/lib/Auth/UserPassBase.php index 2eb860bf3..b3477aafe 100644 --- a/modules/core/lib/Auth/UserPassBase.php +++ b/modules/core/lib/Auth/UserPassBase.php @@ -36,12 +36,16 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * * A forced username cannot be changed by the user. * If this is NULL, we won't force any username. + * + * @var string|null */ private $forcedUsername; /** * Links to pages from login page. * From configuration + * + * @var array */ protected $loginLinks; @@ -49,6 +53,7 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * Storage for authsource config option remember.username.enabled * loginuserpass.php and loginuserpassorg.php pages/templates use this option to * present users with a checkbox to save their username for the next login request. + * * @var bool */ protected $rememberUsernameEnabled = false; @@ -57,6 +62,7 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * Storage for authsource config option remember.username.checked * loginuserpass.php and loginuserpassorg.php pages/templates use this option * to default the remember username checkbox to checked or not. + * * @var bool */ protected $rememberUsernameChecked = false; @@ -67,6 +73,7 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * users with a checkbox to keep their session alive across * different browser sessions (that is, closing and opening the * browser again). + * * @var bool */ protected $rememberMeEnabled = false; @@ -75,6 +82,7 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * Storage for general config option session.rememberme.checked. * loginuserpass.php page/template uses this option to default * the "remember me" checkbox to checked or not. + * * @var bool */ protected $rememberMeChecked = false; @@ -286,10 +294,13 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source assert(is_string($password)); // Here we retrieve the state array we saved in the authenticate-function. + /** @var array $state */ $state = Auth\State::loadState($authStateId, self::STAGEID); // Retrieve the authentication source we are executing. assert(array_key_exists(self::AUTHID, $state)); + + /** @var \SimpleSAML\Module\core\Auth\UserPassBase|null $source */ $source = Auth\Source::getById($state[self::AUTHID]); if ($source === null) { throw new \Exception('Could not find authentication source with id '.$state[self::AUTHID]); diff --git a/modules/core/lib/Auth/UserPassOrgBase.php b/modules/core/lib/Auth/UserPassOrgBase.php index a91109755..552e91650 100644 --- a/modules/core/lib/Auth/UserPassOrgBase.php +++ b/modules/core/lib/Auth/UserPassOrgBase.php @@ -44,6 +44,8 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source * 'none': Force the user to select the correct organization from the dropdown box. * 'allow': Allow the user to enter the organization as part of the username. * 'force': Remove the dropdown box. + * + * @var string */ private $usernameOrgMethod; @@ -51,6 +53,7 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source * Storage for authsource config option remember.username.enabled * loginuserpass.php and loginuserpassorg.php pages/templates use this option to * present users with a checkbox to save their username for the next login request. + * * @var bool */ protected $rememberUsernameEnabled = false; @@ -59,6 +62,7 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source * Storage for authsource config option remember.username.checked * loginuserpass.php and loginuserpassorg.php pages/templates use this option * to default the remember username checkbox to checked or not. + * * @var bool */ protected $rememberUsernameChecked = false; @@ -67,6 +71,7 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source * Storage for authsource config option remember.organization.enabled * loginuserpassorg.php page/template use this option to present users * with a checkbox to save their organization choice for the next login request. + * * @var bool */ protected $rememberOrganizationEnabled = false; @@ -75,6 +80,7 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source * Storage for authsource config option remember.organization.checked * loginuserpassorg.php page/template use this option to * default the remember organization checkbox to checked or not. + * * @var bool */ protected $rememberOrganizationChecked = false; @@ -272,10 +278,13 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source assert(is_string($organization)); /* Retrieve the authentication state. */ + /** @var array $state */ $state = Auth\State::loadState($authStateId, self::STAGEID); /* Find authentication source. */ assert(array_key_exists(self::AUTHID, $state)); + + /** @var \SimpleSAML\Module\core\Auth\UserPassOrgBase|null $source */ $source = Auth\Source::getById($state[self::AUTHID]); if ($source === null) { throw new \Exception('Could not find authentication source with id '.$state[self::AUTHID]); @@ -328,10 +337,13 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source assert(is_string($authStateId)); /* Retrieve the authentication state. */ + /** @var array $state */ $state = Auth\State::loadState($authStateId, self::STAGEID); /* Find authentication source. */ assert(array_key_exists(self::AUTHID, $state)); + + /** @var \SimpleSAML\Module\core\Auth\UserPassOrgBase|null $source */ $source = Auth\Source::getById($state[self::AUTHID]); if ($source === null) { throw new \Exception('Could not find authentication source with id '.$state[self::AUTHID]); diff --git a/modules/core/lib/Controller.php b/modules/core/lib/Controller.php index 1d88454b0..44f3ead04 100644 --- a/modules/core/lib/Controller.php +++ b/modules/core/lib/Controller.php @@ -141,7 +141,9 @@ class Controller if ($request->get(Auth\State::EXCEPTION_PARAM, false) !== false) { // This is just a simple example of an error + /** @var array $state */ $state = Auth\State::loadExceptionState(); + assert(array_key_exists(Auth\State::EXCEPTION_DATA, $state)); $e = $state[Auth\State::EXCEPTION_DATA]; diff --git a/modules/core/lib/Stats/Output/File.php b/modules/core/lib/Stats/Output/File.php index d3875afd1..242372ab0 100644 --- a/modules/core/lib/Stats/Output/File.php +++ b/modules/core/lib/Stats/Output/File.php @@ -38,13 +38,14 @@ class File extends \SimpleSAML\Stats\Output */ public function __construct(Configuration $config) { - $this->logDir = $config->getPathValue('directory'); - if ($this->logDir === null) { + $logDir = $config->getPathValue('directory'); + if ($logDir === null) { throw new \Exception('Missing "directory" option for core:File'); } - if (!is_dir($this->logDir)) { - throw new \Exception('Could not find log directory: '.var_export($this->logDir, true)); + if (!is_dir($logDir)) { + throw new \Exception('Could not find log directory: '.var_export($logDir, true)); } + $this->logDir = $logDir; } @@ -86,6 +87,10 @@ class File extends \SimpleSAML\Stats\Output { assert(isset($data['time'])); + if ($this->file === false || $this->file === null) { + throw new Error\Exception('Error opening log file: invalid handle'); + } + $time = $data['time']; $milliseconds = (int) (($time - (int) $time) * 1000); diff --git a/modules/core/lib/Storage/SQLPermanentStorage.php b/modules/core/lib/Storage/SQLPermanentStorage.php index ea201bbb4..6bce0baf2 100644 --- a/modules/core/lib/Storage/SQLPermanentStorage.php +++ b/modules/core/lib/Storage/SQLPermanentStorage.php @@ -32,7 +32,7 @@ class SQLPermanentStorage $config = Configuration::getInstance(); } - $datadir = $config->getPathValue('datadir', 'data/'); + $datadir = $config->getPathValue('datadir', 'data/') ?: 'data/'; if (!is_dir($datadir)) { throw new \Exception('Data directory ['.$datadir.'] does not exist'); @@ -51,7 +51,7 @@ class SQLPermanentStorage if ($q === false) { $this->db->exec(' CREATE TABLE data ( - key1 text, + key1 text, key2 text, type text, value text, diff --git a/modules/core/templates/frontpage_config.tpl.php b/modules/core/templates/frontpage_config.tpl.php index 10f7463cf..8227e8cd0 100644 --- a/modules/core/templates/frontpage_config.tpl.php +++ b/modules/core/templates/frontpage_config.tpl.php @@ -5,7 +5,7 @@ $this->includeAtTemplateBase('includes/header.php'); ?> -<!-- +<!-- <div id="tabdiv"> <ul> <li><a href="#welcome"><?php echo $this->t('{core:frontpage:welcome}'); ?></a></li> diff --git a/modules/core/templates/loginuserpass.php b/modules/core/templates/loginuserpass.tpl.php similarity index 100% rename from modules/core/templates/loginuserpass.php rename to modules/core/templates/loginuserpass.tpl.php diff --git a/modules/core/templates/logout-iframe-wrapper.php b/modules/core/templates/logout-iframe-wrapper.tpl.php similarity index 100% rename from modules/core/templates/logout-iframe-wrapper.php rename to modules/core/templates/logout-iframe-wrapper.tpl.php diff --git a/modules/core/templates/logout-iframe.php b/modules/core/templates/logout-iframe.tpl.php similarity index 100% rename from modules/core/templates/logout-iframe.php rename to modules/core/templates/logout-iframe.tpl.php diff --git a/modules/core/templates/short_sso_interval.php b/modules/core/templates/short_sso_interval.tpl.php similarity index 100% rename from modules/core/templates/short_sso_interval.php rename to modules/core/templates/short_sso_interval.tpl.php diff --git a/modules/core/www/authenticate.php b/modules/core/www/authenticate.php index e1484e1fe..03b461fcd 100644 --- a/modules/core/www/authenticate.php +++ b/modules/core/www/authenticate.php @@ -20,7 +20,9 @@ if (array_key_exists('logout', $_REQUEST)) { if (array_key_exists(\SimpleSAML\Auth\State::EXCEPTION_PARAM, $_REQUEST)) { // This is just a simple example of an error + /** @var array $state */ $state = \SimpleSAML\Auth\State::loadExceptionState(); + assert(array_key_exists(\SimpleSAML\Auth\State::EXCEPTION_DATA, $state)); $e = $state[\SimpleSAML\Auth\State::EXCEPTION_DATA]; diff --git a/modules/core/www/cardinality_error.php b/modules/core/www/cardinality_error.php index b3a3e5b99..7ce8cce96 100644 --- a/modules/core/www/cardinality_error.php +++ b/modules/core/www/cardinality_error.php @@ -10,6 +10,7 @@ if (!array_key_exists('StateId', $_REQUEST)) { throw new \SimpleSAML\Error\BadRequest('Missing required StateId query parameter.'); } $id = $_REQUEST['StateId']; +/** @var array $state */ $state = \SimpleSAML\Auth\State::loadState($id, 'core:cardinality'); $session = \SimpleSAML\Session::getSessionFromRequest(); diff --git a/modules/core/www/frontpage_config.php b/modules/core/www/frontpage_config.php index bb6e6ae5a..3ed2fffce 100644 --- a/modules/core/www/frontpage_config.php +++ b/modules/core/www/frontpage_config.php @@ -75,8 +75,9 @@ if ($config->getBoolean('admin.checkforupdates', true) && $current !== 'master') curl_setopt($ch, CURLOPT_PROXYUSERPWD, $config->getValue('proxy.auth', null)); $response = curl_exec($ch); - if (curl_getinfo($ch, CURLINFO_HTTP_CODE) === 200) { - $latest = json_decode($response, true); + if (curl_getinfo($ch, CURLINFO_RESPONSE_CODE) === 200) { + /** @psalm-suppress InvalidScalarArgument */ + $latest = json_decode(strval($response), true); $session->setData("core:latest_simplesamlphp_version", "version", $latest); } curl_close($ch); diff --git a/modules/core/www/idp/logout-iframe-post.php b/modules/core/www/idp/logout-iframe-post.php index 2081dfe26..6dc9c8d35 100644 --- a/modules/core/www/idp/logout-iframe-post.php +++ b/modules/core/www/idp/logout-iframe-post.php @@ -51,6 +51,7 @@ if ($encryptNameId) { $bindings = [\SAML2\Constants::BINDING_HTTP_POST]; +/** @var array $dst */ $dst = $spMetadata->getDefaultEndpoint('SingleLogoutService', $bindings); $binding = \SAML2\Binding::getBinding($dst['Binding']); $lr->setDestination($dst['Location']); diff --git a/modules/core/www/idp/logout-iframe.php b/modules/core/www/idp/logout-iframe.php index 27195af6b..5e58666ae 100644 --- a/modules/core/www/idp/logout-iframe.php +++ b/modules/core/www/idp/logout-iframe.php @@ -115,9 +115,9 @@ foreach ($state['core:Logout-IFrame:Associations'] as $association) { $globalConfig = \SimpleSAML\Configuration::getInstance(); if ($type === 'nojs') { - $t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:logout-iframe-wrapper.php'); + $t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:logout-iframe-wrapper.tpl.php'); } else { - $t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:logout-iframe.php'); + $t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:logout-iframe.tpl.php'); } /** diff --git a/modules/core/www/loginuserpass.php b/modules/core/www/loginuserpass.php index 747506f89..ce14bf1ed 100644 --- a/modules/core/www/loginuserpass.php +++ b/modules/core/www/loginuserpass.php @@ -14,8 +14,10 @@ if (!array_key_exists('AuthState', $_REQUEST)) { throw new \SimpleSAML\Error\BadRequest('Missing AuthState parameter.'); } $authStateId = $_REQUEST['AuthState']; +/** @var array $state */ $state = \SimpleSAML\Auth\State::loadState($authStateId, \SimpleSAML\Module\core\Auth\UserPassBase::STAGEID); +/** @var \SimpleSAML\Module\core\Auth\UserPassBase|null $source */ $source = \SimpleSAML\Auth\Source::getById($state[\SimpleSAML\Module\core\Auth\UserPassBase::AUTHID]); if ($source === null) { throw new \Exception( @@ -23,7 +25,6 @@ if ($source === null) { ); } - if (array_key_exists('username', $_REQUEST)) { $username = $_REQUEST['username']; } elseif ($source->getRememberUsernameEnabled() && array_key_exists($source->getAuthId().'-username', $_COOKIE)) { @@ -60,7 +61,7 @@ if (!empty($_REQUEST['username']) || !empty($password)) { if ($source->getRememberUsernameEnabled()) { $sessionHandler = \SimpleSAML\SessionHandler::getSessionHandler(); $params = $sessionHandler->getCookieParams(); - + if (isset($_REQUEST['remember_username']) && $_REQUEST['remember_username'] == 'Yes') { $params['expire'] = time() + 31536000; } else { @@ -98,7 +99,7 @@ if (!empty($_REQUEST['username']) || !empty($password)) { } $globalConfig = \SimpleSAML\Configuration::getInstance(); -$t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:loginuserpass.php'); +$t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:loginuserpass.tpl.php'); $t->data['stateparams'] = ['AuthState' => $authStateId]; if (array_key_exists('forcedUsername', $state)) { $t->data['username'] = $state['forcedUsername']; diff --git a/modules/core/www/loginuserpassorg.php b/modules/core/www/loginuserpassorg.php index 2b9ec4dba..e9423c71c 100644 --- a/modules/core/www/loginuserpassorg.php +++ b/modules/core/www/loginuserpassorg.php @@ -14,8 +14,11 @@ if (!array_key_exists('AuthState', $_REQUEST)) { throw new \SimpleSAML\Error\BadRequest('Missing AuthState parameter.'); } $authStateId = $_REQUEST['AuthState']; + +/** @var array $state */ $state = \SimpleSAML\Auth\State::loadState($authStateId, \SimpleSAML\Module\core\Auth\UserPassOrgBase::STAGEID); +/** @var \SimpleSAML\Module\core\Auth\UserPassOrgBase $source */ $source = \SimpleSAML\Auth\Source::getById($state[\SimpleSAML\Module\core\Auth\UserPassOrgBase::AUTHID]); if ($source === null) { throw new \Exception( @@ -124,7 +127,7 @@ if ($organizations === null || !empty($organization)) { } $globalConfig = \SimpleSAML\Configuration::getInstance(); -$t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:loginuserpass.php'); +$t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:loginuserpass.tpl.php'); $t->data['stateparams'] = ['AuthState' => $authStateId]; $t->data['username'] = $username; $t->data['forceUsername'] = false; diff --git a/modules/core/www/no_cookie.php b/modules/core/www/no_cookie.php index 3575d85f6..7ae3889d6 100644 --- a/modules/core/www/no_cookie.php +++ b/modules/core/www/no_cookie.php @@ -11,8 +11,15 @@ $globalConfig = \SimpleSAML\Configuration::getInstance(); $t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:no_cookie.tpl.php'); $translator = $t->getTranslator(); -$t->data['header'] = htmlspecialchars($translator->t('{core:no_cookie:header}')); -$t->data['description'] = htmlspecialchars($translator->t('{core:no_cookie:description}')); -$t->data['retry'] = htmlspecialchars($translator->t('{core:no_cookie:retry}')); +/** @var string $header */ +$header = $translator->t('{core:no_cookie:header}'); +/** @var string $desc */ +$desc = $translator->t('{core:no_cookie:description}'); +/** @var string $retry */ +$retry = $translator->t('{core:no_cookie:retry}'); + +$t->data['header'] = htmlspecialchars($header); +$t->data['description'] = htmlspecialchars($desc); +$t->data['retry'] = htmlspecialchars($retry); $t->data['retryURL'] = $retryURL; $t->show(); diff --git a/modules/core/www/short_sso_interval.php b/modules/core/www/short_sso_interval.php index a9da1cae1..919210f53 100644 --- a/modules/core/www/short_sso_interval.php +++ b/modules/core/www/short_sso_interval.php @@ -11,7 +11,10 @@ if (!array_key_exists('StateId', $_REQUEST)) { throw new \SimpleSAML\Error\BadRequest('Missing required StateId query parameter.'); } $id = $_REQUEST['StateId']; + +/** @var array $state */ $state = \SimpleSAML\Auth\State::loadState($id, 'core:short_sso_interval'); + $session = \SimpleSAML\Session::getSessionFromRequest(); if (array_key_exists('continue', $_REQUEST)) { @@ -20,7 +23,7 @@ if (array_key_exists('continue', $_REQUEST)) { } $globalConfig = \SimpleSAML\Configuration::getInstance(); -$t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:short_sso_interval.php'); +$t = new \SimpleSAML\XHTML\Template($globalConfig, 'core:short_sso_interval.tpl.php'); $translator = $t->getTranslator(); $t->data['target'] = \SimpleSAML\Module::getModuleURL('core/short_sso_interval.php'); $t->data['params'] = ['StateId' => $id]; diff --git a/modules/cron/lib/Cron.php b/modules/cron/lib/Cron.php index 9956a87d4..bbaa4e7c3 100644 --- a/modules/cron/lib/Cron.php +++ b/modules/cron/lib/Cron.php @@ -17,6 +17,7 @@ class Cron */ private $cronconfig; + /* * @param \SimpleSAML\Configuration $cronconfig The cron configuration to use. If not specified defaults * to `config/module_cron.php` diff --git a/modules/cron/templates/croninfo-result.php b/modules/cron/templates/croninfo-result.tpl.php similarity index 100% rename from modules/cron/templates/croninfo-result.php rename to modules/cron/templates/croninfo-result.tpl.php diff --git a/modules/cron/www/cron.php b/modules/cron/www/cron.php index 5dd7439d0..2f0dae8b5 100644 --- a/modules/cron/www/cron.php +++ b/modules/cron/www/cron.php @@ -29,7 +29,7 @@ if ($cronconfig->getValue('sendemail', true) && count($summary) > 0) { } if (isset($_REQUEST['output']) && $_REQUEST['output'] == "xhtml") { - $t = new \SimpleSAML\XHTML\Template($config, 'cron:croninfo-result.php', 'cron:cron'); + $t = new \SimpleSAML\XHTML\Template($config, 'cron:croninfo-result.tpl.php', 'cron:cron'); $t->data['tag'] = $croninfo['tag']; $t->data['time'] = $time; $t->data['url'] = $url; diff --git a/modules/exampleauth/lib/Auth/Source/External.php b/modules/exampleauth/lib/Auth/Source/External.php index 1b37ad7a8..3d3d5cf7b 100644 --- a/modules/exampleauth/lib/Auth/Source/External.php +++ b/modules/exampleauth/lib/Auth/Source/External.php @@ -34,6 +34,7 @@ class External extends \SimpleSAML\Auth\Source */ const AUTHID = 'SimpleSAML\Module\exampleautth\Auth\Sourc\External.AuthId'; + /** * Constructor for this authentication source. * @@ -186,7 +187,6 @@ class External extends \SimpleSAML\Auth\Source * This function resumes the authentication process after the user has * entered his or her credentials. * - * @param array &$state The authentication state. * @return void * @throws \SimpleSAML\Error\BadRequest * @throws \SimpleSAML\Error\Exception @@ -205,6 +205,7 @@ class External extends \SimpleSAML\Auth\Source * Once again, note the second parameter to the loadState function. This must * match the string we used in the saveState-call above. */ + /** @var array $state */ $state = Auth\State::loadState($_REQUEST['State'], 'exampleauth:External'); /* diff --git a/modules/exampleauth/lib/Auth/Source/StaticSource.php b/modules/exampleauth/lib/Auth/Source/StaticSource.php index 7d196b62d..1a29fcd2f 100644 --- a/modules/exampleauth/lib/Auth/Source/StaticSource.php +++ b/modules/exampleauth/lib/Auth/Source/StaticSource.php @@ -17,9 +17,11 @@ class StaticSource extends \SimpleSAML\Auth\Source { /** * The attributes we return. + * @var array */ private $attributes; + /** * Constructor for this authentication source. * diff --git a/modules/exampleauth/lib/Auth/Source/UserPass.php b/modules/exampleauth/lib/Auth/Source/UserPass.php index 76adc8324..4761e65da 100644 --- a/modules/exampleauth/lib/Auth/Source/UserPass.php +++ b/modules/exampleauth/lib/Auth/Source/UserPass.php @@ -20,9 +20,12 @@ class UserPass extends \SimpleSAML\Module\core\Auth\UserPassBase /** * Our users, stored in an associative array. The key of the array is "<username>:<password>", * while the value of each element is a new array with the attributes for each user. + * + * @var array */ private $users; + /** * Constructor for this authentication source. * @@ -66,6 +69,7 @@ class UserPass extends \SimpleSAML\Module\core\Auth\UserPassBase } } + /** * Attempt to log in using the given username and password. * diff --git a/modules/exampleauth/www/redirecttest.php b/modules/exampleauth/www/redirecttest.php index 9d605277d..ff06102cd 100644 --- a/modules/exampleauth/www/redirecttest.php +++ b/modules/exampleauth/www/redirecttest.php @@ -10,6 +10,8 @@ if (!array_key_exists('StateId', $_REQUEST)) { throw new \SimpleSAML\Error\BadRequest('Missing required StateId query parameter.'); } + +/** @var array $state */ $state = \SimpleSAML\Auth\State::loadState($_REQUEST['StateId'], 'exampleauth:redirectfilter-test'); $state['Attributes']['RedirectTest2'] = ['OK']; diff --git a/modules/multiauth/lib/Auth/Source/MultiAuth.php b/modules/multiauth/lib/Auth/Source/MultiAuth.php index 31e76b202..02a017cfc 100644 --- a/modules/multiauth/lib/Auth/Source/MultiAuth.php +++ b/modules/multiauth/lib/Auth/Source/MultiAuth.php @@ -40,6 +40,7 @@ class MultiAuth extends \SimpleSAML\Auth\Source /** * Array of sources we let the user chooses among. + * @var array */ private $sources; @@ -237,7 +238,7 @@ class MultiAuth extends \SimpleSAML\Auth\Source $source = Auth\Source::getById($authId); if ($source === null) { - throw new \Exception('Invalid authentication source during logout: '.$source); + throw new \Exception('Invalid authentication source during logout: '.$authId); } // Then, do the logout on it $source->logout($state); diff --git a/modules/multiauth/templates/selectsource.php b/modules/multiauth/templates/selectsource.tpl.php similarity index 100% rename from modules/multiauth/templates/selectsource.php rename to modules/multiauth/templates/selectsource.tpl.php diff --git a/modules/multiauth/www/selectsource.php b/modules/multiauth/www/selectsource.php index f18d60fbe..2038e3087 100644 --- a/modules/multiauth/www/selectsource.php +++ b/modules/multiauth/www/selectsource.php @@ -15,6 +15,8 @@ if (!array_key_exists('AuthState', $_REQUEST)) { throw new \SimpleSAML\Error\BadRequest('Missing AuthState parameter.'); } $authStateId = $_REQUEST['AuthState']; + +/** @var array $state */ $state = \SimpleSAML\Auth\State::loadState($authStateId, \SimpleSAML\Module\multiauth\Auth\Source\MultiAuth::STAGEID); if (array_key_exists("\SimpleSAML\Auth\Source.id", $state)) { @@ -49,7 +51,7 @@ if (array_key_exists('multiauth:preselect', $state)) { } $globalConfig = \SimpleSAML\Configuration::getInstance(); -$t = new \SimpleSAML\XHTML\Template($globalConfig, 'multiauth:selectsource.php'); +$t = new \SimpleSAML\XHTML\Template($globalConfig, 'multiauth:selectsource.tpl.php'); $defaultLanguage = $globalConfig->getString('language.default', 'en'); $language = $t->getTranslator()->getLanguage()->getLanguage(); diff --git a/modules/portal/hooks/hook_htmlinject.php b/modules/portal/hooks/hook_htmlinject.php index 72235d733..e23e0d1e1 100644 --- a/modules/portal/hooks/hook_htmlinject.php +++ b/modules/portal/hooks/hook_htmlinject.php @@ -16,6 +16,8 @@ function portal_hook_htmlinject(&$hookinfo) $links = ['links' => []]; \SimpleSAML\Module::callHooks('frontpage', $links); + assert(is_array($links)); + $portalConfig = \SimpleSAML\Configuration::getOptionalConfig('module_portal.php'); $allLinks = []; diff --git a/modules/portal/lib/Portal.php b/modules/portal/lib/Portal.php index 69bcf63e2..0bbf78a7f 100644 --- a/modules/portal/lib/Portal.php +++ b/modules/portal/lib/Portal.php @@ -50,6 +50,9 @@ class Portal */ public function isPortalized($thispage) { + if (!isset($this->config)) { + return false; + } foreach ($this->config as $set) { if (in_array($thispage, $set, true)) { return true; @@ -95,15 +98,19 @@ class Portal if (isset($page['shorttext'])) { $name = $page['shorttext']; } + + /** @var string $name */ + $name = $t->t($name); + if (!isset($page['href'])) { $text .= '<li class="ui-state-default ui-corner-top ui-tabs-selected ui-state-active"><a href="#">'. - $t->t($name).'</a></li>'; + $name.'</a></li>'; } elseif ($pageid === $thispage) { $text .= '<li class="ui-state-default ui-corner-top ui-tabs-selected ui-state-active"><a href="#">'. - $t->t($name).'</a></li>'; + $name.'</a></li>'; } else { $text .= '<li class="ui-state-default ui-corner-top"><a href="'.$page['href'].'">'. - $t->t($name).'</a></li>'; + $name.'</a></li>'; } } $text .= '</ul>'; diff --git a/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php b/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php index 7fde9ab41..535ae1f0b 100644 --- a/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php +++ b/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php @@ -73,7 +73,7 @@ class ExpectedAuthnContextClassRef extends \SimpleSAML\Auth\ProcessingFilter public function process(&$request) { assert(is_array($request)); - assert(array_key_exists('Attributes', $request)); + assert(array_key_exists('saml:sp:State', $request)); $this->AuthnContextClassRef = $request['saml:sp:State']['saml:sp:AuthnContext']; @@ -99,7 +99,7 @@ class ExpectedAuthnContextClassRef extends \SimpleSAML\Auth\ProcessingFilter protected function unauthorized(&$request) { Logger::error( - 'ExpectedAuthnContextClassRef: Invalid authentication context: '.$this->AuthnContextClassRef. + 'ExpectedAuthnContextClassRef: Invalid authentication context: '.strval($this->AuthnContextClassRef). '. Accepted values are: '.var_export($this->accepted, true) ); diff --git a/modules/saml/lib/Auth/Process/FilterScopes.php b/modules/saml/lib/Auth/Process/FilterScopes.php index 7427be412..fd4089b92 100644 --- a/modules/saml/lib/Auth/Process/FilterScopes.php +++ b/modules/saml/lib/Auth/Process/FilterScopes.php @@ -60,41 +60,43 @@ class FilterScopes extends \SimpleSAML\Auth\ProcessingFilter $validScopes = $src['scope']; } - foreach ($this->scopedAttributes as $attribute) { - if (!isset($request['Attributes'][$attribute])) { - continue; - } - - $values = $request['Attributes'][$attribute]; - $newValues = []; - foreach ($values as $value) { - $ep = Utils\Config\Metadata::getDefaultEndpoint($request['Source']['SingleSignOnService']); - $loc = $ep['Location']; - $host = parse_url($loc, PHP_URL_HOST); - if ($host === null) { - $host = ''; + $ep = Utils\Config\Metadata::getDefaultEndpoint($request['Source']['SingleSignOnService']); + if ($ep !== null) { + foreach ($this->scopedAttributes as $attribute) { + if (!isset($request['Attributes'][$attribute])) { + continue; } - $value_a = explode('@', $value, 2); - if (count($value_a) < 2) { - $newValues[] = $value; - continue; // there's no scope + + $values = $request['Attributes'][$attribute]; + $newValues = []; + foreach ($values as $value) { + $loc = $ep['Location']; + $host = parse_url($loc, PHP_URL_HOST); + if ($host === null) { + $host = ''; + } + $value_a = explode('@', $value, 2); + if (count($value_a) < 2) { + $newValues[] = $value; + continue; // there's no scope + } + $scope = $value_a[1]; + if (in_array($scope, $validScopes, true)) { + $newValues[] = $value; + } elseif (strpos($host, $scope) === strlen($host) - strlen($scope)) { + $newValues[] = $value; + } else { + Logger::warning("Removing value '$value' for attribute '$attribute'. Undeclared scope."); + } } - $scope = $value_a[1]; - if (in_array($scope, $validScopes, true)) { - $newValues[] = $value; - } elseif (strpos($host, $scope) === strlen($host) - strlen($scope)) { - $newValues[] = $value; + + if (empty($newValues)) { + Logger::warning("No suitable values for attribute '$attribute', removing it."); + unset($request['Attributes'][$attribute]); // remove empty attributes } else { - Logger::warning("Removing value '$value' for attribute '$attribute'. Undeclared scope."); + $request['Attributes'][$attribute] = $newValues; } } - - if (empty($newValues)) { - Logger::warning("No suitable values for attribute '$attribute', removing it."); - unset($request['Attributes'][$attribute]); // remove empty attributes - } else { - $request['Attributes'][$attribute] = $newValues; - } } } } diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php index d65b72c52..f749bc63e 100644 --- a/modules/saml/lib/Auth/Source/SP.php +++ b/modules/saml/lib/Auth/Source/SP.php @@ -52,7 +52,7 @@ class SP extends \SimpleSAML\Auth\Source /** * Flag to indicate whether to disable sending the Scoping element. * - * @var boolean|FALSE + * @var bool */ private $disable_scoping; @@ -409,6 +409,8 @@ class SP extends \SimpleSAML\Auth\Source $this->protocols[] = Constants::NS_SAMLP; } break; + default: + $acs = []; } $acs['index'] = $index; $endpoints[] = $acs; @@ -554,23 +556,29 @@ class SP extends \SimpleSAML\Auth\Source } $nameId = $state['saml:NameID']; - $nid = new NameID(); - if (!array_key_exists('Value', $nameId)) { - throw new \InvalidArgumentException('Missing "Value" in array, cannot create NameID from it.'); - } + if (is_array($nameId)) { + // Must be an array > convert to object - $nid->setValue($nameId['Value']); - if (array_key_exists('NameQualifier', $nameId) && $nameId['NameQualifier'] !== null) { - $nid->setNameQualifier($nameId['NameQualifier']); - } - if (array_key_exists('SPNameQualifier', $nameId) && $nameId['SPNameQualifier'] !== null) { - $nid->setSPNameQualifier($nameId['SPNameQualifier']); - } - if (array_key_exists('SPProvidedID', $nameId) && $nameId['SPProvidedId'] !== null) { - $nid->setSPProvidedID($nameId['SPProvidedID']); - } - if (array_key_exists('Format', $nameId) && $nameId['Format'] !== null) { - $nid->setFormat($nameId['Format']); + $nid = new NameID(); + if (!array_key_exists('Value', $nameId)) { + throw new \InvalidArgumentException('Missing "Value" in array, cannot create NameID from it.'); + } + + $nid->setValue($nameId['Value']); + if (array_key_exists('NameQualifier', $nameId) && $nameId['NameQualifier'] !== null) { + $nid->setNameQualifier($nameId['NameQualifier']); + } + if (array_key_exists('SPNameQualifier', $nameId) && $nameId['SPNameQualifier'] !== null) { + $nid->setSPNameQualifier($nameId['SPNameQualifier']); + } + if (array_key_exists('SPProvidedID', $nameId) && $nameId['SPProvidedId'] !== null) { + $nid->setSPProvidedID($nameId['SPProvidedID']); + } + if (array_key_exists('Format', $nameId) && $nameId['Format'] !== null) { + $nid->setFormat($nameId['Format']); + } + } else { + $nid = $nameId; } $ar->setNameId($nid); @@ -650,6 +658,7 @@ class SP extends \SimpleSAML\Auth\Source // Select appropriate SSO endpoint if ($ar->getProtocolBinding() === Constants::BINDING_HOK_SSO) { + /** @var array $dst */ $dst = $idpMetadata->getDefaultEndpoint( 'SingleSignOnService', [ @@ -657,6 +666,7 @@ class SP extends \SimpleSAML\Auth\Source ] ); } else { + /** @var array $dst */ $dst = $idpMetadata->getEndpointPrioritizedByBinding( 'SingleSignOnService', [ @@ -829,6 +839,10 @@ class SP extends \SimpleSAML\Auth\Source $session = Session::getSessionFromRequest(); $data = $session->getAuthState($this->authId); + if ($data === null) { + throw new Error\NoState(); + } + foreach ($data as $k => $v) { $state[$k] = $v; } @@ -1022,6 +1036,7 @@ class SP extends \SimpleSAML\Auth\Source $idpMetadata = $this->getIdPMetadata($idp); + /** @var array $endpoint */ $endpoint = $idpMetadata->getEndpointPrioritizedByBinding('SingleLogoutService', [ Constants::BINDING_HTTP_REDIRECT, Constants::BINDING_HTTP_POST], false); @@ -1182,6 +1197,8 @@ class SP extends \SimpleSAML\Auth\Source $state = $authProcState['saml:sp:State']; $sourceId = $state['saml:sp:AuthId']; + + /** @var \SimpleSAML\Module\saml\Auth\Source\SP $source */ $source = Auth\Source::getById($sourceId); if ($source === null) { throw new \Exception('Could not find authentication source with id '.$sourceId); diff --git a/modules/saml/lib/IdP/SAML1.php b/modules/saml/lib/IdP/SAML1.php index 1254f0d16..0cb5cf5ea 100644 --- a/modules/saml/lib/IdP/SAML1.php +++ b/modules/saml/lib/IdP/SAML1.php @@ -58,6 +58,7 @@ class SAML1 $hasNewCert = true; } + /** @var array $certInfo */ $certInfo = Utils\Crypto::loadPublicKey($config, true); $keys[] = [ 'type' => 'X509Certificate', diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php index f6cf7b5aa..0a5c220d9 100644 --- a/modules/saml/lib/IdP/SAML2.php +++ b/modules/saml/lib/IdP/SAML2.php @@ -371,6 +371,10 @@ class SAML2 ); } elseif ($issuer instanceof Issuer) { $spEntityId = $issuer->getValue(); + if ($spEntityId === null) { + /* Without an issuer we have no way to respond to the message. */ + throw new Error\BadRequest('Received message on logout endpoint without issuer.'); + } } else { // we got a string, old case $spEntityId = $issuer; } @@ -430,6 +434,9 @@ class SAML2 $protocolBinding, $consumerIndex ); + if ($acsEndpoint === null) { + throw new \Exception('Unable to use any of the ACS endpoints found for SP \''.$spEntityId.'\''); + } $IDPList = array_unique(array_merge($IDPList, $spMetadata->getArrayizeString('IDPList', []))); if ($ProxyCount === null) { @@ -509,6 +516,7 @@ class SAML2 'idpEntityID' => $idpMetadata->getString('entityid'), ]); + /** @var array $dst */ $dst = $spMetadata->getEndpointPrioritizedByBinding( 'SingleLogoutService', [ @@ -564,6 +572,8 @@ class SAML2 'idpEntityID' => $idpMetadata->getString('entityid'), 'partial' => $partial ]); + + /** @var array $dst */ $dst = $spMetadata->getEndpointPrioritizedByBinding( 'SingleLogoutService', [ @@ -601,6 +611,10 @@ class SAML2 throw new Error\BadRequest('Received message on logout endpoint without issuer.'); } elseif ($issuer instanceof Issuer) { $spEntityId = $issuer->getValue(); + if ($spEntityId === null) { + /* Without an issuer we have no way to respond to the message. */ + throw new Error\BadRequest('Received message on logout endpoint without issuer.'); + } } else { $spEntityId = $issuer; } @@ -682,6 +696,8 @@ class SAML2 Constants::BINDING_HTTP_REDIRECT, Constants::BINDING_HTTP_POST ]; + + /** @var array $dst */ $dst = $spMetadata->getEndpointPrioritizedByBinding('SingleLogoutService', $bindings); if ($dst['Binding'] === Constants::BINDING_HTTP_POST) { @@ -793,6 +809,7 @@ class SAML2 $hasNewCert = true; } + /** @var array $certInfo */ $certInfo = Utils\Crypto::loadPublicKey($config, true); $keys[] = [ 'type' => 'X509Certificate', @@ -803,6 +820,7 @@ class SAML2 ]; if ($config->hasValue('https.certificate')) { + /** @var array $httpsCert */ $httpsCert = Utils\Crypto::loadPublicKey($config, true, 'https.'); $keys[] = [ 'type' => 'X509Certificate', diff --git a/modules/saml/lib/Message.php b/modules/saml/lib/Message.php index 5ef132ed2..42632885e 100644 --- a/modules/saml/lib/Message.php +++ b/modules/saml/lib/Message.php @@ -44,9 +44,11 @@ class Message $dstPrivateKey = $dstMetadata->getString('signature.privatekey', null); if ($dstPrivateKey !== null) { + /** @var array $keyArray */ $keyArray = Utils\Crypto::loadPrivateKey($dstMetadata, true, 'signature.'); $certArray = Utils\Crypto::loadPublicKey($dstMetadata, false, 'signature.'); } else { + /** @var array $keyArray */ $keyArray = Utils\Crypto::loadPrivateKey($srcMetadata, true); $certArray = Utils\Crypto::loadPublicKey($srcMetadata, false); } @@ -415,7 +417,11 @@ class Message } } - /** @var \Exception $lastException */ + /** + * The annotation below is not working - See vimeo/psalm#1909 + * @psalm-suppress InvalidThrow + * @var \Exception $lastException + */ throw $lastException; } diff --git a/modules/saml/templates/metadata.php b/modules/saml/templates/metadata.tpl.php similarity index 100% rename from modules/saml/templates/metadata.php rename to modules/saml/templates/metadata.tpl.php diff --git a/modules/saml/templates/proxy/invalid_session.php b/modules/saml/templates/proxy/invalid_session.tpl.php similarity index 100% rename from modules/saml/templates/proxy/invalid_session.php rename to modules/saml/templates/proxy/invalid_session.tpl.php diff --git a/modules/saml/www/idp/certs.php b/modules/saml/www/idp/certs.php index 7a2597519..7d4ba82af 100644 --- a/modules/saml/www/idp/certs.php +++ b/modules/saml/www/idp/certs.php @@ -18,19 +18,22 @@ $idpmeta = $metadata->getMetaDataConfig($idpentityid, 'saml20-idp-hosted'); switch ($_SERVER['PATH_INFO']) { case '/new_idp.crt': - $certInfo = SimpleSAML\Utils\Crypto::loadPublicKey($idpmeta, false, 'new_'); + /** @var array $certInfo */ + $certInfo = SimpleSAML\Utils\Crypto::loadPublicKey($idpmeta, true, 'new_'); break; case '/idp.crt': + /** @var array $certInfo */ $certInfo = SimpleSAML\Utils\Crypto::loadPublicKey($idpmeta, true); break; case '/https.crt': + /** @var array $certInfo */ $certInfo = SimpleSAML\Utils\Crypto::loadPublicKey($idpmeta, true, 'https.'); break; default: throw new \SimpleSAML\Error\NotFound('Unknown certificate.'); } - header('Content-Disposition: attachment; filename='.substr($_SERVER['PATH_INFO'], 1)); header('Content-Type: application/x-x509-ca-cert'); + echo $certInfo['PEM']; exit(0); diff --git a/modules/saml/www/proxy/invalid_session.php b/modules/saml/www/proxy/invalid_session.php index 1afc151ae..3b9c09610 100644 --- a/modules/saml/www/proxy/invalid_session.php +++ b/modules/saml/www/proxy/invalid_session.php @@ -16,9 +16,11 @@ if (!array_key_exists('AuthState', $_REQUEST)) { try { // try to get the state + /** @var array $state State can never be null without a third argument */ $state = \SimpleSAML\Auth\State::loadState($_REQUEST['AuthState'], 'saml:proxy:invalid_idp'); } catch (\Exception $e) { // the user probably hit the back button after starting the logout, try to recover the state with another stage + /** @var array $state State can never be null without a third argument */ $state = \SimpleSAML\Auth\State::loadState($_REQUEST['AuthState'], 'core:Logout:afterbridge'); // success! Try to continue with reauthentication, since we no longer have a valid session here @@ -38,14 +40,15 @@ if (isset($_POST['cancel'])) { } if (isset($_POST['continue'])) { - // log the user out before being able to login again - $as = \SimpleSAML\Auth\Source::getById($state['saml:sp:AuthId'], '\SimpleSAML\Module\saml\Auth\Source\SP'); /** @var \SimpleSAML\Module\saml\Auth\Source\SP $as */ + $as = \SimpleSAML\Auth\Source::getById($state['saml:sp:AuthId'], '\SimpleSAML\Module\saml\Auth\Source\SP'); + + // log the user out before being able to login again $as->reauthLogout($state); } $cfg = \SimpleSAML\Configuration::getInstance(); -$template = new \SimpleSAML\XHTML\Template($cfg, 'saml:proxy/invalid_session.php'); +$template = new \SimpleSAML\XHTML\Template($cfg, 'saml:proxy/invalid_session.tpl.php'); $translator = $template->getTranslator(); $template->data['AuthState'] = (string) $_REQUEST['AuthState']; diff --git a/modules/saml/www/sp/discoresp.php b/modules/saml/www/sp/discoresp.php index 94ac7c1d7..1b53506e7 100644 --- a/modules/saml/www/sp/discoresp.php +++ b/modules/saml/www/sp/discoresp.php @@ -11,7 +11,12 @@ if (!array_key_exists('AuthID', $_REQUEST)) { if (!array_key_exists('idpentityid', $_REQUEST)) { throw new \SimpleSAML\Error\BadRequest('Missing idpentityid to discovery service response handler'); } + +/** @var array $state */ $state = \SimpleSAML\Auth\State::loadState($_REQUEST['AuthID'], 'saml:sp:sso'); +if ($state === null) { + throw new \SimpleSAML\Error\NoState(); +} // Find authentication source assert(array_key_exists('saml:sp:AuthId', $state)); diff --git a/modules/saml/www/sp/metadata.php b/modules/saml/www/sp/metadata.php index a9557379d..ebf1a9481 100644 --- a/modules/saml/www/sp/metadata.php +++ b/modules/saml/www/sp/metadata.php @@ -208,9 +208,11 @@ if ($spconfig->hasValue('contacts')) { // add technical contact $email = $config->getString('technicalcontact_email', 'na@example.org'); if ($email && $email !== 'na@example.org') { - $techcontact['emailAddress'] = $email; - $techcontact['name'] = $config->getString('technicalcontact_name', null); - $techcontact['contactType'] = 'technical'; + $techcontact = [ + 'emailAddress' => $email, + 'name' => $config->getString('technicalcontact_name', null), + 'contactType' => 'technical' + ]; $metaArray20['contacts'][] = \SimpleSAML\Utils\Config\Metadata::getContact($techcontact); } @@ -268,7 +270,7 @@ if (isset($metaArray20['attributes']) && is_array($metaArray20['attributes'])) { $xml = \SimpleSAML\Metadata\Signer::sign($xml, $spconfig->toArray(), 'SAML 2 SP'); if (array_key_exists('output', $_REQUEST) && $_REQUEST['output'] == 'xhtml') { - $t = new \SimpleSAML\XHTML\Template($config, 'saml:metadata.php', 'admin'); + $t = new \SimpleSAML\XHTML\Template($config, 'saml:metadata.tpl.php', 'admin'); $t->data['clipboard.js'] = true; $t->data['header'] = 'saml20-sp'; // TODO: Replace with headerString in 2.0 diff --git a/modules/saml/www/sp/saml1-acs.php b/modules/saml/www/sp/saml1-acs.php index db18627fd..6b981774c 100644 --- a/modules/saml/www/sp/saml1-acs.php +++ b/modules/saml/www/sp/saml1-acs.php @@ -21,6 +21,7 @@ if ($end === false) { } $sourceId = substr($sourceId, 1, $end - 1); +/** @var \SimpleSAML\Module\saml\Auth\Source\SP $source */ $source = \SimpleSAML\Auth\Source::getById($sourceId, '\SimpleSAML\Module\saml\Auth\Source\SP'); SimpleSAML\Logger::debug('Received SAML1 response'); @@ -35,6 +36,7 @@ if (preg_match('@^https?://@i', $target)) { 'saml:sp:RelayState' => \SimpleSAML\Utils\HTTP::checkURLAllowed($target), ]; } else { + /** @var array $state State can never be null without a third argument */ $state = \SimpleSAML\Auth\State::loadState($_REQUEST['TARGET'], 'saml:sp:sso'); // Check that the authentication source is correct @@ -66,7 +68,7 @@ if (array_key_exists('SAMLart', $_REQUEST)) { $responseXML = base64_decode($responseXML); $isValidated = false; /* Must check signature on response. */ } else { - assert(false); + throw new \SimpleSAML\Error\BadRequest('Missing SAMLResponse or SAMLart parameter.'); } $response = new \SimpleSAML\XML\Shib13\AuthnResponse(); diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php index 34c80bc76..c2ec998ff 100644 --- a/modules/saml/www/sp/saml2-acs.php +++ b/modules/saml/www/sp/saml2-acs.php @@ -9,9 +9,11 @@ if (!array_key_exists('PATH_INFO', $_SERVER)) { } $sourceId = substr($_SERVER['PATH_INFO'], 1); + +/** @var \SimpleSAML\Module\saml\Auth\Source\SP $source */ $source = \SimpleSAML\Auth\Source::getById($sourceId, '\SimpleSAML\Module\saml\Auth\Source\SP'); -$spMetadata = $source->getMetadata(); +$spMetadata = $source->getMetadata(); try { $b = \SAML2\Binding::getCurrentBinding(); } catch (Exception $e) { @@ -51,14 +53,17 @@ if ($issuer === null) { } } -$idp = $issuer; if ($issuer instanceof \SAML2\XML\saml\Issuer) { - $idp = $idp->getValue(); + $issuer = $issuer->getValue(); + if ($issuer === null) { + // no issuer found in the assertions + throw new Exception('Missing <saml:Issuer> in message delivered to AssertionConsumerService.'); + } } $session = \SimpleSAML\Session::getSessionFromRequest(); $prevAuth = $session->getAuthData($sourceId, 'saml:sp:prevAuth'); -if ($prevAuth !== null && $prevAuth['id'] === $response->getId() && $prevAuth['issuer'] === $idp) { +if ($prevAuth !== null && $prevAuth['id'] === $response->getId() && $prevAuth['issuer'] === $issuer) { /* OK, it looks like this message has the same issuer * and ID as the SP session we already have active. We * therefore assume that the user has somehow triggered @@ -103,8 +108,8 @@ if ($state) { // check that the issuer is the one we are expecting assert(array_key_exists('ExpectedIssuer', $state)); - if ($state['ExpectedIssuer'] !== $idp) { - $idpMetadata = $source->getIdPMetadata($idp); + if ($state['ExpectedIssuer'] !== $issuer) { + $idpMetadata = $source->getIdPMetadata($issuer); $idplist = $idpMetadata->getArrayize('IDPList', []); if (!in_array($state['ExpectedIssuer'], $idplist, true)) { SimpleSAML\Logger::warning('The issuer of the response not match to the identity provider we sent the request to.'); @@ -124,10 +129,10 @@ if ($state) { ]; } -SimpleSAML\Logger::debug('Received SAML2 Response from '.var_export($idp, true).'.'); +SimpleSAML\Logger::debug('Received SAML2 Response from '.var_export($issuer, true).'.'); if (empty($idpMetadata)) { - $idpMetadata = $source->getIdPmetadata($idp); + $idpMetadata = $source->getIdPmetadata($issuer); } try { @@ -138,13 +143,13 @@ try { \SimpleSAML\Auth\State::throwException($state, $e); } - $authenticatingAuthority = null; $nameId = null; $sessionIndex = null; $expire = null; $attributes = []; $foundAuthnStatement = false; + foreach ($assertions as $assertion) { // check for duplicate assertion (replay attack) $store = \SimpleSAML\Store::getInstance(); @@ -165,7 +170,6 @@ foreach ($assertions as $assertion) { $store->set('saml.AssertionReceived', $aID, true, $notOnOrAfter); } - if ($authenticatingAuthority === null) { $authenticatingAuthority = $assertion->getAuthenticatingAuthority(); } @@ -186,6 +190,7 @@ foreach ($assertions as $assertion) { $foundAuthnStatement = true; } } +$assertion = end($assertions); if (!$foundAuthnStatement) { $e = new \SimpleSAML\Error\Exception('No AuthnStatement found in assertion(s).'); @@ -206,7 +211,7 @@ if (!empty($nameId)) { // we need to save the NameID and SessionIndex for logout $logoutState = [ 'saml:logout:Type' => 'saml2', - 'saml:logout:IdP' => $idp, + 'saml:logout:IdP' => $issuer, 'saml:logout:NameID' => $nameId, 'saml:logout:SessionIndex' => $sessionIndex, ]; @@ -233,7 +238,7 @@ if (!empty($nameId)) { $state['LogoutState'] = $logoutState; $state['saml:AuthenticatingAuthority'] = $authenticatingAuthority; -$state['saml:AuthenticatingAuthority'][] = $idp; +$state['saml:AuthenticatingAuthority'][] = $issuer; $state['PersistentAuthData'][] = 'saml:AuthenticatingAuthority'; $state['saml:AuthnInstant'] = $assertion->getAuthnInstant(); $state['PersistentAuthData'][] = 'saml:AuthnInstant'; @@ -249,7 +254,7 @@ if ($expire !== null) { // note some information about the authentication, in case we receive the same response again $state['saml:sp:prevAuth'] = [ 'id' => $response->getId(), - 'issuer' => $idp, + 'issuer' => $issuer, ]; if (isset($state['\SimpleSAML\Auth\Source.ReturnURL'])) { $state['saml:sp:prevAuth']['redirect'] = $state['\SimpleSAML\Auth\Source.ReturnURL']; @@ -258,5 +263,5 @@ if (isset($state['\SimpleSAML\Auth\Source.ReturnURL'])) { } $state['PersistentAuthData'][] = 'saml:sp:prevAuth'; -$source->handleResponse($state, $idp, $attributes); +$source->handleResponse($state, $issuer, $attributes); assert(false); diff --git a/modules/saml/www/sp/saml2-logout.php b/modules/saml/www/sp/saml2-logout.php index e18148412..9f835d370 100644 --- a/modules/saml/www/sp/saml2-logout.php +++ b/modules/saml/www/sp/saml2-logout.php @@ -12,11 +12,11 @@ if (!array_key_exists('PATH_INFO', $_SERVER)) { $sourceId = substr($_SERVER['PATH_INFO'], 1); +/** @var \SimpleSAML\Module\saml\Auth\Source\SP $source */ $source = \SimpleSAML\Auth\Source::getById($sourceId); if ($source === null) { throw new \Exception('Could not find authentication source with id '.$sourceId); -} -if (!($source instanceof \SimpleSAML\Module\saml\Auth\Source\SP)) { +} elseif (!($source instanceof \SimpleSAML\Module\saml\Auth\Source\SP)) { throw new \SimpleSAML\Error\Exception('Source type changed?'); } @@ -35,15 +35,18 @@ try { $message = $binding->receive(); $issuer = $message->getIssuer(); -if ($issuer === null) { - // Without an issuer we have no way to respond to the message. - throw new \SimpleSAML\Error\BadRequest('Received message on logout endpoint without issuer.'); -} elseif ($issuer instanceof \SAML2\XML\saml\Issuer) { +if ($issuer instanceof \SAML2\XML\saml\Issuer) { $idpEntityId = $issuer->getValue(); } else { $idpEntityId = $issuer; } +if ($idpEntityId === null) { + // Without an issuer we have no way to respond to the message. + throw new \SimpleSAML\Error\BadRequest('Received message on logout endpoint without issuer.'); +} + +/** @var \SimpleSAML\Module\saml\Auth\Source\SP $source */ $spEntityId = $source->getEntityId(); $metadata = \SimpleSAML\Metadata\MetaDataStorageHandler::getMetadataHandler(); @@ -122,6 +125,7 @@ if ($message instanceof \SAML2\LogoutResponse) { \SimpleSAML\Logger::warning('Logged out of '.$numLoggedOut.' of '.count($sessionIndexes).' sessions.'); } + /** @var array $dst */ $dst = $idpMetadata->getEndpointPrioritizedByBinding( 'SingleLogoutService', [ diff --git a/psalm.xml b/psalm.xml index c80d81a44..9d3da2f5c 100644 --- a/psalm.xml +++ b/psalm.xml @@ -20,11 +20,23 @@ <directory name="lib/SimpleSAML/Utils" /> <directory name="lib/SimpleSAML/XHTML" /> <directory name="lib/SimpleSAML/XML" /> + <directory name="modules/admin" /> + <directory name="modules/core" /> + <directory name="modules/cron" /> + <directory name="modules/exampleauth" /> + <directory name="modules/multiauth" /> + <directory name="modules/portal" /> + <directory name="modules/saml" /> <!-- Ignore deprecated classes --> <ignoreFiles> <file name="lib/SimpleSAML/Auth/DefaultAuth.php" /> <file name="lib/SimpleSAML/Utilities.php" /> + + <!-- Ignore deprecated PHP-templates - Remove for 2.0 --> + <file name="modules/**/templates/*.tpl.php" /> + <file name="modules/saml/templates/proxy/*.tpl.php" /> + <file name="modules/saml/templates/sp/*.tpl.php" /> </ignoreFiles> </projectFiles> @@ -52,14 +64,29 @@ <DocblockTypeContradiction errorLevel="suppress" /> <RedundantConditionGivenDocblockType errorLevel="suppress" /> - <!-- Suppress false-positive --> - <UnresolvableInclude errorLevel="suppress" /> <!-- See #1141 --> <MissingDependency> <errorLevel type="suppress"> <file name="lib/SimpleSAML/HTTP/Router.php" /> </errorLevel> </MissingDependency> + + <!-- Ignore UnresolvableInclude on CLI-scripts --> + <UnresolvableInclude> + <errorLevel type="suppress"> + <file name="bin/*.php" /> + <file name="lib/SimpleSAML/XHTML/Template.php" /> + <file name="modules/*/bin/*.php" /> + </errorLevel> + </UnresolvableInclude> + + <!-- Ignore MissingFile on www-scripts - Remove for 2.0 --> + <MissingFile> + <errorLevel type="suppress"> + <file name="www/*.php" /> + <file name="modules/*/www/*.php" /> + </errorLevel> + </MissingFile> </issueHandlers> <stubs> -- GitLab