diff --git a/features/fakes/FakeIdBrokerClient.php b/features/fakes/FakeIdBrokerClient.php index 8e825b9..5245e2f 100644 --- 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; @@ -80,11 +81,11 @@ public function updateUserLastLogin(string $employeeId): array * Create a new MFA configuration * @param string $employee_id * @param string $type - * @param string $label + * @param string|null $label * @return array|null * @throws Exception */ - public function mfaCreate($employee_id, $type, $label = null) + public function mfaCreate(string $employee_id, string $type, string $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): array { return [ [ diff --git a/modules/expirychecker/src/Auth/Process/ExpiryDate.php b/modules/expirychecker/src/Auth/Process/ExpiryDate.php index 4aa6d11..d4cae92 100644 --- 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 index e3c3a5f..5b9471f 100644 --- 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 index f3b05bf..21ee838 100644 --- 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 index a777cf7..ed90a68 100644 --- a/modules/mfa/public/send-manager-mfa.php +++ b/modules/mfa/public/send-manager-mfa.php @@ -23,8 +23,13 @@ $logger = LoggerFactory::getAccordingToState($state); +$errorMessage = null; 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, @@ -37,7 +42,7 @@ $t = new Template($globalConfig, 'mfa:send-manager-mfa'); $t->data['manager_email'] = $state['managerEmail']; -$t->data['error_message'] = $errorMessage ?? null; +$t->data['error_message'] = $errorMessage; $t->send(); $logger->info(json_encode([ diff --git a/modules/mfa/src/Auth/Process/Mfa.php b/modules/mfa/src/Auth/Process/Mfa.php index 2c73905..a0a71a1 100644 --- 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,7 +418,7 @@ 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. @@ -424,7 +426,8 @@ protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl) * @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 index 35d8cd3..36186a0 100644 --- 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 index b99276b..1b56eef 100644 --- 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. diff --git a/modules/profilereview/src/Auth/Process/ProfileReview.php b/modules/profilereview/src/Auth/Process/ProfileReview.php index 8aad1d9..78635c0 100644 --- a/modules/profilereview/src/Auth/Process/ProfileReview.php +++ b/modules/profilereview/src/Auth/Process/ProfileReview.php @@ -60,8 +60,8 @@ public function __construct(array $config, mixed $reserved) } /** - * @param $config - * @param $attributes + * @param array $config + * @param array $attributes * @throws Exception */ protected function loadValuesFromConfig(array $config, array $attributes): void @@ -85,7 +85,7 @@ protected function loadValuesFromConfig(array $config, array $attributes): void * @param LoggerInterface $logger The logger. * @throws Exception */ - public static function validateConfigValue($attribute, $value, $logger) + public static function validateConfigValue(string $attribute, mixed $value, LoggerInterface $logger): void { if (empty($value) || !is_string($value)) { $exception = new Exception(sprintf( @@ -110,7 +110,7 @@ public static function validateConfigValue($attribute, $value, $logger) * @param array $state The state data. * @return mixed The attribute value, or null if not found. */ - protected function getAttribute($attributeName, $state) + protected function getAttribute(string $attributeName, array $state): mixed { $attributeData = $state['Attributes'][$attributeName] ?? null; @@ -133,7 +133,7 @@ protected function getAttribute($attributeName, $state) * @return array|null The attribute's value(s), or null if the attribute was * not found. */ - protected function getAttributeAllValues($attributeName, $state) + protected function getAttributeAllValues(string $attributeName, array $state): ?array { $attributeData = $state['Attributes'][$attributeName] ?? null; @@ -148,23 +148,23 @@ protected function getAttributeAllValues($attributeName, $state) * @returns string * @return mixed|string */ - protected static function getRelayStateUrl($state) + protected static function getRelayStateUrl(array $state): mixed { 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; } } return ''; } - protected function initComposerAutoloader() + protected function initComposerAutoloader(): void { $path = __DIR__ . '/../../../vendor/autoload.php'; if (file_exists($path)) { @@ -172,12 +172,12 @@ protected function initComposerAutoloader() } } - protected static function isHeadedToProfileUrl($state, $ProfileUrl) + protected static function isHeadedToProfileUrl($state, $ProfileUrl): bool { if (array_key_exists('saml:RelayState', $state)) { $currentDestination = self::getRelayStateUrl($state); if (!empty($currentDestination)) { - return (strpos($currentDestination, $ProfileUrl) === 0); + return (str_starts_with($currentDestination, $ProfileUrl)); } } return false; @@ -188,7 +188,7 @@ protected static function isHeadedToProfileUrl($state, $ProfileUrl) * * @param array $state */ - public static function redirectToProfile(&$state) + public static function redirectToProfile(array $state): void { $profileUrl = $state['ProfileUrl']; // Tell the profile-setup URL where the user is ultimately trying to go (if known). @@ -214,6 +214,7 @@ public static function redirectToProfile(&$state) /** * @inheritDoc + * @throws Exception */ public function process(array &$state): void { @@ -256,7 +257,6 @@ public function process(array &$state): void unset($state['Attributes']['method']); unset($state['Attributes']['mfa']); - return; } /** @@ -320,7 +320,10 @@ protected function getAllMfaOptionsExceptManager(array $state): array return $mfaOptions; } - public static function hasSeenSplashPageRecently(string $page) + /** + * @throws Exception + */ + public static function hasSeenSplashPageRecently(string $page): bool { $session = Session::getSessionFromRequest(); return (bool)$session->getData( @@ -329,7 +332,10 @@ public static function hasSeenSplashPageRecently(string $page) ); } - public static function skipSplashPagesFor($seconds, string $page) + /** + * @throws Exception + */ + public static function skipSplashPagesFor(int $seconds, string $page): void { $session = Session::getSessionFromRequest(); $session->setData( @@ -341,7 +347,10 @@ public static function skipSplashPagesFor($seconds, string $page) $session->save(); } - public static function needToShow($flag, $page) + /** + * @throws Exception + */ + public static function needToShow(?string $flag, string $page): bool { $oneDay = 24 * 60 * 60; if ($flag === 'yes' && !self::hasSeenSplashPageRecently($page)) { diff --git a/modules/profilereview/src/LoggerFactory.php b/modules/profilereview/src/LoggerFactory.php index 5ab4585..5bb920f 100644 --- a/modules/profilereview/src/LoggerFactory.php +++ b/modules/profilereview/src/LoggerFactory.php @@ -2,6 +2,7 @@ namespace SimpleSAML\Module\profilereview; +use InvalidArgumentException; use Psr\Log\LoggerInterface; use Sil\Psr3Adapters\Psr3SamlLogger; diff --git a/modules/silauth/src/Auth/Source/SilAuth.php b/modules/silauth/src/Auth/Source/SilAuth.php index 00b3714..cf11869 100644 --- a/modules/silauth/src/Auth/Source/SilAuth.php +++ b/modules/silauth/src/Auth/Source/SilAuth.php @@ -2,17 +2,19 @@ namespace SimpleSAML\Module\silauth\Auth\Source; +use Exception; use Sil\Psr3Adapters\Psr3StdOutLogger; +use SimpleSAML\Auth\State; +use SimpleSAML\Error\Error; +use SimpleSAML\Module; +use SimpleSAML\Module\core\Auth\UserPassBase; use SimpleSAML\Module\silauth\Auth\Source\auth\Authenticator; use SimpleSAML\Module\silauth\Auth\Source\auth\IdBroker; use SimpleSAML\Module\silauth\Auth\Source\captcha\Captcha; use SimpleSAML\Module\silauth\Auth\Source\config\ConfigManager; use SimpleSAML\Module\silauth\Auth\Source\http\Request; -use SimpleSAML\Auth\State; -use SimpleSAML\Error\Error; -use SimpleSAML\Module; -use SimpleSAML\Module\core\Auth\UserPassBase; use SimpleSAML\Utils\HTTP; +use yii\base\InvalidConfigException; /** * Class SimpleSAML\Module\silauth\Auth\Source\SilAuth. @@ -37,6 +39,7 @@ class SilAuth extends UserPassBase * * @param array $info Information about this authentication source. * @param array $config Configuration for this authentication source. + * @throws InvalidConfigException */ public function __construct(array $info, array $config) { @@ -101,6 +104,10 @@ protected function getTrustedIpAddresses(): array return $trustedIpAddresses; } + /** + * @throws Error + * @throws Exception + */ protected function login(string $username, string $password): array { $logger = new Psr3StdOutLogger(); diff --git a/modules/silauth/src/Auth/Source/auth/IdBroker.php b/modules/silauth/src/Auth/Source/auth/IdBroker.php index 5a6938a..685b61e 100644 --- a/modules/silauth/src/Auth/Source/auth/IdBroker.php +++ b/modules/silauth/src/Auth/Source/auth/IdBroker.php @@ -2,6 +2,7 @@ namespace SimpleSAML\Module\silauth\Auth\Source\auth; +use Exception; use Psr\Log\LoggerInterface; use Sil\Idp\IdBroker\Client\IdBrokerClient; use Sil\SspBase\Features\fakes\FakeIdBrokerClient; @@ -30,6 +31,7 @@ class IdBroker * the ID Broker API. * @param bool $assertValidIp (Optional:) Whether or not to assert that the * IP address for the ID Broker API is trusted. + * @throws Exception */ public function __construct( string $baseUri, @@ -63,7 +65,7 @@ public function __construct( * @param string $username The username. * @param string $password The password. * @return array|null The user's attributes (if successful), otherwise null. - * @throws \Exception + * @throws Exception */ public function getAuthenticatedUser(string $username, string $password): ?array { diff --git a/modules/silauth/src/Auth/Source/config/ConfigManager.php b/modules/silauth/src/Auth/Source/config/ConfigManager.php index 9cb0954..43620d2 100644 --- a/modules/silauth/src/Auth/Source/config/ConfigManager.php +++ b/modules/silauth/src/Auth/Source/config/ConfigManager.php @@ -3,6 +3,7 @@ namespace SimpleSAML\Module\silauth\Auth\Source\config; use SimpleSAML\Module\silauth\Auth\Source\text\Text; +use yii\base\InvalidConfigException; use yii\console\Application as ConsoleApplication; use yii\web\Application as WebApplication; @@ -79,6 +80,9 @@ private static function initializeYiiClass(): void } } + /** + * @throws InvalidConfigException + */ public static function getYii2ConsoleApp(array $customConfig): ConsoleApplication { self::initializeYiiClass(); @@ -86,6 +90,9 @@ public static function getYii2ConsoleApp(array $customConfig): ConsoleApplicatio return new ConsoleApplication($mergedYii2Config); } + /** + * @throws InvalidConfigException + */ public static function initializeYii2WebApp(array $customConfig = []): void { self::initializeYiiClass(); diff --git a/modules/silauth/src/Auth/Source/csrf/CsrfProtector.php b/modules/silauth/src/Auth/Source/csrf/CsrfProtector.php index 653041f..d83bfc1 100644 --- a/modules/silauth/src/Auth/Source/csrf/CsrfProtector.php +++ b/modules/silauth/src/Auth/Source/csrf/CsrfProtector.php @@ -2,6 +2,7 @@ namespace SimpleSAML\Module\silauth\Auth\Source\csrf; +use Exception; use SimpleSAML\Session; /** @@ -24,6 +25,9 @@ public function __construct(Session $session) $this->session = $session; } + /** + * @throws Exception + */ public function changeMasterToken(): void { $newMasterToken = $this->generateToken(); @@ -40,6 +44,7 @@ protected function generateToken(): string * will be generated and stored in the session. * * @return string The master (aka. authoritative) CSRF token. + * @throws Exception */ public function getMasterToken(): string { @@ -65,12 +70,16 @@ protected function getTokenFromSession(): mixed * @param string $submittedToken The CSRF protection token provided by the * HTTP request. * @return bool + * @throws Exception */ public function isTokenCorrect(string $submittedToken): bool { return hash_equals($this->getMasterToken(), $submittedToken); } + /** + * @throws Exception + */ protected function setTokenInSession(string $masterToken): void { $this->session->setData( diff --git a/modules/silauth/src/Auth/Source/models/FailedLoginIpAddress.php b/modules/silauth/src/Auth/Source/models/FailedLoginIpAddress.php index e72fbe9..0334f20 100644 --- a/modules/silauth/src/Auth/Source/models/FailedLoginIpAddress.php +++ b/modules/silauth/src/Auth/Source/models/FailedLoginIpAddress.php @@ -34,7 +34,7 @@ public function behaviors(): array { return [ [ - 'class' => CreatedAtUtcBehavior::className(), + 'class' => CreatedAtUtcBehavior::class, 'attributes' => [ Model::EVENT_BEFORE_VALIDATE => 'occurred_at_utc', ], @@ -42,6 +42,9 @@ public function behaviors(): array ]; } + /** + * @throws Exception + */ public static function countRecentFailedLoginsFor(string $ipAddress): int { $count = self::find()->where([ @@ -90,6 +93,7 @@ public static function getMostRecentFailedLoginFor(string $ipAddress): ?FailedLo * * @param string $ipAddress The IP address in question * @return int The number of seconds + * @throws Exception */ public static function getSecondsUntilUnblocked(string $ipAddress): int { @@ -107,6 +111,9 @@ public function init(): void parent::init(); } + /** + * @throws Exception + */ public static function isCaptchaRequiredFor(string $ipAddress): bool { return Authenticator::isEnoughFailedLoginsToRequireCaptcha( @@ -114,6 +121,9 @@ public static function isCaptchaRequiredFor(string $ipAddress): bool ); } + /** + * @throws Exception + */ public static function isCaptchaRequiredForAnyOfThese(array $ipAddresses): bool { foreach ($ipAddresses as $ipAddress) { @@ -124,12 +134,18 @@ public static function isCaptchaRequiredForAnyOfThese(array $ipAddresses): bool return false; } + /** + * @throws Exception + */ public static function isRateLimitBlocking(string $ipAddress): bool { $secondsUntilUnblocked = self::getSecondsUntilUnblocked($ipAddress); return ($secondsUntilUnblocked > 0); } + /** + * @throws Exception + */ public static function isRateLimitBlockingAnyOfThese(array $ipAddresses): bool { foreach ($ipAddresses as $ipAddress) { @@ -140,6 +156,9 @@ public static function isRateLimitBlockingAnyOfThese(array $ipAddresses): bool return false; } + /** + * @throws \yii\db\Exception + */ public static function recordFailedLoginBy( array $ipAddresses, LoggerInterface $logger diff --git a/modules/silauth/src/Auth/Source/models/FailedLoginUsername.php b/modules/silauth/src/Auth/Source/models/FailedLoginUsername.php index 1391b6a..e02e5c2 100644 --- a/modules/silauth/src/Auth/Source/models/FailedLoginUsername.php +++ b/modules/silauth/src/Auth/Source/models/FailedLoginUsername.php @@ -31,7 +31,7 @@ public function behaviors(): array { return [ [ - 'class' => CreatedAtUtcBehavior::className(), + 'class' => CreatedAtUtcBehavior::class, 'attributes' => [ Model::EVENT_BEFORE_VALIDATE => 'occurred_at_utc', ], @@ -39,6 +39,9 @@ public function behaviors(): array ]; } + /** + * @throws Exception + */ public static function countRecentFailedLoginsFor(string $username): int { $count = self::find()->where([ @@ -86,6 +89,7 @@ public static function getMostRecentFailedLoginFor(string $username): ?FailedLog * * @param string $username The username in question * @return int The number of seconds + * @throws Exception */ public static function getSecondsUntilUnblocked(string $username): int { @@ -108,6 +112,7 @@ public function init(): void * * @param string $username The username * @return bool + * @throws Exception */ public static function isRateLimitBlocking(string $username): bool { @@ -115,6 +120,9 @@ public static function isRateLimitBlocking(string $username): bool return ($secondsUntilUnblocked > 0); } + /** + * @throws Exception + */ public static function isCaptchaRequiredFor(?string $username): bool { if (empty($username)) { @@ -125,6 +133,9 @@ public static function isCaptchaRequiredFor(?string $username): bool ); } + /** + * @throws \yii\db\Exception + */ public static function recordFailedLoginBy( string $username, LoggerInterface $logger diff --git a/modules/silauth/src/Auth/Source/rebuildbasemodels.sh b/modules/silauth/src/Auth/Source/rebuildbasemodels.sh index 50729c9..06837e9 100644 --- a/modules/silauth/src/Auth/Source/rebuildbasemodels.sh +++ b/modules/silauth/src/Auth/Source/rebuildbasemodels.sh @@ -1,14 +1,11 @@ #!/usr/bin/env bash -TABLES=(user previous_password) -SUFFIX="Base" - declare -A models models["failed_login_username"]="FailedLoginUsernameBase" models["failed_login_ip_address"]="FailedLoginIpAddressBase" for i in "${!models[@]}"; do CMD="/data/src/yii gii/model --tableName=$i --modelClass=${models[$i]} --generateRelations=all --enableI18N=1 --overwrite=1 --interactive=0 --ns=SimpleSAML\Module\silauth\Auth\Source\models" - echo $CMD + echo "$CMD" $CMD done diff --git a/modules/silauth/src/Auth/Source/system/System.php b/modules/silauth/src/Auth/Source/system/System.php index d7dd282..061b36d 100644 --- a/modules/silauth/src/Auth/Source/system/System.php +++ b/modules/silauth/src/Auth/Source/system/System.php @@ -34,6 +34,9 @@ protected function isDatabaseOkay(): bool } } + /** + * @throws Exception + */ protected function isRequiredConfigPresent(): bool { $globalConfig = Configuration::getInstance(); diff --git a/modules/silauth/src/Auth/Source/text/Text.php b/modules/silauth/src/Auth/Source/text/Text.php index 8b292d5..44b9f23 100644 --- a/modules/silauth/src/Auth/Source/text/Text.php +++ b/modules/silauth/src/Auth/Source/text/Text.php @@ -14,7 +14,8 @@ class Text public static function sanitizeString(mixed $input): string { $inputAsString = is_string($input) ? $input : ''; - $output = filter_var($inputAsString, FILTER_SANITIZE_STRING, [ + $tagsStripped = strip_tags($inputAsString); + $output = filter_var($tagsStripped, FILTER_DEFAULT, [ 'flags' => FILTER_FLAG_STRIP_LOW | FILTER_FLAG_STRIP_BACKTICK, ]); return trim($output); diff --git a/modules/silauth/src/Auth/Source/time/UtcTime.php b/modules/silauth/src/Auth/Source/time/UtcTime.php index 04c5543..44140cc 100644 --- a/modules/silauth/src/Auth/Source/time/UtcTime.php +++ b/modules/silauth/src/Auth/Source/time/UtcTime.php @@ -4,14 +4,15 @@ use DateTime; use DateTimeZone; +use Exception; use InvalidArgumentException; class UtcTime { const DATE_TIME_FORMAT = 'Y-m-d H:i:s'; - private $utc; - private $dateTime; + private DateTimeZone $utc; + private DateTime $dateTime; /** * Create an object representing a date/time in Coordinated Universal Time @@ -72,7 +73,7 @@ public static function getRemainingSeconds(int $totalSeconds, int $elapsedSecond * time occurred before this UTC time. If they are the same, zero will be * returned. Otherwise, a negative number will be returned. * - * @param \SimpleSAML\Module\silauth\Auth\Source\time\UtcTime $otherUtcTime The other UTC time + * @param UtcTime $otherUtcTime The other UTC time * (presumably in the past, though not necessarily). * @return int The number of seconds */ @@ -117,9 +118,10 @@ public function getTimestamp(): int * Get the current date/time (UTC) as a formatted string * * @return string + * @throws Exception */ public static function now(): string { - return self::format('now'); + return self::format(); } } diff --git a/modules/silauth/src/Auth/Source/traits/LoggerAwareTrait.php b/modules/silauth/src/Auth/Source/traits/LoggerAwareTrait.php index f6b8090..507668c 100644 --- a/modules/silauth/src/Auth/Source/traits/LoggerAwareTrait.php +++ b/modules/silauth/src/Auth/Source/traits/LoggerAwareTrait.php @@ -21,7 +21,7 @@ public function initializeLogger(): void * Set a logger for this class instance to use. * * @param LoggerInterface $logger A PSR-3 compliant logger. - * @return null + * @return void */ public function setLogger(LoggerInterface $logger): void { diff --git a/modules/silauth/src/Auth/Source/yii b/modules/silauth/src/Auth/Source/yii index 736df0b..1bb19d7 100755 --- a/modules/silauth/src/Auth/Source/yii +++ b/modules/silauth/src/Auth/Source/yii @@ -8,8 +8,8 @@ * @license http://www.yiiframework.com/license/ */ -define('YII_DEBUG', false); -define('YII_ENV', 'production'); +const YII_DEBUG = false; +const YII_ENV = 'production'; // fcgi doesn't have STDIN and STDOUT defined by default defined('STDIN') or define('STDIN', fopen('php://stdin', 'r')); diff --git a/modules/sildisco/public/metadata.php b/modules/sildisco/public/metadata.php index 618a815..58f8757 100644 --- a/modules/sildisco/public/metadata.php +++ b/modules/sildisco/public/metadata.php @@ -28,8 +28,7 @@ //} try { - $idpentityid = isset($_GET['idpentityid']) ? - $_GET['idpentityid'] : + $idpentityid = $_GET['idpentityid'] ?? $metadata->getMetaDataCurrentEntityID('saml20-idp-hosted'); $idpmeta = $metadata->getMetaDataConfig($idpentityid, 'saml20-idp-hosted'); @@ -57,7 +56,7 @@ $keys[] = array( 'type' => 'X509Certificate', 'signing' => true, - 'encryption' => ($hasNewCert ? false : true), + 'encryption' => !$hasNewCert, 'X509Certificate' => $certInfo['certData'], ); @@ -144,7 +143,7 @@ if ($idpmeta->hasValue('OrganizationName')) { $metaArray['OrganizationName'] = $idpmeta->getLocalizedString('OrganizationName'); - $metaArray['OrganizationDisplayName'] = $idpmeta->getLocalizedString( + $metaArray['OrganizationDisplayName'] = $idpmeta->getOptionalLocalizedString( 'OrganizationDisplayName', $metaArray['OrganizationName'] ); @@ -220,14 +219,12 @@ header('Content-Type: application/xml'); echo $metaxml; - exit(0); } else { - header('Content-Type: text/html; charset=utf-8'); echo '
' . print_r($metaflat, true) . '
'; - exit(0); } + exit(0); } catch (Exception $exception) { throw new Error\Error('METADATA', $exception); } diff --git a/modules/sildisco/src/Auth/Process/AddIdp2NameId.php b/modules/sildisco/src/Auth/Process/AddIdp2NameId.php index b41f01c..b6e5f4a 100644 --- a/modules/sildisco/src/Auth/Process/AddIdp2NameId.php +++ b/modules/sildisco/src/Auth/Process/AddIdp2NameId.php @@ -5,6 +5,7 @@ use SAML2\XML\saml\NameID; use SimpleSAML\Auth\ProcessingFilter; use SimpleSAML\Error; +use SimpleSAML\Error\Exception; use SimpleSAML\Logger; use SimpleSAML\Metadata\MetaDataStorageHandler; @@ -66,8 +67,6 @@ class AddIdp2NameId extends ProcessingFilter * The format of this NameID. * * This property must be initialized the subclass. - * - * @var string */ protected ?string $format; @@ -113,12 +112,12 @@ public function appendIdp(NameID $nameId, string $IDPNamespace): void $suffix = self::DELIMITER . $IDPNamespace; $value = $nameId->getValue(); $nameId->setValue($value . $suffix); - return; } /** * @inheritDoc + * @throws Exception */ public function process(array &$state): void { diff --git a/modules/sildisco/src/Auth/Process/LogUser.php b/modules/sildisco/src/Auth/Process/LogUser.php index 6d59643..4437948 100644 --- a/modules/sildisco/src/Auth/Process/LogUser.php +++ b/modules/sildisco/src/Auth/Process/LogUser.php @@ -4,7 +4,9 @@ use Aws\DynamoDb\Marshaler; use Aws\Sdk; +use Exception; use SimpleSAML\Auth\ProcessingFilter; +use SimpleSAML\Error\MetadataNotFound; use SimpleSAML\Logger; use SimpleSAML\Metadata\MetaDataStorageHandler; @@ -136,8 +138,8 @@ public function process(array &$state): void ]; try { - $result = $dynamodb->putItem($params); - } catch (\Exception $e) { + $dynamodb->putItem($params); + } catch (Exception $e) { Logger::error("Unable to add item: " . $e->getMessage()); } } @@ -159,7 +161,10 @@ private function configsAreValid(): bool return true; } - private function getIdp(array &$state) + /** + * @throws MetadataNotFound + */ + private function getIdp(array $state) { if (empty($state[self::IDP_KEY])) { return 'No IDP available'; @@ -203,9 +208,7 @@ private function getUserAttributes(array $state): array $userAttrs = $this->addUserAttribute($userAttrs, "CN", $cn); $userAttrs = $this->addUserAttribute($userAttrs, "EduPersonPrincipalName", $eduPersonPrincipalName); - $userAttrs = $this->addUserAttribute($userAttrs, "EmployeeNumber", $employeeNumber); - - return $userAttrs; + return $this->addUserAttribute($userAttrs, "EmployeeNumber", $employeeNumber); } private function getAttributeFrom(array $attributes, string $oidKey, string $friendlyKey): string diff --git a/modules/sildisco/src/Auth/Process/TagGroup.php b/modules/sildisco/src/Auth/Process/TagGroup.php index 791d76a..a4694e8 100644 --- a/modules/sildisco/src/Auth/Process/TagGroup.php +++ b/modules/sildisco/src/Auth/Process/TagGroup.php @@ -3,6 +3,7 @@ namespace SimpleSAML\Module\sildisco\Auth\Process; use SimpleSAML\Auth\ProcessingFilter; +use SimpleSAML\Error\MetadataNotFound; use SimpleSAML\Metadata\MetaDataStorageHandler; /** @@ -34,6 +35,7 @@ public function prependIdp2Groups(array $attributes, string $attributeLabel, str * Apply filter to copy attributes. * * @inheritDoc + * @throws MetadataNotFound */ public function process(array &$state): void { diff --git a/modules/sildisco/src/Auth/Process/TrackIdps.php b/modules/sildisco/src/Auth/Process/TrackIdps.php index e14a415..546c340 100644 --- a/modules/sildisco/src/Auth/Process/TrackIdps.php +++ b/modules/sildisco/src/Auth/Process/TrackIdps.php @@ -2,6 +2,7 @@ namespace SimpleSAML\Module\sildisco\Auth\Process; +use Exception; use SimpleSAML\Auth\ProcessingFilter; use SimpleSAML\Session; @@ -16,6 +17,7 @@ class TrackIdps extends ProcessingFilter * Apply filter to save IDPs to session. * * @inheritDoc + * @throws Exception */ public function process(array &$state): void { diff --git a/modules/sildisco/src/Controller/SilDisco.php b/modules/sildisco/src/Controller/SilDisco.php index ba92fe3..b0b8646 100644 --- a/modules/sildisco/src/Controller/SilDisco.php +++ b/modules/sildisco/src/Controller/SilDisco.php @@ -13,8 +13,9 @@ class SilDisco { /** - * @param \Symfony\Component\HttpFoundation\Request $request The current request. - * @return \SimpleSAML\HTTP\RunnableResponse + * @param Request $request The current request. + * @return RunnableResponse + * @throws Error\Error */ public function main(Request $request): RunnableResponse { diff --git a/modules/sildisco/src/IdPDisco.php b/modules/sildisco/src/IdPDisco.php index 97eccd8..ebb298b 100644 --- a/modules/sildisco/src/IdPDisco.php +++ b/modules/sildisco/src/IdPDisco.php @@ -5,6 +5,7 @@ use Sil\SspUtils\AnnouncementUtils; use Sil\SspUtils\Utils; use SimpleSAML\Auth; +use SimpleSAML\Error\ConfigurationError; use SimpleSAML\Error\MetadataNotFound; use SimpleSAML\Error\NoState; use SimpleSAML\Logger; @@ -38,7 +39,7 @@ protected function log(string $message): void /** * @throws NoState - * @throws Exception + * @throws Exception|MetadataNotFound */ private function getSPEntityIDAndReducedIdpList(): array { @@ -68,6 +69,8 @@ private function getSPEntityIDAndReducedIdpList(): array /** * @inheritDoc + * @throws MetadataNotFound|ConfigurationError + * @throws \Exception */ public function handleRequest(): void { @@ -96,7 +99,7 @@ public function handleRequest(): void $metadata = MetaDataStorageHandler::getMetadataHandler(); $sp = $metadata->getMetaData($spEntityId, 'saml20-sp-remote'); - $t = new Template($this->config, 'selectidp-links', 'disco'); + $t = new Template($this->config, 'selectidp-links'); $t->data['idp_list'] = $idpList; $t->data['return'] = $this->returnURL; @@ -140,7 +143,7 @@ protected function validateIdP(?string $idp): ?string $spEntityIdParam = 'spentityid=' . $spEntityId; - if (strpos($requestReturn, $spEntityIdParam) === false) { + if (!str_contains($requestReturn, $spEntityIdParam)) { $message = 'Invalid SP entity id [' . $spEntityId . ']. ' . 'Could not find in return value. ' . PHP_EOL . $requestReturn; $this->log($message);