Skip to content

Commit facb328

Browse files
nathanjrobertsonnathanjrobertson
committed
Support password_verify() in version 2. Provide v1 compat interfaces.
1 parent 2286d34 commit facb328

13 files changed

+465
-108
lines changed

‎src/Auth/Source/PasswordVerify1Compat.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Module\sqlauth\Auth\Source;
6+
7+
/**
8+
* @package SimpleSAMLphp
9+
*/
10+
11+
class PasswordVerify1Compat extends SQL2
12+
{
13+
/**
14+
* Constructor for this authentication source.
15+
*
16+
* @param array $info Information about this authentication source.
17+
* @param array $config Configuration.
18+
*/
19+
public function __construct(array $info, array $config)
20+
{
21+
/* Transform PasswordVerify (version 1) config to SQL2 config
22+
* Version 1 supported only one database, but multiple queries. The first query was defined
23+
* to be the "authentication query", all subsequent queries were "attribute queries".
24+
*/
25+
$v2config = [
26+
'sqlauth:SQL2',
27+
'databases' => [
28+
'default' => [
29+
'dsn' => $config['dsn'],
30+
'username' => $config['username'],
31+
'password' => $config['password'],
32+
]
33+
],
34+
35+
'auth_queries' => [
36+
'default' => [
37+
'database' => 'default',
38+
'query' => is_array($config['query']) ? $config['query'][0] : $config['query'],
39+
'password_verify_hash_column' => 'passwordhash',
40+
]
41+
],
42+
];
43+
44+
if (array_key_exists('username_regex', $config)) {
45+
$v2config['auth_queries']['default']['username_regex'] = $config['username_regex'];
46+
}
47+
48+
// Override the default passwordhash column if configured
49+
if (array_key_exists('passwordhash_column', $config)) {
50+
$v2config['auth_queries']['default']['password_verify_hash_column'] = $config['passwordhash_column'];
51+
}
52+
53+
if (is_array($config['query']) && count($config['query']) > 1) {
54+
$v2config['attr_queries'] = [];
55+
for ($i = 1; $i < count($config['query']); $i++) {
56+
$v2config['attr_queries']['query' . $i] = [
57+
'database' => 'default',
58+
'query' => $config['query'][$i],
59+
];
60+
}
61+
}
62+
63+
parent::__construct($info, $v2config);
64+
}
65+
}

‎src/Auth/Source/SQL1Compat.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,6 @@
55
namespace SimpleSAML\Module\sqlauth\Auth\Source;
66

77
/**
8-
* Simple SQL authentication source
9-
*
10-
* This class is an example authentication source which authenticates an user
11-
* against a SQL database.
12-
*
138
* @package SimpleSAMLphp
149
*/
1510

‎src/Auth/Source/SQL2.php‎

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,14 @@ public function __construct(array $info, array $config)
179179
$this->authQueries[$authQueryName]['extract_userid_from'] = $authQueryConfig['extract_userid_from'];
180180
}
181181

