Opened 9 years ago

Closed 7 years ago

Last modified 6 years ago

#354 closed defect (fixed)

Option to stay logged in

Reported by: Jef van Schendel Owned by:
Priority: minor Milestone: 0.5.0
Component: programming Keywords:
Cc: Parent Tickets:

Description (last modified by Elrond)

Users don't stay logged in after leaving a MediaGoblin site. There
is no option to stay logged in (via cookies, or whatever the best
way to do this is).

Subtickets

Change History (9)

comment:1 Changed 9 years ago by Will Kahn-Greene

The original url for this bug was http://bugs.foocorp.net/issues/707 .

comment:2 Changed 7 years ago by Jakob Kramer

Component: programming
Owner: set to Jakob Kramer
Status: acceptedin_progress

Started working on this.

comment:3 Changed 7 years ago by Jakob Kramer

Owner: Jakob Kramer deleted
Status: in_progressreview

This is my solution: branch 354-save-login. Maybe it would be better if you could set the expiry time in the site configuration.

Last edited 7 years ago by Jakob Kramer (previous) (diff)

comment:4 Changed 7 years ago by Elrond

Description: modified (diff)
Status: reviewaccepted

Good idea.

BUT: It wont work fully, I think.

request.session is a new, fresh object for each request, even of the same user/session. Because it is created from the contents of the sent cookie. So the extra .max_age will only be set on the session straight on the login action. So yes, it will affect the cookie that is set at login.

But if the cookie is modified later on by modifying the session, then there wont be any max_age and the cookie will be sent as a session cookie. I don't know, what browsers do, when cookies change from having an expire time to not having one.
If you want to test: Do anything that creates a message at the top. For example post an empty comment.

So the max_age should possibly be put in the session itself, so that it can be used later on again?

comment:5 Changed 7 years ago by Jakob Kramer

Status: acceptedreview

I pushed another, hopefully working, commit to the same branch.

comment:6 Changed 7 years ago by Elrond

This looks very good from my quick glance of it.

I am not able to review this further these days, really.

For someone wanting to review this:

  • Boot up firebug, get the http headers thing ready. Watch out for the session cookie.
  • Maybe also get up the cookie dialog to see the current cookies.
  • Log in with and without the "stay logged in" option set.
  • After logging in (both with and without), do something that will set a new session, like posting an empty comment.

comment:7 Changed 7 years ago by rodney757

Everything looks good to me and works as expected.

I have rebased against master and pushed it here: https://github.com/rodney757/mediagoblin/tree/save_login

I also added a commit using the new render_divs functionality.

Last edited 7 years ago by rodney757 (previous) (diff)

comment:8 Changed 7 years ago by rodney757

Resolution: fixed
Status: reviewclosed

Okay, I merged this. Thanks!

comment:9 Changed 7 years ago by rodney757

Milestone: 0.5.0
Note: See TracTickets for help on using tickets.