Expires Value that occurs in the past not deleting

[ssnukala]

Hi Thanks for the code. This works pretty well.

I was testing this out for my application and while testing some use cases I bumped into a potential issue, just want to bring this to your attention and see if this is intended, if not would like to propose a fix.

Here are my steps

1. I login with remember me
2. I am using database so it inserts a record in a database table that I specify and stores the triplet
3. I hit logout - and it deletes the remember me record from the database all is well so far

Now I am looking to test the auto login so I manipulate the database record a little

1. I login with remember me
2. I am using database so it inserts a record in a database table that I specify and stores the triplet
3. I go to the table and update the expires date to some date in the past manually
4. Now I hit logout - even though I am invoking CleanTriplet at logout, it does not Find the record because of the " $sql = " AND {$this->expiresColumn} > NOW() LIMIT 1"; on line 29 https://github.com/gbirke/rememberme/blob/master/src/Rememberme/Storage/PDO.php#L29 So it does not delete the record.
5. If I try to login again with "remember me" checked again, it creates a 2nd record. for the same user, with different tokens.

But

1. IF i update the database record and make the Expires column to a future value, the SQL above will detect the record and logs me in.

So the issue here is if the token expiry date is in the past it just keeps the record forever.

So I am thinking the "AND {$this->expiresColumn} > NOW() " should be removed from the Where clause and you should check the resulting record to see if it expired, if it does then delete the record and return false.

That way we can make sure we don't leave zombie records that will never be deleted.

Please let me know if there is another use case where the "AND {$this->expiresColumn} > NOW() " is applicable.

If you look at the File option, it is just a check to see if the file is there or not, we are not looking into the file to see if it has an expiry date that is before today, but with database records we have that flexibility.

Hope this helps.

[ssnukala]

I submitted a fix, please review and let me know if that works and you can merge it to your branch.

[gbirke]

Thanks for pointing out this issue!

I won't merge your propesed fix since I'd rather integrate the expiration of tokens into the storage adapter interface and the authenticator. This is definitely a planned feature for the next version.

[gbirke]

Implemented in Version 2.0