From f46ed9479b6b3cb355b64d2bf2b3bdb73269a549 Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Thu, 6 Feb 2025 17:10:48 +0100 Subject: [PATCH] feat: stateless logout --- consent/handler.go | 23 +-- consent/handler_test.go | 55 ------- consent/manager.go | 4 +- consent/sdk_test.go | 27 ++-- consent/strategy_default.go | 62 ++++--- consent/strategy_logout_test.go | 26 +-- consent/strategy_oauth_test.go | 2 +- consent/test/manager_test_helpers.go | 68 +------- .../TestLogoutRequest_MarshalJSON.json | 2 +- flow/consent_types.go | 58 ++----- go.mod | 2 +- go.sum | 4 +- internal/httpclient/api/openapi.yaml | 10 +- .../httpclient/docs/OAuth2LogoutRequest.md | 28 +--- .../model_o_auth2_logout_request.go | 39 +---- oauth2/flowctx/encoding.go | 4 + .../challenge-0009.json | 10 -- .../challenge-0010.json | 10 -- .../challenge-0011.json | 10 -- .../challenge-0012.json | 10 -- .../challenge-0013.json | 10 -- .../challenge-0014.json | 10 -- .../challenge-20240916105610000001.json | 10 -- persistence/sql/migratest/migration_test.go | 15 +- persistence/sql/persister_consent.go | 69 ++++---- persistence/sql/persister_nid_test.go | 151 ------------------ persistence/sql/persister_test.go | 2 +- spec/api.json | 12 +- spec/swagger.json | 12 +- 29 files changed, 128 insertions(+), 617 deletions(-) delete mode 100644 persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0009.json delete mode 100644 persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0010.json delete mode 100644 persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0011.json delete mode 100644 persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0012.json delete mode 100644 persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0013.json delete mode 100644 persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0014.json delete mode 100644 persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-20240916105610000001.json diff --git a/consent/handler.go b/consent/handler.go index f2605b405e9..ed0c04771a2 100644 --- a/consent/handler.go +++ b/consent/handler.go @@ -911,9 +911,7 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ // Accept OAuth 2.0 Logout Request // // swagger:parameters acceptOAuth2LogoutRequest -// -//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions -type acceptOAuth2LogoutRequest struct { +type _ struct { // OAuth 2.0 Logout Request Challenge // // in: query @@ -943,23 +941,21 @@ func (h *Handler) acceptOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque r.URL.Query().Get("challenge"), ) - c, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge) + verifier, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge) if err != nil { h.r.Writer().WriteError(w, r, err) return } h.r.Writer().Write(w, r, &flow.OAuth2RedirectTo{ - RedirectTo: urlx.SetQuery(urlx.AppendPaths(h.c.PublicURL(r.Context()), "/oauth2/sessions/logout"), url.Values{"logout_verifier": {c.Verifier}}).String(), + RedirectTo: urlx.SetQuery(urlx.AppendPaths(h.c.PublicURL(r.Context()), "/oauth2/sessions/logout"), url.Values{"logout_verifier": {verifier}}).String(), }) } // Reject OAuth 2.0 Logout Request // // swagger:parameters rejectOAuth2LogoutRequest -// -//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions -type rejectOAuth2LogoutRequest struct { +type _ struct { // in: query // required: true Challenge string `json:"logout_challenge"` @@ -999,9 +995,7 @@ func (h *Handler) rejectOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque // Get OAuth 2.0 Logout Request // // swagger:parameters getOAuth2LogoutRequest -// -//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions -type getOAuth2LogoutRequest struct { +type _ struct { // in: query // required: true Challenge string `json:"logout_challenge"` @@ -1039,13 +1033,6 @@ func (h *Handler) getOAuth2LogoutRequest(w http.ResponseWriter, r *http.Request, request.Client.Secret = "" } - if request.WasHandled { - h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{ - RedirectTo: request.RequestURL, - }) - return - } - h.r.Writer().Write(w, r, request) } diff --git a/consent/handler_test.go b/consent/handler_test.go index 45ba2b7733a..d74231f9837 100644 --- a/consent/handler_test.go +++ b/consent/handler_test.go @@ -27,61 +27,6 @@ import ( "github.com/ory/x/sqlxx" ) -func TestGetLogoutRequest(t *testing.T) { - for k, tc := range []struct { - exists bool - handled bool - status int - }{ - {false, false, http.StatusNotFound}, - {true, false, http.StatusOK}, - {true, true, http.StatusGone}, - } { - t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { - ctx := context.Background() - key := fmt.Sprint(k) - challenge := "challenge" + key - requestURL := "http://192.0.2.1" - - conf := testhelpers.NewConfigurationWithDefaults() - reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{}) - - if tc.exists { - cl := &client.Client{ID: "client" + key} - require.NoError(t, reg.ClientManager().CreateClient(ctx, cl)) - require.NoError(t, reg.ConsentManager().CreateLogoutRequest(context.TODO(), &flow.LogoutRequest{ - Client: cl, - ID: challenge, - WasHandled: tc.handled, - RequestURL: requestURL, - })) - } - - h := NewHandler(reg, conf) - r := x.NewRouterAdmin(conf.AdminURL) - h.SetRoutes(r) - ts := httptest.NewServer(r) - defer ts.Close() - - c := &http.Client{} - resp, err := c.Get(ts.URL + "/admin" + LogoutPath + "?challenge=" + challenge) - require.NoError(t, err) - require.EqualValues(t, tc.status, resp.StatusCode) - - if tc.handled { - var result flow.OAuth2RedirectTo - require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) - require.Equal(t, requestURL, result.RedirectTo) - } else if tc.exists { - var result flow.LogoutRequest - require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) - require.Equal(t, challenge, result.ID) - require.Equal(t, requestURL, result.RequestURL) - } - }) - } -} - func TestGetLoginRequest(t *testing.T) { for k, tc := range []struct { exists bool diff --git a/consent/manager.go b/consent/manager.go index fe4b018352e..f887a759343 100644 --- a/consent/manager.go +++ b/consent/manager.go @@ -55,9 +55,9 @@ type ( ListUserAuthenticatedClientsWithFrontChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error) ListUserAuthenticatedClientsWithBackChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error) - CreateLogoutRequest(ctx context.Context, request *flow.LogoutRequest) error + CreateLogoutChallenge(ctx context.Context, request *flow.LogoutRequest) (challenge string, err error) GetLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error) - AcceptLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error) + AcceptLogoutRequest(ctx context.Context, challenge string) (verifier string, err error) RejectLogoutRequest(ctx context.Context, challenge string) error VerifyAndInvalidateLogoutRequest(ctx context.Context, verifier string) (*flow.LogoutRequest, error) } diff --git a/consent/sdk_test.go b/consent/sdk_test.go index 0f30d16e7c8..d552fcc5964 100644 --- a/consent/sdk_test.go +++ b/consent/sdk_test.go @@ -147,13 +147,6 @@ func TestSDK(t *testing.T) { _, err = m.VerifyAndInvalidateConsentRequest(context.Background(), consentVerifier(cr4Flow)) require.NoError(t, err) - lur1 := test.MockLogoutRequest("testsdk-1", true, network) - require.NoError(t, reg.ClientManager().CreateClient(context.Background(), lur1.Client)) - require.NoError(t, m.CreateLogoutRequest(context.Background(), lur1)) - - lur2 := test.MockLogoutRequest("testsdk-2", false, network) - require.NoError(t, m.CreateLogoutRequest(context.Background(), lur2)) - cr1.ID = consentChallenge(cr1Flow) crGot := execute[hydra.OAuth2ConsentRequest](t, sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(cr1.ID)) compareSDKConsentRequest(t, cr1, *crGot) @@ -213,19 +206,25 @@ func TestSDK(t *testing.T) { require.NoError(t, err) assert.Equal(t, 0, len(csGot)) - luGot, _, err := sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(makeID("challenge", network, "testsdk-1")).Execute() + lur1 := test.MockLogoutRequest("testsdk-1", true, network) + require.NoError(t, reg.ClientManager().CreateClient(context.Background(), lur1.Client)) + loc1, err := m.CreateLogoutChallenge(context.Background(), lur1) require.NoError(t, err) - compareSDKLogoutRequest(t, lur1, luGot) - luaGot, _, err := sdk.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(makeID("challenge", network, "testsdk-1")).Execute() + luGot, _, err := sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(loc1).Execute() require.NoError(t, err) - assert.EqualValues(t, "https://www.ory.sh/oauth2/sessions/logout?logout_verifier="+makeID("verifier", network, "testsdk-1"), luaGot.RedirectTo) + compareSDKLogoutRequest(t, lur1, luGot) - _, err = sdk.OAuth2API.RejectOAuth2LogoutRequest(ctx).LogoutChallenge(lur2.ID).Execute() + luaGot, _, err := sdk.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(loc1).Execute() require.NoError(t, err) + assert.Contains(t, luaGot.RedirectTo, "https://www.ory.sh/oauth2/sessions/logout?logout_verifier=") - _, _, err = sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(lur2.ID).Execute() - require.Error(t, err) + lur2 := test.MockLogoutRequest("testsdk-2", false, network) + loc2, err := m.CreateLogoutChallenge(context.Background(), lur2) + require.NoError(t, err) + lur2Got, err := sdk.OAuth2API.RejectOAuth2LogoutRequest(ctx).LogoutChallenge(loc2).Execute() + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, lur2Got.StatusCode) } func compareSDKLoginRequest(t *testing.T, expected *LoginRequest, got hydra.OAuth2LoginRequest) { diff --git a/consent/strategy_default.go b/consent/strategy_default.go index 8acf7f7c2fa..53e75aafbf0 100644 --- a/consent/strategy_default.go +++ b/consent/strategy_default.go @@ -21,14 +21,13 @@ import ( "github.com/sirupsen/logrus" "go.opentelemetry.io/otel/trace" - "github.com/ory/hydra/v2/flow" - "github.com/ory/hydra/v2/oauth2/flowctx" - "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" "github.com/ory/hydra/v2/client" "github.com/ory/hydra/v2/driver/config" + "github.com/ory/hydra/v2/flow" + "github.com/ory/hydra/v2/oauth2/flowctx" "github.com/ory/hydra/v2/x" "github.com/ory/x/errorsx" "github.com/ory/x/mapx" @@ -855,21 +854,18 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon return nil, err } - challenge := uuid.New() - if err := s.r.ConsentManager().CreateLogoutRequest(r.Context(), &flow.LogoutRequest{ - RequestURL: r.URL.String(), - ID: challenge, - 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() + now := time.Now().UTC().Round(time.Second) + challenge, err := s.r.ConsentManager().CreateLogoutChallenge(ctx, &flow.LogoutRequest{ + RequestURL: r.URL.String(), + Subject: session.Subject, + SessionID: session.ID, + RequestedAt: now, + ExpiresAt: now.Add(s.c.ConsentRequestMaxAge(ctx)), + RPInitiated: false, PostLogoutRedirectURI: redir, - }); err != nil { - return nil, err + }) + if err != nil { + return nil, errors.WithStack(err) } s.r.AuditLogger(). @@ -895,13 +891,13 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon ) } - now := time.Now().UTC().Unix() - if !claims.VerifyIssuedAt(now, true) { + now := time.Now().UTC() + if !claims.VerifyIssuedAt(now.Unix(), true) { return nil, errorsx.WithStack(fosite.ErrInvalidRequest. WithHintf( `Logout failed because iat claim value '%.0f' from query parameter id_token_hint is before now ('%d').`, mapx.GetFloat64Default(mksi, "iat", float64(0)), - now, + now.Unix(), ), ) } @@ -939,6 +935,7 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon return nil, errorsx.WithStack(fosite.ErrInvalidRequest. WithHint("Logout failed because none of the listed audiences is a registered OAuth 2.0 Client.")) } + cl.Secret = "" // We don't want to expose the client secret. if len(requestedRedir) > 0 { var f *url.URL @@ -979,20 +976,19 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon return nil, err } - challenge := uuid.New() - if err := s.r.ConsentManager().CreateLogoutRequest(r.Context(), &flow.LogoutRequest{ - RequestURL: r.URL.String(), - ID: challenge, - SessionID: hintSid, - Subject: session.Subject, - Verifier: uuid.New(), - Client: cl, - RPInitiated: true, - - // PostLogoutRedirectURI is set to the value from config.Provider().LogoutRedirectURL() + now = time.Now().UTC().Round(time.Second) + challenge, err := s.r.ConsentManager().CreateLogoutChallenge(ctx, &flow.LogoutRequest{ + RequestURL: r.URL.String(), + Subject: session.Subject, + SessionID: hintSid, + RequestedAt: now, + ExpiresAt: now.Add(s.c.ConsentRequestMaxAge(ctx)), + RPInitiated: true, PostLogoutRedirectURI: redir, - }); err != nil { - return nil, err + Client: cl, + }) + if err != nil { + return nil, errors.WithStack(err) } http.Redirect(w, r, urlx.SetQuery(s.c.LogoutURL(ctx), url.Values{"logout_challenge": {challenge}}).String(), http.StatusFound) diff --git a/consent/strategy_logout_test.go b/consent/strategy_logout_test.go index 80e633e7bf6..7c227bb08d4 100644 --- a/consent/strategy_logout_test.go +++ b/consent/strategy_logout_test.go @@ -157,20 +157,21 @@ func TestLogoutFlows(t *testing.T) { return &wg } - setupCheckAndAcceptLogoutHandler := func(t *testing.T, wg *sync.WaitGroup, cb func(*testing.T, *hydra.OAuth2LogoutRequest, error)) { + setupCheckAndAcceptLogoutHandler := func(t *testing.T, wg *sync.WaitGroup, cb func(_ *testing.T, challenge string, _ *hydra.OAuth2LogoutRequest, _ error)) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if wg != nil { defer wg.Done() } - res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute() + challenge := r.URL.Query().Get("logout_challenge") + res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(challenge).Execute() if cb != nil { - cb(t, res, err) + cb(t, challenge, res, err) } else { require.NoError(t, err) } - v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute() + v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(challenge).Execute() require.NoError(t, err) require.NotEmpty(t, v.RedirectTo) http.Redirect(w, r, v.RedirectTo, http.StatusFound) @@ -244,7 +245,7 @@ func TestLogoutFlows(t *testing.T) { acceptLoginAs(t, subject) wg := newWg(2) - setupCheckAndAcceptLogoutHandler(t, wg, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) { + setupCheckAndAcceptLogoutHandler(t, wg, func(t *testing.T, challenge string, res *hydra.OAuth2LogoutRequest, err error) { require.NoError(t, err) assert.EqualValues(t, subject, *res.Subject) assert.NotEmpty(t, subject, res.Sid) @@ -277,20 +278,19 @@ func TestLogoutFlows(t *testing.T) { acceptLoginAs(t, subject) browser := createBrowserWithSession(t, createSampleClient(t)) - var logoutReq *hydra.OAuth2LogoutRequest - setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, req *hydra.OAuth2LogoutRequest, err error) { + var logoutChallenge string + setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, challenge string, _ *hydra.OAuth2LogoutRequest, err error) { require.NoError(t, err) - logoutReq = req + logoutChallenge = challenge }) // run once to log out logoutAndExpectPostLogoutPage(t, browser, http.MethodGet, url.Values{}, defaultRedirectedMessage) - // run again to ensure that the logout challenge is invalid - _, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(logoutReq.GetChallenge()).Execute() - assert.Error(t, err) + require.NotZero(t, logoutChallenge) - v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(logoutReq.GetChallenge()).Execute() + // double-submit: still works + v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(logoutChallenge).Execute() require.NoError(t, err) require.NotEmpty(t, v.RedirectTo) @@ -485,7 +485,7 @@ func TestLogoutFlows(t *testing.T) { c := createSampleClient(t) acceptLoginAs(t, subject) - setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) { + setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, _ string, _ *hydra.OAuth2LogoutRequest, _ error) { t.Fatalf("Logout should not have been called") }) browser := createBrowserWithSession(t, c) diff --git a/consent/strategy_oauth_test.go b/consent/strategy_oauth_test.go index a2e39d5b6ec..92a3dfc7623 100644 --- a/consent/strategy_oauth_test.go +++ b/consent/strategy_oauth_test.go @@ -53,7 +53,7 @@ func TestStrategyLoginConsentNext(t *testing.T) { adminClient := hydra.NewAPIClient(hydra.NewConfiguration()) adminClient.GetConfig().Servers = hydra.ServerConfigurations{{URL: adminTS.URL}} - oauth2Config := func(t *testing.T, c *client.Client) *oauth2.Config { + oauth2Config := func(_ *testing.T, c *client.Client) *oauth2.Config { return &oauth2.Config{ ClientID: c.GetID(), ClientSecret: c.Secret, diff --git a/consent/test/manager_test_helpers.go b/consent/test/manager_test_helpers.go index 986b4f3144c..c57e0534bdf 100644 --- a/consent/test/manager_test_helpers.go +++ b/consent/test/manager_test_helpers.go @@ -118,15 +118,13 @@ func MockLogoutRequest(key string, withClient bool, network string) (c *flow.Log } return &flow.LogoutRequest{ Subject: "subject" + key, - ID: makeID("challenge", network, key), - Verifier: makeID("verifier", network, key), SessionID: makeID("session", network, key), RPInitiated: true, RequestURL: "http://request-me/", PostLogoutRedirectURI: "http://redirect-me/", - WasHandled: false, - Accepted: false, Client: cl, + RequestedAt: time.Now().UTC().Add(-time.Minute), + ExpiresAt: time.Now().UTC().Add(time.Hour), } } @@ -987,66 +985,6 @@ func ManagerTests(deps Deps, m consent.Manager, clientManager client.Manager, fo }) } }) - - t.Run("case=LogoutRequest", func(t *testing.T) { - for k, tc := range []struct { - key string - authAt bool - withClient bool - }{ - {"LogoutRequest-1", true, true}, - {"LogoutRequest-2", true, true}, - {"LogoutRequest-3", true, true}, - {"LogoutRequest-4", true, true}, - {"LogoutRequest-5", true, false}, - {"LogoutRequest-6", false, false}, - } { - t.Run("key="+tc.key, func(t *testing.T) { - challenge := makeID("challenge", network, tc.key) - verifier := makeID("verifier", network, tc.key) - c := MockLogoutRequest(tc.key, tc.withClient, network) - if tc.withClient { - require.NoError(t, clientManager.CreateClient(ctx, c.Client)) // Ignore errors that are caused by duplication - } - - _, err := m.GetLogoutRequest(ctx, challenge) - require.Error(t, err) - - require.NoError(t, m.CreateLogoutRequest(ctx, c)) - - got2, err := m.GetLogoutRequest(ctx, challenge) - require.NoError(t, err) - assert.False(t, got2.WasHandled) - assert.False(t, got2.Accepted) - compareLogoutRequest(t, c, got2) - - if k%2 == 0 { - got2, err = m.AcceptLogoutRequest(ctx, challenge) - require.NoError(t, err) - assert.True(t, got2.Accepted) - compareLogoutRequest(t, c, got2) - - got3, err := m.VerifyAndInvalidateLogoutRequest(ctx, verifier) - require.NoError(t, err) - assert.True(t, got3.Accepted) - assert.True(t, got3.WasHandled) - compareLogoutRequest(t, c, got3) - - _, err = m.VerifyAndInvalidateLogoutRequest(ctx, verifier) - require.NoError(t, err) - - got2, err = m.GetLogoutRequest(ctx, challenge) - require.NoError(t, err) - compareLogoutRequest(t, got3, got2) - assert.True(t, got2.WasHandled) - } else { - require.NoError(t, m.RejectLogoutRequest(ctx, challenge)) - _, err = m.GetLogoutRequest(ctx, challenge) - require.Error(t, err) - } - }) - } - }) }) t.Run("case=foreign key regression", func(t *testing.T) { @@ -1111,9 +1049,7 @@ func compareLogoutRequest(t *testing.T, a, b *flow.LogoutRequest) { assert.EqualValues(t, a.Client.GetID(), b.Client.GetID()) } - assert.EqualValues(t, a.ID, b.ID) assert.EqualValues(t, a.Subject, b.Subject) - assert.EqualValues(t, a.Verifier, b.Verifier) assert.EqualValues(t, a.RequestURL, b.RequestURL) assert.EqualValues(t, a.PostLogoutRedirectURI, b.PostLogoutRedirectURI) assert.EqualValues(t, a.RPInitiated, b.RPInitiated) diff --git a/flow/.snapshots/TestLogoutRequest_MarshalJSON.json b/flow/.snapshots/TestLogoutRequest_MarshalJSON.json index 4132efb0269..9e71bfe7b40 100644 --- a/flow/.snapshots/TestLogoutRequest_MarshalJSON.json +++ b/flow/.snapshots/TestLogoutRequest_MarshalJSON.json @@ -1 +1 @@ -"{\"challenge\":\"\",\"subject\":\"\",\"request_url\":\"\",\"rp_initiated\":false,\"expires_at\":null,\"requested_at\":null,\"client\":null}" +"{\"request_url\":\"\",\"rp_initiated\":false,\"expires_at\":\"0001-01-01T00:00:00Z\",\"requested_at\":\"0001-01-01T00:00:00Z\",\"client\":null}" diff --git a/flow/consent_types.go b/flow/consent_types.go index 399938a8020..77bebe20806 100644 --- a/flow/consent_types.go +++ b/flow/consent_types.go @@ -4,21 +4,18 @@ package flow import ( - "database/sql" "database/sql/driver" "encoding/json" "fmt" "net/http" "time" - "github.com/gobuffalo/pop/v6" "github.com/gofrs/uuid" "github.com/ory/x/errorsx" "github.com/ory/fosite" "github.com/ory/hydra/v2/client" - "github.com/ory/x/sqlcon" "github.com/ory/x/sqlxx" ) @@ -479,59 +476,24 @@ func (n *OAuth2ConsentRequestOpenIDConnectContext) Value() (driver.Value, error) // // swagger:model oAuth2LogoutRequest type LogoutRequest struct { - // Challenge is the identifier ("logout challenge") of the logout authentication request. It is used to - // identify the session. - ID string `json:"challenge" db:"challenge"` - NID uuid.UUID `json:"-" db:"nid"` - - // Subject is the user for whom the logout was request. - Subject string `json:"subject" db:"subject"` + // Subject is the user for whom the logout was requested. + Subject string `json:"subject,omitempty"` // SessionID is the login session ID that was requested to log out. - SessionID string `json:"sid,omitempty" db:"sid"` + SessionID string `json:"sid,omitempty"` // RequestURL is the original Logout URL requested. - RequestURL string `json:"request_url" db:"request_url"` + RequestURL string `json:"request_url"` // RPInitiated is set to true if the request was initiated by a Relying Party (RP), also known as an OAuth 2.0 Client. - RPInitiated bool `json:"rp_initiated" db:"rp_initiated"` - - // If set to true means that the request was already handled. This - // can happen on form double-submit or other errors. If this is set - // we recommend redirecting the user to `request_url` to re-initiate - // the flow. - WasHandled bool `json:"-" db:"was_used"` + RPInitiated bool `json:"rp_initiated"` - Verifier string `json:"-" db:"verifier"` - PostLogoutRedirectURI string `json:"-" db:"redir_url"` - 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:"-"` -} + ExpiresAt time.Time `json:"expires_at"` + RequestedAt time.Time `json:"requested_at"` + Client *client.Client `json:"client"` -func (LogoutRequest) TableName() string { - return "hydra_oauth2_logout_request" -} - -func (r *LogoutRequest) BeforeSave(_ *pop.Connection) error { - if r.Client != nil { - r.ClientID = sql.NullString{ - Valid: true, - String: r.Client.GetID(), - } - } - return nil -} - -func (r *LogoutRequest) AfterFind(c *pop.Connection) error { - if r.ClientID.Valid { - r.Client = &client.Client{} - return sqlcon.HandleError(c.Where("id = ?", r.ClientID.String).First(r.Client)) - } - return nil + // swagger:ignore + PostLogoutRedirectURI string `json:"redir_url,omitempty"` } // Returned when the log out request was used. diff --git a/go.mod b/go.mod index 06714a3dc32..f4ebc3faa66 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,7 @@ require ( github.com/ory/hydra-client-go/v2 v2.2.1 github.com/ory/jsonschema/v3 v3.0.8 github.com/ory/kratos-client-go v1.2.1 - github.com/ory/x v0.0.675 + github.com/ory/x v0.0.694 github.com/pborman/uuid v1.2.1 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.19.1 diff --git a/go.sum b/go.sum index bad2adc7cee..b610d19ae1f 100644 --- a/go.sum +++ b/go.sum @@ -394,8 +394,8 @@ github.com/ory/kratos-client-go v1.2.1 h1:Q3T/adfAfAkHFcV1LGLnwz4QkY6ghBdX9zde5T github.com/ory/kratos-client-go v1.2.1/go.mod h1:WiQYlrqW4Atj6Js7oDN5ArbZxo0nTO2u/e1XaDv2yMI= github.com/ory/pop/v6 v6.2.1-0.20241121111754-e5dfc0f3344b h1:BIzoOe2/wynZBQak1po0tzgvARseIKsR2bF6b+SZoKE= github.com/ory/pop/v6 v6.2.1-0.20241121111754-e5dfc0f3344b/go.mod h1:okVAYKGtgunD/wbW3NGhZTndJCS+6FqO+cA89rQ4doc= -github.com/ory/x v0.0.675 h1:K6GpVo99BXBFv2UiwMjySNNNqCFKGswynrt7vWQJFU8= -github.com/ory/x v0.0.675/go.mod h1:zJmnDtKje2FCP4EeFvRsKk94XXiqKCSGJMZcirAfhUs= +github.com/ory/x v0.0.694 h1:by3cFPttn9K8LsTWbzLLBJQlJWGr2fd+4ksHEv3onG8= +github.com/ory/x v0.0.694/go.mod h1:CLnDxtKBkO2MoOYRbQe9HASUlqTBl7AP9r7YcmyD1CY= github.com/pborman/uuid v1.2.1 h1:+ZZIw58t/ozdjRaXh/3awHfmWRbzYxJoAdNJxe/3pvw= github.com/pborman/uuid v1.2.1/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8= diff --git a/internal/httpclient/api/openapi.yaml b/internal/httpclient/api/openapi.yaml index ae5ad4dd6f3..93c31869cf2 100644 --- a/internal/httpclient/api/openapi.yaml +++ b/internal/httpclient/api/openapi.yaml @@ -3559,7 +3559,6 @@ components: example: expires_at: 2000-01-23T04:56:07.000+00:00 subject: subject - challenge: challenge client: metadata: "" token_endpoint_auth_signing_alg: token_endpoint_auth_signing_alg @@ -3630,23 +3629,16 @@ components: requested_at: 2000-01-23T04:56:07.000+00:00 sid: sid properties: - challenge: - description: |- - Challenge is the identifier ("logout challenge") of the logout authentication request. It is used to - identify the session. - type: string client: $ref: '#/components/schemas/oAuth2Client' expires_at: format: date-time - title: NullTime implements sql.NullTime functionality. type: string request_url: description: RequestURL is the original Logout URL requested. type: string requested_at: format: date-time - title: NullTime implements sql.NullTime functionality. type: string rp_initiated: description: "RPInitiated is set to true if the request was initiated by\ @@ -3657,7 +3649,7 @@ components: out. type: string subject: - description: Subject is the user for whom the logout was request. + description: Subject is the user for whom the logout was requested. type: string title: Contains information about an ongoing logout request. type: object diff --git a/internal/httpclient/docs/OAuth2LogoutRequest.md b/internal/httpclient/docs/OAuth2LogoutRequest.md index 65a48ecb29d..ca66c6b6eeb 100644 --- a/internal/httpclient/docs/OAuth2LogoutRequest.md +++ b/internal/httpclient/docs/OAuth2LogoutRequest.md @@ -4,14 +4,13 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- -**Challenge** | Pointer to **string** | Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to identify the session. | [optional] **Client** | Pointer to [**OAuth2Client**](OAuth2Client.md) | | [optional] **ExpiresAt** | Pointer to **time.Time** | | [optional] **RequestUrl** | Pointer to **string** | RequestURL is the original Logout URL requested. | [optional] **RequestedAt** | Pointer to **time.Time** | | [optional] **RpInitiated** | Pointer to **bool** | RPInitiated is set to true if the request was initiated by a Relying Party (RP), also known as an OAuth 2.0 Client. | [optional] **Sid** | Pointer to **string** | SessionID is the login session ID that was requested to log out. | [optional] -**Subject** | Pointer to **string** | Subject is the user for whom the logout was request. | [optional] +**Subject** | Pointer to **string** | Subject is the user for whom the logout was requested. | [optional] ## Methods @@ -32,31 +31,6 @@ NewOAuth2LogoutRequestWithDefaults instantiates a new OAuth2LogoutRequest object This constructor will only assign default values to properties that have it defined, but it doesn't guarantee that properties required by API are set -### GetChallenge - -`func (o *OAuth2LogoutRequest) GetChallenge() string` - -GetChallenge returns the Challenge field if non-nil, zero value otherwise. - -### GetChallengeOk - -`func (o *OAuth2LogoutRequest) GetChallengeOk() (*string, bool)` - -GetChallengeOk returns a tuple with the Challenge field if it's non-nil, zero value otherwise -and a boolean to check if the value has been set. - -### SetChallenge - -`func (o *OAuth2LogoutRequest) SetChallenge(v string)` - -SetChallenge sets Challenge field to given value. - -### HasChallenge - -`func (o *OAuth2LogoutRequest) HasChallenge() bool` - -HasChallenge returns a boolean if a field has been set. - ### GetClient `func (o *OAuth2LogoutRequest) GetClient() OAuth2Client` diff --git a/internal/httpclient/model_o_auth2_logout_request.go b/internal/httpclient/model_o_auth2_logout_request.go index 6ff85eb4062..42342fe5780 100644 --- a/internal/httpclient/model_o_auth2_logout_request.go +++ b/internal/httpclient/model_o_auth2_logout_request.go @@ -21,8 +21,6 @@ var _ MappedNullable = &OAuth2LogoutRequest{} // OAuth2LogoutRequest struct for OAuth2LogoutRequest type OAuth2LogoutRequest struct { - // Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to identify the session. - Challenge *string `json:"challenge,omitempty"` Client *OAuth2Client `json:"client,omitempty"` ExpiresAt *time.Time `json:"expires_at,omitempty"` // RequestURL is the original Logout URL requested. @@ -32,7 +30,7 @@ type OAuth2LogoutRequest struct { RpInitiated *bool `json:"rp_initiated,omitempty"` // SessionID is the login session ID that was requested to log out. Sid *string `json:"sid,omitempty"` - // Subject is the user for whom the logout was request. + // Subject is the user for whom the logout was requested. Subject *string `json:"subject,omitempty"` } @@ -53,38 +51,6 @@ func NewOAuth2LogoutRequestWithDefaults() *OAuth2LogoutRequest { return &this } -// GetChallenge returns the Challenge field value if set, zero value otherwise. -func (o *OAuth2LogoutRequest) GetChallenge() string { - if o == nil || IsNil(o.Challenge) { - var ret string - return ret - } - return *o.Challenge -} - -// GetChallengeOk returns a tuple with the Challenge field value if set, nil otherwise -// and a boolean to check if the value has been set. -func (o *OAuth2LogoutRequest) GetChallengeOk() (*string, bool) { - if o == nil || IsNil(o.Challenge) { - return nil, false - } - return o.Challenge, true -} - -// HasChallenge returns a boolean if a field has been set. -func (o *OAuth2LogoutRequest) HasChallenge() bool { - if o != nil && !IsNil(o.Challenge) { - return true - } - - return false -} - -// SetChallenge gets a reference to the given string and assigns it to the Challenge field. -func (o *OAuth2LogoutRequest) SetChallenge(v string) { - o.Challenge = &v -} - // GetClient returns the Client field value if set, zero value otherwise. func (o *OAuth2LogoutRequest) GetClient() OAuth2Client { if o == nil || IsNil(o.Client) { @@ -319,9 +285,6 @@ func (o OAuth2LogoutRequest) MarshalJSON() ([]byte, error) { func (o OAuth2LogoutRequest) ToMap() (map[string]interface{}, error) { toSerialize := map[string]interface{}{} - if !IsNil(o.Challenge) { - toSerialize["challenge"] = o.Challenge - } if !IsNil(o.Client) { toSerialize["client"] = o.Client } diff --git a/oauth2/flowctx/encoding.go b/oauth2/flowctx/encoding.go index 8a1f8cbf270..bdebef3f410 100644 --- a/oauth2/flowctx/encoding.go +++ b/oauth2/flowctx/encoding.go @@ -27,6 +27,8 @@ const ( loginVerifier consentChallenge consentVerifier + logoutChallenge + logoutVerifier ) func withPurpose(purpose purpose) CodecOption { return func(ad *data) { ad.Purpose = purpose } } @@ -36,6 +38,8 @@ var ( AsLoginVerifier = withPurpose(loginVerifier) AsConsentChallenge = withPurpose(consentChallenge) AsConsentVerifier = withPurpose(consentVerifier) + AsLogoutChallenge = withPurpose(logoutChallenge) + AsLogoutVerifier = withPurpose(logoutVerifier) ) func additionalDataFromOpts(opts ...CodecOption) []byte { diff --git a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0009.json b/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0009.json deleted file mode 100644 index 7681dc70e21..00000000000 --- a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0009.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "challenge": "challenge-0009", - "subject": "subject-0009", - "sid": "session_id-0009", - "request_url": "http://request/0009", - "rp_initiated": true, - "expires_at": null, - "requested_at": null, - "client": null -} diff --git a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0010.json b/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0010.json deleted file mode 100644 index d1cb5f6aa61..00000000000 --- a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0010.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "challenge": "challenge-0010", - "subject": "subject-0010", - "sid": "session_id-0010", - "request_url": "http://request/0010", - "rp_initiated": true, - "expires_at": null, - "requested_at": null, - "client": null -} diff --git a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0011.json b/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0011.json deleted file mode 100644 index 3c81d38cb47..00000000000 --- a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0011.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "challenge": "challenge-0011", - "subject": "subject-0011", - "sid": "session_id-0011", - "request_url": "http://request/0011", - "rp_initiated": true, - "expires_at": null, - "requested_at": null, - "client": null -} diff --git a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0012.json b/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0012.json deleted file mode 100644 index 67c839b88a0..00000000000 --- a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0012.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "challenge": "challenge-0012", - "subject": "subject-0012", - "sid": "session_id-0012", - "request_url": "http://request/0012", - "rp_initiated": true, - "expires_at": null, - "requested_at": null, - "client": null -} diff --git a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0013.json b/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0013.json deleted file mode 100644 index f8b84db4b56..00000000000 --- a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0013.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "challenge": "challenge-0013", - "subject": "subject-0013", - "sid": "session_id-0013", - "request_url": "http://request/0013", - "rp_initiated": true, - "expires_at": null, - "requested_at": null, - "client": null -} diff --git a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0014.json b/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0014.json deleted file mode 100644 index c5194805b6a..00000000000 --- a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-0014.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "challenge": "challenge-0014", - "subject": "subject-0014", - "sid": "session_id-0014", - "request_url": "http://request/0014", - "rp_initiated": true, - "expires_at": null, - "requested_at": null, - "client": null -} diff --git a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-20240916105610000001.json b/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-20240916105610000001.json deleted file mode 100644 index 8b1841c9be6..00000000000 --- a/persistence/sql/migratest/fixtures/hydra_oauth2_logout_request/challenge-20240916105610000001.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "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 -} diff --git a/persistence/sql/migratest/migration_test.go b/persistence/sql/migratest/migration_test.go index 71435d95687..0833e4182d3 100644 --- a/persistence/sql/migratest/migration_test.go +++ b/persistence/sql/migratest/migration_test.go @@ -85,7 +85,7 @@ func TestMigrations(t *testing.T) { }) } - var test = func(db string, c *pop.Connection) func(t *testing.T) { + var test = func(_ string, c *pop.Connection) func(t *testing.T) { return func(t *testing.T) { ctx := context.Background() x.CleanSQLPop(t, c) @@ -168,19 +168,6 @@ func TestMigrations(t *testing.T) { } }) - t.Run("case=hydra_oauth2_logout_request", func(t *testing.T) { - lrs := []flow.LogoutRequest{} - require.NoError(t, c.All(&lrs)) - require.Equal(t, 7, len(lrs)) - - for _, s := range lrs { - testhelpersuuid.AssertUUID(t, s.NID) - s.NID = uuid.Nil - s.Client = nil - CompareWithFixture(t, s, "hydra_oauth2_logout_request", s.ID) - } - }) - t.Run("case=hydra_oauth2_jti_blacklist", func(t *testing.T) { bjtis := []oauth2.BlacklistedJTI{} require.NoError(t, c.All(&bjtis)) diff --git a/persistence/sql/persister_consent.go b/persistence/sql/persister_consent.go index ad3c25b3d72..e15a9a4f169 100644 --- a/persistence/sql/persister_consent.go +++ b/persistence/sql/persister_consent.go @@ -663,79 +663,70 @@ WHERE return cs, nil } -func (p *Persister) CreateLogoutRequest(ctx context.Context, request *flow.LogoutRequest) (err error) { - ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.CreateLogoutRequest") +func (p *Persister) CreateLogoutChallenge(ctx context.Context, request *flow.LogoutRequest) (challenge string, err error) { + ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.CreateLogoutChallenge") defer otelx.End(span, &err) - return errorsx.WithStack(p.CreateWithNetwork(ctx, request)) + challenge, err = flowctx.Encode(ctx, p.r.FlowCipher(), request, flowctx.AsLogoutChallenge) + if err != nil { + return "", errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithHintf("Failed to encrypt the logout challenge.")) + } + return challenge, nil } -func (p *Persister) AcceptLogoutRequest(ctx context.Context, challenge string) (_ *flow.LogoutRequest, err error) { +func (p *Persister) AcceptLogoutRequest(ctx context.Context, challenge string) (verifier string, err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.AcceptLogoutRequest") defer otelx.End(span, &err) - if err := p.Connection(ctx).RawQuery("UPDATE hydra_oauth2_logout_request SET accepted=true, rejected=false WHERE challenge=? AND nid = ?", challenge, p.NetworkID(ctx)).Exec(); err != nil { - return nil, sqlcon.HandleError(err) + req, err := flowctx.Decode[flow.LogoutRequest](ctx, p.r.FlowCipher(), challenge, flowctx.AsLogoutChallenge) + if err != nil { + return "", errorsx.WithStack(x.ErrNotFound.WithWrap(err).WithHintf("Failed to decrypt the logout challenge.")) + } + + verifier, err = flowctx.Encode(ctx, p.r.FlowCipher(), req, flowctx.AsLogoutVerifier) + if err != nil { + return "", errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithHintf("Failed to encrypty the logout verifier.")) } - return p.GetLogoutRequest(ctx, challenge) + return verifier, nil } func (p *Persister) RejectLogoutRequest(ctx context.Context, challenge string) (err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.RejectLogoutRequest") defer otelx.End(span, &err) - count, err := p.Connection(ctx). - RawQuery("UPDATE hydra_oauth2_logout_request SET rejected=true, accepted=false WHERE challenge=? AND nid = ?", challenge, p.NetworkID(ctx)). - ExecWithCount() - if count == 0 { - return errorsx.WithStack(x.ErrNotFound) - } else { - return errorsx.WithStack(err) + _, err = flowctx.Decode[flow.LogoutRequest](ctx, p.r.FlowCipher(), challenge, flowctx.AsLogoutChallenge) + if err != nil { + return errorsx.WithStack(x.ErrNotFound.WithWrap(err).WithHintf("Failed to decrypt the logout challenge.")) } + return nil } func (p *Persister) GetLogoutRequest(ctx context.Context, challenge string) (_ *flow.LogoutRequest, err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.GetLogoutRequest") defer otelx.End(span, &err) - var lr flow.LogoutRequest - return &lr, sqlcon.HandleError(p.QueryWithNetwork(ctx).Where("challenge = ? AND rejected = FALSE", challenge).First(&lr)) + request, err := flowctx.Decode[flow.LogoutRequest](ctx, p.r.FlowCipher(), challenge, flowctx.AsLogoutChallenge) + if err != nil { + return nil, errorsx.WithStack(x.ErrNotFound.WithWrap(err).WithHintf("Failed to decrypt the logout challenge.")) + } + return request, nil } func (p *Persister) VerifyAndInvalidateLogoutRequest(ctx context.Context, verifier string) (_ *flow.LogoutRequest, err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.VerifyAndInvalidateLogoutRequest") defer otelx.End(span, &err) - var lr flow.LogoutRequest - if count, err := p.Connection(ctx).RawQuery(` -UPDATE hydra_oauth2_logout_request - SET was_used = TRUE -WHERE nid = ? - AND verifier = ? - AND accepted = TRUE - AND rejected = FALSE`, - p.NetworkID(ctx), - verifier, - ).ExecWithCount(); count == 0 && err == nil { - return nil, errorsx.WithStack(x.ErrNotFound) - } else if err != nil { - return nil, sqlcon.HandleError(err) - } - - err = sqlcon.HandleError(p.QueryWithNetwork(ctx).Where("verifier = ?", verifier).First(&lr)) + lr, err := flowctx.Decode[flow.LogoutRequest](ctx, p.r.FlowCipher(), verifier, flowctx.AsLogoutVerifier) if err != nil { - return nil, err + return nil, errorsx.WithStack(x.ErrNotFound.WithWrap(err).WithHintf("Failed to decrypt the logout verifier.")) } - 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()) { + if lr.ExpiresAt.Before(time.Now()) { return nil, errorsx.WithStack(flow.ErrorLogoutFlowExpired) } - return &lr, nil + return lr, nil } func (p *Persister) FlushInactiveLoginConsentRequests(ctx context.Context, notAfter time.Time, limit int, batchSize int) (err error) { diff --git a/persistence/sql/persister_nid_test.go b/persistence/sql/persister_nid_test.go index 93bccdcfe58..79db37b8193 100644 --- a/persistence/sql/persister_nid_test.go +++ b/persistence/sql/persister_nid_test.go @@ -83,29 +83,6 @@ func (s *PersisterTestSuite) TearDownTest() { } } -func (s *PersisterTestSuite) TestAcceptLogoutRequest() { - t := s.T() - lr := newLogoutRequest() - - for k, r := range s.registries { - t.Run("dialect="+k, func(*testing.T) { - require.NoError(t, r.ConsentManager().CreateLogoutRequest(s.t1, lr)) - - expected, err := r.ConsentManager().GetLogoutRequest(s.t1, lr.ID) - require.NoError(t, err) - require.Equal(t, false, expected.Accepted) - - lrAccepted, err := r.ConsentManager().AcceptLogoutRequest(s.t2, lr.ID) - require.Error(t, err) - require.Equal(t, &flow.LogoutRequest{}, lrAccepted) - - actual, err := r.ConsentManager().GetLogoutRequest(s.t1, lr.ID) - require.NoError(t, err) - require.Equal(t, expected, actual) - }) - } -} - func (s *PersisterTestSuite) TestAddKeyGetKeyDeleteKey() { t := s.T() key := newKey("test-ks", "test") @@ -456,27 +433,6 @@ func (s *PersisterTestSuite) TestCreateLoginSession() { }) } } - -func (s *PersisterTestSuite) TestCreateLogoutRequest() { - t := s.T() - for k, r := range s.registries { - t.Run(k, func(t *testing.T) { - client := &client.Client{ID: "client-id"} - lr := flow.LogoutRequest{ - // TODO there is not FK for SessionID so we don't need it here; TODO make sure the missing FK is intentional - ID: uuid.Must(uuid.NewV4()).String(), - ClientID: sql.NullString{Valid: true, String: client.ID}, - } - - require.NoError(t, r.Persister().CreateClient(s.t1, client)) - require.NoError(t, r.Persister().CreateLogoutRequest(s.t1, &lr)) - actual, err := r.Persister().GetLogoutRequest(s.t1, lr.ID) - require.NoError(t, err) - require.Equal(t, s.t1NID, actual.NID) - }) - } -} - func (s *PersisterTestSuite) TestCreateOpenIDConnectSession() { t := s.T() for k, r := range s.registries { @@ -1235,30 +1191,6 @@ func (s *PersisterTestSuite) TestGetLoginRequest() { } } -func (s *PersisterTestSuite) TestGetLogoutRequest() { - t := s.T() - for k, r := range s.registries { - t.Run(k, func(t *testing.T) { - client := &client.Client{ID: "client-id"} - lr := flow.LogoutRequest{ - ID: uuid.Must(uuid.NewV4()).String(), - ClientID: sql.NullString{Valid: true, String: client.ID}, - } - - require.NoError(t, r.Persister().CreateClient(s.t1, client)) - require.NoError(t, r.Persister().CreateLogoutRequest(s.t1, &lr)) - - actual, err := r.Persister().GetLogoutRequest(s.t2, lr.ID) - require.Error(t, err) - require.Equal(t, &flow.LogoutRequest{}, actual) - - actual, err = r.Persister().GetLogoutRequest(s.t1, lr.ID) - require.NoError(t, err) - require.NotEqual(t, &flow.LogoutRequest{}, actual) - }) - } -} - func (s *PersisterTestSuite) TestGetOpenIDConnectSession() { t := s.T() for k, r := range s.registries { @@ -1722,27 +1654,6 @@ func (s *PersisterTestSuite) TestQueryWithNetwork() { }) } } - -func (s *PersisterTestSuite) TestRejectLogoutRequest() { - t := s.T() - for k, r := range s.registries { - t.Run(k, func(t *testing.T) { - lr := newLogoutRequest() - require.NoError(t, r.ConsentManager().CreateLogoutRequest(s.t1, lr)) - - require.Error(t, r.ConsentManager().RejectLogoutRequest(s.t2, lr.ID)) - actual, err := r.ConsentManager().GetLogoutRequest(s.t1, lr.ID) - require.NoError(t, err) - require.Equal(t, lr, actual) - - require.NoError(t, r.ConsentManager().RejectLogoutRequest(s.t1, lr.ID)) - actual, err = r.ConsentManager().GetLogoutRequest(s.t1, lr.ID) - require.Error(t, err) - require.Equal(t, &flow.LogoutRequest{}, actual) - }) - } -} - func (s *PersisterTestSuite) TestRevokeAccessToken() { t := s.T() for k, r := range s.registries { @@ -2135,62 +2046,6 @@ func (s *PersisterTestSuite) TestVerifyAndInvalidateLoginRequest() { } } -func (s *PersisterTestSuite) TestVerifyAndInvalidateLogoutRequest() { - t := s.T() - for k, r := range s.registries { - t.Run(k, func(t *testing.T) { - 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) - } - - t.Run("case=legacy logout request without expiry", func(t *testing.T) { - lr := newLogoutRequest() - run(t, lr) - }) - - 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) - }) - - 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) - }) - }) - } -} - func (s *PersisterTestSuite) TestWithFallbackNetworkID() { t := s.T() for k, r := range s.registries { @@ -2253,12 +2108,6 @@ func newGrant(keySet string, keyID string) trust.Grant { } } -func newLogoutRequest() *flow.LogoutRequest { - return &flow.LogoutRequest{ - ID: uuid.Must(uuid.NewV4()).String(), - } -} - func newKey(ksID string, use string) jose.JSONWebKey { ks, err := jwk.GenerateJWK(context.Background(), jose.RS256, ksID, use) if err != nil { diff --git a/persistence/sql/persister_test.go b/persistence/sql/persister_test.go index b4c88ef01c3..95a02c472b5 100644 --- a/persistence/sql/persister_test.go +++ b/persistence/sql/persister_test.go @@ -38,7 +38,7 @@ func init() { func testRegistry(t *testing.T, ctx context.Context, k string, t1 driver.Registry, t2 driver.Registry) { t.Run("package=client/manager="+k, func(t *testing.T) { - t.Run("case=create-get-update-delete", client.TestHelperCreateGetUpdateDeleteClient(k, t1.Persister().Connection(context.Background()), t1.ClientManager(), t2.ClientManager())) + t.Run("case=create-get-update-delete", client.TestHelperCreateGetUpdateDeleteClient(k, t1.Persister().Connection(ctx), t1.ClientManager(), t2.ClientManager())) t.Run("case=autogenerate-key", client.TestHelperClientAutoGenerateKey(k, t1.ClientManager())) diff --git a/spec/api.json b/spec/api.json index f1f96c7c1ba..517e2bb002f 100644 --- a/spec/api.json +++ b/spec/api.json @@ -1030,22 +1030,20 @@ }, "oAuth2LogoutRequest": { "properties": { - "challenge": { - "description": "Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to\nidentify the session.", - "type": "string" - }, "client": { "$ref": "#/components/schemas/oAuth2Client" }, "expires_at": { - "$ref": "#/components/schemas/nullTime" + "format": "date-time", + "type": "string" }, "request_url": { "description": "RequestURL is the original Logout URL requested.", "type": "string" }, "requested_at": { - "$ref": "#/components/schemas/nullTime" + "format": "date-time", + "type": "string" }, "rp_initiated": { "description": "RPInitiated is set to true if the request was initiated by a Relying Party (RP), also known as an OAuth 2.0 Client.", @@ -1056,7 +1054,7 @@ "type": "string" }, "subject": { - "description": "Subject is the user for whom the logout was request.", + "description": "Subject is the user for whom the logout was requested.", "type": "string" } }, diff --git a/spec/swagger.json b/spec/swagger.json index 4128c56448e..cc836e6f28f 100755 --- a/spec/swagger.json +++ b/spec/swagger.json @@ -3025,22 +3025,20 @@ "type": "object", "title": "Contains information about an ongoing logout request.", "properties": { - "challenge": { - "description": "Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to\nidentify the session.", - "type": "string" - }, "client": { "$ref": "#/definitions/oAuth2Client" }, "expires_at": { - "$ref": "#/definitions/nullTime" + "type": "string", + "format": "date-time" }, "request_url": { "description": "RequestURL is the original Logout URL requested.", "type": "string" }, "requested_at": { - "$ref": "#/definitions/nullTime" + "type": "string", + "format": "date-time" }, "rp_initiated": { "description": "RPInitiated is set to true if the request was initiated by a Relying Party (RP), also known as an OAuth 2.0 Client.", @@ -3051,7 +3049,7 @@ "type": "string" }, "subject": { - "description": "Subject is the user for whom the logout was request.", + "description": "Subject is the user for whom the logout was requested.", "type": "string" } }