session icon indicating copy to clipboard operation
session copied to clipboard

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

Open Allendar opened this issue 7 years ago • 5 comments

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)

Allendar avatar Oct 28 '17 08:10 Allendar

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

icza avatar Oct 29 '17 06:10 icza

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.

Allendar avatar Oct 29 '17 09:10 Allendar

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.

icza avatar Oct 30 '17 13:10 icza

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.

Allendar avatar Oct 30 '17 21:10 Allendar

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

icza avatar Nov 01 '17 01:11 icza