Skip to content

Commit

Permalink
fix: CookieStore MaxAge value (#2485) (#2488)
Browse files Browse the repository at this point in the history
CookieStore MaxAge is set to 86400 * 30 by default. This prevents secure cookies retrieval with expiration > 30 days. MaxAge: 0 disables MaxAge check by SecureCookie, thus allowing sessions lasting > 30 days.

Closes  #2485

Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com>
  • Loading branch information
romanlytvyn and aeneasr authored Apr 25, 2021

Verified

This commit was signed with the committer’s verified signature.
frapell Franco Pellegrini
1 parent b6c34e0 commit aafc901
Showing 2 changed files with 21 additions and 1 deletion.
9 changes: 8 additions & 1 deletion driver/registry_base.go
Original file line number Diff line number Diff line change
@@ -239,7 +239,14 @@ func (m *RegistryBase) KeyCipher() *jwk.AEAD {

func (m *RegistryBase) CookieStore() sessions.Store {
if m.cs == nil {
m.cs = sessions.NewCookieStore(m.C.GetCookieSecrets()...)
cs := sessions.NewCookieStore(m.C.GetCookieSecrets()...)
// CookieStore MaxAge is set to 86400 * 30 by default. This prevents secure cookies retrieval with expiration > 30 days.
// MaxAge(0) disables internal MaxAge check by SecureCookie, see:
//
// https://github.com/ory/hydra/pull/2488#discussion_r618992698
cs.MaxAge(0)

m.cs = cs
m.csPrev = m.C.GetCookieSecrets()
}
return m.cs
13 changes: 13 additions & 0 deletions driver/registry_base_test.go
Original file line number Diff line number Diff line change
@@ -12,6 +12,8 @@ import (
"github.com/ory/hydra/driver/config"
"github.com/ory/x/configx"
"github.com/ory/x/logrusx"

"github.com/gorilla/sessions"
)

func TestRegistryBase_newKeyStrategy_handlesNetworkError(t *testing.T) {
@@ -42,3 +44,14 @@ func TestRegistryBase_newKeyStrategy_handlesNetworkError(t *testing.T) {
assert.Equal(t, logrus.FatalLevel, hook.LastEntry().Level)
assert.Contains(t, hook.LastEntry().Message, "A network error occurred, see error for specific details.")
}

func TestRegistryBase_CookieStore_MaxAgeZero(t *testing.T) {
// Test ensures that CookieStore MaxAge option is equal to zero after initialization

r := new(RegistryBase)
r.WithConfig(config.MustNew(logrusx.New("", "")))

cs := r.CookieStore().(*sessions.CookieStore)

assert.Equal(t, cs.Options.MaxAge, 0)
}

0 comments on commit aafc901

Please sign in to comment.