374978 - AMO should offer "Remember Login" when logging in

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Stefan Breunig]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: 

Currently AMO3 cookies expire at the end of the session without offering to save them for a longer time.
At end of session is fine when accessing AMO via public computers, it's annoying however when being at home and having to sign in every time you want to view the sandbox. AMO should provide a checkbox on the login page that allows the users to stay logged in for a longer period of time (1 month probably?).

Reproducible: Always

Comment 2

[WildcatRay]

My comments from Bug 434322:

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre)
Gecko/2008051706 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre)
Gecko/2008051706 Minefield/3.0pre ID:2008051706

At present, users' log ins to addons.mozilla.org (AMO) does not persist over
multiple browser sessions. Offer the option to users to remain logged in (i.e.,
"Remember me" check box when logging in).

Reproducible: Always

Steps to Reproduce:
1. Go to AMO home page. Click "Log in" link and log in to AMO.
2. Close browser.
3. Start browser again. Go to AMO. User is not logged in. User must log in,
again, as in Step 1.
Actual Results:  
See Step 3, above.


I can think of two ways to handle this.

1. Add a "Remember me" type check box on login page to offer the option for
users on their own computers to remain logged in like such sites as
forums.mozillazine.org and gmail.

2. If the preference is to not allow the "Remember me" check box option,
eliminate the separate login page in favor of adding Login and Password boxes
to the AMO home page. At least with this option, users will only have to click
once to log in and be a the AMO home page.

Comment 3

[Basil Hashem [:baz]]

Let's change the cookie expiration to 2 weeks esp if there's a remember me checkbox enabled.

Comment 4

[WildcatRay]

Why only 2 weeks? Bugzilla sets (what appears to be) an unexpiring cookie. Why shouldn't registered AMO users have the same ability?

Comment 5

[u88484]

So now we want to be like yahoo and annoy the hell out of people by making them log back in every two weeks.  Most people probably won't even come back within those two weeks and will once again forget their login information and just say screw it to signing up for a new account.

Comment 6

[WildcatRay]

At least our login and password can be saved to the password manager, but I agree that 2 weeks will be only slightly less annoying than the current unpersistent login we have to deal with now. It just does not make much sense to not offer this.

Comment 7

[Justin Scott [:fligtar]]

I think 2 weeks is better than nothing at all, and after this is implemented it will be easy to increase the number if it goes well. We have had issues with cookies and our netscaler in the past, so we want to be careful implementing this.

Comment 8

[WildcatRay]

A technical issue is a different story. Good implementations seem to be bugzilla and mozillazine forums. Is there a possibility mimic one of those to handle this?

Comment 9

[Fred Wenzel [:wenzel]]

