fix: re-issue outdated cookie in /whoami (#2598)

Closes #2562

Co-authored-by: aeneasr <[email protected]>
Co-authored-by: ory-bot <[email protected]>

Author: 3 people

session/handler.go

@@ -199,6 +199,12 @@ func (h *Handler) whoami(w http.ResponseWriter, r *http.Request, ps httprouter.P
 	// Set userId as the X-Kratos-Authenticated-Identity-Id header.
 	w.Header().Set("X-Kratos-Authenticated-Identity-Id", s.Identity.ID.String())
 
+	if err := h.r.SessionManager().RefreshCookie(r.Context(), w, r, s); err != nil {
+		h.r.Audit().WithRequest(r).WithError(err).Info("Could not re-issue cookie.")
+		h.r.Writer().WriteError(w, r, err)
+		return
+	}
+
 	h.r.Writer().Write(w, r, s)
 }
 

session/handler_test.go

@@ -635,6 +635,54 @@ func TestHandlerSelfServiceSessionManagement(t *testing.T) {
 			assert.Equal(t, http.StatusNoContent, resp.StatusCode, "case=%d", j)
 		}
 	})
+
+	t.Run("case=whoami should not issue cookie for up to date session", func(t *testing.T) {
+		client, _, _ := setup(t)
+
+		req, _ := http.NewRequest("GET", ts.URL+"/sessions/whoami", nil)
+		resp, err := client.Do(req)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+		assert.Empty(t, resp.Cookies())
+	})
+
+	t.Run("case=whoami should reissue cookie for outdated session", func(t *testing.T) {
+		client, _, session := setup(t)
+		oldExpires := session.ExpiresAt
+
+		session.ExpiresAt = time.Now().Add(time.Hour * 24 * 30).UTC().Round(time.Hour)
+		err := reg.SessionPersister().UpsertSession(context.Background(), session)
+		require.NoError(t, err)
+
+		resp, err := client.Get(ts.URL + "/sessions/whoami")
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+		require.Len(t, resp.Cookies(), 1)
+		for _, c := range resp.Cookies() {
+			assert.WithinDuration(t, session.ExpiresAt, c.Expires, 5*time.Second, "Ensure the expiry does not deviate +- 5 seconds from the expiry of the session for cookie: %s", c.Name)
+			assert.NotEqual(t, oldExpires, c.Expires, "%s", c.Name)
+		}
+	})
+
+	t.Run("case=whoami should not issue cookie if request is token based", func(t *testing.T) {
+		_, _, session := setup(t)
+
+		session.ExpiresAt = time.Now().Add(time.Hour * 24 * 30).UTC().Round(time.Hour)
+		err := reg.SessionPersister().UpsertSession(context.Background(), session)
+		require.NoError(t, err)
+
+		req, err := http.NewRequest("GET", ts.URL+"/sessions/whoami", nil)
+		require.NoError(t, err)
+		req.Header.Set("Authorization", "Bearer "+session.Token)
+
+		resp, err := http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+		require.Len(t, resp.Cookies(), 0)
+	})
 }
 
 func TestHandlerRefreshSessionBySessionID(t *testing.T) {

session/manager.go

@@ -91,6 +91,9 @@ type Manager interface {
 	// Also regenerates CSRF tokens due to assumed principal change.
 	IssueCookie(context.Context, http.ResponseWriter, *http.Request, *Session) error
 
+	// RefreshCookie checks if the request uses an outdated cookie and refreshes the cookie if needed.
+	RefreshCookie(context.Context, http.ResponseWriter, *http.Request, *Session) error
+
 	// FetchFromRequest creates an HTTP session using cookies.
 	FetchFromRequest(context.Context, *http.Request) (*Session, error)
 

session/manager_http.go

@@ -4,6 +4,9 @@ import (
 	"context"
 	"net/http"
 	"net/url"
+	"time"
+
+	"github.com/gorilla/sessions"
 
 	"github.com/ory/x/urlx"
 
@@ -58,6 +61,29 @@ func (s *ManagerHTTP) UpsertAndIssueCookie(ctx context.Context, w http.ResponseW
 	return nil
 }
 
+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) {
+		return nil
+	}
+
+	cookie, err := s.getCookie(r)
+	if err != nil {
+		return err
+	}
+
+	expiresAt := getCookieExpiry(cookie)
+	if expiresAt == nil || expiresAt.Before(session.ExpiresAt) {
+		if err := s.IssueCookie(ctx, w, r, session); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (s *ManagerHTTP) IssueCookie(ctx context.Context, w http.ResponseWriter, r *http.Request, session *Session) error {
 	cookie, err := s.r.CookieManager(r.Context()).Get(r, s.cookieName(ctx))
 	// Fix for https://github.com/ory/kratos/issues/1695
@@ -94,28 +120,51 @@ func (s *ManagerHTTP) IssueCookie(ctx context.Context, w http.ResponseWriter, r
 
 	cookie.Options.MaxAge = 0
 	if s.r.Config(ctx).SessionPersistentCookie() {
-		cookie.Options.MaxAge = int(s.r.Config(ctx).SessionLifespan().Seconds())
+		if session.ExpiresAt.IsZero() {
+			cookie.Options.MaxAge = int(s.r.Config(ctx).SessionLifespan().Seconds())
+		} else {
+			cookie.Options.MaxAge = int(time.Until(session.ExpiresAt).Seconds())
+		}
 	}
 
 	cookie.Values["session_token"] = session.Token
+	cookie.Values["expires_at"] = session.ExpiresAt.UTC().Format(time.RFC3339Nano)
+
 	if err := cookie.Save(r, w); err != nil {
 		return errors.WithStack(err)
 	}
 	return nil
 }
 
-func (s *ManagerHTTP) extractToken(r *http.Request) string {
-	if token := r.Header.Get("X-Session-Token"); len(token) > 0 {
-		return token
+func getCookieExpiry(s *sessions.Session) *time.Time {
+	expiresAt, ok := s.Values["expires_at"].(string)
+	if !ok {
+		return nil
 	}
 
+	n, err := time.Parse(time.RFC3339Nano, expiresAt)
+	if err != nil {
+		return nil
+	}
+	return &n
+}
+
+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}}
 	}
 
-	cookie, err := s.r.CookieManager(r.Context()).Get(r, s.cookieName(r.Context()))
+	return s.r.CookieManager(r.Context()).Get(r, s.cookieName(r.Context()))
+}
+
+func (s *ManagerHTTP) extractToken(r *http.Request) string {
+	if token := r.Header.Get("X-Session-Token"); len(token) > 0 {
+		return token
+	}
+
+	cookie, err := s.getCookie(r)
 	if err != nil {
 		token, _ := bearerTokenFromRequest(r)
 		return token