Skip to content

Commit 54fe050

Browse files
brisktbriskt
authored
Merge pull request #283 from silinternational/clear-minor-warnings
Release 10.1.4 -- resolve lint warnings
2 parents c27ccdf + 246b29f commit 54fe050

29 files changed

+194
-93
lines changed

‎features/fakes/FakeIdBrokerClient.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Sil\SspBase\Features\fakes;
44

5+
use Exception;
56
use InvalidArgumentException;
67
use Sil\Idp\IdBroker\Client\ServiceException;
78

@@ -80,11 +81,11 @@ public function updateUserLastLogin(string $employeeId): array
8081
* Create a new MFA configuration
8182
* @param string $employee_id
8283
* @param string $type
83-
* @param string $label
84+
* @param string|null $label
8485
* @return array|null
8586
* @throws Exception
8687
*/
87-
public function mfaCreate($employee_id, $type, $label = null)
88+
public function mfaCreate(string $employee_id, string $type, string $label = null): ?array
8889
{
8990
if (empty($employee_id)) {
9091
throw new InvalidArgumentException('employee_id is required');
@@ -125,9 +126,8 @@ public function mfaCreate($employee_id, $type, $label = null)
125126
* Get a list of MFA configurations for given user
126127
* @param string $employee_id
127128
* @return array
128-
* @throws ServiceException
129129
*/
130-
public function mfaList($employee_id)
130+
public function mfaList(string $employee_id): array
131131
{
132132
return [
133133
[

‎modules/expirychecker/src/Auth/Process/ExpiryDate.php

+16-3
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class ExpiryDate extends ProcessingFilter
3737
*
3838
* @param array $config Configuration information about this filter.
3939
* @param mixed $reserved For future use.
40+
* @throws Exception
4041
*/
4142
public function __construct(array $config, mixed $reserved)
4243
{
@@ -65,6 +66,9 @@ public function __construct(array $config, mixed $reserved)
6566
]);
6667
}
6768

69+
/**
70+
* @throws Exception
71+
*/
6872
protected function loadValuesFromConfig(array $config, array $attributeRules): void
6973
{
7074
foreach ($attributeRules as $attribute => $rules) {
@@ -141,6 +145,9 @@ protected function getExpiryTimestamp(string $expiryDateAttr, array $state): int
141145
return $expiryTimestamp;
142146
}
143147

148+
/**
149+
* @throws Exception
150+
*/
144151
public static function hasSeenSplashPageRecently(): bool
145152
{
146153
$session = Session::getSessionFromRequest();
@@ -150,6 +157,9 @@ public static function hasSeenSplashPageRecently(): bool
150157
);
151158
}
152159

160+
/**
161+
* @throws Exception
162+
*/
153163
public static function skipSplashPagesFor(int $seconds): void
154164
{
155165
$session = Session::getSessionFromRequest();
@@ -162,6 +172,9 @@ public static function skipSplashPagesFor(int $seconds): void
162172
$session->save();
163173
}
164174

175+
/**
176+
* @throws Exception
177+
*/
165178
protected function initLogger(array $config): void
166179
{
167180
$loggerClass = $config['loggerClass'] ?? Psr3SamlLogger::class;
@@ -215,6 +228,7 @@ public function isTimeToWarn(int $expiryTimestamp, int $warnDaysBefore): bool
215228

216229
/**
217230
* @inheritDoc
231+
* @throws Exception
218232
*/
219233
public function process(array &$state): void
220234
{
@@ -244,7 +258,7 @@ public function process(array &$state): void
244258
]));
245259

246260
if ($this->isExpired($expiryTimestamp)) {
247-
$this->redirectToExpiredPage($state, $accountName, $expiryTimestamp);
261+
$this->redirectToExpiredPage($state, $accountName);
248262
}
249263

250264
// Display a password expiration warning page if it's time to do so.
@@ -263,9 +277,8 @@ public function process(array &$state): void
263277
*
264278
* @param array $state The state data.
265279
* @param string $accountName The name of the user account.
266-
* @param int $expiryTimestamp When the password expired.
267280
*/
268-
public function redirectToExpiredPage(array &$state, string $accountName, int $expiryTimestamp): void
281+
public function redirectToExpiredPage(array &$state, string $accountName): void
269282
{
270283
assert('is_array($state)');
271284

‎modules/material/src/MaterialController.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace SimpleSAML\Module\material;
44

5+
use Exception;
56
use SimpleSAML\Configuration;
67
use SimpleSAML\XHTML\TemplateControllerInterface;
78
use Twig\Environment;
@@ -11,7 +12,7 @@ class MaterialController implements TemplateControllerInterface
1112
/**
1213
* Modify the twig environment after its initialization (e.g. add filters or extensions).
1314
*
14-
* @param \Twig\Environment $twig The current twig environment.
15+
* @param Environment $twig The current twig environment.
1516
* @return void
1617
*/
1718
public function setUpTwig(Environment &$twig): void
@@ -24,6 +25,7 @@ public function setUpTwig(Environment &$twig): void
2425
*
2526
* @param array $data The current data used by the template.
2627
* @return void
28+
* @throws Exception
2729
*/
2830
public function display(array &$data): void
2931
{

‎modules/mfa/public/prompt-for-mfa.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
if (filter_has_var(INPUT_POST, 'submitMfa')) {
6969
/* @var string|array $mfaSubmission */
7070
$mfaSubmission = filter_input(INPUT_POST, 'mfaSubmission');
71-
if (substr($mfaSubmission, 0, 1) == '{') {
71+
if (str_starts_with($mfaSubmission, '{')) {
7272
$mfaSubmission = json_decode($mfaSubmission, true);
7373
}
7474

‎modules/mfa/public/send-manager-mfa.php

+7-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,13 @@
2323

2424
$logger = LoggerFactory::getAccordingToState($state);
2525

26+
$errorMessage = null;
2627
if (filter_has_var(INPUT_POST, 'send')) {
27-
$errorMessage = Mfa::sendManagerCode($state, $logger);
28+
try {
29+
Mfa::sendManagerCode($state, $logger);
30+
} catch (Exception $e) {
31+
$errorMessage = $e->getMessage();
32+
}
2833
} elseif (filter_has_var(INPUT_POST, 'cancel')) {
2934
$moduleUrl = SimpleSAML\Module::getModuleURL('mfa/prompt-for-mfa.php', [
3035
'StateId' => $stateId,
@@ -37,7 +42,7 @@
3742

3843
$t = new Template($globalConfig, 'mfa:send-manager-mfa');
3944
$t->data['manager_email'] = $state['managerEmail'];
40-
$t->data['error_message'] = $errorMessage ?? null;
45+
$t->data['error_message'] = $errorMessage;
4146
$t->send();
4247

4348
$logger->info(json_encode([

‎modules/mfa/src/Auth/Process/Mfa.php

+29-24
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use SimpleSAML\Module;
1818
use SimpleSAML\Module\mfa\LoggerFactory;
1919
use SimpleSAML\Utils\HTTP;
20+
use Throwable;
2021

2122
/**
2223
* Filter which prompts the user for MFA credentials.
@@ -25,9 +26,7 @@
2526
*/
2627
class Mfa extends ProcessingFilter
2728
{
28-
const SESSION_TYPE = 'mfa';
2929
const STAGE_SENT_TO_LOW_ON_BACKUP_CODES_NAG = 'mfa:sent_to_low_on_backup_codes_nag';
30-
const STAGE_SENT_TO_MFA_CHANGE_URL = 'mfa:sent_to_mfa_change_url';
3130
const STAGE_SENT_TO_MFA_NEEDED_MESSAGE = 'mfa:sent_to_mfa_needed_message';
3231
const STAGE_SENT_TO_MFA_PROMPT = 'mfa:sent_to_mfa_prompt';
3332
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)
7978
$this->idBrokerClientClass = $config['idBrokerClientClass'] ?? IdBrokerClient::class;
8079
}
8180

81+
/**
82+
* @throws Exception
83+
*/
8284
protected function loadValuesFromConfig(array $config, array $attributes): void
8385
{
8486
foreach ($attributes as $attribute) {
@@ -159,7 +161,7 @@ protected function getAttributeAllValues(string $attributeName, array $state): ?
159161
* Get an ID Broker client.
160162
*
161163
* @param array $idBrokerConfig
162-
* @return IdBrokerClient
164+
* @return IdBrokerClient|FakeIdBrokerClient
163165
*/
164166
private static function getIdBrokerClient(array $idBrokerConfig): IdBrokerClient|FakeIdBrokerClient
165167
{
@@ -194,7 +196,7 @@ public static function getMfaOptionById(array $mfaOptions, int $mfaId): array
194196
}
195197

196198
foreach ($mfaOptions as $mfaOption) {
197-
if ((int)$mfaOption['id'] === (int)$mfaId) {
199+
if ((int)$mfaOption['id'] === $mfaId) {
198200
return $mfaOption;
199201
}
200202
}
@@ -310,16 +312,16 @@ public static function getTemplateFor(string $mfaType): string
310312
* @param array $state
311313
* @return string
312314
*/
313-
protected static function getRelayStateUrl($state)
315+
protected static function getRelayStateUrl(array $state): string
314316
{
315317
if (array_key_exists('saml:RelayState', $state)) {
316318
$samlRelayState = $state['saml:RelayState'];
317319

318-
if (strpos($samlRelayState, "http://") === 0) {
320+
if (str_starts_with($samlRelayState, "http://")) {
319321
return $samlRelayState;
320322
}
321323

322-
if (strpos($samlRelayState, "https://") === 0) {
324+
if (str_starts_with($samlRelayState, "https://")) {
323325
return $samlRelayState;
324326
}
325327
}
@@ -349,7 +351,7 @@ public static function giveUserNewBackupCodes(array &$state, LoggerInterface $lo
349351
'event' => 'New backup codes result: succeeded',
350352
'employeeId' => $state['employeeId'],
351353
]));
352-
} catch (\Throwable $t) {
354+
} catch (Throwable $t) {
353355
$logger->error(json_encode([
354356
'event' => 'New backup codes result: failed',
355357
'employeeId' => $state['employeeId'],
@@ -383,7 +385,7 @@ public static function hasMfaOptionsOtherThan(string $excludedMfaType, array $st
383385
{
384386
$mfaOptions = $state['mfaOptions'] ?? [];
385387
foreach ($mfaOptions as $mfaOption) {
386-
if (strval($mfaOption['type']) !== strval($excludedMfaType)) {
388+
if (strval($mfaOption['type']) !== $excludedMfaType) {
387389
return true;
388390
}
389391
}
@@ -398,12 +400,12 @@ protected function initComposerAutoloader(): void
398400
}
399401
}
400402

401-
protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl)
403+
protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl): bool
402404
{
403405
if (array_key_exists('saml:RelayState', $state)) {
404406
$currentDestination = self::getRelayStateUrl($state);
405407
if (!empty($currentDestination)) {
406-
return (strpos($currentDestination, $mfaSetupUrl) === 0);
408+
return (str_starts_with($currentDestination, $mfaSetupUrl));
407409
}
408410
}
409411
return false;
@@ -416,15 +418,16 @@ protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl)
416418
*
417419
* @param int $mfaId The ID of the MFA option used.
418420
* @param string $employeeId The Employee ID that this MFA option belongs to.
419-
* @param string $mfaSubmission The value of the MFA submission.
421+
* @param string|array $mfaSubmission The value of the MFA submission.
420422
* @param array $state The array of state information.
421423
* @param bool $rememberMe Whether or not to set remember me cookies
422424
* @param LoggerInterface $logger A PSR-3 compatible logger.
423425
* @param string $mfaType The type of the MFA ('webauthn', 'totp', 'backupcode').
424426
* @param string $rpOrigin The Relying Party Origin (for WebAuthn)
425427
* @return string If validation fails, an error message to show to the
426428
* end user will be returned.
427-
* @throws \Sil\PhpEnv\EnvVarNotFoundException
429+
* @throws EnvVarNotFoundException
430+
* @throws Exception
428431
*/
429432
public static function validateMfaSubmission(
430433
int $mfaId,
@@ -458,7 +461,7 @@ public static function validateMfaSubmission(
458461
if ($mfaDataFromBroker === true || count($mfaDataFromBroker) > 0) {
459462
$idBrokerClient->updateUserLastLogin($employeeId);
460463
}
461-
} catch (\Throwable $t) {
464+
} catch (Throwable $t) {
462465
$message = 'Something went wrong while we were trying to do the '
463466
. '2-step verification.';
464467
if ($t instanceof ServiceException) {
@@ -539,7 +542,7 @@ public static function validateMfaSubmission(
539542
*
540543
* @param array $state
541544
*/
542-
public static function redirectToMfaSetup(array &$state): void
545+
public static function redirectToMfaSetup(array $state): void
543546
{
544547
$mfaSetupUrl = $state['mfaSetupUrl'];
545548

@@ -566,6 +569,7 @@ public static function redirectToMfaSetup(array &$state): void
566569

567570
/**
568571
* @inheritDoc
572+
* @throws Exception
569573
*/
570574
public function process(array &$state): void
571575
{
@@ -686,7 +690,7 @@ protected function redirectToMfaPrompt(array &$state, string $employeeId, array
686690
* @param array $mfaOptions
687691
* @param array $state
688692
* @return bool
689-
* @throws EnvVarNotFoundException
693+
* @throws EnvVarNotFoundException|ServiceException
690694
*/
691695
public static function isRememberMeCookieValid(
692696
string $cookieHash,
@@ -700,7 +704,7 @@ public static function isRememberMeCookieValid(
700704
if ((int)$expireDate > time()) {
701705
$expectedString = self::generateRememberMeCookieString($rememberSecret, $state['employeeId'], $expireDate, $mfaOptions);
702706
$isValid = password_verify($expectedString, $cookieHash);
703-
707+
704708
if ($isValid) {
705709
$idBrokerClient = self::getIdBrokerClient($state['idBrokerConfig']);
706710
$idBrokerClient->updateUserLastLogin($state['employeeId']);
@@ -734,8 +738,7 @@ public static function generateRememberMeCookieString(
734738
}
735739
}
736740

737-
$string = $rememberSecret . $employeeId . $expireDate . $allMfaIds;
738-
return $string;
741+
return $rememberSecret . $employeeId . $expireDate . $allMfaIds;
739742
}
740743

741744
/**
@@ -789,7 +792,7 @@ protected static function redirectToOutOfBackupCodesMessage(array &$state, strin
789792
* @param string $employeeId
790793
* @param array $mfaOptions
791794
* @param string $rememberDuration
792-
* @throws \Sil\PhpEnv\EnvVarNotFoundException
795+
* @throws EnvVarNotFoundException
793796
*/
794797
public static function setRememberMeCookies(
795798
string $employeeId,
@@ -818,8 +821,9 @@ protected static function shouldPromptForMfa(array $mfa): bool
818821
*
819822
* @param array $state The state data.
820823
* @param LoggerInterface $logger A PSR-3 compatible logger.
824+
* @throws Exception
821825
*/
822-
public static function sendManagerCode(array &$state, LoggerInterface $logger): string
826+
public static function sendManagerCode(array &$state, LoggerInterface $logger): void
823827
{
824828
try {
825829
$idBrokerClient = self::getIdBrokerClient($state['idBrokerConfig']);
@@ -830,13 +834,14 @@ public static function sendManagerCode(array &$state, LoggerInterface $logger):
830834
'event' => 'Manager rescue code sent',
831835
'employeeId' => $state['employeeId'],
832836
]));
833-
} catch (\Throwable $t) {
837+
} catch (Throwable $t) {
834838
$logger->error(json_encode([
835839
'event' => 'Manager rescue code: failed',
836840
'employeeId' => $state['employeeId'],
837841
'error' => $t->getCode() . ': ' . $t->getMessage(),
838842
]));
839-
return 'There was an unexpected error. Please try again or contact tech support for assistance';
843+
throw new Exception('There was an unexpected error. Please try again ' .
844+
'or contact tech support for assistance');
840845
}
841846

842847
$mfaOptions = $state['mfaOptions'];
@@ -932,7 +937,7 @@ protected static function updateStateWithNewMfaData(array &$state, LoggerInterfa
932937

933938
try {
934939
$newMfaOptions = $idBrokerClient->mfaList($state['employeeId']);
935-
} catch (Exception $e) {
940+
} catch (Exception) {
936941
$log['status'] = 'failed: id-broker exception';
937942
$logger->error(json_encode($log));
938943
return;

‎modules/mfa/src/LoggerFactory.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44

55
use InvalidArgumentException;
66
use Psr\Log\LoggerInterface;
7-
use Psr\Log\Psr3SamlLogger;
8-
use SimpleSAML\Module\mfa\Assert;
7+
use Sil\Psr3Adapters\Psr3SamlLogger;
98

109
class LoggerFactory
1110
{

0 commit comments

Comments
 (0)