(In reply to comment #5)
> So now we want to be like yahoo and annoy the hell out of people by making them
> log back in every two weeks.  Most people probably won't even come back within
> those two weeks and will once again forget their login information and just say
> screw it to signing up for a new account.

Keeping people from needing to remember their passwords is not the job of an extended cookie expiration date.

Comment 10

[u88484]

(In reply to comment #9)
> (In reply to comment #5)
> > So now we want to be like yahoo and annoy the hell out of people by making them
> > log back in every two weeks.  Most people probably won't even come back within
> > those two weeks and will once again forget their login information and just say
> > screw it to signing up for a new account.
> 
> Keeping people from needing to remember their passwords is not the job of an
> extended cookie expiration date.
> 
yeah I thought about that after I committed my reply.  But still it will be annoying and I still don't understand why sites like to "log people out" after two weeks.  It is very, very annoying and makes end-users think it is a Firefox problem.

Comment 11

[Justin Scott [:fligtar]]

(In reply to comment #10)
> But still it will be annoying and I still don't understand why
> sites like to "log people out" after two weeks.

The first thing that comes to mind is security. Having someone permanently logged into a website is just a bad idea from a security perspective. Anyone can come along and open your browser and be logged in as you.

And to respond to what will probably be posted after I say this, yes, someone logging into someone else's AMO account would be a big security problem. If that someone was a developer or editor or admin and pushed a malicious update to an extension with 4 million active users, that would be a big deal.

So, let's try out 2 weeks and see how it goes.

Comment 12

[Stefan Breunig]

Stop.

I believe that this would lead to security problems in the current version of AMO. It's possible to be logged in multiple times, just log in using Firefox and fire up any other browser installed on your system and log in, too. You're not kicked out in Firefox.
Now, say someone went to an internet cafe and forgot to delete the cookies. Effectively anyone using this computer has access to the AMO profile for two weeks unless the cafe is re-visited. Even a script-kiddie can figure out how to extend this period indefinitely. 

I'm not sure how AMO's handles this exactly, but it shouldn't be possible to do that. Is there already a bug on this? If so, can someone point me to it? In any case, this bug depends on the issue being fixed prior.
A solution would be to store a user-specific token in the database that expires on each login and is attached to the cookie /on login/. If the user logs in somewhere else, the token is updated and thus all older "stored logins" are invalidated.

Comment 13

[Justin Scott [:fligtar]]

Open up Gmail or Facebook in another browser and you won't be logged out there either. 

If we did log people out when they log in somewhere else and someone goes to an internet cafe and forgets to logout/delete the cookies but doesn't log in anywhere else for 2 weeks, we're in the same place.

I would say that we need to require the current password to change your email address and that if someone changes their password, it should log out any other sessions.

But it's fairly common practice to not log out people if they log in multiple times (see Gmail, Facebook). We can look into it further before implementation.

Comment 14

[Stefan Breunig]

I don't use Facebook, but I already suggested this to the Gmail team. Everytime I get asked about this, most people realize that this a decision that makes sense or at least isn't stupid.

Still, AMO should invalidate ALL logins on "Log Out", i.e. not just delete the cookies on your current computer/browser. (Using the above implementation, a new toked would be generated every log out). On the upside, this requires less user-interaction than my previous suggestion, on the downside users have to care themselves that "old" cookies get invalidated.

Comment 15

[Ryan Doherty (:rdoherty)]

I vote for not logging out other computers when logging into AMO. There are legitimate reasons for someone logging into AMO from multiple computers (debugging, multiple browsers, multiple personal computers). It's very non-standard and will probably create many user complaints.

We can default to expiring the cookies at the end of the browser session and only if a user checks 'remember me' we'll set the expiration for 2 weeks. We could even add a message next to it stating something like "don't use this on a public computer".

I think 2 weeks is necessary to guarantee old cookies are removed after some time. 

If we want to be truly secure, we would need a completely different login and authentication system.

Comment 16

[Ryan Doherty (:rdoherty)]

Attachment: Remember me screenshot

Here's a first round of text. Not 100% sure if we should add "(don't use this on a public computer)" or something similar.

Comment 17

[Ryan Doherty (:rdoherty)]

Attachment: Patch

Patch for remember me checkbox.

Comment 18

[Fred Wenzel [:wenzel]]

Comment on attachment 329001 [details] [diff] [review]
Patch

I've not tried your patch (sorry) because I think you're setting a high expiration time here for the wrong cookie: You're setting it for the AMO app name cookie which is used to remember the application the user was in when they go into the developer CP (APP agnostic) and go back to the public pages from there.

What you want to do is make the login cookie expire later, not the one above. (Sorry, not sure off the top of my head how to do that with Cake's Session component, but there should be a way).

Comment 19

[Ryan Doherty (:rdoherty)]

(In reply to comment #18)
> What you want to do is make the login cookie expire later, not the one above.
> (Sorry, not sure off the top of my head how to do that with Cake's Session
> component, but there should be a way).
> 

Crap, thanks for catching that. Will fix tmrw.

Comment 20

[Stefan Breunig]

Oh by the way, the GMail team has found his own solution to this "logged in on mutliple computers" problem:
http://lifehacker.com/398039/gmail-adds-multiple-session-info-and-remote-signout

Comment 21

[Ryan Doherty (:rdoherty)]

(In reply to comment #20)
> Oh by the way, the GMail team has found his own solution to this "logged in on
> mutliple computers" problem:
> http://lifehacker.com/398039/gmail-adds-multiple-session-info-and-remote-signout
> 

While this would be a cool feature for AMO, it's outside the scope of this bug.

Comment 22

[Stefan Breunig]

> While this would be a cool feature for AMO, it's outside the scope of this bug.

Created bug 445169

Comment 23

[Ryan Doherty (:rdoherty)]

Attachment: Patch

New patch. Created from trunk/site, not trunk/site/app like normal because I had to patch cake.

Comment 24

[Fred Wenzel [:wenzel]]

Comment on attachment 329916 [details] [diff] [review]
Patch

Works very well. Good job.

Comment 25

[Ryan Doherty (:rdoherty)]

Commited in r17103

Comment 26

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; tested with:

* Firefox 2.0.0.16 / Firefox 3.01
* IE 6 / 7
* Opera 9.51
* Safari 3.1.2

(I think I'd like to see the text right-aligned, or something; anyone else? -- that's a separate bug, however.)

Comment 27

[Fred Wenzel [:wenzel]]

(In reply to comment #26)
> (I think I'd like to see the text right-aligned, or something; anyone else? --
> that's a separate bug, however.)

Yeah, I think it should align with the text fields. Especially considering in other languages, it won't be aligned centered under the text fields when the text length differs.

Comment 28

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #27)

<snip>
> Yeah, I think it should align with the text fields. Especially considering in
> other languages, it won't be aligned centered under the text fields when the
> text length differs.

I filed bug 445847.

Comment 29

[Ryan Doherty (:rdoherty)]

Attachment: Screenshot of more text

I'm not quite following the comments about text-alignment. It works with extra text fine...

Comment 30

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #29)
> Created an attachment (id=330100) [details]
> Screenshot of more text
> 
> I'm not quite following the comments about text-alignment. It works with extra
> text fine...

I mean to propose in bug 445847 that the end of the phrase aligns vertically with the end of the textfield...guess I could hack up a mock :-)

Comment 31

[Cameron]

(In reply to comment #11)
> The first thing that comes to mind is security. Having someone permanently
> logged into a website is just a bad idea from a security perspective. Anyone
> can come along and open your browser and be logged in as you.
> 
> And to respond to what will probably be posted after I say this, yes, someone
> logging into someone else's AMO account would be a big security problem. If
> that someone was a developer or editor or admin and pushed a malicious update
> to an extension with 4 million active users, that would be a big deal.
> 

Filed Bug 446289