499276 - [Extension] Remember Login

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Collector Extension, defect)

Description

[Brian King [:kinger]]

From Ken (spin out from bug 498850):

Adding a status bar button would be a great help to display whether or not a
user is logged in and to provide quick access to the Subscriptions window, and
the Add-ons Collector Settings.
An option to stay logged in would be great too.

Comment 1

[Brian King [:kinger]]

I'm not sure about the status bar icon, we can debate that.

But I would like to see a checkbox on the login screen to remember login, and have the extension poll status at Firefox startup and auto-login if the credentials are saved. I don't think we are using the Login Manager currently.

One possible issue is if the user has more than one AMO account saved.

Comment 2

[Justin Scott [:fligtar]]

We can do auto-login. I was under the impression we kept the token and it was good for a long time, but I never restart my browser so I haven't noticed.

I don't think we need the status bar icon.

Comment 3

[Brian King [:kinger]]

(In reply to comment #2)
> We can do auto-login. I was under the impression we kept the token and it was
> good for a long time, but I never restart my browser so I haven't noticed.

Dave,

Do you know when the token should expire? It is not every Firefox session, but I do need to re-login every so often.

My proposal is to keep the users Login credentials in the Login Manager if they choose so (checkbox), and when the session expires we can check there and auto-login.

Thoughts?

Comment 4

[David McNamara [:mackers]]

(In reply to comment #3)
> Do you know when the token should expire? It is not every Firefox session, but
> I do need to re-login every so often.

It shouldn't expire. We keep it in a preference, so it should never go away.

What happens is that when we periodically check for updates, the API tells us we're not authenticated and the extension kicks back to the login screen.

> My proposal is to keep the users Login credentials in the Login Manager if they
> choose so (checkbox), and when the session expires we can check there and
> auto-login.

That seems like a good solution.

Comment 5

[Justin Scott [:fligtar]]

Bumping to Collector 1.0.4

Comment 6

[David McNamara [:mackers]]

Some work on this in r50584

1) Added a "notAuthorized" observer to all API responses. UI will log itself out when it receives a response with the error "unauthorized". (This was previously only the behaviour on some API calls.) This should help maintain a more current UI state. 

2) Fixed some slight visual (focus) anomalies during login.

Comment 7

[David McNamara [:mackers]]

I believe I have found an elegant solution for this using the login manager and respecting the user preference. It goes like this:

1) The user visits the AMO site using the browser.
2) The user logins into AMO. If she wants to remember her password she ticks "Remember me on this computer", if not, she leaves it unticked. (Ticking it has the effect of storing the username and password in FF's Login Manager).
3) The user installs the extension and, on first run, is presented with the login screen.
4) If the user has previously ticked "Remember me" and the AMO password is in the Login Manager, then the BW extension will auto-populate the username and password fields with the correct values (same as AMO login).
5) The user then just has to click "Log in" in the extension.
6) If the session expires for whatever reason, the login fields will be populated again.

This is a neat solution because a) we don't need another tickbox in the extension (the user has already specified her preference to remember. b) it doesn't stop the user from clearing the default values and entering the details of another account here.

I have implemented this in r50593.

Comment 8

[Brian King [:kinger]]

(In reply to comment #7)
> This is a neat solution because a) we don't need another tickbox in the
> extension (the user has already specified her preference to remember. b) it
> doesn't stop the user from clearing the default values and entering the details
> of another account here.
> 
> I have implemented this in r50593.

I'm all for reducing UI, but didn't we in general want to keep the site login separate from the extension (API) login?

Comment 9

[David McNamara [:mackers]]

(In reply to comment #8)
> I'm all for reducing UI, but didn't we in general want to keep the site login
> separate from the extension (API) login?

I'm not sure I understand what you mean.

They are separate, but this fix lets the extension take a hint from the website login, which is likely to be the same 99% of the time.

Comment 10

[Justin Scott [:fligtar]]

Yeah, I am fine with taking the login info as a hint, although I'd ideally like the user to be automatically logged in across sessions.

The main point of this is to be able to get subscription notifications when browsing around Firefox without having to remember to manually log in every session.

Comment 11

[David McNamara [:mackers]]

In theory, the user should only have to log in once to receive their auth token. The token is stored in a pref and should remain valid indefinitely and across sessions. 

It seems sometimes the token expires and is rejected by the API for reasons unknown.

Comment 12

[Brian King [:kinger]]

(In reply to comment #11) 
> It seems sometimes the token expires and is rejected by the API for reasons
> unknown.

Is this something the API team needs to look into?

Anything left client side to do here?

Comment 13

[David McNamara [:mackers]]

(In reply to comment #12)
> Is this something the API team needs to look into?

If we decide its an actual issue.
 
> Anything left client side to do here?

Nope, I'm happy if you're happy with the solution as above.

Comment 14

[Brian King [:kinger]]

(In reply to comment #13)
> (In reply to comment #12)
> > Is this something the API team needs to look into?
> 
> If we decide its an actual issue.

I suggest filing a bug against Les and letting him decide.
 
> > Anything left client side to do here?
> 
> Nope, I'm happy if you're happy with the solution as above.

Ok, feel free to close this off then.

Comment 15

[David McNamara [:mackers]]

Bug 515393