Skip to content

DoctrineTokenProvider issue with postgresql #21467

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

Closed
PF4Public opened this issue Jan 30, 2017 · 11 comments
Closed

DoctrineTokenProvider issue with postgresql #21467

PF4Public opened this issue Jan 30, 2017 · 11 comments

Comments

@PF4Public
Copy link
Contributor

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? yes
Symfony version 3.2.1

There is an issue with DoctrineTokenProvider if using postgresql.

The problem is that by default postgresql is not case-sensitive unless you apply quotes to your arguments.
Thus creating lastUsed table as suggested might result in creating it all-lowercase. All is fine untill you hit:

    return new PersistentToken($row['class'], $row['username'], $series, $row['value'], new \DateTime($row['lastUsed']));

which will throw exception as there's no such field in table. Looks like changing the field to be exactly lastUsed might help, but for me exceptions start happening even earlier, since this time some other queries could not obtain this field properly. I ended up replacing 'lastUsed' to be 'lastused' in that particular line.

Therefore I wonder is there any better approach to fix this?

As I'm afraid fixing this way might break this code for mysql or any other database.

@sstok
Copy link
Contributor

sstok commented Feb 2, 2017

Status: Works for me

PostgreSQL is indeed case-insensitive by default.

Are you using a custom UserProvider of sorts? Because there is no UserProviderInterface for PDO/PostgreSQL provided by default. If so, you need to update your query, if you are using Doctrine ORM make sure to use `lastUsed` in name option of the column, so Doctrine quotes it using the correct platform.

@PF4Public
Copy link
Contributor Author

I tried to use custom user provider, but using in-memory at the moment. And indeed reverted that line back and it still works properly. Maybe something was wrong in custom user provider. I'll give it another try with custom user provider soon.
Thanks

@PF4Public
Copy link
Contributor Author

Aaaand gotcha!
It works fine in prod environment, but just now I tried to access my dev environment with rememberme cookie and it fails now. Opening another tab with the same cookie, but accessing prod environment goes just fine! Trying dev environment again - it fails.
The error is

 Notice: Undefined index: lastUsed
 500 Internal Server Error - ContextErrorException 

in vendor/symfony/symfony/src/Symfony/Bridge/Doctrine/Security/RememberMe/DoctrineTokenProvider.php at line 71  -
at DoctrineTokenProvider ->loadTokenBySeries ('***')
in vendor/symfony/symfony/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php at line 71  +
at PersistentTokenBasedRememberMeServices ->processAutoLoginCookie (array('***', '***'), object(Request))
in vendor/symfony/symfony/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php at line 119  +
at AbstractRememberMeServices ->autoLogin (object(Request))
in vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/RememberMeListener.php at line 75  +
at RememberMeListener ->handle (object(GetResponseEvent))
in var/cache/dev/classes.php at line 5211  +

As I've mentioned earlier I'm still using in-memory provider.
Clearing symfony cache didn't help.

That happens with the column in question being all-lower-case.

Should it be modified to be exactly "lastUsed" in the database, another error shows up:

An exception occurred while executing 'SELECT class, username, value, lastUsed FROM rememberme_token WHERE series=?' with params ["***"]:

SQLSTATE[42703]: Undefined column: 7 ERROR: column "lastused" does not exist
LINE 1: SELECT class, username, value, lastUsed FROM rememberme_toke...
^
500 Internal Server Error - InvalidFieldNameException
2 linked Exceptions:

    PDOException » PDOException » 

Also, now that the column is exactly "lastUsed" it is now broken for prod environment as well.

Reverting the column in the database back to all-lower-case fixes it for prod environment, but not for the dev environment, which is still broken with aforementioned "Notice: Undefined index: lastUsed" message.

Now another experiment.
Clear cookies and login via dev environment - all is fine. Until I open prod environment with the same cookies. After that accessing dev environment fails with "Notice: Undefined index: lastUsed" message.

What is wrong here?

@PF4Public PF4Public reopened this Feb 17, 2017
@PF4Public
Copy link
Contributor Author

Looks like
$sql = 'SELECT class, username, value, lastUsed'
is inconsistent with
return new PersistentToken($row['class'], $row['username'], $series, $row['value'], new \DateTime($row['lastUsed']));

@jakzal
Copy link
Contributor

