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

[clarkphp]

Here is the issue report which this pull request addresses - #6428

One-line addition to the AbstractAdapter, a set of unit tests specific for IBM i / DB2 (run on the IBM, of course, against a DB2 table), and some additional TestConfiguration constants defined for controlling the tests.

Bear with me: this is my first ZF2 pull request!

[Ocramius]

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)

[Ocramius]

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.

[clarkphp]

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?

[Ocramius]

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.

[clarkphp]

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?

[clarkphp]

Is this failing the build because the relevant extension does not exist on the build system? Those constants shouldn't be referenced, though, because TESTS_ZEND_AUTH_ADAPTER_DBTABLE_DB2_ENABLED is set to false in TestConfiguration.php.dist. Or am I missing something obvious. I'm rather good at that.

[samsonasik]

change to PSR-2 cs

[clarkphp]

Ah, thanks. I see now. I had missed this in the Travis output: 2) ZendTest/Authentication/Adapter/DbTable/CredentialTreatmentAdapterDb2Test.php (indentation, braces)

There is another code style failure in a file I did not change.

Two questions:
a) Do I issue a separate, unrelated pull request for that file, or include a code style fix into an update of this pull request (putting it with this PR would be polluting it a bit, I think).
b) How do I update this pull request with the code style fix, since another commit will be needed? I'll look at GitHub Help in the meantime, and scour ZF2 contributor information.

[samsonasik]

you don't need separate PR, just add more commit and do git push to your branch, the PR will automatically updated

[clarkphp]

Now to question a) above, to deal with this Travis build (Code style) failure:

1. Zend/View/Stream.php (short_tag)

I did not make a change to that file, and the Code Style failure appears to be spurious, anyway.
From Zend/View/Stream.php

        /**
         * Convert <?= ?> to long-form <?php echo ?> and <?php ?> to <?php ?>
         *
         */
        $this->data = preg_replace('/\<\?\=/',          "<?php echo ",  $this->data);
        $this->data = preg_replace('/<\?(?!xml|php)/s', '<?php ',       $this->data);

The php-cs-fixer is not configured to ignore short tags found in comments.

[samsonasik]

You should modify only your changed/added file. Fixing not related file to your PR should in separate PR

Warm regards,

Abdul Malik Ikhsan

Pada 2 Agt 2014, pukul 20.07, Clark Everetts notifications@github.com menulis:

Now to question a) above, to deal with this Travis build (Code style) failure:

1. Zend/View/Stream.php (short_tag)

I did not make a change to that file, and the Code Style failure appears to be spurious, anyway.
From Zend/View/Stream.php

    /**
     * Convert <?= ?> to long-form <?php echo ?> and <?php ?> to <?php ?>
     *
     */
    $this->data = preg_replace('/\<\?\=/',          "<?php echo ",  $this->data);
    $this->data = preg_replace('/<\?(?!xml|php)/s', '<?php ',       $this->data);

The php-cs-fixer is not configured to ignore short tags found in comments.

—
Reply to this email directly or view it on GitHub.

[weierophinney]

These two need to be defined in init() after testing for DB 2 support, as otherwise, they raise notices due to not being present if DB2 is not available. I can change that on merge.