feat: stateless logout

Author: alnr

consent/handler.go

@@ -932,9 +932,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
@@ -964,23 +962,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"`
@@ -1020,9 +1016,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"`
@@ -1060,13 +1054,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)
 }
 

consent/handler_test.go

@@ -31,61 +31,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

consent/manager.go

@@ -57,9 +57,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)
 

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"
@@ -883,21 +882,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().
@@ -923,13 +919,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().Round(time.Second)
+	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(),
 			),
 		)
 	}
@@ -967,6 +963,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
@@ -1007,20 +1004,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)

consent/strategy_logout_test.go

@@ -16,21 +16,19 @@ import (
 	"testing"
 	"time"
 
-	"github.com/ory/hydra/v2/internal/kratos"
-	"github.com/ory/x/pointerx"
-
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/tidwall/gjson"
 
 	jwtgo "github.com/ory/fosite/token/jwt"
-
 	hydra "github.com/ory/hydra-client-go/v2"
 	"github.com/ory/hydra/v2/client"
 	"github.com/ory/hydra/v2/driver/config"
+	"github.com/ory/hydra/v2/internal/kratos"
 	"github.com/ory/hydra/v2/internal/testhelpers"
 	"github.com/ory/x/contextx"
 	"github.com/ory/x/ioutilx"
+	"github.com/ory/x/pointerx"
 )
 
 func TestLogoutFlows(t *testing.T) {
@@ -163,14 +161,17 @@ func TestLogoutFlows(t *testing.T) {
 				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)
 			} else {
 				require.NoError(t, err)
 			}
+			require.NotNil(t, res)
+			require.NotNil(t, res.Challenge)
 
-			v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute()
+			v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(*res.Challenge).Execute()
 			require.NoError(t, err)
 			require.NotEmpty(t, v.RedirectTo)
 			http.Redirect(w, r, v.RedirectTo, http.StatusFound)
@@ -277,20 +278,20 @@ func TestLogoutFlows(t *testing.T) {
 		acceptLoginAs(t, subject)
 		browser := createBrowserWithSession(t, createSampleClient(t))
 
-		var logoutReq *hydra.OAuth2LogoutRequest
+		var logoutChallenge string
 		setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, req *hydra.OAuth2LogoutRequest, err error) {
 			require.NoError(t, err)
-			logoutReq = req
+			require.NotNil(t, req.Challenge)
+			logoutChallenge = *req.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 +486,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, _ *hydra.OAuth2LogoutRequest, _ error) {
 			t.Fatalf("Logout should not have been called")
 		})
 		browser := createBrowserWithSession(t, c)

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,