Skip to content

Commit

Permalink
feat: add expiry and requested times to logout table
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Sep 16, 2024
1 parent ee427ed commit f51abee
Show file tree
Hide file tree
Showing 19 changed files with 93 additions and 20 deletions.
2 changes: 2 additions & 0 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion flow/.snapshots/TestLogoutRequest_MarshalJSON.json
Original file line number Diff line number Diff line change
@@ -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}"
2 changes: 2 additions & 0 deletions flow/consent_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
}

Expand Down
5 changes: 5 additions & 0 deletions flow/error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package flow

import "github.com/ory/fosite"

var ErrorLogoutFlowExpired = fosite.ErrRequestUnauthorized.WithHint("The logout request has expired, please try the flow again.")
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@
"sid": "session_id-0009",
"request_url": "http://request/0009",
"rp_initiated": true,
"expires_at": null,
"requested_at": null,
"client": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@
"sid": "session_id-0010",
"request_url": "http://request/0010",
"rp_initiated": true,
"expires_at": null,
"requested_at": null,
"client": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@
"sid": "session_id-0011",
"request_url": "http://request/0011",
"rp_initiated": true,
"expires_at": null,
"requested_at": null,
"client": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@
"sid": "session_id-0012",
"request_url": "http://request/0012",
"rp_initiated": true,
"expires_at": null,
"requested_at": null,
"client": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@
"sid": "session_id-0013",
"request_url": "http://request/0013",
"rp_initiated": true,
"expires_at": null,
"requested_at": null,
"client": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@
"sid": "session_id-0014",
"request_url": "http://request/0014",
"rp_initiated": true,
"expires_at": null,
"requested_at": null,
"client": null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"challenge": "challenge-20240916105610000001",
"subject": "subject-0014",
"sid": "session_id-0014",
"request_url": "http://request/0014",
"rp_initiated": true,
"expires_at": null,
"requested_at": null,
"client": null
}
2 changes: 1 addition & 1 deletion persistence/sql/migratest/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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, 'now()', 'now()');
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE hydra_oauth2_logout_request DROP expires_at;
ALTER TABLE hydra_oauth2_logout_request DROP requested_at;
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 7 additions & 0 deletions persistence/sql/persister_consent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
63 changes: 45 additions & 18 deletions persistence/sql/persister_nid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
}
}
Expand Down

0 comments on commit f51abee

Please sign in to comment.