Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change session expiration if user is logged in via issuer API #855

Open
cmcavoy opened this issue Apr 27, 2013 · 13 comments

Comments

Projects
None yet
7 participants
@cmcavoy
Copy link
Contributor

commented Apr 27, 2013

If the user logs in through the issuer API, set the session expiration to expire as soon as possible.

/cc @brianloveswords does this seem like a reasonable response to the 'computer lab conundrum?'

@brianloveswords

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2013

It doesn't solve the problem of signing out of persona, which is the problem they are having. Our signout endpoint is accessible via GET request (which is probably considered a bug – it changes state and should be behind a POST with CSRF protection), so they've been able to automate the signout process from their end. Their problem is that a user has signed into persona at that point, and we (nor they) have a way to programatically sign a user out of persona.

@cmcavoy

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2013

@toolness

This comment has been minimized.

Copy link
Contributor

commented May 1, 2013

I've noticed that when I log into Persona for the first time with a new browser/device, it asks me if I'm on a public computer. According to the documentation for navigator.id.watch(), "Persona always believes that a user wants to be logged in or does not want to be logged in to your site." This means that if we rely on Persona to have an accurate representation of the user's login state (e.g. if they told Persona they're on a public computer, or if they manually logged themselves out of Persona at persona.org), we actually don't have to do a whole lot to solve the "computer lab conundrum".

Reading the Persona documentation, it looks like a prudent strategy for us might be to always have our sessions expire early, regardless of whether the Issuer API is being used, but rely on Persona's observer API to automatically re-login the current user, effectively "refreshing" the login state. This way we're always cautious and delegate to Persona to tell us whether the user still wants to be logged in or not.

All of this blocks on #511, though, which was blocking on mozilla/persona#2999, but that now seems to be resolved. So, my suggestion is to just implement #511, make the Backpack's sessions last a very short time (e.g. 24 hours), and then rely on Persona to transparently trigger the re-login of the user as needed.

Coincidentally, all this research has made me realize why Persona's Observer API is actually useful, so that's cool.

@toolness

This comment has been minimized.

Copy link
Contributor

commented May 1, 2013

Another sort-of-funky solution is to ask the user, at the end of an Issuer API session, whether they'd like to log out of the backpack. That would make it even easier to not give potential attackers a 24-hour window in which to access the user's backpack on a public computer, but it'd also complicate the UX by giving the user more choices. Oy.

Either way, though, I still think that the solution I proposed in my last comment is good for security.

@brianloveswords

This comment has been minimized.

Copy link
Contributor

commented May 1, 2013

I like everything except the example time limit – 24 hours doesn't solve the computer lab problem. I think it'd have to be something like 30 minutes or an hour at most.

In addition to being time-limited, they should be browser-session-limited, so when the browser closes, the session is destroyed.

Oh, I also don't think I like the "ask the user if she wants to sign out" at the end of the issuer API workflow – I feel like the issuing workflow is already a little clunky.

Another way to get around this is to encourage people to use backpack connect. That reduces the amount of signing-in that user has to do in order to receive badges (though there is still the first time).

@toolness

This comment has been minimized.

Copy link
Contributor

commented May 1, 2013

Rad! Cool, I agree with all that.

Should I go ahead and finish #511 then?

@brianloveswords

This comment has been minimized.

Copy link
Contributor

commented May 1, 2013

From my perspective, sure, though I'd wait on any comments from @stenington and prioritization from @cmcavoy.

@cmcavoy

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2013

If @stenington has the changes ready to go (I believe he does) then I think we should launch this asap (/cc @jdotpz).

I'll mention backpack-connect to our friends that cited this issue.

@cmcavoy

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2013

Also, thanks for the followup... 👍 🐤 🍰 👯 🎲

@iamjessklein

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2013

@stenington @jdotpz has this been launched?

@jdotpz

This comment has been minimized.

Copy link

commented Jun 27, 2013

Unknown on my front.

@mattdigitalme

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2016

@auralon relevant anymore? If not, then clsoe

@auralon

This comment has been minimized.

Copy link
Member

commented Dec 16, 2016

Still relevant, I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.