This could be a module setting per site.

I read about this idea in Barry's blog, and I looks to me nice. :)

CommentFileSizeAuthor
#2 persistent_login-442782.patch3.09 KBmarkus_petrux
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented
Status: Active » Needs review

Well, here's a patch that implements this feature.

1) Adds a new module option "Persistent Logins per user limit"
2) Adds code to remove PL records beyond the specified limit per user.
3) Adds a new index to the PL table by uid and expires to optime the previous query.

I would appreciate if someone else could test it before being committed. It requires running update.php to create the new index.

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented
FileSize
persistent_login-442782.patch3.09 KB
markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented

I would like to quote Barry's comment on his blog (link above) so that this issue can be self documented :)

Clicking Logout deletes your current PL record from the database, so if the goal is to fill up the database with useless PL records, that won't work. However, the user could simply log in, erase all session cookies, and log in again over and over to achieve the same effect.

If this becomes a problem, it would be a simple matter to limit the number of unique PL records in the database per user.

And this is what the patch in #2 is trying to address :)

Easy in concept, but there was a tricky thing here, and that's the fact that we add a query to every page, and that needs to be as light as possible. Hence, I had to add an index to the PL table.

If there's no objection in a few days I'll go ahead and commit, so that I can focus on the other issues. :)

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented
Title: Limit the number of unique PL records in the database per user » Limit the number of PL records in the database per user
Status: Needs review » Fixed

Committed to CVS

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

dragonwize’s picture

dragonwize CreditAttribution: dragonwize commented

Love this patch.

Besides helping keep the PL table clean it helps with security by allowing the site admin to only allow the user to be logged in on one computer at any given time.

The part I am missing though is the reason there needs to be a query run on every page. As Barry describes, the only time another record is created is when the user logs in again without having logged out but has no cookie. ie. either they cleared their cookie or they are on another computer.

So would it not be better to preform the check on login instead of on every page?

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented

AFAICT, the check is performed at login time, just after a PL cookie has been created. Other than that, I'm not really sure what you mean. :-|

dragonwize’s picture

dragonwize CreditAttribution: dragonwize commented

I am questioning this comment from you above:

that's the fact that we add a query to every page

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented

Oh, I see. I was wrong in that comment. Who knows what I had in mind when I wrote it. The index was a good thing, anyway. :)