From 86d6db36cc2d61bec5ae18641ab7d1d242e487fd Mon Sep 17 00:00:00 2001 From: briskt <3172830+briskt@users.noreply.github.com> Date: Fri, 14 Jun 2024 15:03:56 +0800 Subject: [PATCH 1/4] update process() typehinting in Auth\ProcessingFilter inherited classes --- modules/expirychecker/src/Auth/Process/ExpiryDate.php | 6 ++---- modules/mfa/src/Auth/Process/Mfa.php | 10 +++------- .../profilereview/src/Auth/Process/ProfileReview.php | 8 ++------ modules/sildisco/src/Auth/Process/AddIdp2NameId.php | 6 ++---- modules/sildisco/src/Auth/Process/LogUser.php | 4 ++-- modules/sildisco/src/Auth/Process/TagGroup.php | 5 +++-- modules/sildisco/src/Auth/Process/TrackIdps.php | 5 +++-- 7 files changed, 17 insertions(+), 27 deletions(-) diff --git a/modules/expirychecker/src/Auth/Process/ExpiryDate.php b/modules/expirychecker/src/Auth/Process/ExpiryDate.php index 30322af4..567b31fa 100644 --- a/modules/expirychecker/src/Auth/Process/ExpiryDate.php +++ b/modules/expirychecker/src/Auth/Process/ExpiryDate.php @@ -298,11 +298,9 @@ public function redirect2PasswordChange( } /** - * Apply this AuthProc Filter. - * - * @param array &$state The current state. + * @inheritDoc */ - public function process(&$state): void + public function process(array &$state): void { $employeeId = $this->getAttribute($this->employeeIdAttr, $state); diff --git a/modules/mfa/src/Auth/Process/Mfa.php b/modules/mfa/src/Auth/Process/Mfa.php index f4a23bb5..363df9a5 100644 --- a/modules/mfa/src/Auth/Process/Mfa.php +++ b/modules/mfa/src/Auth/Process/Mfa.php @@ -566,15 +566,11 @@ public static function redirectToMfaSetup(array &$state): void HTTP::redirectTrustedURL($mfaSetupUrl); } - + /** - * Apply this AuthProc Filter. It will either return (indicating that it - * has completed) or it will redirect the user, in which case it will - * later call `SimpleSAML\Auth\ProcessingChain::resumeProcessing($state)`. - * - * @param array &$state The current state. + * @inheritDoc */ - public function process(&$state): void + public function process(array &$state): void { // Get the necessary info from the state data. $employeeId = $this->getAttribute($this->employeeIdAttr, $state); diff --git a/modules/profilereview/src/Auth/Process/ProfileReview.php b/modules/profilereview/src/Auth/Process/ProfileReview.php index 789f6838..56ed70f5 100644 --- a/modules/profilereview/src/Auth/Process/ProfileReview.php +++ b/modules/profilereview/src/Auth/Process/ProfileReview.php @@ -211,13 +211,9 @@ public static function redirectToProfile(&$state) } /** - * Apply this AuthProc Filter. It will either return (indicating that it - * has completed) or it will redirect the user, in which case it will - * later call `SimpleSAML\Auth\ProcessingChain::resumeProcessing($state)`. - * - * @param array &$state The current state. + * @inheritDoc */ - public function process(&$state) + public function process(array &$state): void { // Get the necessary info from the state data. $employeeId = $this->getAttribute($this->employeeIdAttr, $state); diff --git a/modules/sildisco/src/Auth/Process/AddIdp2NameId.php b/modules/sildisco/src/Auth/Process/AddIdp2NameId.php index e3c8408e..3516881d 100644 --- a/modules/sildisco/src/Auth/Process/AddIdp2NameId.php +++ b/modules/sildisco/src/Auth/Process/AddIdp2NameId.php @@ -110,11 +110,9 @@ public function appendIdp(NameID $nameId, string $IDPNamespace): void /** - * Apply filter to copy attributes. - * - * @param array &$state The current state array + * @inheritDoc */ - public function process(&$state): void + public function process(array &$state): void { assert('is_array($state)'); diff --git a/modules/sildisco/src/Auth/Process/LogUser.php b/modules/sildisco/src/Auth/Process/LogUser.php index 835ed287..86ae4171 100644 --- a/modules/sildisco/src/Auth/Process/LogUser.php +++ b/modules/sildisco/src/Auth/Process/LogUser.php @@ -65,9 +65,9 @@ public function __construct(array $config, mixed $reserved) /** * Log info for a user's login to Dynamodb * - * @param array &$state The current state array + * @inheritDoc */ - public function process(&$state): void + public function process(array &$state): void { if (! $this->configsAreValid()) { return; diff --git a/modules/sildisco/src/Auth/Process/TagGroup.php b/modules/sildisco/src/Auth/Process/TagGroup.php index 5d8ccf68..abfbdd12 100644 --- a/modules/sildisco/src/Auth/Process/TagGroup.php +++ b/modules/sildisco/src/Auth/Process/TagGroup.php @@ -31,9 +31,10 @@ public function prependIdp2Groups(array $attributes, string $attributeLabel, str /** * Apply filter to copy attributes. * - * @param array &$state The current request + * @inheritDoc */ - public function process(&$state) { + public function process(array &$state): void + { assert('is_array($request)'); assert('array_key_exists("Attributes", $request)'); diff --git a/modules/sildisco/src/Auth/Process/TrackIdps.php b/modules/sildisco/src/Auth/Process/TrackIdps.php index 4a84d1d9..7d574845 100644 --- a/modules/sildisco/src/Auth/Process/TrackIdps.php +++ b/modules/sildisco/src/Auth/Process/TrackIdps.php @@ -11,9 +11,10 @@ class TrackIdps extends \SimpleSAML\Auth\ProcessingFilter { /** * Apply filter to save IDPs to session. * - * @param array &$request The current request + * @inheritDoc */ - public function process(&$request) { + public function process(array &$state): void + { // get the authenticating Idp and add it to the list of previous ones $session = \SimpleSAML\Session::getSessionFromRequest(); $sessionDataType = "sildisco:authentication"; From f7be2a92be515dbf83e2c42bfd50293b98d28223 Mon Sep 17 00:00:00 2001 From: briskt <3172830+briskt@users.noreply.github.com> Date: Fri, 14 Jun 2024 15:20:32 +0800 Subject: [PATCH 2/4] update login() and authenticated() method types in SilAuth --- modules/silauth/src/Auth/Source/SilAuth.php | 14 +++----------- .../silauth/src/Auth/Source/auth/Authenticator.php | 2 +- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/modules/silauth/src/Auth/Source/SilAuth.php b/modules/silauth/src/Auth/Source/SilAuth.php index f7c0624f..3f8d05ed 100644 --- a/modules/silauth/src/Auth/Source/SilAuth.php +++ b/modules/silauth/src/Auth/Source/SilAuth.php @@ -59,15 +59,7 @@ public function __construct(array $info, array $config) ]]]); } - /** - * Initialize login. - * - * This function saves the information about the login, and redirects to a - * login page. - * - * @param array &$state Information about the current authentication. - */ - public function authenticate(&$state): void + public function authenticate(array &$state): void { assert('is_array($state)'); @@ -107,8 +99,8 @@ protected function getTrustedIpAddresses(): array } return $trustedIpAddresses; } - - protected function login($username, $password): ?array + + protected function login(string $username, string $password): array { $logger = new Psr3StdOutLogger(); $captcha = new Captcha($this->recaptchaConfig['secret'] ?? null); diff --git a/modules/silauth/src/Auth/Source/auth/Authenticator.php b/modules/silauth/src/Auth/Source/auth/Authenticator.php index dc8d10ca..0d917f4a 100644 --- a/modules/silauth/src/Auth/Source/auth/Authenticator.php +++ b/modules/silauth/src/Auth/Source/auth/Authenticator.php @@ -213,7 +213,7 @@ public static function getSecondsUntilUnblocked( * * @throws \Exception */ - public function getUserAttributes(): ?array + public function getUserAttributes(): array { if ($this->userAttributes === null) { throw new \Exception( From 158c2405b820c3a313ea84a2d0c47f55a402c1be Mon Sep 17 00:00:00 2001 From: briskt <3172830+briskt@users.noreply.github.com> Date: Fri, 14 Jun 2024 15:23:06 +0800 Subject: [PATCH 3/4] update log() and validateIdP() method types in IdPDisco --- modules/sildisco/src/IdPDisco.php | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/modules/sildisco/src/IdPDisco.php b/modules/sildisco/src/IdPDisco.php index 4808a8ff..c17cff9a 100644 --- a/modules/sildisco/src/IdPDisco.php +++ b/modules/sildisco/src/IdPDisco.php @@ -38,13 +38,9 @@ class IdPDisco extends \SimpleSAML\XHTML\IdPDisco /** - * Log a message. - * - * This is an helper function for logging messages. It will prefix the messages with our discovery service type. - * - * @param string $message The message which should be logged. + * @inheritDoc */ - protected function log($message): void + protected function log(string $message): void { \SimpleSAML\Logger::info('SildiscoIdPDisco.'.$this->instance.': '.$message); } @@ -72,9 +68,7 @@ private function getSPEntityIDAndReducedIdpList(): array } /** - * Handles a request to this discovery service. - * - * The IdP disco parameters should be set before calling this function. + * @inheritDoc */ public function handleRequest(): void { @@ -160,16 +154,9 @@ public static function enableBetaEnabled(array $idpList, ?bool $isBetaTester=nul } /** - * Validates the given IdP entity id. - * - * Takes a string with the IdP entity id, and returns the entity id if it is valid, or - * null if not. Ensures that the selected IdP is allowed for the current SP - * - * @param string|null $idp The entity id we want to validate. This can be null, in which case we will return null. - * - * @return string|null The entity id if it is valid, null if not. + * @inheritDoc */ - protected function validateIdP($idp): ?string + protected function validateIdP(?string $idp): ?string { if ($idp === null) { return null; From 4e045f0a403757f8223dfd06bd8095e3751ef976 Mon Sep 17 00:00:00 2001 From: briskt <3172830+briskt@users.noreply.github.com> Date: Fri, 14 Jun 2024 15:31:35 +0800 Subject: [PATCH 4/4] add types to FakeSession class inherited from SimpleSAML\Session [skip ci] --- .../Source/tests/unit/csrf/FakeSession.php | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/modules/silauth/src/Auth/Source/tests/unit/csrf/FakeSession.php b/modules/silauth/src/Auth/Source/tests/unit/csrf/FakeSession.php index 41d1fa48..08b329e3 100644 --- a/modules/silauth/src/Auth/Source/tests/unit/csrf/FakeSession.php +++ b/modules/silauth/src/Auth/Source/tests/unit/csrf/FakeSession.php @@ -2,35 +2,32 @@ namespace SimpleSAML\Module\silauth\Auth\Source\tests\unit\csrf; +use SimpleSAML\Session; + /** * Class to mimic the bare basics of the SimpleSAML\Session class in order to * allow good testing of the CsrfProtector class. */ -class FakeSession extends \SimpleSAML\Session +class FakeSession extends Session { - private $inMemoryDataStore; + private array $inMemoryDataStore; - private function __construct($transient = false) + private function __construct(bool $transient = false) { $this->inMemoryDataStore = []; } - /** - * @param string $type - * @param string|null $id - * @return mixed - */ - public function getData($type, $id) + public function getData(string $type, ?string $id): mixed { return $this->inMemoryDataStore[$type][$id] ?? null; } - public static function getSessionFromRequest($sessionId = null) + public static function getSessionFromRequest(): Session { return new self(); } - public function setData($type, $id, $data, $timeout = null) + public function setData(string $type, string $id, mixed $data, int|string|null $timeout = null): void { // Make sure an array exists for that type of data. $this->inMemoryDataStore[$type] = $this->inMemoryDataStore[$type] ?? [];