Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow case-sensitive check for token #4

Merged
merged 2 commits into from Mar 25, 2014
Merged

Allow case-sensitive check for token #4

merged 2 commits into from Mar 25, 2014

Conversation

bakura10
Copy link
Member

ping @Ocramius @mac_nibblet

I realized that by default, SQL do case-insenstivie checks for token. This can be an issue as tokens are fetched by the token itself.

I couldn't find anything in Doctrine 2 mapping to force having utf8_bin (which makes case-senstivie checks instead of default utf8_general), so I used a columnDefinition.

@@ -6,7 +6,7 @@
http://doctrine-project.org/schemas/orm/doctrine-mapping.xsd">

<mapped-superclass name="ZfrOAuth2\Server\Entity\AbstractToken">
<id name="token" type="string" length="40" />
<id name="token" type="string" column-definition="VARCHAR(40) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL" />
Copy link
Member

Choose a reason for hiding this comment

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

This is not portable, and will break any storage layer that isn't MySQL. No-go IMO.

Instead, fetch the token and compare it after fetch?

Copy link
Member

Choose a reason for hiding this comment

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

@bakura10
Copy link
Member Author

So options={"collation"="utf8_bin"} is portable?

@Ocramius
Copy link
Member

@bakura10 no, but at least it won't crash DDL statements for other storage layers.

The only portable approach is most probably by fetching and checking PHP-side

bakura10 added a commit that referenced this pull request Mar 25, 2014
Allow case-sensitive check for token
@bakura10 bakura10 merged commit 1d2c248 into master Mar 25, 2014
@bakura10 bakura10 deleted the utf8-bin branch March 25, 2014 09:02
@Ocramius
Copy link
Member

@bakura10 this is still not fixed btw - it needs checks in the repository as well.

@macnibblet
Copy link
Contributor

@bakura10 you just broke my entire project with this 👎 SQLITE does not support it thus integration tests fails :/

@bakura10
Copy link
Member Author

Really? Even with the "options" thing? @Ocramius told us it should not break anything.

@Ocramius
Copy link
Member

@bakura10 I was honestly expecting this to be ignored on sqlite =_=

Yeah, so I guess the right fix is really fetching and comparing

@bakura10
Copy link
Member Author

That's a bit strange, ZfrOAuth2 is unit-tested with SQLite, and all tests are passing.

@macnibblet
Copy link
Contributor

@bakura10 Any chance you can revert this with a notice for now so that it does not break my application.

1) AppIntegrationTest\Campaign\RestResource\CampaignResourceTest::testDelete
Doctrine\ORM\Tools\ToolsException: Schema-Tool failed with Error 'An exception occurred while executing 'CREATE TABLE oauth_access_tokens (token VARCHAR(40) NOT NULL COLLATE utf8_bin, client_id VARCHAR(60) DEFAULT NULL, owner_id INTEGER DEFAULT NULL, scopes CLOB NOT NULL, expires_at DATETIME DEFAULT NULL, PRIMARY KEY(token), CONSTRAINT FK_CA42527C19EB6921 FOREIGN KEY (client_id) REFERENCES oauth_clients (id) NOT DEFERRABLE INITIALLY IMMEDIATE, CONSTRAINT FK_CA42527C7E3C61F9 FOREIGN KEY (owner_id) REFERENCES users (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE)':

SQLSTATE[HY000]: General error: 1 no such collation sequence: utf8_bin' while executing DDL: CREATE TABLE oauth_access_tokens (token VARCHAR(40) NOT NULL COLLATE utf8_bin, client_id VARCHAR(60) DEFAULT NULL, owner_id INTEGER DEFAULT NULL, scopes CLOB NOT NULL, expires_at DATETIME DEFAULT NULL, PRIMARY KEY(token), CONSTRAINT FK_CA42527C19EB6921 FOREIGN KEY (client_id) REFERENCES oauth_clients (id) NOT DEFERRABLE INITIALLY IMMEDIATE, CONSTRAINT FK_CA42527C7E3C61F9 FOREIGN KEY (owner_id) REFERENCES users (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE)

/application/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/ToolsException.php:39
/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:116
/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:832
/application/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/SchemaTool.php:93
/application/tests/Util/DataFixtureTrait.php:60
/application/tests/Util/DataFixtureTrait.php:71
/application/tests/AppIntegrationTest/Campaign/RestResource/CampaignResourceTest.php:27

Caused by
Doctrine\DBAL\Exception\DriverException: An exception occurred while executing 'CREATE TABLE oauth_access_tokens (token VARCHAR(40) NOT NULL COLLATE utf8_bin, client_id VARCHAR(60) DEFAULT NULL, owner_id INTEGER DEFAULT NULL, scopes CLOB NOT NULL, expires_at DATETIME DEFAULT NULL, PRIMARY KEY(token), CONSTRAINT FK_CA42527C19EB6921 FOREIGN KEY (client_id) REFERENCES oauth_clients (id) NOT DEFERRABLE INITIALLY IMMEDIATE, CONSTRAINT FK_CA42527C7E3C61F9 FOREIGN KEY (owner_id) REFERENCES users (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE)':

SQLSTATE[HY000]: General error: 1 no such collation sequence: utf8_bin

/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractSQLiteDriver.php:83
/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:829
/application/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/SchemaTool.php:93
/application/tests/Util/DataFixtureTrait.php:60
/application/tests/Util/DataFixtureTrait.php:71
/application/tests/AppIntegrationTest/Campaign/RestResource/CampaignResourceTest.php:27

Caused by
Doctrine\DBAL\Driver\PDOException: SQLSTATE[HY000]: General error: 1 no such collation sequence: utf8_bin

/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:94
/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:92
/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:829
/application/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/SchemaTool.php:93
/application/tests/Util/DataFixtureTrait.php:60
/application/tests/Util/DataFixtureTrait.php:71
/application/tests/AppIntegrationTest/Campaign/RestResource/CampaignResourceTest.php:27

Caused by
PDOException: SQLSTATE[HY000]: General error: 1 no such collation sequence: utf8_bin

/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:92
/application/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:829
/application/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/SchemaTool.php:93
/application/tests/Util/DataFixtureTrait.php:60
/application/tests/Util/DataFixtureTrait.php:71
/application/tests/AppIntegrationTest/Campaign/RestResource/CampaignResourceTest.php:27

@bakura10
Copy link
Member Author

Ok so :/. Please see the other PR. I'll merge that in a few minutes.

@bakura10
Copy link
Member Author

I'm still curious about why ZfrOAuth2 unit tests do not fail :/.

@macnibblet
Copy link
Contributor

Different versions of sqlite perhaps ?

@Ocramius
Copy link
Member

@macnibblet probably an old sqlite version? Also, this PR should be reverted

@macnibblet
Copy link
Contributor

SQLite3 support => enabled
SQLite3 module version => 0.7-dev
SQLite Library => 3.7.13

@bakura10
Copy link
Member Author

I have version 3.7.13 too... :/.

@macnibblet
Copy link
Contributor

Wtf

@bakura10
Copy link
Member Author

nvm. You're right. Actually there is no functional tests in ZfrOAuth2. I experienced the same issue in my code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants