I would like a "Remember Me" feature for Logintoboggan -- i.e., a cookie that stores the users' login details.

Comments

Ogredude’s picture

Normally, I can't stand "me too" posts... But this seems like a good place for one.

Me too!

hunmonk’s picture

Version: 4.6.x-1.x-dev » 4.7.x-1.x-dev
Priority: Normal » Minor

i have no interest in coding this, but if somebody wants to submit a patch for the feature, then i'll review it for possible inclusion. i think it would be best if it was an optional feature. also, i won't add new features to the 4.6 version, so moving this to a 4.7 request.

FiReaNGeL’s picture

I'd be interested in this also. It's a shame that currently there's no way to have a 'remember me' login in Drupal... really, every system and website out there use this scheme. It must be for a (good) reason! You can't trust users signing out on public terminals :(

joshk’s picture

Just a bump, because this is a good idea, but also to explain why this is hard.

The way the login system works is off PHP's built-in session system. This is good. It's secure and robust. However, the lifetime of this session cookie is set by php (at compile-time and through php.ini and .htaccess files), and modified a bit in session.inc.

One way to do this would be for logintoboggan to have it's own "remembered sessions" table and it's own cookie. Another way would be to have LT implement some kind of code to change the session lifetime site-wide.

One easy thing, which I think I'm going to code today would be a separate session table that just remembers usernames. This would let the login name field be pre-filled when people return, but still prompt them for the password. It's more secure that way. Beyond that, it would be a simple modification to actually have that cookie generate a login rather than just pre-fill the name. However, I think the username thing is a good first step because I'm wary of session-hijacking and would like some of the more security-minded folks to weigh in.

joshk’s picture

Ok. Got the thumbs-up from chx, with sepeck and rokerr confirming what I thought. I'll start rolling a patch now.

Here's the IRC log on security question:

[11:32] 	josh_k	chx: is there any outstanding liability in a "remember me" checkbox for user logins
[11:32] 	josh_k	that would store a random token (akin the PHPSESID, but a different string probably)
[11:32] 	josh_k	and log people in automagically for, say, two weeks?
[11:33] 	josh_k	obviously it's a bigger window for session-hijacking
[11:33] 	josh_k	but other than that?
[11:33] 	rokerr	that already happens, if you set the session lifetime on the server for 2 weeks
[11:33] 	josh_k	right
[11:33] 	josh_k	the idea here
[11:33] 	josh_k	is to let the user choose
[11:33] 	josh_k	if they want the two weeks or not
[11:33] 	rokerr	right..
[11:33] 	sepeck	doesn't login toboggin have something silly like that?
[11:33] 	josh_k	not yet ;)
[11:34] 	rokerr	only issue is if the user leaves their account logged in when they didn't want to be, that i can think of
[11:34] 	rokerr	you'd have to check your extra "2 week" value and start a new session anyway
[11:34] 	josh_k	right
[11:34] 	sepeck	here's the security deal.  you are relying on your users to be educated enough to make educated desicians on when to store the string locally
[11:34] 	josh_k	that's what I'm thinking
[11:35] 	josh_k	right, so if they do it on a public terminal, they're opening up a can of worms
[11:35] 	josh_k	I'm just wondering whether this opens new doors for more artful malicious activity from the outside
[11:35] 	josh_k	clearly it gives the user a bigger hammer to hit their thumb with, so to speak
[11:36] 	rokerr	that's what i'd say
[11:36] 	josh_k	but in this case, the info on-site isn't particularly sensitive, and people are having a hard time staying logged in, so...
[11:36] 	rokerr	so yeah let them set a cookie that lasts longer than the server session
[11:37] 	sepeck	everyone says the info on their site isn't sensitive or that big a deal until they've been hacked
[11:38] 	josh_k	heh
[11:38] 	josh_k	indeed
[11:38] 	rokerr	sepeck: I don't think it makes the site more susceptible to "hacking"
[11:38] 	sepeck	:D
[11:39] 	chx	i do not think there is a bigger problem with two week logins
[11:39] 	chx	i would simply leave the time window big
[11:39] 	chx	neclimdul: yes
[11:39] 	josh_k	chx: w00t
[11:39] 	josh_k	thanks, /me feels beter about this now  ;)
joshk’s picture

Assigned: Unassigned » joshk
Status: Active » Needs review
StatusFileSize
new18.73 KB

Here's a patch. It's a biggun, so let me break it out:

1) Updates the .install file to create a table to store rememberme data: a sid, a username, a uid and a timestamp

2) Adds a rememberme tab to the admin/settings/logintoboggan screen with the following functions:

- Tells admins how long PHP's sessions will run for
- Activate rememberme for logins? No / With Checkbox / Always
- Lifetime for remembering logins
- Activate rememberme for usernames? No / With Checkbox / Always
- Lifetime for remembering usernames

3) Adds a checkbox to user login forms if either of the settings are "with checkbox"

4) If user logs in, their login and/or uid are stored in the rememberme sessions table and a cookie is set.

5) If an anonymous user comes to the site and a LTRM_session cookie is present and a session row is present with a user id and the lifetime for remembering logins (#2) has not expired, the user is seamlessly logged back in. A watchdog notice fires here as well.

6) If an anonymous user comes to the /user/login PAGE and a LTRM_session cookie is present and a session row is present with a login name and the lifetime is good, the username field is pre-filled. Also, if page caching is OFF, we'll try to pre-fill user-login BLOCKS as well.

THINGS TO IMPROVE/REVIEW:

A) Currently I'm using hook_init to do the re-login. It seems to work fine, but I'd like some vetting on this.

B) Also in hook_init, I am keeping the cache clear for the /user page, which is necessary for the pre-fill option to work.

C) I've tried to copy best-practices in terms of how the cookie is set and the session lifetime maintained, but review is probably in order here.

I've commented the code a fair amount. Let me know how this looks! I have yet to dig into the changes for HEAD/5.0, but will do if this is acceptable.

joshk’s picture

StatusFileSize
new18.8 KB

Fixed bug in .install file's upgrade hook. Use this patch instead.

fatfish’s picture

followUp

joshk’s picture

I've been using this now for a week on two of my sites. No problems so far.

hunmonk’s picture

Status: Needs review » Needs work

couple of things:

  1. patch no longer applies to the HEAD version of the module. if the feature were accepted, it should be put into both 4.7 and HEAD versions.
  2. have you looked at how this feature was implemented in core before it was removed? just curious to know if you based your work on that. if you haven't looked, it might be a good idea to see if anything can be learned from the previous implementation.
  3. a cursory look at the code suggests to me that this could be made into it's own module. i'm personally opposed to throwing every single login feature into LoginToboggan--it makes the code harder to maintain.
joshk’s picture

There's actually a separate module that does this already, although I don't know how good it is or if it plays well w/the toboggan but (sylistic difference) I'm a fan of convergance rather than a million modules. But then I'm not the maintainer, so I see your point too. ;)

This is a bit different from the old 4.3-era checkbox, in that the method then was to change the session cookie's lifespan, like so:

if ($edit["remember_me"]) {
        setcookie(session_name(), session_id(), time() + 3600 * 24 * 365, $path);
      }

This didn't always work very well, and is incompatible with ussing PHPs own built-in sessioning I think. My patch uses it's own independent cookie, which works every time.

I'll probably just try and improve the other module (or be ashamed I didn't look for it before writng my own code).

senthiln’s picture

Title: "Remember Me" feature » "Remember Me" feature logintoboggan for drupal 4.6
Version: 4.7.x-1.x-dev » 4.6.x-1.x-dev
Component: User interface » Code
Assigned: joshk » senthiln
Category: feature » task
Status: Needs work » Fixed
StatusFileSize
new46.91 KB

Most of the discussion here is for adding remember me feature to logintoboggan for version 4.7.x So I applied a similar change to logintoboggan 4.6 version. Now remember me feature works fine for my drupal 4.6 site. If you have any doubt, look at http://supplychain-logistics.com Feel free to change the code or contact me. Code is attached to this post.

hunmonk’s picture

Category: task » feature
Status: Fixed » Needs review

why is this marked fixed? it hasn't been applied to the CVS version of the module, so it certainly isn't fixed. :)

also, this is a feature request and not a task.

Gary Feldman’s picture

Status: Needs review » Closed (won't fix)

It appears that the Persistent Login module (http://drupal.org/project/persistent_login) already provides this behavior for 4.7 and 5.0. If absolutely needed for 4.6, it would probably make more sense to backport that module than add it here.

So I'm marking this "won't fix."