feat: add expiry and requested times to logout table (#3837)

consent/strategy_default.go

@@ -860,6 +860,8 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
 			Subject:     session.Subject,
 			SessionID:   session.ID,
 			Verifier:    uuid.New(),
+			RequestedAt: sqlxx.NullTime(time.Now().UTC().Round(time.Second)),
+			ExpiresAt:   sqlxx.NullTime(time.Now().UTC().Round(time.Second).Add(s.c.ConsentRequestMaxAge(ctx))),
 			RPInitiated: false,
 
 			// PostLogoutRedirectURI is set to the value from config.Provider().LogoutRedirectURL()

flow/.snapshots/TestLogoutRequest_MarshalJSON.json

@@ -1 +1 @@
-"{\"challenge\":\"\",\"subject\":\"\",\"request_url\":\"\",\"rp_initiated\":false,\"client\":null}"
+"{\"challenge\":\"\",\"subject\":\"\",\"request_url\":\"\",\"rp_initiated\":false,\"expires_at\":null,\"requested_at\":null,\"client\":null}"

flow/consent_types.go

@@ -507,6 +507,8 @@ type LogoutRequest struct {
 	Accepted              bool           `json:"-" db:"accepted"`
 	Rejected              bool           `db:"rejected" json:"-"`
 	ClientID              sql.NullString `json:"-" db:"client_id"`
+	ExpiresAt             sqlxx.NullTime `json:"expires_at" db:"expires_at"`
+	RequestedAt           sqlxx.NullTime `json:"requested_at" db:"requested_at"`
 	Client                *client.Client `json:"client" db:"-"`
 }
 

flow/error.go

@@ -0,0 +1,8 @@
+// Copyright © 2024 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package flow
+
+import "github.com/ory/fosite"
+
+var ErrorLogoutFlowExpired = fosite.ErrRequestUnauthorized.WithHint("The logout request has expired, please try the flow again.")

persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0009.json

@@ -4,5 +4,7 @@
   "sid": "session_id-0009",
   "request_url": "http://request/0009",
   "rp_initiated": true,
+  "expires_at": null,
+  "requested_at": null,
   "client": null
 }

persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0010.json

@@ -4,5 +4,7 @@
   "sid": "session_id-0010",
   "request_url": "http://request/0010",
   "rp_initiated": true,
+  "expires_at": null,
+  "requested_at": null,
   "client": null
 }

persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0011.json

@@ -4,5 +4,7 @@
   "sid": "session_id-0011",
   "request_url": "http://request/0011",
   "rp_initiated": true,
+  "expires_at": null,
+  "requested_at": null,
   "client": null
 }

persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0012.json

@@ -4,5 +4,7 @@
   "sid": "session_id-0012",
   "request_url": "http://request/0012",
   "rp_initiated": true,
+  "expires_at": null,
+  "requested_at": null,
   "client": null
 }

persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0013.json

@@ -4,5 +4,7 @@
   "sid": "session_id-0013",
   "request_url": "http://request/0013",
   "rp_initiated": true,
+  "expires_at": null,
+  "requested_at": null,
   "client": null
 }

persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0014.json

@@ -4,5 +4,7 @@
   "sid": "session_id-0014",
   "request_url": "http://request/0014",
   "rp_initiated": true,
+  "expires_at": null,
+  "requested_at": null,
   "client": null
 }

persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-20240916105610000001.json

@@ -0,0 +1,10 @@
+{
+  "challenge": "challenge-20240916105610000001",
+  "subject": "subject-0014",
+  "sid": "session_id-0014",
+  "request_url": "http://request/0014",
+  "rp_initiated": true,
+  "expires_at": "2022-02-15T22:20:20Z",
+  "requested_at": "2022-02-15T22:20:20Z",
+  "client": null
+}

persistence/sql/migratest/migration_test.go

