Skip to content

Commit

Permalink
Refactor the session AutoCreate behaviour. This is a breaking change.
Browse files Browse the repository at this point in the history
  • Loading branch information
knadh committed May 21, 2024
1 parent 205c1e0 commit cf1786d
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 231 deletions.
55 changes: 52 additions & 3 deletions manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ 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.
EnableAutoCreate bool

// CookieName sets http cookie name. This is also sent as cookie name in `GetCookie` callback.
CookieName string
Expand Down Expand Up @@ -94,6 +94,36 @@ func (m *Manager) RegisterSetCookie(cb func(*http.Cookie, interface{}) error) {
m.setCookieCb = cb
}

// NewSession creates a new session. Reads cookie info from `GetCookie“ callback
// and validate the session with current store. If cookie not set then it creates
// new session and calls `SetCookie“ callback. If `DisableAutoSet` is set then it
// skips new session creation and should be manually done using `Create` method.
// If a cookie is found but its invalid in store then `ErrInvalidSession` error is returned.
func (m *Manager) NewSession(r, w interface{}) (*Session, error) {
var (
sess = &Session{
manager: m,
reader: r,
writer: w,
values: make(map[string]interface{}),
}
)

// 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)
}

// Write cookie.
if err := sess.WriteCookie(id); err != nil {
return nil, err
}

return sess, nil
}

// 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
Expand Down Expand Up @@ -124,5 +154,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,
values: make(map[string]interface{}),
}, nil
}

// If auto-creation is disabled, return an error.
if !m.opts.EnableAutoCreate {
return nil, ErrInvalidSession
}

return m.NewSession(r, w)
}
79 changes: 54 additions & 25 deletions manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestNewManagerWithDefaultOptions(t *testing.T) {

func TestManagerNewManagerWithOptions(t *testing.T) {
opts := Options{
DisableAutoSet: true,
EnableAutoCreate: true,
CookieName: "testcookiename",
CookieDomain: "somedomain",
CookiePath: "/abc/123",
Expand All @@ -36,7 +36,7 @@ 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.EnableAutoCreate, opts.EnableAutoCreate)
assert.Equal(m.opts.CookieName, opts.CookieName)
assert.Equal(m.opts.CookieDomain, opts.CookieDomain)
assert.Equal(m.opts.CookiePath, opts.CookiePath)
Expand All @@ -60,12 +60,12 @@ 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)
Expand All @@ -81,7 +81,7 @@ func TestManagerRegisterSetCookie(t *testing.T) {
assert := assert.New(t)
m := New(Options{})

testCookie := &http.Cookie{
ck := &http.Cookie{
Name: "testcookie",
}

Expand All @@ -91,15 +91,15 @@ 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)
}

func TestManagerAcquireFails(t *testing.T) {
assert := assert.New(t)
m := New(Options{})
m := New(Options{EnableAutoCreate: false})

_, err := m.Acquire(nil, nil, nil)
assert.Error(err, "session store is not set")
Expand All @@ -108,32 +108,60 @@ func TestManagerAcquireFails(t *testing.T) {
_, err = m.Acquire(nil, nil, nil)
assert.Error(err, "callback `GetCookie` not set")

getCb := func(string, interface{}) (*http.Cookie, error) {
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")

m.RegisterSetCookie(func(*http.Cookie, interface{}) error {
return nil
})
_, err = m.Acquire(nil, nil, nil)
assert.ErrorIs(err, ErrInvalidSession)
}

func TestManagerAcquireSucceeds(t *testing.T) {
m := New(Options{})
func TestManagerAcquireNoAutocreate(t *testing.T) {
m := New(Options{EnableAutoCreate: false})
m.UseStore(&MockStore{
isValid: true,
id: "somerandomid",
})

getCb := func(string, interface{}) (*http.Cookie, error) {
m.RegisterGetCookie(func(string, interface{}) (*http.Cookie, error) {
return &http.Cookie{
Name: "testcookie",
Value: "somerandomid",
}, nil
})

m.RegisterSetCookie(func(*http.Cookie, interface{}) error {
return nil
})

_, err := m.Acquire(nil, nil, nil)
assert := assert.New(t)
assert.NoError(err)
}

func TestManagerAcquireAutocreate(t *testing.T) {
m := New(Options{EnableAutoCreate: true})
m.UseStore(&MockStore{
isValid: true,
id: "somerandomid",
})

m.RegisterGetCookie(func(string, interface{}) (*http.Cookie, error) {
return &http.Cookie{
Name: "testcookie",
Value: "",
}, nil
}
m.RegisterGetCookie(getCb)
})

setCb := func(*http.Cookie, interface{}) error {
return http.ErrNoCookie
}
m.RegisterSetCookie(setCb)
m.RegisterSetCookie(func(*http.Cookie, interface{}) error {
return nil
})

_, err := m.Acquire(nil, nil, nil)
assert := assert.New(t)
Expand All @@ -142,9 +170,10 @@ func TestManagerAcquireSucceeds(t *testing.T) {

func TestManagerAcquireFromContext(t *testing.T) {
assert := assert.New(t)
m := New(Options{})
m := New(Options{EnableAutoCreate: true})
m.UseStore(&MockStore{
isValid: true,
id: "somerandomid",
})

getCb := func(string, interface{}) (*http.Cookie, error) {
Expand All @@ -156,20 +185,20 @@ func TestManagerAcquireFromContext(t *testing.T) {
m.RegisterGetCookie(getCb)

setCb := func(*http.Cookie, interface{}) error {
return http.ErrNoCookie
return nil
}
m.RegisterSetCookie(setCb)

sess, err := m.Acquire(nil, nil, nil)
assert.NoError(err)
sess.cookie.Value = "updated"
sess.id = "updated"

sessNew, err := m.Acquire(nil, nil, nil)
assert.NoError(err)
assert.NotEqual(sessNew.cookie.Value, sess.cookie.Value)
assert.NotEqual(sessNew.id, sess.id)

ctx := context.Background()
ctx = context.WithValue(ctx, ContextName, sess)
sessNext, err := m.Acquire(nil, nil, ctx)
assert.Equal(sessNext.cookie.Value, sess.cookie.Value)
assert.Equal(sessNext.id, sess.id)
}
Loading

0 comments on commit cf1786d

Please sign in to comment.