182+
if (array_key_exists('password_verify_hash_column', $authQueryConfig)) {
183+
if (!is_string($authQueryConfig['password_verify_hash_column'])) {
184+
throw new Exception('Optional parameter \'password_verify_hash_column\' for authentication source ' .
185+
$this->authId . ' was provided and is expected to be a string. Instead it was: ' .
186+
var_export($authQueryConfig['password_verify_hash_column'], true));
187+
}
188+
$this->authQueries[$authQueryName]['password_verify_hash_column'] = $authQueryConfig['password_verify_hash_column'];
189+
}
182190
}
183191
} else {
184192
throw new Exception('Missing required attribute \'auth_queries\' for authentication source ' . $this->authId);
@@ -408,7 +416,14 @@ protected function login(
408416
$db = $this->connect($queryConfig['database']);
409417

410418
try {
411-
$data = $this->executeQuery($db, $queryConfig['query'], ['username' => $username, 'password' => $password]);
419+
if (array_key_exists('password_verify_hash_column', $queryConfig)) {
420+
// We will verify the password using password_verify() later, so we do not
421+
// pass the password to the query.
422+
$data = $this->executeQuery($db, $queryConfig['query'], ['username' => $username]);
423+
} else {
424+
// Pass both username and password to the query
425+
$data = $this->executeQuery($db, $queryConfig['query'], ['username' => $username, 'password' => $password]);
426+
}
412427
} catch (PDOException $e) {
413428
Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
414429
' failed with error: ' . $e->getMessage());
@@ -417,14 +432,64 @@ protected function login(
417432

418433
// If we got any rows, the authentication succeeded. If not, try the next query.
419434
if (count($data) > 0) {
435+
/* This is where we need to run password_verify() if we are using password_verify() to
436+
* authenticate hashed passwords that are only stored in the database. */
437+
if (array_key_exists('password_verify_hash_column', $queryConfig)) {
438+
$hashColumn = $queryConfig['password_verify_hash_column'];
439+
if (!array_key_exists($hashColumn, $data[0])) {
440+
Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
441+
' did not return expected hash column \'' . $hashColumn . '\'');
442+
throw new Error\Error('WRONGUSERPASS');
443+
}
444+
445+
$passwordHash = null;
446+
foreach ($data as $row) {
447+
if ((!array_key_exists($hashColumn, $row)) || is_null($row[$hashColumn])) {
448+
Logger::error(sprintf(
449+
'sqlauth:%s: column `%s` must be in every result tuple.',
450+
$this->authId,
451+
$hashColumn,
452+
));
453+
throw new Error\Error('WRONGUSERPASS');
454+
}
455+
if ($passwordHash === null) {
456+
$passwordHash = $row[$hashColumn];
457+
} elseif ($passwordHash != $row[$hashColumn]) {
458+
Logger::error(sprintf(
459+
'sqlauth:%s: column %s must be THE SAME in every result tuple.',
460+
$this->authId,
461+
$hashColumn,
462+
));
463+
throw new Error\Error('WRONGUSERPASS');
464+
}
465+
}
466+
467+
if (($passwordHash === null) || (!password_verify($password, $passwordHash))) {
468+
Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
469+
' password verification failed');
470+
/* Authentication with verify_password() failed, however that only means that
471+
* this auth query did not succeed. We should try the next auth query if any. */
472+
continue;
473+
}
474+
475+
Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
476+
' password verification using password_verify() succeeded');
477+
}
478+
420479
Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
421480
' succeeded with ' . count($data) . ' rows');
422481
$queryConfig['_winning_auth_query'] = true;
482+
423483
if (array_key_exists('extract_userid_from', $queryConfig)) {
424484
$queryConfig['_extracted_userid'] = $data[0][$queryConfig['extract_userid_from']];
425485
}
426486
$winning_auth_query = $queryname;
427-
$this->extractAttributes($attributes, $data, []);
487+
488+
$forbiddenAttributes = [];
489+
if (array_key_exists('password_verify_hash_column', $queryConfig)) {
490+
$forbiddenAttributes[] = $queryConfig['password_verify_hash_column'];
491+
}
492+
$this->extractAttributes($attributes, $data, $forbiddenAttributes);
428493

429494
// The first auth query that succeeds is the winning one, so we can stop here.
430495
break;

‎tests/bootstrap.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,20 @@
55
$projectRoot = dirname(__DIR__);
66
require_once($projectRoot . '/vendor/autoload.php');
77

8+
89
// Load our wrapper class to get around login() being declared protected in SQL.php
910
require_once($projectRoot . '/tests/src/Auth/Source/SQLWrapper.php');
1011
require_once($projectRoot . '/tests/src/Auth/Source/SQL2Wrapper.php');
1112
require_once($projectRoot . '/tests/src/Auth/Source/SQL1CompatWrapper.php');
1213
require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerifyWrapper.php');
14+
require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerify1CompatWrapper.php');
1315

14-
// Get around SQL1CompatTest extending SQLTest and not being able to find SQLTest
16+
// We use inheritance quite extensively in our test cases, so we need to
17+
// make sure all the classes that are subclassed are loaded before we run any tests.
18+
require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerifyTest.php');
1519
require_once($projectRoot . '/tests/src/Auth/Source/SQLTest.php');
20+
require_once($projectRoot . '/tests/src/Auth/Source/SQL2SimpleTest.php');
21+
require_once($projectRoot . '/tests/src/Auth/Source/SQL2SingleAuthTest.php');
1622