@@ -170,7 +170,7 @@ func TestMigrations(t *testing.T) {
 				t.Run("case=hydra_oauth2_logout_request", func(t *testing.T) {
 					lrs := []flow.LogoutRequest{}
 					c.All(&lrs)
-					require.Equal(t, 6, len(lrs))
+					require.Equal(t, 7, len(lrs))
 
 					for _, s := range lrs {
 						testhelpersuuid.AssertUUID(t, s.NID)

persistence/sql/migratest/testdata/20240916105610_testdata.sql

@@ -0,0 +1,4 @@
+INSERT INTO hydra_oauth2_logout_request (challenge, verifier, subject, sid, client_id, nid, request_url, redir_url,
+                                         was_used, accepted, rejected, rp_initiated, expires_at, requested_at)
+VALUES ('challenge-20240916105610000001', 'verifier-20240916105610000001', 'subject-0014', 'session_id-0014', 'client-0014',
+        (SELECT id FROM networks LIMIT 1), 'http://request/0014', 'http://post_logout/0014', true, true, false, true, '2022-02-15 22:20:20', '2022-02-15 22:20:20');

persistence/sql/migrations/20240916105610000001_add_logout_request_timestamps.down.sql

@@ -0,0 +1,2 @@
+ALTER TABLE hydra_oauth2_logout_request DROP expires_at;
+ALTER TABLE hydra_oauth2_logout_request DROP requested_at;

persistence/sql/migrations/20240916105610000001_add_logout_request_timestamps.up.sql

@@ -0,0 +1,2 @@
+ALTER TABLE hydra_oauth2_logout_request ADD expires_at timestamp NULL;
+ALTER TABLE hydra_oauth2_logout_request ADD requested_at timestamp NULL;

persistence/sql/persister_consent.go

@@ -735,6 +735,13 @@ WHERE nid = ?
 		return nil, err
 	}
 
+	if expiry := time.Time(lr.ExpiresAt);
+	// If the expiry is unset, we are in a legacy use case (allow logout).
+	// TODO: Remove this in the future.
+	!expiry.IsZero() && expiry.Before(time.Now().UTC()) {
+		return nil, errorsx.WithStack(flow.ErrorLogoutFlowExpired)
+	}
+
 	return &lr, nil
 }
 

persistence/sql/persister_nid_test.go

@@ -2071,27 +2071,54 @@ func (s *PersisterTestSuite) TestVerifyAndInvalidateLogoutRequest() {
 	t := s.T()
 	for k, r := range s.registries {
 		t.Run(k, func(t *testing.T) {
-			lr := newLogoutRequest()
-			lr.Verifier = uuid.Must(uuid.NewV4()).String()
-			lr.Accepted = true
-			lr.Rejected = false
-			require.NoError(t, r.ConsentManager().CreateLogoutRequest(s.t1, lr))
+			run := func(t *testing.T, lr *flow.LogoutRequest) {
+				lr.Verifier = uuid.Must(uuid.NewV4()).String()
+				lr.Accepted = true
+				lr.Rejected = false
+				require.NoError(t, r.ConsentManager().CreateLogoutRequest(s.t1, lr))
+
+				expected, err := r.ConsentManager().GetLogoutRequest(s.t1, lr.ID)
+				require.NoError(t, err)
+
+				lrInvalidated, err := r.ConsentManager().VerifyAndInvalidateLogoutRequest(s.t2, lr.Verifier)
+				require.Error(t, err)
+				require.Nil(t, lrInvalidated)
+				actual := &flow.LogoutRequest{}
+				require.NoError(t, r.Persister().Connection(context.Background()).Find(actual, lr.ID))
+				require.Equal(t, expected, actual)
+
+				lrInvalidated, err = r.ConsentManager().VerifyAndInvalidateLogoutRequest(s.t1, lr.Verifier)
+				require.NoError(t, err)
+				require.NoError(t, r.Persister().Connection(context.Background()).Find(actual, lr.ID))
+				require.Equal(t, lrInvalidated, actual)
+				require.Equal(t, true, actual.WasHandled)
+			}
 
-			expected, err := r.ConsentManager().GetLogoutRequest(s.t1, lr.ID)
-			require.NoError(t, err)
+			t.Run("case=legacy logout request without expiry", func(t *testing.T) {
+				lr := newLogoutRequest()
+				run(t, lr)
+			})
 
-			lrInvalidated, err := r.ConsentManager().VerifyAndInvalidateLogoutRequest(s.t2, lr.Verifier)
-			require.Error(t, err)
-			require.Nil(t, lrInvalidated)
-			actual := &flow.LogoutRequest{}
-			require.NoError(t, r.Persister().Connection(context.Background()).Find(actual, lr.ID))
-			require.Equal(t, expected, actual)
+			t.Run("case=logout request with expiry", func(t *testing.T) {
+				lr := newLogoutRequest()
+				lr.ExpiresAt = sqlxx.NullTime(time.Now().Add(time.Hour))
+				run(t, lr)
+			})
 
-			lrInvalidated, err = r.ConsentManager().VerifyAndInvalidateLogoutRequest(s.t1, lr.Verifier)
-			require.NoError(t, err)
-			require.NoError(t, r.Persister().Connection(context.Background()).Find(actual, lr.ID))
-			require.Equal(t, lrInvalidated, actual)
-			require.Equal(t, true, actual.WasHandled)
+			t.Run("case=logout request that expired returns error", func(t *testing.T) {
+				lr := newLogoutRequest()
+				lr.ExpiresAt = sqlxx.NullTime(time.Now().Add(-time.Hour))
+				lr.Verifier = uuid.Must(uuid.NewV4()).String()
+				lr.Accepted = true
+				lr.Rejected = false
+				require.NoError(t, r.ConsentManager().CreateLogoutRequest(s.t1, lr))
+
+				_, err := r.ConsentManager().VerifyAndInvalidateLogoutRequest(s.t2, lr.Verifier)
+				require.ErrorIs(t, err, x.ErrNotFound)
+
+				_, err = r.ConsentManager().VerifyAndInvalidateLogoutRequest(s.t1, lr.Verifier)
+				require.ErrorIs(t, err, flow.ErrorLogoutFlowExpired)
+			})
 		})
 	}
 }