clear some of the lint warnings identified by PhpStorm

silinternational · Oct 9, 2024 · 59594d6 · 59594d6
1 parent 6664fdc
commit 59594d6
Show file tree

Hide file tree

Showing 30 changed files with 192 additions and 94 deletions.
diff --git a/features/fakes/FakeIdBrokerClient.php b/features/fakes/FakeIdBrokerClient.php
@@ -2,6 +2,7 @@
 
 namespace Sil\SspBase\Features\fakes;
 
+use Exception;
 use InvalidArgumentException;
 use Sil\Idp\IdBroker\Client\ServiceException;
 
@@ -84,7 +85,7 @@ public function updateUserLastLogin(string $employeeId): array
      * @return array|null
      * @throws Exception
      */
-    public function mfaCreate($employee_id, $type, $label = null)
+    public function mfaCreate($employee_id, $type, $label = null): ?array
     {
         if (empty($employee_id)) {
             throw new InvalidArgumentException('employee_id is required');
@@ -125,9 +126,8 @@ public function mfaCreate($employee_id, $type, $label = null)
      * Get a list of MFA configurations for given user
      * @param string $employee_id
      * @return array
-     * @throws ServiceException
      */
-    public function mfaList($employee_id)
+    public function mfaList(string $employee_id)
     {
         return [
             [

diff --git a/modules/expirychecker/src/Auth/Process/ExpiryDate.php b/modules/expirychecker/src/Auth/Process/ExpiryDate.php
@@ -37,6 +37,7 @@ class ExpiryDate extends ProcessingFilter
      *
      * @param array $config Configuration information about this filter.
      * @param mixed $reserved For future use.
+     * @throws Exception
      */
     public function __construct(array $config, mixed $reserved)
     {
@@ -65,6 +66,9 @@ public function __construct(array $config, mixed $reserved)
         ]);
     }
 
+    /**
+     * @throws Exception
+     */
     protected function loadValuesFromConfig(array $config, array $attributeRules): void
     {
         foreach ($attributeRules as $attribute => $rules) {
@@ -141,6 +145,9 @@ protected function getExpiryTimestamp(string $expiryDateAttr, array $state): int
         return $expiryTimestamp;
     }
 
+    /**
+     * @throws Exception
+     */
     public static function hasSeenSplashPageRecently(): bool
     {
         $session = Session::getSessionFromRequest();
@@ -150,6 +157,9 @@ public static function hasSeenSplashPageRecently(): bool
         );
     }
 
+    /**
+     * @throws Exception
+     */
     public static function skipSplashPagesFor(int $seconds): void
     {
         $session = Session::getSessionFromRequest();
@@ -162,6 +172,9 @@ public static function skipSplashPagesFor(int $seconds): void
         $session->save();
     }
 
+    /**
+     * @throws Exception
+     */
     protected function initLogger(array $config): void
     {
         $loggerClass = $config['loggerClass'] ?? Psr3SamlLogger::class;
@@ -215,6 +228,7 @@ public function isTimeToWarn(int $expiryTimestamp, int $warnDaysBefore): bool
 
     /**
      * @inheritDoc
+     * @throws Exception
      */
     public function process(array &$state): void
     {
@@ -244,7 +258,7 @@ public function process(array &$state): void
         ]));
 
         if ($this->isExpired($expiryTimestamp)) {
-            $this->redirectToExpiredPage($state, $accountName, $expiryTimestamp);
+            $this->redirectToExpiredPage($state, $accountName);
         }
 
         // Display a password expiration warning page if it's time to do so.
@@ -263,9 +277,8 @@ public function process(array &$state): void
      *
      * @param array $state The state data.
      * @param string $accountName The name of the user account.
-     * @param int $expiryTimestamp When the password expired.
      */
-    public function redirectToExpiredPage(array &$state, string $accountName, int $expiryTimestamp): void
+    public function redirectToExpiredPage(array &$state, string $accountName): void
     {
         assert('is_array($state)');
 

diff --git a/modules/material/src/MaterialController.php b/modules/material/src/MaterialController.php
@@ -2,6 +2,7 @@
 
 namespace SimpleSAML\Module\material;
 
+use Exception;
 use SimpleSAML\Configuration;
 use SimpleSAML\XHTML\TemplateControllerInterface;
 use Twig\Environment;
@@ -11,7 +12,7 @@ class MaterialController implements TemplateControllerInterface
     /**
      * Modify the twig environment after its initialization (e.g. add filters or extensions).
      *
-     * @param \Twig\Environment $twig The current twig environment.
+     * @param Environment $twig The current twig environment.
      * @return void
      */
     public function setUpTwig(Environment &$twig): void
@@ -24,6 +25,7 @@ public function setUpTwig(Environment &$twig): void
      *
      * @param array $data The current data used by the template.
      * @return void
+     * @throws Exception
      */
     public function display(array &$data): void
     {

diff --git a/modules/mfa/public/prompt-for-mfa.php b/modules/mfa/public/prompt-for-mfa.php
@@ -68,7 +68,7 @@
 if (filter_has_var(INPUT_POST, 'submitMfa')) {
     /* @var string|array $mfaSubmission */
     $mfaSubmission = filter_input(INPUT_POST, 'mfaSubmission');
-    if (substr($mfaSubmission, 0, 1) == '{') {
+    if (str_starts_with($mfaSubmission, '{')) {
         $mfaSubmission = json_decode($mfaSubmission, true);
     }
 

diff --git a/modules/mfa/public/send-manager-mfa.php b/modules/mfa/public/send-manager-mfa.php
@@ -24,7 +24,11 @@
 $logger = LoggerFactory::getAccordingToState($state);
 
 if (filter_has_var(INPUT_POST, 'send')) {
-    $errorMessage = Mfa::sendManagerCode($state, $logger);
+    try {
+        Mfa::sendManagerCode($state, $logger);
+    } catch (Exception $e) {
+        $errorMessage = $e->getMessage();
+    }
 } elseif (filter_has_var(INPUT_POST, 'cancel')) {
     $moduleUrl = SimpleSAML\Module::getModuleURL('mfa/prompt-for-mfa.php', [
         'StateId' => $stateId,

diff --git a/modules/mfa/src/Auth/Process/Mfa.php b/modules/mfa/src/Auth/Process/Mfa.php
@@ -17,6 +17,7 @@
 use SimpleSAML\Module;
 use SimpleSAML\Module\mfa\LoggerFactory;
 use SimpleSAML\Utils\HTTP;
+use Throwable;
 
 /**
  * Filter which prompts the user for MFA credentials.
@@ -25,9 +26,7 @@
  */
 class Mfa extends ProcessingFilter
 {
-    const SESSION_TYPE = 'mfa';
     const STAGE_SENT_TO_LOW_ON_BACKUP_CODES_NAG = 'mfa:sent_to_low_on_backup_codes_nag';
-    const STAGE_SENT_TO_MFA_CHANGE_URL = 'mfa:sent_to_mfa_change_url';
     const STAGE_SENT_TO_MFA_NEEDED_MESSAGE = 'mfa:sent_to_mfa_needed_message';
     const STAGE_SENT_TO_MFA_PROMPT = 'mfa:sent_to_mfa_prompt';
     const STAGE_SENT_TO_NEW_BACKUP_CODES_PAGE = 'mfa:sent_to_new_backup_codes_page';
@@ -79,6 +78,9 @@ public function __construct(array $config, mixed $reserved)
         $this->idBrokerClientClass = $config['idBrokerClientClass'] ?? IdBrokerClient::class;
     }
 
+    /**
+     * @throws Exception
+     */
     protected function loadValuesFromConfig(array $config, array $attributes): void
     {
         foreach ($attributes as $attribute) {
@@ -159,7 +161,7 @@ protected function getAttributeAllValues(string $attributeName, array $state): ?
      * Get an ID Broker client.
      *
      * @param array $idBrokerConfig
-     * @return IdBrokerClient
+     * @return IdBrokerClient|FakeIdBrokerClient
      */
     private static function getIdBrokerClient(array $idBrokerConfig): IdBrokerClient|FakeIdBrokerClient
     {
@@ -194,7 +196,7 @@ public static function getMfaOptionById(array $mfaOptions, int $mfaId): array
         }
 
         foreach ($mfaOptions as $mfaOption) {
-            if ((int)$mfaOption['id'] === (int)$mfaId) {
+            if ((int)$mfaOption['id'] === $mfaId) {
                 return $mfaOption;
             }
         }
@@ -310,16 +312,16 @@ public static function getTemplateFor(string $mfaType): string
      * @param array $state
      * @return string
      */
-    protected static function getRelayStateUrl($state)
+    protected static function getRelayStateUrl(array $state): string
     {
         if (array_key_exists('saml:RelayState', $state)) {
             $samlRelayState = $state['saml:RelayState'];
 
-            if (strpos($samlRelayState, "http://") === 0) {
+            if (str_starts_with($samlRelayState, "http://")) {
                 return $samlRelayState;
             }
 
-            if (strpos($samlRelayState, "https://") === 0) {
+            if (str_starts_with($samlRelayState, "https://")) {
                 return $samlRelayState;
             }
         }
@@ -349,7 +351,7 @@ public static function giveUserNewBackupCodes(array &$state, LoggerInterface $lo
                 'event' => 'New backup codes result: succeeded',
                 'employeeId' => $state['employeeId'],
             ]));
-        } catch (\Throwable $t) {
+        } catch (Throwable $t) {
             $logger->error(json_encode([
                 'event' => 'New backup codes result: failed',
                 'employeeId' => $state['employeeId'],
@@ -383,7 +385,7 @@ public static function hasMfaOptionsOtherThan(string $excludedMfaType, array $st
     {
         $mfaOptions = $state['mfaOptions'] ?? [];
         foreach ($mfaOptions as $mfaOption) {
-            if (strval($mfaOption['type']) !== strval($excludedMfaType)) {
+            if (strval($mfaOption['type']) !== $excludedMfaType) {
                 return true;
             }
         }
@@ -398,12 +400,12 @@ protected function initComposerAutoloader(): void
         }
     }
 
-    protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl)
+    protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl): bool
     {
         if (array_key_exists('saml:RelayState', $state)) {
             $currentDestination = self::getRelayStateUrl($state);
             if (!empty($currentDestination)) {
-                return (strpos($currentDestination, $mfaSetupUrl) === 0);
+                return (str_starts_with($currentDestination, $mfaSetupUrl));
             }
         }
         return false;
@@ -416,15 +418,16 @@ protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl)
      *
      * @param int $mfaId The ID of the MFA option used.
      * @param string $employeeId The Employee ID that this MFA option belongs to.
-     * @param string $mfaSubmission The value of the MFA submission.
+     * @param string|array $mfaSubmission The value of the MFA submission.
      * @param array $state The array of state information.
      * @param bool $rememberMe Whether or not to set remember me cookies
      * @param LoggerInterface $logger A PSR-3 compatible logger.
      * @param string $mfaType The type of the MFA ('webauthn', 'totp', 'backupcode').
      * @param string $rpOrigin The Relying Party Origin (for WebAuthn)
      * @return string If validation fails, an error message to show to the
      *     end user will be returned.
-     * @throws \Sil\PhpEnv\EnvVarNotFoundException
+     * @throws EnvVarNotFoundException
+     * @throws Exception
      */
     public static function validateMfaSubmission(
         int             $mfaId,
@@ -458,7 +461,7 @@ public static function validateMfaSubmission(
             if ($mfaDataFromBroker === true || count($mfaDataFromBroker) > 0) {
                 $idBrokerClient->updateUserLastLogin($employeeId);
             }
-        } catch (\Throwable $t) {
+        } catch (Throwable $t) {
             $message = 'Something went wrong while we were trying to do the '
                 . '2-step verification.';
             if ($t instanceof ServiceException) {
@@ -539,7 +542,7 @@ public static function validateMfaSubmission(
      *
      * @param array $state
      */
-    public static function redirectToMfaSetup(array &$state): void
+    public static function redirectToMfaSetup(array $state): void
     {
         $mfaSetupUrl = $state['mfaSetupUrl'];
 
@@ -566,6 +569,7 @@ public static function redirectToMfaSetup(array &$state): void
 
     /**
      * @inheritDoc
+     * @throws Exception
      */
     public function process(array &$state): void
     {
@@ -686,7 +690,7 @@ protected function redirectToMfaPrompt(array &$state, string $employeeId, array
      * @param array $mfaOptions
      * @param array $state
      * @return bool
-     * @throws EnvVarNotFoundException
+     * @throws EnvVarNotFoundException|ServiceException
      */
     public static function isRememberMeCookieValid(
         string $cookieHash,
@@ -700,7 +704,7 @@ public static function isRememberMeCookieValid(
             if ((int)$expireDate > time()) {
                 $expectedString = self::generateRememberMeCookieString($rememberSecret, $state['employeeId'], $expireDate, $mfaOptions);
                 $isValid = password_verify($expectedString, $cookieHash);
-                
+
                 if ($isValid) {
                     $idBrokerClient = self::getIdBrokerClient($state['idBrokerConfig']);
                     $idBrokerClient->updateUserLastLogin($state['employeeId']);
@@ -734,8 +738,7 @@ public static function generateRememberMeCookieString(
             }
         }
 
-        $string = $rememberSecret . $employeeId . $expireDate . $allMfaIds;
-        return $string;
+        return $rememberSecret . $employeeId . $expireDate . $allMfaIds;
     }
 
     /**
@@ -789,7 +792,7 @@ protected static function redirectToOutOfBackupCodesMessage(array &$state, strin
      * @param string $employeeId
      * @param array $mfaOptions
      * @param string $rememberDuration
-     * @throws \Sil\PhpEnv\EnvVarNotFoundException
+     * @throws EnvVarNotFoundException
      */
     public static function setRememberMeCookies(
         string $employeeId,
@@ -818,8 +821,9 @@ protected static function shouldPromptForMfa(array $mfa): bool
      *
      * @param array $state The state data.
      * @param LoggerInterface $logger A PSR-3 compatible logger.
+     * @throws Exception
      */
-    public static function sendManagerCode(array &$state, LoggerInterface $logger): string
+    public static function sendManagerCode(array &$state, LoggerInterface $logger): void
     {
         try {
             $idBrokerClient = self::getIdBrokerClient($state['idBrokerConfig']);
@@ -830,13 +834,14 @@ public static function sendManagerCode(array &$state, LoggerInterface $logger):
                 'event' => 'Manager rescue code sent',
                 'employeeId' => $state['employeeId'],
             ]));
-        } catch (\Throwable $t) {
+        } catch (Throwable $t) {
             $logger->error(json_encode([
                 'event' => 'Manager rescue code: failed',
                 'employeeId' => $state['employeeId'],
                 'error' => $t->getCode() . ': ' . $t->getMessage(),
             ]));
-            return 'There was an unexpected error. Please try again or contact tech support for assistance';
+            throw new Exception('There was an unexpected error. Please try again ' .
+                'or contact tech support for assistance');
         }
 
         $mfaOptions = $state['mfaOptions'];
@@ -932,7 +937,7 @@ protected static function updateStateWithNewMfaData(array &$state, LoggerInterfa
 
         try {
             $newMfaOptions = $idBrokerClient->mfaList($state['employeeId']);
-        } catch (Exception $e) {
+        } catch (Exception) {
             $log['status'] = 'failed: id-broker exception';
             $logger->error(json_encode($log));
             return;

diff --git a/modules/mfa/src/LoggerFactory.php b/modules/mfa/src/LoggerFactory.php
@@ -4,8 +4,7 @@
 
 use InvalidArgumentException;
 use Psr\Log\LoggerInterface;
-use Psr\Log\Psr3SamlLogger;
-use SimpleSAML\Module\mfa\Assert;
+use Sil\Psr3Adapters\Psr3SamlLogger;
 
 class LoggerFactory
 {

diff --git a/modules/profilereview/src/Assert.php b/modules/profilereview/src/Assert.php
@@ -2,6 +2,8 @@
 
 namespace SimpleSAML\Module\profilereview;
 
+use InvalidArgumentException;
+
 /**
  * Simple assertion class intended for use in production code, not merely tests.
  * Inspired in part by PHPUnit's assertions and Webmozart\Assert.