1723
// Symlink module into ssp vendor lib so that templates and urls can resolve correctly
1824
$linkPath = $projectRoot . '/vendor/simplesamlphp/simplesamlphp/modules/sqlauth';

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\Module\sqlauth\Auth\Source;
6+
7+
/**
8+
* Test for the core:AttributeLimit filter.
9+
*
10+
* @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit
11+
*/
12+
class PasswordVerify1CompatTest extends PasswordVerifyTest
13+
{
14+
protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\PasswordVerify1CompatWrapper';
15+
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\Module\sqlauth\Auth\Source;
6+
7+
use SimpleSAML\Module\sqlauth\Auth\Source\PasswordVerify1Compat;
8+
9+
/**
10+
* This class only exists to allow us to call the protected login() method.
11+
* The calling method in UserPassBase.php doesn't return, so can't be used
12+
* from PHPUnit. So we do this just to be able to unit test the login()
13+
* method in SQL.php
14+
*/
15+
16+
class PasswordVerify1CompatWrapper extends PasswordVerify1Compat
17+
{
18+
/**
19+
* @param array<mixed> $info
20+
* @param array<mixed> $config
21+
*/
22+
public function __construct(array $info, array $config)
23+
{
24+
parent::__construct($info, $config);
25+
}
26+
27+
28+
/**
29+
* @return array<mixed>
30+
*/
31+
public function callLogin(string $username, string $password): array
32+
{
33+
return $this->login($username, $password);
34+
}
35+
}

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
*/
1515
class PasswordVerifyTest extends TestCase
1616
{
17+
// Subclasses can override this to test other wrapper classes
18+
protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\PasswordVerifyWrapper';
19+
1720
/** @var array<string, string> */
1821
private array $info = ['AuthId' => 'testAuthId'];
1922

@@ -83,7 +86,7 @@ public function testBasicSingleSuccess(): void
8386
{
8487
// Correct username/password
8588
$this->config['query'] = "select givenName, email, passwordhash from users where uid=:username";
86-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1');
89+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1');
8790
asort($ret);
8891
$this->assertCount(2, $ret);
8992
$this->assertEquals($ret, [
@@ -98,7 +101,7 @@ public function testBasicSingleFailedLogin(): void
98101
$this->expectException(\SimpleSAML\Error\Error::class);
99102
// Wrong username/password
100103
$this->config['query'] = "select givenName, email, passwordhash from users where uid=:username";
101-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice', 'wrong');
104+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong');
102105
$this->assertCount(0, $ret);
103106
}
104107

@@ -107,7 +110,7 @@ public function testBasicSingleFailedLoginNonExisting(): void
107110
$this->expectException(\SimpleSAML\Error\Error::class);
108111
// Wrong username/password
109112
$this->config['query'] = "select givenName, email, passwordhash from users where uid=:username";
110-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('henry', 'boo');
113+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('henry', 'boo');
111114
$this->assertCount(0, $ret);
112115
}
113116

@@ -117,7 +120,7 @@ public function testBasicSingleFailedLoginNonExistingNoPassword(): void
117120
$this->expectException(\SimpleSAML\Error\Error::class);
118121
// Wrong username/password
119122
$this->config['query'] = "select givenName, email, passwordhash from users where uid=:username";
120-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice2', '');
123+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice2', '');
121124
$this->assertCount(0, $ret);
122125
}
123126

@@ -129,7 +132,7 @@ public function testJoinSingleSuccess(): void
129132
select u.givenName, u.email, ug.groupname, passwordhash
130133
from users u left join usergroups ug on (u.uid=ug.uid)
131134
where u.uid=:username ";
132-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1');
135+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1');
133136
asort($ret);
134137
asort($ret['groupname']);
135138
$this->assertCount(3, $ret);
@@ -148,7 +151,7 @@ public function testJoinSingleFailedLogin(): void
148151
select u.givenName, u.email, ug.groupname, passwordhash
149152
from users u left join usergroups ug on (u.uid=ug.uid)
150153
where u.uid=:username";
151-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice', 'wrong');
154+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong');
152155
$this->assertCount(0, $ret);
153156
}
154157

@@ -159,7 +162,7 @@ public function testMultiQuerySuccess(): void
159162
"select givenName, email, passwordhash from users where uid=:username",
160163
"select groupname from usergroups where uid=:username",
161164
];
162-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1');
165+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1');
163166
asort($ret);
164167
asort($ret['groupname']);
165168
$this->assertCount(3, $ret);
@@ -178,7 +181,7 @@ public function testMultiQueryFailedLogin(): void
178181
"select givenName, email, passwordhash from users where uid=:username",
179182
"select groupname from usergroups where uid=:username",
180183
];
181-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice', 'wrong');
184+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong');
182185
$this->assertCount(0, $ret);
183186
}
184187

@@ -191,7 +194,7 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void
191194
"select groupname from usergroups where uid=:username and groupname like '%nomatch%'",
192195
"select groupname from usergroups where uid=:username and groupname like 'stud%'",
193196
];
194-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1');
197+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1');
195198
asort($ret);
196199
asort($ret['groupname']);
197200
$this->assertCount(3, $ret);
@@ -210,7 +213,7 @@ public function testMultiQuerySubsequentAppendSuccess(): void
210213
"select groupname from usergroups where uid=:username and groupname like 'stud%'",
211214
"select groupname from usergroups where uid=:username and groupname like '%sers'",
212215
];
213-
$ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1');
216+
$ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1');
214217
asort($ret);
215218
asort($ret['groupname']);
216219
$this->assertCount(3, $ret);

0 commit comments

Comments
 (0)