Skip to content

Commit

Permalink
fix: do not panic if cookiemanager returns a nil cookie
Browse files Browse the repository at this point in the history
Closes #1695
  • Loading branch information
aeneasr committed Aug 31, 2021
1 parent 0da2006 commit 6ea5678
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
6 changes: 5 additions & 1 deletion session/manager_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ func (s *ManagerHTTP) CreateAndIssueCookie(ctx context.Context, w http.ResponseW
}

func (s *ManagerHTTP) IssueCookie(ctx context.Context, w http.ResponseWriter, r *http.Request, session *Session) error {
cookie, _ := s.r.CookieManager(r.Context()).Get(r, s.cookieName(ctx))
cookie, err := s.r.CookieManager(r.Context()).Get(r, s.cookieName(ctx))
// Fix for https://github.com/ory/kratos/issues/1695
if err != nil && cookie == nil {
return errors.WithStack(err)
}

if s.r.Config(ctx).SessionPath() != "" {
cookie.Options.Path = s.r.Config(ctx).SessionPath()
Expand Down
22 changes: 22 additions & 0 deletions session/manager_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,28 @@ func TestManagerHTTP(t *testing.T) {
assert.EqualValues(t, http.StatusOK, res.StatusCode)
})

t.Run("case=no panic on invalid cookie name", func(t *testing.T) {
conf.MustSet(config.ViperKeySessionLifespan, "1m")
conf.MustSet(config.ViperKeySessionName, "$%˜\"")
t.Cleanup(func() {
conf.MustSet(config.ViperKeySessionName, "")
})

rp.GET("/session/set/invalid", func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
require.Error(t, reg.SessionManager().CreateAndIssueCookie(r.Context(), w, r, s))
w.WriteHeader(http.StatusInternalServerError)
})

i := identity.Identity{Traits: []byte("{}")}
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), &i))
s, _ = session.NewActiveSession(&i, conf, time.Now())

c := testhelpers.NewClientWithCookies(t)
res, err := c.Get(pts.URL + "/session/set/invalidç")
require.NoError(t, err)
assert.EqualValues(t, http.StatusInternalServerError, res.StatusCode)
})

t.Run("case=valid and uses x-session-cookie", func(t *testing.T) {
conf.MustSet(config.ViperKeySessionLifespan, "1m")

Expand Down

0 comments on commit 6ea5678

Please sign in to comment.