Skip to content

Commit

Permalink
fix: remove unused x-session-cookie parameter (#2983)
Browse files Browse the repository at this point in the history
This patch removes the undocumented and experimental `X-Session-Cookie` header from the `/sessions/whoami` endpoint.
  • Loading branch information
aeneasr authored Dec 26, 2022
1 parent a3096c7 commit 56b5c26
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 44 deletions.
9 changes: 1 addition & 8 deletions session/manager_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ func (s *ManagerHTTP) UpsertAndIssueCookie(ctx context.Context, w http.ResponseW

func (s *ManagerHTTP) RefreshCookie(ctx context.Context, w http.ResponseWriter, r *http.Request, session *Session) error {
// If it is a session token there is nothing to do.
cookieHeader := r.Header.Get("X-Session-Cookie")
_, cookieErr := r.Cookie(s.cookieName(r.Context()))
if len(cookieHeader) == 0 && errors.Is(cookieErr, http.ErrNoCookie) {
if errors.Is(cookieErr, http.ErrNoCookie) {
return nil
}

Expand Down Expand Up @@ -156,12 +155,6 @@ func getCookieExpiry(s *sessions.Session) *time.Time {
}

func (s *ManagerHTTP) getCookie(r *http.Request) (*sessions.Session, error) {
if cookie := r.Header.Get("X-Session-Cookie"); len(cookie) > 0 {
rr := *r
r = &rr
r.Header = http.Header{"Cookie": []string{s.cookieName(r.Context()) + "=" + cookie}}
}

return s.r.CookieManager(r.Context()).Get(r, s.cookieName(r.Context()))
}

Expand Down
36 changes: 0 additions & 36 deletions session/manager_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (

"github.com/ory/kratos/driver"

"github.com/ory/x/urlx"

"github.com/julienschmidt/httprouter"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -278,40 +276,6 @@ func TestManagerHTTP(t *testing.T) {
assert.EqualValues(t, http.StatusInternalServerError, res.StatusCode)
})

t.Run("case=valid and uses x-session-cookie", func(t *testing.T) {
req := x.NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil)
conf.MustSet(ctx, config.ViperKeySessionLifespan, "1m")

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

c := testhelpers.NewClientWithCookies(t)
testhelpers.MockHydrateCookieClient(t, c, pts.URL+"/session/set")

cookies := c.Jar.Cookies(urlx.ParseOrPanic(pts.URL))
require.Len(t, cookies, 2, "expect two cookies, one csrf, one session")

var cookie *http.Cookie
for _, c := range cookies {
if c.Name == "ory_kratos_session" {
cookie = c
break
}
}
require.NotNil(t, cookie, "must find the kratos session cookie")

assert.Equal(t, "ory_kratos_session", cookie.Name)

req, err := http.NewRequest("GET", pts.URL+"/session/get", nil)
require.NoError(t, err)
req.Header.Set("Cookie", "ory_kratos_session=not-valid")
req.Header.Set("X-Session-Cookie", cookie.Value)
res, err := http.DefaultClient.Do(req)
require.NoError(t, err)
assert.EqualValues(t, http.StatusOK, res.StatusCode)
})

t.Run("case=valid bearer auth as fallback", func(t *testing.T) {
req := x.NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil)
conf.MustSet(ctx, config.ViperKeySessionLifespan, "1m")
Expand Down

0 comments on commit 56b5c26

Please sign in to comment.