jakzal commented Mar 1, 2017

@PF4Public could you please fork the standard edition and reproduce this issue on a new branch with a minimum amount of code? I'd be happy to look into this, but first need a reproducible scenario.

@PF4Public
Copy link
Contributor Author

@jakzal Please inspect my fork here: https://github.com/PF4Public/symfony-standard
You can see my minimal project there. I didn't commit "vendors" tree as well as I didn't bother to remove unused "use" statements. There you can find sql file for postgres database table. Note that the field in question created as "lastUsed".
After navigating to / location it properly redirects to /login location. Which presents login page. However entering proper credentials (according to security.yml) fails with

InvalidFieldNameException: An exception occurred while executing 'INSERT INTO rememberme_token (class, username, series, value, lastUsed) VALUES (?, ?, ?, ?, ?)' with params ["Symfony\Component\Security\Core\User\User", "test", ....

SQLSTATE[42703]: Undefined column: 7 ERROR: column "lastused" of relation "rememberme_token" does not exist
LINE 1: ...rememberme_token (class, username, series, value, lastUsed) ...

So I still suppose that all sql statements do not adequately quote the "lastUsed" field. Which is partially solved if replacing lastUsed with lastused in table, but still stumbles on $row['lastUsed'] which expects it to be camel-cased.

@xabbuh
Copy link
Member

xabbuh commented Apr 14, 2017

This looks really weird (as you can see in the exception message the statement does not use lastusedbut indeedlastUsed`).

@PF4Public
Copy link
Contributor Author

It does not look weird to me.
Postgresql will interpret the line INSERT INTO rememberme_token (class, username, series, value, lastUsed) as INSERT INTO rememberme_token (class, username, series, value, lastused) unless some further function call arranges quotes for lastUsed, but this never happened, thus the exception.

@fabpot
Copy link
Member

fabpot commented Feb 22, 2018

@PF4Public Would you be interested in working on a patch for this issue?

@PF4Public
Copy link
Contributor Author

As I've already mentioned, replacing 'lastUsed' with 'lastused' with all-lowercase fields in database makes it work for me.

It looks like the key issue here is the difference between how MySQL and PostgreSQL treat unquoted strings in field names in SQL queries. Therefore statements from doctrine-bridge / Security / RememberMe / DoctrineTokenProvider.php work differently for different databases.
So to fix this there are two approaches IMHO:

  1. rewrite DoctrineTokenProvider.php to use lowercase field names only
  2. ensure proper quoting on a higher level in database abstraction

As for the 1 point - it is relatively easily done, and I believe will not interfere with any other Symfony code.
Point 2 however is the proper way of fixing this IMHO, but it requires a better knowledge of Symfony code and I'm afraid I cannot help with it for I'm not familiar enough with Symfony internals.

@sstok
Copy link
Contributor

sstok commented Mar 10, 2018

It looks like the key issue here is the difference between how MySQL and PostgreSQL treat unquoted strings in field names in SQL queries.

Like I said "PostgreSQL is indeed case-insensitive by default.", when you quote the field/table name it's used as intended (the casing is preserved) otherwise it's lowercase. This is conform the SQL99 standard btw 👍

The SQL:2008 and SQL-99 standards define databases to be case insensitive for identifiers unless they are quoted. Lower case characters may be used in identifiers and keywords, but are considered to be their upper case counterparts.

MySQL can actually work bit a funky here when your on a Case sensitive filesystem and using MyISAM, because the table structure is stored in a single file which equals the table name. So the table does exist despite the casing difference 😅

I think we can safely change lastUsed to lowercase, or otherwise use something like isset($row['lastUsed']) ? $row['lastUsed'] : $row['lastused']?

@fabpot fabpot closed this as completed Nov 26, 2018
fabpot added a commit that referenced this issue Nov 26, 2018
…DoctrineTokenProvider (PF4Public)

This PR was merged into the 2.8 branch.

Discussion
----------

[DoctrineBridge] fix case sensitivity issue in RememberMe\DoctrineTokenProvider

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | ?
| Fixed tickets | #21467
| License       | MIT
| Doc PR        | -

Commits
-------

0248d4f [DoctrineBridge] fix case sensitivity issue in RememberMe\DoctrineTokenProvider
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants