feat: remove login session cookie

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
-}