Refactor the session AutoCreate behaviour. This is a breaking change.

manager.go

@@ -7,15 +7,17 @@ import (
 	"time"
 )
 
+type ctxNameType string
+
 const (
 	// Default cookie name used to store session.
 	defaultCookieName = "session"
 
 	// ContextName is the key used to store session in context passed to acquire method.
-	ContextName = "_simple_session"
+	ContextName ctxNameType = "_simple_session"
 )
 
-// Manager is a utility to scaffold session and store.
+// Manager handles the storage and management of HTTP cookies.
 type Manager struct {
 	// Store to be used.
 	store Store
@@ -32,8 +34,9 @@ type Manager struct {
 
 // Options are available options to configure Manager.
 type Options struct {
-	// DisableAutoSet skips creation of session cookie in frontend and new session in store if session is not already set.
-	DisableAutoSet bool
+	// If enabled, Acquire() will always create and return a new session if one doesn't already exist.
+	// If disabled then new session can only be created using NewSession() method.
+	EnableAutoCreate bool
 
 	// CookieName sets http cookie name. This is also sent as cookie name in `GetCookie` callback.
 	CookieName string
@@ -82,27 +85,56 @@ func (m *Manager) UseStore(str Store) {
 	m.store = str
 }
 
-// RegisterGetCookie sets a callback to get http cookie from any reader interface which
-// is sent on session acquisition using `Acquire` method.
+// RegisterGetCookie sets a callback to retrieve an HTTP cookie during session acquisition.
 func (m *Manager) RegisterGetCookie(cb func(string, interface{}) (*http.Cookie, error)) {
 	m.getCookieCb = cb
 }
 
-// RegisterSetCookie sets a callback to set cookie from http writer interface which
-// is sent on session acquisition using `Acquire` method.
+// RegisterSetCookie sets a callback to set an HTTP cookie during session acquisition.
 func (m *Manager) RegisterSetCookie(cb func(*http.Cookie, interface{}) error) {
 	m.setCookieCb = cb
 }
 
-// Acquire gets a `Session` for current session cookie from store.
-// If `Session` is not found on store then it creates a new session and sets on store.
-// If 'DisableAutoSet` is set in options then session has to be explicitly created before
-// using `Session` for getting or setting.
-// `r` and `w` is request and response interfaces which are sent back in GetCookie and SetCookie callbacks respectively.
-// In case of net/http `r` will be r`
-// Optionally context can be passed around which is used to get already loaded session. This is useful when
-// handler is wrapped with multiple middlewares and `Acquire` is already called in any of the middleware.
-func (m *Manager) Acquire(r, w interface{}, c context.Context) (*Session, error) {
+// NewSession creates a new `Session` and updates the cookie with a new session ID,
+// replacing any existing session ID if it exists.
+func (m *Manager) NewSession(r, w interface{}) (*Session, error) {
+	// Check if any store is set
+	if m.store == nil {
+		return nil, fmt.Errorf("session store is not set")
+	}
+
+	if m.setCookieCb == nil {
+		return nil, fmt.Errorf("callback `SetCookie` not set")
+	}
+
+	// Create new cookie in store and write to front.
+	// Store also calls `WriteCookie`` to write to http interface.
+	id, err := m.store.Create()
+	if err != nil {
+		return nil, errAs(err)
+	}
+
+	var sess = &Session{
+		id:      id,
+		manager: m,
+		reader:  r,
+		writer:  w,
+		cache:   nil,
+	}
+	// Write cookie.
+	if err := sess.WriteCookie(id); err != nil {
+		return nil, err
+	}
+
+	return sess, nil
+}
+
+// Acquire retrieves a `Session` from the store using the current session cookie.
+// If not found and `opt.EnableAutoCreate` is true, a new session is created and stored.
+// If not found and `opt.EnableAutoCreate` is false which is the default, it returns ErrInvalidSession.
+// `r` and `w` are request and response interfaces which is passed back in in GetCookie and SetCookie callbacks.
+// Optionally, a context can be passed to get an already loaded session, useful in middleware chains.
+func (m *Manager) Acquire(c context.Context, r, w interface{}) (*Session, error) {
 	// Check if any store is set
 	if m.store == nil {
 		return nil, fmt.Errorf("session store is not set")
@@ -124,5 +156,24 @@ func (m *Manager) Acquire(r, w interface{}, c context.Context) (*Session, error)
 		}
 	}
 
-	return NewSession(m, r, w)
+	// Get existing HTTP session cookie.
+	// If there's no error and there's a session ID (unvalidated at this point),
+	// return a session object.
+	ck, err := m.getCookieCb(m.opts.CookieName, r)
+	if err == nil && ck != nil && ck.Value != "" {
+		return &Session{
+			manager: m,
+			reader:  r,
+			writer:  w,
+			id:      ck.Value,
+			cache:   nil,
+		}, nil
+	}
+
+	// If auto-creation is disabled, return an error.
+	if !m.opts.EnableAutoCreate {
+		return nil, ErrInvalidSession
+	}
+
+	return m.NewSession(r, w)
 }

manager_test.go

@@ -9,19 +9,48 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+const mockSessionID = "sometestcookievalue"
+
+func newMockStore() *MockStore {
+	return &MockStore{
+		id:   mockSessionID,
+		data: map[string]interface{}{},
+		err:  nil,
+	}
+}
+
+func newMockManager(store *MockStore) *Manager {
+	m := New(Options{})
+	m.UseStore(store)
+	m.RegisterGetCookie(mockGetCookieCb)
+	m.RegisterSetCookie(mockSetCookieCb)
+	return m
+}
+
+func mockGetCookieCb(name string, r interface{}) (*http.Cookie, error) {
+	return &http.Cookie{
+		Name:  name,
+		Value: mockSessionID,
+	}, nil
+}
+
+func mockSetCookieCb(*http.Cookie, interface{}) error {
+	return nil
+}
+
 func TestNewManagerWithDefaultOptions(t *testing.T) {
 	m := New(Options{})
 
 	assert := assert.New(t)
 	// Default cookie path is set to root
-	assert.Equal(m.opts.CookiePath, "/")
+	assert.Equal("/", m.opts.CookiePath)
 	// Default cookie name is set
-	assert.Equal(m.opts.CookieName, defaultCookieName)
+	assert.Equal(defaultCookieName, m.opts.CookieName)
 }
 
 func TestManagerNewManagerWithOptions(t *testing.T) {
 	opts := Options{
-		DisableAutoSet:   true,
+		EnableAutoCreate: true,
 		CookieName:       "testcookiename",
 		CookieDomain:     "somedomain",
 		CookiePath:       "/abc/123",
@@ -36,36 +65,33 @@ func TestManagerNewManagerWithOptions(t *testing.T) {
 	assert := assert.New(t)
 
 	// Default cookie path is set to root
-	assert.Equal(m.opts.DisableAutoSet, opts.DisableAutoSet)
-	assert.Equal(m.opts.CookieName, opts.CookieName)
-	assert.Equal(m.opts.CookieDomain, opts.CookieDomain)
-	assert.Equal(m.opts.CookiePath, opts.CookiePath)
-	assert.Equal(m.opts.IsSecureCookie, opts.IsSecureCookie)
-	assert.Equal(m.opts.SameSite, opts.SameSite)
-	assert.Equal(m.opts.IsHTTPOnlyCookie, opts.IsHTTPOnlyCookie)
-	assert.Equal(m.opts.CookieLifetime, opts.CookieLifetime)
+	assert.Equal(opts.EnableAutoCreate, m.opts.EnableAutoCreate)
+	assert.Equal(opts.CookieName, m.opts.CookieName)
+	assert.Equal(opts.CookieDomain, m.opts.CookieDomain)
+	assert.Equal(opts.CookiePath, m.opts.CookiePath)
+	assert.Equal(opts.IsSecureCookie, m.opts.IsSecureCookie)
+	assert.Equal(opts.SameSite, m.opts.SameSite)
+	assert.Equal(opts.IsHTTPOnlyCookie, m.opts.IsHTTPOnlyCookie)
+	assert.Equal(opts.CookieLifetime, m.opts.CookieLifetime)
 }
 
 func TestManagerUseStore(t *testing.T) {
 	assert := assert.New(t)
-	mockStr := &MockStore{}
-	assert.Implements((*Store)(nil), mockStr)
-
-	m := New(Options{})
-	m.UseStore(mockStr)
-	assert.Equal(m.store, mockStr)
+	s := newMockStore()
+	m := newMockManager(s)
+	assert.Equal(s, m.store)
 }
 
 func TestManagerRegisterGetCookie(t *testing.T) {
 	assert := assert.New(t)
 	m := New(Options{})
 
-	testCookie := &http.Cookie{
+	ck := &http.Cookie{
 		Name: "testcookie",
 	}
 
 	cb := func(string, interface{}) (*http.Cookie, error) {
-		return testCookie, http.ErrNoCookie
+		return ck, http.ErrNoCookie
 	}
 
 	m.RegisterGetCookie(cb)
@@ -81,7 +107,7 @@ func TestManagerRegisterSetCookie(t *testing.T) {
 	assert := assert.New(t)
 	m := New(Options{})
 
-	testCookie := &http.Cookie{
+	ck := &http.Cookie{
 		Name: "testcookie",
 	}
 
@@ -91,8 +117,8 @@ func TestManagerRegisterSetCookie(t *testing.T) {
 
 	m.RegisterSetCookie(cb)
 
-	expectCbErr := cb(testCookie, nil)
-	actualCbErr := m.setCookieCb(testCookie, nil)
+	expectCbErr := cb(ck, nil)
+	actualCbErr := m.setCookieCb(ck, nil)
 
 	assert.Equal(expectCbErr, actualCbErr)
 }
@@ -101,75 +127,61 @@ func TestManagerAcquireFails(t *testing.T) {
 	assert := assert.New(t)
 	m := New(Options{})
 
-	_, err := m.Acquire(nil, nil, nil)
-	assert.Error(err, "session store is not set")
+	// Fail if store is not assigned.
+	_, err := m.Acquire(context.Background(), nil, nil)
+	assert.Equal("session store is not set", err.Error())
 
+	// Fail if getCookie callback is not assigned.
 	m.UseStore(&MockStore{})
-	_, err = m.Acquire(nil, nil, nil)
-	assert.Error(err, "callback `GetCookie` not set")
+	_, err = m.Acquire(context.Background(), nil, nil)
+	assert.Equal("callback `GetCookie` not set", err.Error())
 
-	getCb := func(string, interface{}) (*http.Cookie, error) {
+	// Assign getCookie, returns nil cookie to make sure it
+	// fails in create session with invalid session.
+	m.RegisterGetCookie(func(string, interface{}) (*http.Cookie, error) {
 		return nil, nil
-	}
-	m.RegisterGetCookie(getCb)
-	_, err = m.Acquire(nil, nil, nil)
-	assert.Error(err, "callback `SetCookie` not set")
-}
+	})
 
-func TestManagerAcquireSucceeds(t *testing.T) {
-	m := New(Options{})
-	m.UseStore(&MockStore{
-		isValid: true,
+	// Fail if setCookie callback is not assigned.
+	_, err = m.Acquire(context.Background(), nil, nil)
+	assert.Equal("callback `SetCookie` not set", err.Error())
+
+	// Register setCookie callback.
+	m.RegisterSetCookie(func(*http.Cookie, interface{}) error {
+		return nil
 	})
 
-	getCb := func(string, interface{}) (*http.Cookie, error) {
-		return &http.Cookie{
-			Name:  "testcookie",
-			Value: "",
-		}, nil
-	}
-	m.RegisterGetCookie(getCb)
+	// By default EnableAutoCreate is disabled
+	// Check if it returns invalid session.
+	_, err = m.Acquire(context.Background(), nil, nil)
+	assert.ErrorIs(err, ErrInvalidSession)
+}
 
-	setCb := func(*http.Cookie, interface{}) error {
-		return http.ErrNoCookie
-	}
-	m.RegisterSetCookie(setCb)
+func TestManagerAcquireAutocreate(t *testing.T) {
+	m := newMockManager(newMockStore())
+	// Enable autocreate.
+	m.opts.EnableAutoCreate = true
+	m.RegisterGetCookie(func(string, interface{}) (*http.Cookie, error) {
+		return nil, ErrInvalidSession
+	})
 
-	_, err := m.Acquire(nil, nil, nil)
+	// If cookie doesn't exist then should return a new one without error.
+	sess, err := m.Acquire(context.Background(), nil, nil)
 	assert := assert.New(t)
 	assert.NoError(err)
+	assert.Equal(mockSessionID, sess.id)
 }
 
 func TestManagerAcquireFromContext(t *testing.T) {
 	assert := assert.New(t)
-	m := New(Options{})
-	m.UseStore(&MockStore{
-		isValid: true,
-	})
-
-	getCb := func(string, interface{}) (*http.Cookie, error) {
-		return &http.Cookie{
-			Name:  "testcookie",
-			Value: "",
-		}, nil
-	}
-	m.RegisterGetCookie(getCb)
+	m := newMockManager(newMockStore())
 
-	setCb := func(*http.Cookie, interface{}) error {
-		return http.ErrNoCookie
-	}
-	m.RegisterSetCookie(setCb)
-
-	sess, err := m.Acquire(nil, nil, nil)
+	sess, err := m.Acquire(context.Background(), nil, nil)
+	sess.id = "updated"
 	assert.NoError(err)
-	sess.cookie.Value = "updated"
 
-	sessNew, err := m.Acquire(nil, nil, nil)
+	ctx := context.WithValue(context.Background(), ContextName, sess)
+	sessNext, err := m.Acquire(ctx, nil, nil)
+	assert.Equal(sess.id, sessNext.id)
 	assert.NoError(err)
-	assert.NotEqual(sessNew.cookie.Value, sess.cookie.Value)
-
-	ctx := context.Background()
-	ctx = context.WithValue(ctx, ContextName, sess)
-	sessNext, err := m.Acquire(nil, nil, ctx)
-	assert.Equal(sessNext.cookie.Value, sess.cookie.Value)
 }