From 7f0a5f4abc4ea8ff9400a84a590111ec231bcac7 Mon Sep 17 00:00:00 2001 From: Andrew Engelbrecht Date: Wed, 2 Aug 2023 15:16:15 -0400 Subject: [PATCH] strict typing, textual space changes this applies changes to the main source file, based on diffs made to the upstream base repo (see previous commit for details) --- lib/Auth/Source/FSFDrupalAuth.php | 173 +++++++++++++++--------------- 1 file changed, 84 insertions(+), 89 deletions(-) diff --git a/lib/Auth/Source/FSFDrupalAuth.php b/lib/Auth/Source/FSFDrupalAuth.php index 29941d6..efad30f 100644 --- a/lib/Auth/Source/FSFDrupalAuth.php +++ b/lib/Auth/Source/FSFDrupalAuth.php @@ -1,10 +1,13 @@ authId); + throw new Exception('Missing required attribute \'' . $param . + '\' for authentication source ' . $this->authId); } if (!is_string($config[$param])) { - throw new Exception('Expected parameter \''.$param. - '\' for authentication source '.$this->authId. - ' to be a string. Instead it was: '. + throw new Exception('Expected parameter \'' . $param . + '\' for authentication source ' . $this->authId . + ' to be a string. Instead it was: ' . var_export($config[$param], true)); } @@ -153,7 +153,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase * * @return PDO The database connection. */ - private function connect() + private function connect(): PDO { try { $db = new PDO($this->dsn, $this->username, $this->password, $this->options); @@ -189,7 +189,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase * Check the password against a Drupal hash * */ - private function check_password($password, $hash) { + private function check_password(string $password, string $hash): boolean { // // The reason for running a separate process is so that the PHP global @@ -228,16 +228,16 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase // proc_close in order to avoid a deadlock $return_value = proc_close($process); - //Logger::debug('fsfdrupalauth:'.$this->authId.': authenticator stdout: '.$result); + //Logger::debug('fsfdrupalauth:' . $this->authId . ': authenticator stdout: ' . $result); $errors_found_yet = false; if ($errors != "") { - Logger::error('fsfdrupalauth:'.$this->authId.': authenticator stderr: '.$errors); + Logger::error('fsfdrupalauth:' . $this->authId.': authenticator stderr: ' . $errors); $errors_found_yet = true; } if ($return_value != 0) { - Logger::error('fsfdrupalauth:'.$this->authId.': authenticator non-zero return code: '.$return_value); + Logger::error('fsfdrupalauth:' . $this->authId . ': authenticator non-zero return code: ' . $return_value); $errors_found_yet = true; } @@ -245,7 +245,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase } else { - Logger::error('fsfdrupalauth:'.$this->authId.': unable to launch authenticator'); + Logger::error('fsfdrupalauth:' . $this->authId . ': unable to launch authenticator'); return false; } @@ -256,35 +256,32 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase * query the database with arbitrary queries that only require a user name. * */ - private function query_db($queryname, $query_params) + private function query_db(string $queryname, string $query_params): array { - assert(is_string($queryname)); - assert(is_string($username)); - $db = $this->connect(); try { $sth = $db->prepare($this->$queryname); } catch (PDOException $e) { - throw new Exception('fsfdrupalauth:'.$this->authId. - ': - Failed to prepare queryname: '.$queryname.': '.$e->getMessage()); + throw new Exception('fsfdrupalauth:' . $this->authId . + ': - Failed to prepare queryname: ' . $queryname . ': ' . $e->getMessage()); } try { $sth->execute($query_params); } catch (PDOException $e) { - throw new Exception('fsfdrupalauth:'.$this->authId. - ': - Failed to execute queryname: '.$queryname.': '.$e->getMessage()); + throw new Exception('fsfdrupalauth:' . $this->authId . + ': - Failed to execute queryname: ' . $queryname . ': ' . $e->getMessage()); } try { $data = $sth->fetchAll(PDO::FETCH_ASSOC); } catch (PDOException $e) { - throw new Exception('fsfdrupalauth:'.$this->authId. - ': - Failed to fetch result set: '.$e->getMessage()); + throw new Exception('fsfdrupalauth:' . $this->authId . + ': - Failed to fetch result set: ' . $e->getMessage()); } - Logger::info('fsfdrupalauth:'.$this->authId.': Got '.count($data). + Logger::info('fsfdrupalauth:' . $this->authId . ': Got ' . count($data) . ' rows from database'); return $data; @@ -293,7 +290,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase /** * add more CAS attributes to user, such as is_staff and is_member */ - private function add_more_attributes(&$attributes, $username) { + private function add_more_attributes(array &$attributes, string $username): void { // // query on staff @@ -303,7 +300,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase if (count($staff_data) === 0) { // No rows returned - invalid username - Logger::debug('fsfdrupalauth:'.$this->authId. + Logger::debug('fsfdrupalauth:' . $this->authId . ': No rows in result set. Probably not FSF staff.'); } @@ -333,7 +330,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase if (count($membership_data) === 0) { // No rows returned - invalid username - Logger::debug('fsfdrupalauth:'.$this->authId. + Logger::debug('fsfdrupalauth:' . $this->authId . ': No rows in result set. Probably no membership.'); } @@ -364,8 +361,8 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase * @param string $query_name Name of query in authsources * @param array $extra_params Associative array of parameters to include in query */ - $donation_query = function ($query_name, $extra_params) - use ($username) { + $donation_query = function (string $query_name, array $extra_params): array + use (string $username) { $parameters = ['username' => $username]; @@ -378,7 +375,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase $old_membership_query = $donation_query; - $compare_res = function ($result, $amount) { + $compare_res = function (array $result, int $amount): void { foreach ($result[0] as $key => $value) { if (intval($value) >= $amount) { return true; @@ -400,13 +397,13 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase // the time window with a single donation. this approximates whether // the person was, or would have been, a member during the configured // time window. - $nomination_process_analyze_history = function ($selective_donations_history) - use ($nomination_process_start_date, $nomination_process_end_date) { + $nomination_process_analyze_history = function (array $selective_donations_history): boolean + use (string $nomination_process_start_date, string $nomination_process_end_date) { $eligible = false; - Logger::debug('fsfdrupalauth:'.$this->authId. - ': start date: '.$nomination_process_start_date. " end date: ".$nomination_process_end_date); + Logger::debug('fsfdrupalauth:' . $this->authId . + ': start date: ' . $nomination_process_start_date . " end date: " . $nomination_process_end_date); $start_date_obj = new \DateTime($nomination_process_start_date); $end_date_obj = new \DateTime($nomination_process_end_date); @@ -447,14 +444,14 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase return false; }; - $discussion_process_analyze_history = function ($selective_donations_history) - use ($discussion_process_start_date, $discussion_process_end_date) { + $discussion_process_analyze_history = function (array $selective_donations_history): boolean + use (string $discussion_process_start_date, string $discussion_process_end_date) { $eligible = false; $total = 0; - Logger::debug('fsfdrupalauth:'.$this->authId. - ': start date: '.$discussion_process_start_date. " end date: ".$discussion_process_end_date); + Logger::debug('fsfdrupalauth:' . $this->authId . + ': start date: ' . $discussion_process_start_date . " end date: " . $discussion_process_end_date); $start_date_obj = new \DateTime($discussion_process_start_date); $end_date_obj = new \DateTime($discussion_process_end_date); @@ -470,8 +467,8 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase } } - Logger::debug('fsfdrupalauth:'.$this->authId. - ': total amount: $'.$total); + Logger::debug('fsfdrupalauth:' . $this->authId . + ': total amount: $' . $total); if ($total >= $this->discussion_process_donation_amount) { return true; @@ -489,25 +486,25 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase $adhoc_params = ['adhoc_access_group_id' => intval($this->nomination_process_adhoc_access_group_id)]; if ($this->nomination_process_active != 'true' ) { - Logger::debug('fsfdrupalauth:'.$this->authId.': Nomination board process checks not active'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Nomination board process checks not active'); $attributes['nomination_process'] = ['false']; } elseif ($compare_res($donation_query('query_nomination_process_adhoc', $adhoc_params), 1)) { - Logger::debug('fsfdrupalauth:'.$this->authId.': In adhoc list of contacts for nomination board process'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': In adhoc list of contacts for nomination board process'); $attributes['nomination_process'] = ['true']; } elseif ($attributes['is_member'] != ['true']) { - Logger::debug('fsfdrupalauth:'.$this->authId.': Not a current member for nomination board process'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Not a current member for nomination board process'); $attributes['nomination_process'] = ['false']; } elseif ($nomination_process_analyze_history($donation_query('query_nomination_process_donations', $donation_params)) || $compare_res($donation_query('query_nomination_process_gift_receipt', $gift_member_params), 1)) { - Logger::debug('fsfdrupalauth:'.$this->authId.': Past membership / donations meet threshold for nomination board process'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Past membership / donations meet threshold for nomination board process'); $attributes['nomination_process'] = ['true']; } else { - Logger::debug('fsfdrupalauth:'.$this->authId.': Past membership / donations do not meet threshold for nomination board process'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Past membership / donations do not meet threshold for nomination board process'); $attributes['nomination_process'] = ['false']; } @@ -521,29 +518,29 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase $adhoc_params_no = ['adhoc_access_group_id' => intval($this->discussion_process_adhoc_no_access_group_id)]; if ($this->discussion_process_active != 'true' ) { - Logger::debug('fsfdrupalauth:'.$this->authId.': Discussion board process checks not active'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Discussion board process checks not active'); $attributes['discussion_process'] = ['false']; } elseif ($compare_res($donation_query('query_discussion_process_adhoc', $adhoc_params_no), 1)) { - Logger::debug('fsfdrupalauth:'.$this->authId.': Nominee not allowed to participate in board discussion process.'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Nominee not allowed to participate in board discussion process.'); $attributes['discussion_process'] = ['false']; } elseif ($compare_res($donation_query('query_discussion_process_adhoc', $adhoc_params), 1)) { - Logger::debug('fsfdrupalauth:'.$this->authId.': In adhoc list of contacts for discussion board process'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': In adhoc list of contacts for discussion board process'); $attributes['discussion_process'] = ['true']; } elseif ($attributes['is_member'] != ['true']) { - Logger::debug('fsfdrupalauth :'.$this->authId.': Not a member, so not eligible for board nominee discussion process.'); + Logger::debug('fsfdrupalauth :' . $this->authId . ': Not a member, so not eligible for board nominee discussion process.'); $attributes['discussion_process'] = ['false']; } elseif ($compare_res($old_membership_query('query_discussion_process_old_membership', $old_member_params), 1) || $discussion_process_analyze_history($donation_query('query_discussion_process_donations', $donation_params))) { - Logger::debug('fsfdrupalauth:'.$this->authId.': Past membership / donations meet threshold for discussion board process'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Past membership / donations meet threshold for discussion board process'); $attributes['discussion_process'] = ['true']; } else { - Logger::debug('fsfdrupalauth:'.$this->authId.': Past membership / donations do not meet threshold for discussion board process'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Past membership / donations do not meet threshold for discussion board process'); $attributes['discussion_process'] = ['false']; } @@ -554,11 +551,11 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase $adhoc_params = ['adhoc_access_group_id' => intval($this->discussion_moderator_access_group_id)]; if ($compare_res($donation_query('query_discussion_process_adhoc', $adhoc_params), 1)) { - Logger::debug('fsfdrupalauth:'.$this->authId.': In adhoc list of moderators for board discussion forum'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': In adhoc list of moderators for board discussion forum'); $attributes['discussion_moderator'] = ['true']; } else { - Logger::debug('fsfdrupalauth:'.$this->authId.': Not in adhoc list of moderators for board discussion forum'); + Logger::debug('fsfdrupalauth:' . $this->authId . ': Not in adhoc list of moderators for board discussion forum'); $attributes['discussion_moderator'] = ['false']; } @@ -594,13 +591,11 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase * @param string $password The password the user wrote. * @return array Associative array with the users attributes. */ - protected function login($username, $password) + protected function login(string $username, string $password): array { - assert(is_string($username)); - assert(is_string($password)); //// keep this commented when it's not in use. it prints user passwords to the log file - //Logger::debug('fsfdrupalauth:'.$this->authId.': entered password: '.$password); + //Logger::debug('fsfdrupalauth:' . $this->authId . ': entered password: ' . $password); $user_data = $this->query_db('query_main', ['username' => $username]); @@ -608,7 +603,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase if (count($user_data) === 0) { // No rows returned - invalid username - Logger::error('fsfdrupalauth:'.$this->authId. + Logger::error('fsfdrupalauth:' . $this->authId . ': No rows in result set. Probably wrong username.'); throw new Error\Error('WRONGUSERPASS'); } @@ -656,7 +651,7 @@ class FSFDrupalAuth extends \SimpleSAML\Module\core\Auth\UserPassBase $this->add_more_attributes($attributes, $username); - Logger::info('fsfdrupalauth:'.$this->authId.': Attributes: '. + Logger::info('fsfdrupalauth:' . $this->authId . ': Attributes: ' . implode(',', array_keys($attributes))); return $attributes; -- 2.25.1