Error while logging in with "Remember me" using Postgres

[RomeroMsk]

With Postgres as database driver I'm having an issue when trying to log in with "Remember me" checked:

UserFrosting Application Error
The application could not run because of the following error:

Details

Type: PDOException
Code: 42883
Message: SQLSTATE[42883]: Undefined function: 7 ERROR: function sha1(unknown) does not exist LINE 1: ..., token, persistent_token, expires_at) VALUES($1, SHA1($2), ... ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts.
File: .../app/vendor/birke/rememberme/src/Rememberme/Storage/PDO.php
Line: 58

So, using this library (gbirke/rememberme) makes UserFrosting database dependent.

Any chances to get a fix/workaround for this?

[alexweissman]

Does Postgres not support SHA1? Is it possible that there is some extension that you need to install?

[RomeroMsk]

There is no such function in PostgreSQL, unfortunately. The only way I've found is ENCODE(DIGEST(..., 'sha1'), 'hex'), but it requires cryptographic libraries to be installed for Postgres.

[Silic0nS0ldier]

Hmm. Looking at the source code, it has raw SQL queries. Wouldn't surprise me if this had issues on other dbms too.

[RomeroMsk]

Yep, the code of this library doesn't seem to be DB agnostic.

[alexweissman]

If we can find a pre-existing implementation of improved persistent login for Eloquent, I'd be fine if someone wants to work on replacing the gbirke library. Otherwise, we may have to implement our own version.

[RomeroMsk]

I think the easiest way to fix this will be own implementation of PDO storage class for gbirke/rememberme library:

1. Create a corresponding PHP script in app/sprinkles/account/src dir.
2. Copy the existing code from vendor/birke/rememberme/src/Rememberme/Storage/PDO.php and replace all SHA1() in SQL queries with PHP sha1() function calls.
3. Use this new file in UserFrosting\Sprinkle\Account\Authenticate\Authenticator class instead of Birke\Rememberme\Storage\PDO.

[alexweissman]

Did you see this commit: gbirke/rememberme#20 (comment) ?
Not sure if that solves the problem with Postgres as well.

My recommendation would be to fork @gbirke's repo, make the changes, and then submit a PR. He seems pretty willing to merge PRs as long as they adhere to standards and pass unit tests. It would be simpler than for us to try and maintain a separate fork.

[RomeroMsk]

I didn't see this commit itself yet, but I saw this solution with custom SHA1 function before. Unfortunately, it requires an additional Postgresql module to be installed. I'm going to check it when I will be able to get this module installed.
But anyways it is still a compatibility issue that makes your product DB dependent ;)

[alexweissman]

Maybe leave a comment on that commit? Ask them why they didn't just use PHP's sha1 function instead?

[RomeroMsk]

There is an issue already (from me): gbirke/rememberme#19
It was mentioned in that comment, btw.

[gbirke]

The history why I do the hashing in SQL instead of PHP is a bit hazy (that was 2-3 years ago), I dimly remember having problems writing a unit test and then settling on doing it in SQL because it was the easiest. At the moment I have no spare time to change it, but if anyone supplies a patch, I'll gladly merge it and release a new version of the library.

[RomeroMsk]

Well, to test a patch with UF we need to bump the version of gbirke/rememberme used in UF to the latest one.

[Silic0nS0ldier]

Not necessarily. Assuming we are already using the latest version (and therefore there are no breaking changes), its possible to just replace the respective folder in vendor with the updated source. (prevents usage of composer, but fine for testing purposes).

I believe there is also a way of forcing composer to use grab a specific version, regardless of what other dependencies request. (fairly sure I've done this before, not that I remember how...)

[RomeroMsk]

UF uses version 1.0.4. The latest is 2.0. And there are breaking changes (at least with storage classes naming). So the problem is if someone will make a patch for the current version of gbirke/rememberme and @gbirke will release a new version, it will not affect the UF.

[RomeroMsk]

I'm going to check it when I will be able to get this module installed.

After installing the module and adding the SHA1 function to Postgresql I still have an issue while logging in with "Remember me". This time we have a problem with IF:

UserFrosting Application Error
The application could not run because of the following error:

Details

Type: PDOException
Code: 42883
Message: SQLSTATE[42883]: Undefined function: 7 ERROR: function if(boolean, integer, integer) does not exist LINE 1: SELECT IF(SHA1($1) = token, 1, -1) AS token_match FROM uf_pe... ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts.
File: .../app/vendor/birke/rememberme/src/Rememberme/Storage/PDO.php
Line: 32

This was solved in gbirke/rememberme#20 PR mentioned earlier, but to use this fix we (again) need the latest version of gbirke/rememberme.

[alexweissman]

We can talk about updating UF to use gbirke/rememberme 2.0. Can you join us in chat?

[RomeroMsk]

Yes, I can. Please invite me (romeromsk) to the channel related to this topic.

[alexweissman]

Cool, want to talk about it in #general?

[RomeroMsk]

@gbirke would you please add the 2.0 version/tag, so we can update the composer dependency?

[gbirke]

I have created the 2.0.1 release which uses PHPs sha1 function instead of the SQL one.

[RomeroMsk]

Great, tnx @gbirke.
@alexweissman, so now we just need to check it all together. Please change the birke/rememberme version to 2.0.1 in composer.json after reviewing my changes.

[alexweissman]

Cool, I might be able to do it tonight. Btw, you can actually make the change directly and commit to the same branch, and the PR should get updated.

[RomeroMsk]

Ok, done.

[RomeroMsk]

@alexweissman, would you please merge it today?

[alexweissman]

Alright, done! See 9c9316b.

[RomeroMsk]

Tested after deleting the custom SHA1() Postgres function, works. Thanks, @alexweissman and @gbirke!