Skip to content

Commit c0a0e77

Browse files
nathanjrobertsonnathanjrobertson
committed
More phpstan fixes
1 parent de4021b commit c0a0e77

File tree

9 files changed

+73
-36
lines changed

9 files changed

+73
-36
lines changed

‎phpstan-baseline-dev.neon‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
11
parameters:
22
ignoreErrors:
33
-
4-
message: "#^Parameter \\#1 \\$array of function asort expects array, string given\\.$#"
5-
count: 4
4+
message: "#Call to an undefined method object::callLogin().#"
5+
count: 10
66
path: tests/src/Auth/Source/PasswordVerifyTest.php
77

88
-
9-
message: "#^Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\PasswordVerifyTest\\:\\:\\$config \\(array\\<string, string\\|null\\>\\) does not accept array\\<string, array\\<int, string\\>\\|string\\|null\\>\\.$#"
10-
count: 4
11-
path: tests/src/Auth/Source/PasswordVerifyTest.php
9+
message: "#Call to an undefined method object::callLogin().#"
10+
count: 11
11+
path: tests/src/Auth/Source/SQLTest.php
1212

1313
-
14-
message: "#^Parameter \\#1 \\$array of function asort expects array, mixed given\\.$#"
15-
count: 4
16-
path: tests/src/Auth/Source/SQLTest.php
14+
message: "#Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQL2SimpleTest\\:\\:\\$config type has no value type specified in iterable type array\\.#"
15+
count: 1
16+
path: tests/src/Auth/Source/SQL2SimpleTest.php
1717

1818
-
19-
message: "#^Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQLTest\\:\\:\\$config \\(array\\<string, string\\|null\\>\\) does not accept array\\<string, array\\<int, string\\>\\|string\\|null\\>\\.$#"
20-
count: 4
21-
path: tests/src/Auth/Source/SQLTest.php
19+
message: "#Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQL2MultipleAuthTest\\:\\:\\$config type has no value type specified in iterable type array\\.#"
20+
count: 1
21+
path: tests/src/Auth/Source/SQL2MultipleAuthTest.php

‎src/Auth/Source/SQL2.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ protected function login(
460460
throw new Error\Error('WRONGUSERPASS');
461461
}
462462

463+
$validPasswordHashFound = false;
463464
$passwordHash = null;
464465
foreach ($data as $row) {
465466
if ((!array_key_exists($hashColumn, $row)) || is_null($row[$hashColumn])) {
@@ -470,19 +471,27 @@ protected function login(
470471
));
471472
throw new Error\Error('WRONGUSERPASS');
472473
}
473-
if ($passwordHash === null) {
474+
if (($passwordHash === null) && (strlen($row[$hashColumn]) > 0)) {
474475
$passwordHash = $row[$hashColumn];
476+
$validPasswordHashFound = true;
475477
} elseif ($passwordHash != $row[$hashColumn]) {
476478
Logger::error(sprintf(
477479
'sqlauth:%s: column %s must be THE SAME in every result tuple.',
478480
$this->authId,
479481
$hashColumn,
480482
));
481483
throw new Error\Error('WRONGUSERPASS');
484+
} elseif (strlen($row[$hashColumn]) === 0) {
485+
Logger::error(sprintf(
486+
'sqlauth:%s: column `%s` must contain a valid password hash.',
487+
$this->authId,
488+
$hashColumn,
489+
));
490+
throw new Error\Error('WRONGUSERPASS');
482491
}
483492
}
484493

