Skip to content

Commit 0bac78e

Browse files
tvdijentvdijen
authored
Fix Psalm-issues (#1)
* Migrate assertions to Webmozart * Fix Psalm issues
1 parent a863c0f commit 0bac78e

File tree

7 files changed

+79
-55
lines changed

7 files changed

+79
-55
lines changed

‎lib/Auth/Process/Consent.php‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use SimpleSAML\Module;
1616
use SimpleSAML\Stats;
1717
use SimpleSAML\Utils;
18+
use Webmozart\Assert\Assert;
1819

1920
class Consent extends \SimpleSAML\Auth\ProcessingFilter
2021
{
@@ -80,7 +81,7 @@ class Consent extends \SimpleSAML\Auth\ProcessingFilter
8081
*/
8182
public function __construct($config, $reserved)
8283
{
83-
assert(is_array($config));
84+
Assert::isArray($config);
8485
parent::__construct($config, $reserved);
8586

8687
if (array_key_exists('includeValues', $config)) {
@@ -233,13 +234,13 @@ private static function checkDisable($option, $entityId)
233234
*/
234235
public function process(&$state)
235236
{
236-
assert(is_array($state));
237-
assert(array_key_exists('UserID', $state));
238-
assert(array_key_exists('Destination', $state));
239-
assert(array_key_exists('entityid', $state['Destination']));
240-
assert(array_key_exists('metadata-set', $state['Destination']));
241-
assert(array_key_exists('entityid', $state['Source']));
242-
assert(array_key_exists('metadata-set', $state['Source']));
237+
Assert::isArray($state);
238+
Assert::keyExists($state, 'UserID');
239+
Assert::keyExists($state, 'Destination');
240+
Assert::keyExists($state['Destination'], 'entityid');
241+
Assert::keyExists($state['Destination'], 'metadata-set');
242+
Assert::keyExists($state['Source'], 'entityid');
243+
Assert::keyExists($state['Source'], 'metadata-set');
243244

244245
$spEntityId = $state['Destination']['entityid'];
245246
$idpEntityId = $state['Source']['entityid'];

‎lib/Consent/Store/Cookie.php‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace SimpleSAML\Module\consent\Consent\Store;
44

5+
use Webmozart\Assert\Assert;
6+
57
/**
68
* Cookie storage for consent
79
*
@@ -38,9 +40,9 @@ class Cookie extends \SimpleSAML\Module\consent\Store
3840
*/
3941
public function hasConsent($userId, $destinationId, $attributeSet)
4042
{
41-
assert(is_string($userId));
42-
assert(is_string($destinationId));
43-
assert(is_string($attributeSet));
43+
Assert::string($userId);
44+
Assert::string($destinationId);
45+
Assert::string($attributeSet);
4446

4547
$cookieName = self::getCookieName($userId, $destinationId);
4648

@@ -89,21 +91,21 @@ public function hasConsent($userId, $destinationId, $attributeSet)
8991
* @param string $destinationId A string which identifies the destination.
9092
* @param string $attributeSet A hash which identifies the attributes.
9193
*
92-
* @return void
94+
* @return bool
9395
*/
9496
public function saveConsent($userId, $destinationId, $attributeSet)
9597
{
96-
assert(is_string($userId));
97-
assert(is_string($destinationId));
98-
assert(is_string($attributeSet));
98+
Assert::string($userId);
99+
Assert::string($destinationId);
100+
Assert::string($attributeSet);
99101

100102
$name = self::getCookieName($userId, $destinationId);
101103
$value = $userId.':'.$attributeSet.':'.$destinationId;
102104

103105
\SimpleSAML\Logger::debug('Consent cookie - Set ['.$value.']');
104106

105107
$value = self::sign($value);
106-
$this->setConsentCookie($name, $value);
108+
return $this->setConsentCookie($name, $value);
107109
}
108110

109111

@@ -119,8 +121,8 @@ public function saveConsent($userId, $destinationId, $attributeSet)
119121
*/
120122
public function deleteConsent($userId, $destinationId)
121123
{
122-
assert(is_string($userId));
123-
assert(is_string($destinationId));
124+
Assert::string($userId);
125+
Assert::string($destinationId);
124126

125127
$name = self::getCookieName($userId, $destinationId);
126128
$this->setConsentCookie($name, null);
@@ -139,7 +141,7 @@ public function deleteConsent($userId, $destinationId)
139141
*/
140142
public function deleteAllConsents($userId)
141143
{
142-
assert(is_string($userId));
144+
Assert::string($userId);
143145

144146
throw new \Exception(
145147
'The cookie consent handler does not support delete of all consents...'
@@ -158,7 +160,7 @@ public function deleteAllConsents($userId)
158160
*/
159161
public function getConsents($userId)
160162
{
161-
assert(is_string($userId));
163+
Assert::string($userId);
162164

163165
$ret = [];
164166

@@ -206,7 +208,7 @@ public function getConsents($userId)
206208
*/
207209
private static function sign($data)
208210
{
209-
assert(is_string($data));
211+
Assert::string($data);
210212

211213
$secretSalt = \SimpleSAML\Utils\Config::getSecretSalt();
212214

@@ -225,7 +227,7 @@ private static function sign($data)
225227
*/
226228
private static function verify($signedData)
227229
{
228-
assert(is_string($signedData));
230+
Assert::string($signedData);
229231

230232
$data = explode(':', $signedData, 2);
231233
if (count($data) !== 2) {
@@ -256,8 +258,8 @@ private static function verify($signedData)
256258
*/
257259
private static function getCookieName($userId, $destinationId)
258260
{
259-
assert(is_string($userId));
260-
assert(is_string($destinationId));
261+
Assert::string($userId);
262+
Assert::string($destinationId);
261263

262264
return '\SimpleSAML\Module\consent:'.sha1($userId.':'.$destinationId);
263265
}
@@ -269,12 +271,12 @@ private static function getCookieName($userId, $destinationId)
269271
* @param string $name Name of the cookie.
270272
* @param string|null $value Value of the cookie. Set this to null to delete the cookie.
271273
*
272-
* @return void
274+
* @return bool
273275
*/
274276
private function setConsentCookie($name, $value)
275277
{
276-
assert(is_string($name));
277-
assert(is_string($value) || $value === null);
278+
Assert::string($name);
279+
Assert::nullOrString($value);
278280

279281
$globalConfig = \SimpleSAML\Configuration::getInstance();
280282
$params = [
@@ -284,6 +286,11 @@ private function setConsentCookie($name, $value)
284286
'secure' => \SimpleSAML\Utils\HTTP::isHTTPS(),
285287
];
286288

287-
\SimpleSAML\Utils\HTTP::setCookie($name, $value, $params, false);
289+
try {
290+
\SimpleSAML\Utils\HTTP::setCookie($name, $value, $params, false);
291+
return true;
292+
} catch (\SimpleSAML\Error\CannotSetCookie $e) {
293+
return false;
294+
}
288295
}
289296
}

‎lib/Consent/Store/Database.php‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace SimpleSAML\Module\consent\Consent\Store;
44

5+
use Webmozart\Assert\Assert;
6+
57
/**
68
* Store consent in database.
79
*
@@ -166,9 +168,9 @@ public function __sleep()
166168
*/
167169
public function hasConsent($userId, $destinationId, $attributeSet)
168170
{
169-
assert(is_string($userId));
170-
assert(is_string($destinationId));
171-
assert(is_string($attributeSet));
171+
Assert::string($userId);
172+
Assert::string($destinationId);
173+
Assert::string($attributeSet);
172174

173175
$st = $this->execute(
174176
'UPDATE '.$this->table.' '.
@@ -202,13 +204,13 @@ public function hasConsent($userId, $destinationId, $attributeSet)
202204
* @param string $destinationId A string which identifies the destination.
203205
* @param string $attributeSet A hash which identifies the attributes.
204206
*
205-
* @return void|true True if consent is deleted.
207+
* @return bool True if consent is deleted, false otherwise.
206208
*/
207209
public function saveConsent($userId, $destinationId, $attributeSet)
208210
{
209-
assert(is_string($userId));
210-
assert(is_string($destinationId));
211-
assert(is_string($attributeSet));
211+
Assert::string($userId);
212+
Assert::string($destinationId);
213+
Assert::string($attributeSet);
212214

213215
// Check for old consent (with different attribute set)
214216
$st = $this->execute(
@@ -219,13 +221,13 @@ public function saveConsent($userId, $destinationId, $attributeSet)
219221
);
220222

221223
if ($st === false) {
222-
return;
224+
return false;
223225
}
224226

225227
if ($st->rowCount() > 0) {
226228
// Consent has already been stored in the database
227229
\SimpleSAML\Logger::debug('consent:Database - Updated old consent.');
228-
return;
230+
return false;
229231
}
230232

231233
// Add new consent
@@ -254,8 +256,8 @@ public function saveConsent($userId, $destinationId, $attributeSet)
254256
*/
255257
public function deleteConsent($userId, $destinationId)
256258
{
257-
assert(is_string($userId));
258-
assert(is_string($destinationId));
259+
Assert::string($userId);
260+
Assert::string($destinationId);
259261

260262
$st = $this->execute(
261263
'DELETE FROM '.$this->table.' WHERE hashed_user_id = ? AND service_id = ?;',
@@ -285,7 +287,7 @@ public function deleteConsent($userId, $destinationId)
285287
*/
286288
public function deleteAllConsents($userId)
287289
{
288-
assert(is_string($userId));
290+
Assert::string($userId);
289291

290292
$st = $this->execute(
291293
'DELETE FROM '.$this->table.' WHERE hashed_user_id = ?',
@@ -317,7 +319,7 @@ public function deleteAllConsents($userId)
317319
*/
318320
public function getConsents($userId)
319321
{
320-
assert(is_string($userId));
322+
Assert::string($userId);
321323

322324
$ret = [];
323325

@@ -348,12 +350,12 @@ public function getConsents($userId)
348350
* @param string $statement The statement which should be executed.
349351
* @param array $parameters Parameters for the statement.
350352
*
351-
* @return \PDOStatement|bool The statement, or false if execution failed.
353+
* @return \PDOStatement|false The statement, or false if execution failed.
352354
*/
353355
private function execute($statement, $parameters)
354356
{
355-
assert(is_string($statement));
356-
assert(is_array($parameters));
357+
Assert::string($statement);
358+
Assert::isArray($parameters);
357359

358360
$db = $this->getDB();
359361
if ($db === false) {
@@ -477,8 +479,8 @@ private function getDB()
477479
*/
478480
private static function formatError($error)
479481
{
480-
assert(is_array($error));
481-
assert(count($error) >= 3);
482+
Assert::isArray($error);
483+
Assert::greaterThanEq(count($error), 3);
482484

483485
return $error[0].' - '.$error[2].' ('.$error[1].')';
484486
}

‎lib/Store.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace SimpleSAML\Module\consent;
44

5+
use Webmozart\Assert\Assert;
6+
57
/**
68
* Base class for consent storage handlers.
79
*
@@ -21,7 +23,7 @@ abstract class Store
2123
*/
2224
protected function __construct(&$config)
2325
{
24-
assert(is_array($config));
26+
Assert::isArray($config);
2527
}
2628

2729

@@ -144,6 +146,11 @@ public static function parseStoreConfig($config)
144146
);
145147

146148
unset($config[0]);
147-
return new $className($config);
149+
/**
150+
* @psalm-suppress InvalidStringClass
151+
* @var \SimpleSAML\Module\consent\Store $retval
152+
*/
153+
$retval = new $className($config);
154+
return $retval;
148155
}
149156
}

‎templates/consentform.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/**
34
* Template form for giving consent.
45
*
@@ -9,9 +10,12 @@
910
*
1011
* @package SimpleSAMLphp
1112
*/
12-
assert(is_string($this->data['yesTarget']));
13-
assert(is_string($this->data['noTarget']));
14-
assert($this->data['sppp'] === false || is_string($this->data['sppp']));
13+
14+
use Webmozart\Assert\Assert;
15+
16+
Assert::string($this->data['yesTarget']);
17+
Assert::string($this->data['noTarget']);
18+
Assert::true($this->data['sppp'] === false || is_string($this->data['sppp']));
1519

1620
// Parse parameters
1721
$dstName = $this->data['dstName'];

‎www/getconsent.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131
$id = $_REQUEST['StateId'];
3232
$state = \SimpleSAML\Auth\State::loadState($id, 'consent:request');
3333

34-
if (array_key_exists('core:SP', $state)) {
34+
if (is_null($state)) {
35+
throw new \SimpleSAML\Error\NoState;
36+
} elseif (array_key_exists('core:SP', $state)) {
3537
$spentityid = $state['core:SP'];
3638
} elseif (array_key_exists('saml:sp:State', $state)) {
3739
$spentityid = $state['saml:sp:State']['core:SP'];
@@ -209,7 +211,7 @@
209211
*
210212
* @return string HTML representation of the attributes
211213
*/
212-
function present_attributes($t, $attributes, $nameParent)
214+
function present_attributes(\SimpleSAML\XHTML\Template $t, array $attributes, $nameParent)
213215
{
214216
$translator = $t->getTranslator();
215217

@@ -248,6 +250,7 @@ function present_attributes($t, $attributes, $nameParent)
248250
$hiddenId = \SimpleSAML\Utils\Random::generateID();
249251
$str .= '<td><span class="attrvalue hidden" id="hidden_'.$hiddenId.'">';
250252
} else {
253+
$hiddenId = '';
251254
$str .= '<td><span class="attrvalue">';
252255
}
253256

@@ -287,6 +290,6 @@ function present_attributes($t, $attributes, $nameParent)
287290
$str .= '</td></tr>';
288291
} // end else: not child table
289292
} // end foreach
290-
$str .= isset($attributes) ? '</table>' : '';
293+
$str .= '</table>';
291294
return $str;
292295
}

‎www/logout.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@
1414

1515
$idp = \SimpleSAML\IdP::getByState($state);
1616
$idp->handleLogoutRequest($state, null);
17-
assert(false);
17+
throw new \Exception('Should never happen');

0 commit comments

Comments
 (0)