feat: remove login session cookie

.gitignore

@@ -31,3 +31,4 @@ persistence/sql/migrations/schema.sql
 cypress/videos
 cypress/screenshots
 BENCHMARKS.md
+*.sqlite

consent/manager.go

@@ -5,7 +5,6 @@ package consent
 
 import (
 	"context"
-	"time"
 
 	"github.com/gofrs/uuid"
 
@@ -43,7 +42,7 @@ type (
 		CreateLoginSession(ctx context.Context, session *flow.LoginSession) error
 		DeleteLoginSession(ctx context.Context, id string) (deletedSession *flow.LoginSession, err error)
 		RevokeSubjectLoginSession(ctx context.Context, user string) error
-		ConfirmLoginSession(ctx context.Context, session *flow.LoginSession, id string, authTime time.Time, subject string, remember bool) error
+		ConfirmLoginSession(ctx context.Context, loginSession *flow.LoginSession) error
 
 		CreateLoginRequest(ctx context.Context, req *flow.LoginRequest) (*flow.Flow, error)
 		GetLoginRequest(ctx context.Context, challenge string) (*flow.LoginRequest, error)

consent/manager_test_helpers.go

@@ -322,8 +322,8 @@ func TestHelperNID(r interface {
 		require.Error(t, err)
 		_, err = t1ValidNID.HandleLoginRequest(ctx, f, testLR.ID, &testHLR)
 		require.NoError(t, err)
-		require.Error(t, t2InvalidNID.ConfirmLoginSession(ctx, &testLS, testLS.ID, time.Now(), testLS.Subject, true))
-		require.NoError(t, t1ValidNID.ConfirmLoginSession(ctx, &testLS, testLS.ID, time.Now(), testLS.Subject, true))
+		require.Error(t, t2InvalidNID.ConfirmLoginSession(ctx, &testLS))
+		require.NoError(t, t1ValidNID.ConfirmLoginSession(ctx, &testLS))
 		ls, err := t2InvalidNID.DeleteLoginSession(ctx, testLS.ID)
 		require.Error(t, err)
 		assert.Nil(t, ls)
@@ -356,7 +356,7 @@ func ManagerTests(deps Deps, m Manager, clientManager client.Manager, fositeMana
 					Subject:         fmt.Sprintf("subject-%s", k),
 				}
 				require.NoError(t, m.CreateLoginSession(ctx, loginSession))
-				require.NoError(t, m.ConfirmLoginSession(ctx, loginSession, loginSession.ID, time.Now().Round(time.Second).UTC(), loginSession.Subject, true))
+				require.NoError(t, m.ConfirmLoginSession(ctx, loginSession))
 
 				lr[k] = &flow.LoginRequest{
 					ID:              makeID("fk-login-challenge", network, k),
@@ -392,6 +392,7 @@ func ManagerTests(deps Deps, m Manager, clientManager client.Manager, fositeMana
 					},
 				},
 			} {
+				tc := tc
 				t.Run("case=create-get-"+tc.s.ID, func(t *testing.T) {
 					_, err := m.GetRememberedLoginSession(ctx, &tc.s, tc.s.ID)
 					require.EqualError(t, err, x.ErrNotFound.Error(), "%#v", err)
@@ -403,17 +404,28 @@ func ManagerTests(deps Deps, m Manager, clientManager client.Manager, fositeMana
 					require.EqualError(t, err, x.ErrNotFound.Error())
 
 					updatedAuth := time.Time(tc.s.AuthenticatedAt).Add(time.Second)
-					require.NoError(t, m.ConfirmLoginSession(ctx, &tc.s, tc.s.ID, updatedAuth, tc.s.Subject, true))
+					tc.s.AuthenticatedAt = sqlxx.NullTime(updatedAuth)
+					require.NoError(t, m.ConfirmLoginSession(ctx, &flow.LoginSession{
+						ID:              tc.s.ID,
+						AuthenticatedAt: sqlxx.NullTime(updatedAuth),
+						Subject:         tc.s.Subject,
+						Remember:        true,
+					}))
 
 					got, err := m.GetRememberedLoginSession(ctx, nil, tc.s.ID)
 					require.NoError(t, err)
 					assert.EqualValues(t, tc.s.ID, got.ID)
-					assert.Equal(t, updatedAuth.Unix(), time.Time(got.AuthenticatedAt).Unix()) // this was updated from confirm...
+					assert.Equal(t, tc.s.AuthenticatedAt, got.AuthenticatedAt) // this was updated from confirm...
 					assert.EqualValues(t, tc.s.Subject, got.Subject)
 
 					// Make sure AuthAt does not equal...
 					updatedAuth2 := updatedAuth.Add(1 * time.Second).UTC()
-					require.NoError(t, m.ConfirmLoginSession(ctx, nil, tc.s.ID, updatedAuth2, "some-other-subject", true))
+					require.NoError(t, m.ConfirmLoginSession(ctx, &flow.LoginSession{
+						ID:              tc.s.ID,
+						AuthenticatedAt: sqlxx.NullTime(updatedAuth2),
+						Subject:         "some-other-subject",
+						Remember:        true,
+					}))
 
 					got2, err := m.GetRememberedLoginSession(ctx, nil, tc.s.ID)
 					require.NoError(t, err)
@@ -902,7 +914,8 @@ func ManagerTests(deps Deps, m Manager, clientManager client.Manager, fositeMana
 							Subject:         subject,
 						}
 						require.NoError(t, m.CreateLoginSession(ctx, ls))
-						require.NoError(t, m.ConfirmLoginSession(ctx, ls, ls.ID, time.Now(), ls.Subject, true))
+						ls.Remember = true
+						require.NoError(t, m.ConfirmLoginSession(ctx, ls))
 
 						cl := &client.Client{ID: uuid.New().String()}
 						switch k % 4 {
@@ -1042,7 +1055,7 @@ func ManagerTests(deps Deps, m Manager, clientManager client.Manager, fositeMana
 			}
 
 			require.NoError(t, m.CreateLoginSession(ctx, &s))
-			require.NoError(t, m.ConfirmLoginSession(ctx, &s, s.ID, time.Time(s.AuthenticatedAt), s.Subject, false))
+			require.NoError(t, m.ConfirmLoginSession(ctx, &s))
 
 			lr := &flow.LoginRequest{
 				ID:              uuid.New().String(),

consent/sdk_test.go

@@ -104,7 +104,7 @@ func TestSDK(t *testing.T) {
 
 	loginSession3 := &LoginSession{ID: cr3.LoginSessionID.String()}
 	require.NoError(t, m.CreateLoginSession(context.Background(), loginSession3))
-	require.NoError(t, m.ConfirmLoginSession(context.Background(), loginSession3, loginSession3.ID, time.Now(), cr3.Subject, true))
+	require.NoError(t, m.ConfirmLoginSession(context.Background(), loginSession3))
 	cr3Flow, err := m.CreateLoginRequest(context.Background(), &LoginRequest{
 		ID:          cr3.LoginChallenge.String(),
 		Subject:     cr3.Subject,
@@ -117,7 +117,7 @@ func TestSDK(t *testing.T) {
 
 	loginSession4 := &LoginSession{ID: cr4.LoginSessionID.String()}
 	require.NoError(t, m.CreateLoginSession(context.Background(), loginSession4))
-	require.NoError(t, m.ConfirmLoginSession(context.Background(), loginSession4, loginSession4.ID, time.Now(), cr4.Subject, true))
+	require.NoError(t, m.ConfirmLoginSession(context.Background(), loginSession4))
 	cr4Flow, err := m.CreateLoginRequest(context.Background(), &LoginRequest{
 		ID:        cr4.LoginChallenge.String(),
 		Client:    cr4.Client,

consent/strategy_default.go

@@ -108,8 +108,7 @@ func (s *DefaultStrategy) authenticationSession(ctx context.Context, _ http.Resp
 		return nil, errorsx.WithStack(ErrNoAuthenticationSessionFound)
 	}
 
-	sessionFromCookie := s.loginSessionFromCookie(r)
-	session, err := s.r.ConsentManager().GetRememberedLoginSession(r.Context(), sessionFromCookie, sessionID)
+	session, err := s.r.ConsentManager().GetRememberedLoginSession(r.Context(), nil, sessionID)
 	if errors.Is(err, x.ErrNotFound) {
 		s.r.Logger().WithRequest(r).WithError(err).
 			Debug("User logout skipped because cookie exists and session value exist but are not remembered any more.")
@@ -232,15 +231,6 @@ func (s *DefaultStrategy) forwardAuthenticationRequest(ctx context.Context, w ht
 	sessionID := uuid.New()
 	if session != nil {
 		sessionID = session.ID
-	} else {
-		// Create a stub session so that we can later update it.
-		loginSession := &flow.LoginSession{ID: sessionID}
-		if err := s.r.ConsentManager().CreateLoginSession(ctx, loginSession); err != nil {
-			return err
-		}
-		if err := flowctx.SetCookie(ctx, w, s.r, flowctx.LoginSessionCookie(flowctx.SuffixForClient(ar.GetClient())), loginSession); err != nil {
-			return err
-		}
 	}
 
 	// Set the session
@@ -454,16 +444,17 @@ func (s *DefaultStrategy) verifyAuthentication(
 	if !session.LoginRequest.Skip {
 		if time.Time(session.AuthenticatedAt).IsZero() {
 			return nil, errorsx.WithStack(fosite.ErrServerError.WithHint(
-				"Expected the handled login request to contain a valid authenticated_at value but it was zero. This is a bug which should be reported to https://github.com/ory/hydra."))
+				"Expected the handled login request to contain a valid authenticated_at value but it was zero. " +
+					"This is a bug which should be reported to https://github.com/ory/hydra."))
 		}
 
-		loginSession := s.loginSessionFromCookie(r)
-		if loginSession == nil {
-			return nil, fosite.ErrAccessDenied.WithHint("The login session cookie was not found or malformed.")
-		}
-
-		loginSession.IdentityProviderSessionID = sqlxx.NullString(session.IdentityProviderSessionID)
-		if err := s.r.ConsentManager().ConfirmLoginSession(ctx, loginSession, sessionID, time.Time(session.AuthenticatedAt), session.Subject, session.Remember); err != nil {
+		if err := s.r.ConsentManager().ConfirmLoginSession(ctx, &flow.LoginSession{
+			ID:                        sessionID,
+			AuthenticatedAt:           session.AuthenticatedAt,
+			Subject:                   session.Subject,
+			IdentityProviderSessionID: sqlxx.NullString(session.IdentityProviderSessionID),
+			Remember:                  session.Remember,
+		}); err != nil {
 			if errors.Is(err, sqlcon.ErrUniqueViolation) {
 				return nil, errorsx.WithStack(fosite.ErrAccessDenied.WithHint("The login verifier has already been used."))
 			}
@@ -633,6 +624,10 @@ func (s *DefaultStrategy) forwardConsentRequest(
 		return err
 	}
 
+	if f.Client.GetID() != cl.GetID() {
+		return errorsx.WithStack(fosite.ErrInvalidClient.WithHint("The flow client id does not match the authorize request client id."))
+	}
+
 	clientSpecificCookieNameConsentCSRF := fmt.Sprintf("%s_%s", s.r.Config().CookieNameConsentCSRF(ctx), cl.CookieSuffix())
 	if err := createCsrfSession(w, r, s.r.Config(), store, clientSpecificCookieNameConsentCSRF, csrf, s.c.ConsentRequestMaxAge(ctx)); err != nil {
 		return errorsx.WithStack(err)
@@ -970,8 +965,7 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
 
 	// We do not really want to verify if the user (from id token hint) has a session here because it doesn't really matter.
 	// Instead, we'll check this when we're actually revoking the cookie!
-	sessionFromCookie := s.loginSessionFromCookie(r)
-	session, err := s.r.ConsentManager().GetRememberedLoginSession(r.Context(), sessionFromCookie, hintSid)
+	session, err := s.r.ConsentManager().GetRememberedLoginSession(r.Context(), nil, hintSid)
 	if errors.Is(err, x.ErrNotFound) {
 		// Such a session does not exist - maybe it has already been revoked? In any case, we can't do much except
 		// leaning back and redirecting back.
@@ -1101,8 +1095,7 @@ func (s *DefaultStrategy) HandleOpenIDConnectLogout(ctx context.Context, w http.
 }
 
 func (s *DefaultStrategy) HandleHeadlessLogout(ctx context.Context, _ http.ResponseWriter, r *http.Request, sid string) error {
-	sessionFromCookie := s.loginSessionFromCookie(r)
-	loginSession, lsErr := s.r.ConsentManager().GetRememberedLoginSession(ctx, sessionFromCookie, sid)
+	loginSession, lsErr := s.r.ConsentManager().GetRememberedLoginSession(ctx, nil, sid)
 
 	if errors.Is(lsErr, x.ErrNotFound) {
 		// This is ok (session probably already revoked), do nothing!

consent/strategy_logout_test.go

@@ -512,11 +512,11 @@ func TestLogoutFlows(t *testing.T) {
 				defer wg.Done()
 				require.NoError(t, err)
 				assert.False(t, res.Skip)
-				return hydra.AcceptOAuth2LoginRequest{Remember: pointerx.Bool(true)}
+				return hydra.AcceptOAuth2LoginRequest{Remember: pointerx.Ptr(true)}
 			}),
 			checkAndAcceptConsentHandler(t, adminApi, func(t *testing.T, res *hydra.OAuth2ConsentRequest, err error) hydra.AcceptOAuth2ConsentRequest {
 				require.NoError(t, err)
-				return hydra.AcceptOAuth2ConsentRequest{Remember: pointerx.Bool(true)}
+				return hydra.AcceptOAuth2ConsentRequest{Remember: pointerx.Ptr(true)}
 			}))
 
 		// Make an oauth 2 request to trigger the login check.

consent/strategy_oauth_test.go

@@ -19,8 +19,6 @@ import (
 	"golang.org/x/exp/slices"
 	"golang.org/x/oauth2"
 
-	"github.com/ory/hydra/v2/aead"
-	"github.com/ory/hydra/v2/consent"
 	"github.com/ory/x/pointerx"
 
 	"github.com/tidwall/gjson"
@@ -114,7 +112,7 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 	t.Run("case=should fail because a login verifier was given that doesn't exist in the store", func(t *testing.T) {
 		testhelpers.NewLoginConsentUI(t, reg.Config(), testhelpers.HTTPServerNoExpectedCallHandler(t), testhelpers.HTTPServerNoExpectedCallHandler(t))
 		c := createDefaultClient(t)
-		hc := newHTTPClientWithFlowCookie(t, ctx, reg, c)
+		hc := testhelpers.NewEmptyJarClient(t)
 
 		makeRequestAndExpectError(
 			t, hc, c, url.Values{"login_verifier": {"does-not-exist"}},
@@ -128,7 +126,7 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 		// - This should fail because a consent verifier was given but no login verifier
 		testhelpers.NewLoginConsentUI(t, reg.Config(), testhelpers.HTTPServerNoExpectedCallHandler(t), testhelpers.HTTPServerNoExpectedCallHandler(t))
 		c := createDefaultClient(t)
-		hc := newHTTPClientWithFlowCookie(t, ctx, reg, c)
+		hc := testhelpers.NewEmptyJarClient(t)
 
 		makeRequestAndExpectError(
 			t, hc, c, url.Values{"consent_verifier": {"does-not-exist"}},
@@ -252,7 +250,7 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 			require.NoError(t, err)
 			q := res.Request.URL.Query()
 			assert.Equal(t,
-				"The resource owner or authorization server denied the request. The login verifier has already been used.",
+				"The resource owner or authorization server denied the request. The consent verifier has already been used.",
 				q.Get("error_description"), q)
 		})
 
@@ -1122,15 +1120,3 @@ func (d *dropCSRFCookieJar) SetCookies(u *url.URL, cookies []*http.Cookie) {
 func (d *dropCSRFCookieJar) Cookies(u *url.URL) []*http.Cookie {
 	return d.jar.Cookies(u)
 }
-
-// TODO(hperl): rename
-func newHTTPClientWithFlowCookie(t *testing.T, ctx context.Context, reg interface {
-	ConsentManager() consent.Manager
-	Config() *config.DefaultProvider
-	FlowCipher() *aead.XChaCha20Poly1305
-}, c *client.Client) *http.Client {
-
-	hc := testhelpers.NewEmptyJarClient(t)
-
-	return hc
-}

go.mod

@@ -1,6 +1,8 @@
 module github.com/ory/hydra/v2
 
-go 1.20
+go 1.21
+
+toolchain go1.21.3
 
 replace (
 	github.com/jackc/pcmock => github.com/jackc/pgmock v0.0.0-20210724152146-4ad1a8207f65
@@ -16,16 +18,16 @@ require (
 	github.com/cenkalti/backoff/v3 v3.2.2
 	github.com/fatih/structs v1.1.0
 	github.com/go-faker/faker/v4 v4.1.1
-	github.com/go-jose/go-jose/v3 v3.0.0
+	github.com/go-jose/go-jose/v3 v3.0.1
 	github.com/go-swagger/go-swagger v0.30.5
 	github.com/gobuffalo/pop/v6 v6.1.2-0.20230318123913-c85387acc9a0
 	github.com/gobwas/glob v0.2.3
 	github.com/gofrs/uuid v4.4.0+incompatible
 	github.com/golang-jwt/jwt/v5 v5.0.0
 	github.com/golang/mock v1.6.0
-	github.com/google/uuid v1.3.0
-	github.com/gorilla/securecookie v1.1.1
-	github.com/gorilla/sessions v1.2.1
+	github.com/google/uuid v1.4.0
+	github.com/gorilla/securecookie v1.1.2
+	github.com/gorilla/sessions v1.2.2
 	github.com/hashicorp/go-retryablehttp v0.7.4
 	github.com/jackc/pgx/v4 v4.18.1
 	github.com/julienschmidt/httprouter v1.3.0
@@ -42,7 +44,7 @@ require (
 	github.com/ory/hydra-client-go/v2 v2.1.1
 	github.com/ory/jsonschema/v3 v3.0.8
 	github.com/ory/kratos-client-go v0.13.1
-	github.com/ory/x v0.0.582-0.20230816082414-f1e6acad79b5
+	github.com/ory/x v0.0.601
 	github.com/pborman/uuid v1.2.1
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.16.0
@@ -52,23 +54,23 @@ require (
 	github.com/spf13/cobra v1.7.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.4
-	github.com/tidwall/gjson v1.15.0
+	github.com/tidwall/gjson v1.17.0
 	github.com/tidwall/sjson v1.2.5
 	github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80
 	github.com/toqueteos/webbrowser v1.2.0
 	github.com/twmb/murmur3 v1.1.8
 	github.com/urfave/negroni v1.0.0
-	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0
-	go.opentelemetry.io/otel v1.16.0
-	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0
-	go.opentelemetry.io/otel/sdk v1.16.0
-	go.opentelemetry.io/otel/trace v1.16.0
+	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.45.0
+	go.opentelemetry.io/otel v1.19.0
+	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0
+	go.opentelemetry.io/otel/sdk v1.19.0
+	go.opentelemetry.io/otel/trace v1.19.0
 	go.uber.org/automaxprocs v1.5.3
-	golang.org/x/crypto v0.14.0
-	golang.org/x/exp v0.0.0-20230809094429-853ea248256d
-	golang.org/x/oauth2 v0.11.0
-	golang.org/x/sync v0.3.0
-	golang.org/x/tools v0.12.0
+	golang.org/x/crypto v0.15.0
+	golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa
+	golang.org/x/oauth2 v0.14.0
+	golang.org/x/sync v0.5.0
+	golang.org/x/tools v0.15.0
 )
 
 require github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
@@ -108,9 +110,9 @@ require (
 	github.com/fatih/color v1.15.0 // indirect
 	github.com/fatih/structtag v1.2.0 // indirect
 	github.com/felixge/fgprof v0.9.3 // indirect
-	github.com/felixge/httpsnoop v1.0.3 // indirect
+	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
-	github.com/go-logr/logr v1.2.4 // indirect
+	github.com/go-logr/logr v1.3.0 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-openapi/analysis v0.21.4 // indirect
 	github.com/go-openapi/errors v0.20.4 // indirect
@@ -181,7 +183,7 @@ require (
 	github.com/mattn/go-sqlite3 v2.0.3+incompatible // indirect
 	github.com/mattn/goveralls v0.0.12 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
-	github.com/microcosm-cc/bluemonday v1.0.25 // indirect
+	github.com/microcosm-cc/bluemonday v1.0.26 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
@@ -226,21 +228,20 @@ require (
 	go.opentelemetry.io/contrib/propagators/jaeger v1.17.0 // indirect
 	go.opentelemetry.io/contrib/samplers/jaegerremote v0.11.0 // indirect
 	go.opentelemetry.io/otel/exporters/jaeger v1.16.0 // indirect
-	go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 // indirect
-	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0 // indirect
+	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.19.0 // indirect
 	go.opentelemetry.io/otel/exporters/zipkin v1.16.0 // indirect
-	go.opentelemetry.io/otel/metric v1.16.0 // indirect
+	go.opentelemetry.io/otel/metric v1.19.0 // indirect
 	go.opentelemetry.io/proto/otlp v1.0.0 // indirect
-	golang.org/x/mod v0.12.0 // indirect
-	golang.org/x/net v0.17.0 // indirect
-	golang.org/x/sys v0.13.0 // indirect
-	golang.org/x/text v0.13.0 // indirect
+	golang.org/x/mod v0.14.0 // indirect
+	golang.org/x/net v0.18.0 // indirect
+	golang.org/x/sys v0.14.0 // indirect
+	golang.org/x/text v0.14.0 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
-	google.golang.org/genproto v0.0.0-20230807174057-1744710a1577 // indirect
-	google.golang.org/genproto/googleapis/api v0.0.0-20230807174057-1744710a1577 // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20230807174057-1744710a1577 // indirect
-	google.golang.org/grpc v1.57.0 // indirect
+	google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d // indirect
+	google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d // indirect
+	google.golang.org/grpc v1.59.0 // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/op/go-logging.v1 v1.0.0-20160211212156-b2cb9fa56473 // indirect