485-
if ($passwordHash == null || (!password_verify($password, $passwordHash))) {
494+
if ((!$validPasswordHashFound) || (!password_verify($password, $passwordHash))) {
486495
Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
487496
' password verification failed');
488497
/* Authentication with verify_password() failed, however that only means that

‎tests/src/Auth/Source/PasswordVerifyTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class PasswordVerifyTest extends TestCase
2020
/** @var array<string, string> */
2121
private array $info = ['AuthId' => 'testAuthId'];
2222

23-
/** @var array<string, string|null> */
23+
/** @var array<string, list<string>|string|null> */
2424
private array $config = [
2525
"dsn" => 'sqlite:file::memory:?cache=shared',
2626
"username" => "notused",

‎tests/src/Auth/Source/SQL2MultipleAuthTest.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
class SQL2MultipleAuthTest extends TestCase
2626
{
27+
/** @var array<string, string> */
2728
private array $info = ['AuthId' => 'testAuthId'];
2829

2930
protected array $config = []; // Filled out in setUp()
@@ -108,7 +109,7 @@ public function setUp(): void
108109
],
109110
[
110111
'database' => 'studentsdb',
111-
'query' => "select unit_code from units_enrolled where studentid=:userid",
112+
'query' => "select unit_code from units_enrolled where studentid=:userid order by unit_code",
112113
'only_for_auth' => ['auth_query_students'],
113114
],
114115
],
@@ -259,7 +260,6 @@ public function testStudentLoginSuccess(): void
259260
$ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('[email protected]', 'password');
260261
asort($ret);
261262
$this->assertCount(7, $ret);
262-
$this->assertCount(2, $ret['unit_code']);
263263
$this->assertEquals($ret, [
264264
'studentid' => ['1'],
265265
'givenName' => ["Alice"],
@@ -278,7 +278,6 @@ public function testNonPhysicsStaffLoginSuccess(): void
278278
$ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('[email protected]', 'password');
279279
asort($ret);
280280
$this->assertCount(6, $ret);
281-
$this->assertCount(1, $ret['role']);
282281
$this->assertEquals($ret, [
283282
'uid' => ['1'],
284283
'givenName' => ["Eve"],
@@ -296,7 +295,6 @@ public function testPhysicsStaffLoginSuccess(): void
296295
$ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('[email protected]', 'password');
297296
asort($ret);
298297
$this->assertCount(8, $ret);
299-
$this->assertCount(1, $ret['role']);
300298
$this->assertEquals($ret, [
301299
'uid' => ['2'],
302300
'givenName' => ["Mallory"],

‎tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class SQL2PasswordVerifySimpleTest extends SQL2SimpleTest
2323

2424
// We need to not specify the 'password=:password' clause in the WHERE clause,
2525
// as password_verify() does not work that way.
26-
protected string $extraSqlAndClauses = '';
26+
protected string $extraSqlAndClauses = ' ';
2727

2828

2929
public function setUp(): void

‎tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ class SQL2PasswordVerifySingleAuthTest extends SQL2SingleAuthTest
2626
protected string $extraSqlAndClauses = '';
2727

2828

29+
/**
30+
* Different tests require different combinations of databases, auth queries and attr queries.
31+
* This function returns a config with the requested number of each.
32+
*
33+
* @param int $numDatabases
34+
* @param int $numAuthQueries
35+
* @param array<string> $authQueryAttributes
36+
* @param int $numAttrQueries
37+
* @return array<string, mixed>
38+
*/
2939
protected function getConfig(
3040
int $numDatabases,
3141
int $numAuthQueries,
@@ -34,8 +44,10 @@ protected function getConfig(
3444
): array {
3545
$config = parent::getConfig($numDatabases, $numAuthQueries, $authQueryAttributes, $numAttrQueries);
3646

37-
foreach ($config['auth_queries'] as &$query) {
38-
$query['password_verify_hash_column'] = 'password';
47+
// @phpstan-ignore argument.type
48+
foreach (array_keys($config['auth_queries']) as $authQueryName) {
49+
// @phpstan-ignore offsetAccess.nonOffsetAccessible
50+
$config['auth_queries'][$authQueryName]['password_verify_hash_column'] = 'password';
3951
}
4052

4153
return $config;

‎tests/src/Auth/Source/SQL2SimpleTest.php‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@
1616
*/
1717
class SQL2SimpleTest extends TestCase
1818
{
19+
/** @var array<string, string> */
1920
private array $info = ['AuthId' => 'testAuthId'];
2021

2122
protected array $config = []; // Filled out in setUp()
2223

2324
protected string $extraSqlSelectColumns = '';
2425

25-
protected string $extraSqlAndClauses = ' and password=:password';
26+
protected string $extraSqlAndClauses = ' and password=:password ';
2627

2728

2829
public function setUp(): void
@@ -193,10 +194,10 @@ public function testJoinSingleSuccess(): void
193194
$this->config['auth_queries']['auth_query']['query'] = "
194195
select u.givenName, u.email, ug.groupname" . $this->extraSqlSelectColumns . "
195196
from users u left join usergroups ug on (u.uid=ug.uid)
196-
where u.uid=:username" . $this->extraSqlAndClauses;
197+
where u.uid=:username" . $this->extraSqlAndClauses .
198+
"order by ug.groupname";
197199
$ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password');
198200
asort($ret);
199-
asort($ret['groupname']);
200201
$this->assertCount(3, $ret);
201202
$this->assertEquals($ret, [
202203
'email' => ['[email protected]'],
@@ -228,13 +229,14 @@ public function testMultiQuerySuccess(): void
228229
$this->config['attr_queries'] = [
229230
[
230231
'database' => 'defaultdb',
231-
'query' => "select groupname from usergroups where uid=:username",
232+
'query' =>
233+
"select groupname from usergroups where uid=:username " .
234+
"order by groupname",
232235
],
233236
];
234237

235238
$ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password');
236239
asort($ret);
237-
asort($ret['groupname']);
238240
$this->assertCount(3, $ret);
239241
$this->assertEquals($ret, [
240242
'email' => ['[email protected]'],
@@ -271,17 +273,22 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void
271273
$this->config['attr_queries'] = [
272274
[
273275
'database' => 'defaultdb',
274-
'query' => "select groupname from usergroups where uid=:username and groupname like '%nomatch%'",
276+
'query' =>
277+
"select groupname from usergroups " .
278+
"where uid=:username and groupname like '%nomatch%' " .
279+
"order by groupname",
275280
],
276281
[
277282
'database' => 'defaultdb',
278-
'query' => "select groupname from usergroups where uid=:username and groupname like 'stud%'",
283+
'query' =>
284+
"select groupname from usergroups " .
285+
"where uid=:username and groupname like 'stud%' " .
286+
"order by groupname",
279287
],
280288
];
281289

282290
$ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password');
283291
asort($ret);
284-
asort($ret['groupname']);
285292
$this->assertCount(3, $ret);
286293
$this->assertEquals($ret, [
287294
'email' => ['[email protected]'],
@@ -300,16 +307,21 @@ public function testMultiQuerySubsequentAppendSuccess(): void
300307
$this->config['attr_queries'] = [
301308
[
302309
'database' => 'defaultdb',
303-
'query' => "select groupname from usergroups where uid=:username and groupname like 'stud%'",
310+
'query' =>
311+
"select groupname from usergroups " .
312+
"where uid=:username and groupname like 'stud%'" .
313+
" order by groupname",
304314
],
305315
[
306316
'database' => 'defaultdb',
307-
'query' => "select groupname from usergroups where uid=:username and groupname like '%sers'",
317+
'query' =>
318+
"select groupname from usergroups " .
319+
"where uid=:username and groupname like '%sers' " .
320+
"order by groupname",
308321
],
309322
];
310323
$ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password');
311324
asort($ret);
312-
asort($ret['groupname']);
313325
$this->assertCount(3, $ret);
314326
$this->assertEquals($ret, [
315327
'email' => ['[email protected]'],

‎tests/src/Auth/Source/SQL2SingleAuthTest.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,23 @@
1818
*/
1919
class SQL2SingleAuthTest extends TestCase
2020
{
21+
/** @var array<string, string> */
2122
private array $info = ['AuthId' => 'testAuthId'];
2223

2324
protected string $extraSqlSelectColumns = '';
2425

2526
protected string $extraSqlAndClauses = ' and password=:password';
2627

2728

28-
/* Different tests require different combinations of databases, auth queries and attr queries.
29+
/**
30+
* Different tests require different combinations of databases, auth queries and attr queries.
2931
* This function returns a config with the requested number of each.
32+
*
33+
* @param int $numDatabases
34+
* @param int $numAuthQueries
35+
* @param array<string> $authQueryAttributes
36+
* @param int $numAttrQueries
37+
* @return array<string, mixed>
3038
*/
3139
protected function getConfig(
3240
int $numDatabases,
@@ -372,7 +380,6 @@ public function testMultipleAuthQueryStudentWithMultipleEnrolmentsSuccess(): voi
372380
$ret = (new SQL2Wrapper($this->info, $config))->callLogin('3', 'password');
373381
asort($ret);
374382
$this->assertCount(6, $ret);
375-
$this->assertCount(2, $ret['unit_code']);
376383
$this->assertEquals($ret, [
377384
'uid' => ['3'],
378385
'email' => ['[email protected]'],
@@ -412,7 +419,6 @@ public function testMultipleAuthQueryStudentWithSingleEnrolmentSuccess(): void
412419
$ret = (new SQL2Wrapper($this->info, $config))->callLogin('5', 'password');
413420
asort($ret);
414421
$this->assertCount(6, $ret);
415-
$this->assertCount(1, $ret['unit_code']);
416422
$this->assertEquals($ret, [
417423
'uid' => ['5'],
418424
'email' => ['[email protected]'],

‎tests/src/Auth/Source/SQLTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class SQLTest extends TestCase
2020
/** @var array<string, string> */
2121
private array $info = ['AuthId' => 'testAuthId'];
2222

23-
/** @var array<string, string|null> */
23+
/** @var array<string, list<string>|string|null> */
2424
private array $config = [
2525
"dsn" => 'sqlite:file::memory:?cache=shared',
2626
"username" => "notused",

0 commit comments

Comments
 (0)