Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fix 6428 - authenticate() always fails on IBMi when using DB table-based authentication #6510

Merged
merged 4 commits into from Aug 4, 2014
Merged

Fix 6428 - authenticate() always fails on IBMi when using DB table-based authentication #6510

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52422c4
ZF-6428: authenticate() always fails on IBMi when using DB table-based
clarkphp Aug 1, 2014
4c80350
Merge branch 'fix-6428' of https://github.com/clarkphp/zf2 into fix-6428
clarkphp Aug 1, 2014
1218c2b
Correct a PSR-2 Code Style error I had missed before issuing PR.
clarkphp Aug 2, 2014
5d8e023
Format source code to better conform to ZF2 Coding Standards:
clarkphp Aug 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions library/Zend/Authentication/Adapter/DbTable/AbstractAdapter.php
View file
Open in desktop
Expand Up @@ -324,6 +324,11 @@ protected function authenticateQuerySelect(Sql\Select $dbSelect)
$resultIdentities = array();
// iterate result, most cross platform way
foreach ($result as $row) {
// ZF-6428 - account for db engines that by default return uppercase column names
if (isset($row['ZEND_AUTH_CREDENTIAL_MATCH'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this can be pushed down to Zend\Db instead (providing the actual requested column names when the DB layer scrambles the returned ones)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more specific, while I understand why this is happening (I think informix/firebase also do this mess) I don't like that we push these hacks into layers with other responsibilities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see where you're coming from, and initially, I thought the same, but technically, the ZF2 DB layer is not scrambling anything; rather, it is abiding by the column-casing set by the value of the driver option db2_attr_case, which can be one of DB2_CASE_LOWER, DB2_CASE_UPPER, or DB2_CASE_NATURAL. The default is DB2_CASE_UPPER, and is used by nearly every IBM shop we at Zend have worked with.

Since CredentialTreatmentAdapter::authenticateValidateResult() is always looking for a lowercase key, the value of db2_attr_case might or might not allow authenticate() to behave as expected, depending on the setting of db2_attr_case for a given application.

Should this really go somewhere in the DB layer, or should this go in an IBM-specific class overriding CredentialTreatmentAdapter::authenticateQuerySelect(), or should it stay where it is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is DB2_CASE_UPPER, and is used by nearly every IBM shop we at Zend have worked with.

That by far means that DB2 in general is doing it right :-)

The fix is simple, so I won't be too picky about it, but you may as well introduce it as @todo if Zend\Db can't abstract this ugly detail of the RDBMS away from us right now (and by abstracting, I mean reading the db2_attr_case and doing something about it).
I think the keys should just match the input queries, and so should the return values.

If you don't plan to fix that in this PR (an additional if condition is acceptable over a huge rework of Zend\Db), then please open an issue about it and reference it in a @todo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I'm thinking too hard about it, but the more I consider it, the less I see that Zend\Db needs any change. It is doing exactly what it is supposed to do: return column names cased according to db2_attr_case, regardless of the case used when the query was built.

The values returned are whatever was stored in the table; db2_attr_case affects only the column names, and thus the keys of our associative arrays.

I could be totally missing your point, and if so, I apologize, but is your preference special code in Zend\Db that looks for a ZEND_AUTH_CREDENTIAL_MATCH column (either upper or lower case) in a resultset, and forces it lower case, so that the authentication code sees the lowercase array key it is looking for?

$row['zend_auth_credential_match'] = $row['ZEND_AUTH_CREDENTIAL_MATCH'];
unset($row['ZEND_AUTH_CREDENTIAL_MATCH']);
}
$resultIdentities[] = $row;
}
} catch (\Exception $e) {
Expand Down
7 changes: 7 additions & 0 deletions tests/TestConfiguration.php.dist
View file
Open in desktop
Expand Up @@ -43,6 +43,13 @@ defined('TESTS_ZEND_OB_ENABLED') || define('TESTS_ZEND_OB_ENABLED', false);
*/
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_PDO_SQLITE_ENABLED') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_PDO_SQLITE_ENABLED', false);
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_PDO_SQLITE_DATABASE') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_PDO_SQLITE_DATABASE', ':memory:');
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_ENABLED') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_ENABLED', false);
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_HOSTNAME') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_HOSTNAME', '127.0.0.1');
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_PORT') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_PORT', 50000); // 446 and 50000 most common
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_USERNAME') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_USERNAME', ''); // '' == default Apache user QTMHHTTP
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_PASSWORD') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_PASSWORD', '');
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_DATABASE') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_DATABASE', '*LOCAL'); // '' also == *LOCAL
defined('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_CREDENTIAL_TABLE') || define('TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_CREDENTIAL_TABLE', 'YOURLIB.TESTING_USERS');

/**
* Zend\Auth\Adapter\Ldap online tests
Expand Down