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

Deleting and creating a new session on the same request causes dangling? #11

Open
Allendar opened this issue Oct 28, 2017 · 5 comments
Open

Comments

@Allendar
Copy link

Allendar commented Oct 28, 2017

Say I have these two functions for my framework where I need to respawn the session if someone already has session values but chose Remember Me at login. I'm trying to respawn a new session with the old attributes and a new, longer, expiration time:

// New generates a new session for the context. If a session
// already exists it will be discarded.
func New(c context.Context, options *session.SessOptions) {
	Delete(c)
	session.Add(session.NewSessionOptions(options), c.ResponseWriter())
}

// Delete the session.
func Delete(c context.Context) {
	if sess := session.Get(c.Request()); nil != sess {
		session.Remove(sess, c.ResponseWriter())
	}
}

Even tho I expect a delete and an add, the session-logger tells me one is deleted but two times shows me added. When I dump the internal sessions it still has the old (deleted) session too with it's old variables.

The following code is used in my login page (sessiondb is simply a small layer to automatise session existences checks):

if form.IsValid() {
	// If already had a session; need to migrate for remember me new session timeout
	var attrs map[string]interface{}
	if sessiondb.Exists(c) {
		attrs = sessiondb.GetAll(c)
	}
	var expiration time.Duration
	if "1" == form.Field("rememberMe").Value {
		expiration = time.Second * time.Duration(globals.RememberMeExpiration)
	} else {
		expiration = time.Second * time.Duration(globals.CookieExpiration)
	}
	sessiondb.New(c, &session.SessOptions{
		Timeout: expiration,
		Attrs:   attrs,
	})
	sessiondb.Set(c, "UserID", id)

	if referer := sessiondb.Get(c, "LoginReferer"); nil != referer {
		sessiondb.Unset(c, "LoginReferer")
		c.Redirect(referer.(string))
	} else {
		c.Redirect("/")
	}
	return
}

In the above case;

  • the old session still exists with only LoginReferer
  • the new session exists with UserID and LoginReferer, which of course is consumed again right away if it was set to jump back to where to user was before being bounced to the login page (e.a. authorised environment)
@icza
Copy link
Owner

icza commented Oct 29, 2017

I saw your issue, just didn't have time for it yet. Will look into it soon.

@Allendar
Copy link
Author

No pressure :) Thanks for responding!

For now I just patched my fork with:

// SetTimeout modifies the timeout the session currently has.
func (s *Impl) SetTimeout(d time.Duration) {
	s.TimeoutF = d
}

This is probably not the prettiest method, but it works. It seems deleting and creating a new session in the same request/response cycle confuses the manager.

@icza
Copy link
Owner

icza commented Oct 30, 2017

Ok, so some questions.

  • What session manager do you use (session.Manager)? The default, global one which is session.CookieManager?

  • What session store do you use? The default in-memory store returned by session.NewInMemStore()?

I looked into the implementation, and by default (if you use the default manager and store which is NewCookieManager(NewInMemStore())), session.Remove() calls Global.Remove(sess, w), which clears the cookie value (takes effect when response is committed), and also removes the session from the internal store (inmem_store.go, inMemStore.Remove() method).

So session.Remove() properly removes it / clears what needs to be cleared. And adding a new session in the same request-response handler will set a new value for the same cookie, so the HTTP client will change / update the cookie value, pointing to the new session.

@Allendar
Copy link
Author

Allendar commented Oct 30, 2017

I'm using the default .Global entry:

	session.Global.Close()
	sessStore := session.NewInMemStoreOptions(&session.InMemStoreOptions{
		DebugMode: "prod" != *config.ClFlagEnvironment,
	})
	session.Global = session.NewCookieManagerOptions(sessStore, &session.CookieMngrOptions{
		SessIDCookieName: config.Values.Security.SessionCookieName,
	})

By accident I noticed that after doing a redirect after removing the session on logout it also didn't unset, but only after the second navigation/refresh it completed the session/cookie update. Maybe it has something to do with creating/removing and redirecting afterwards? As that's what happens after the login redirect too. And maybe that caused the 2 sessions to come to exist.

@icza
Copy link
Owner

icza commented Nov 1, 2017

I'm not sure, but I guess you can easily test that (by not redirecting).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants