feat: enable simultaneous auth flows by creating client related csrf …

…co… (#3059)

Closes #3019

consent/helper.go

@@ -24,6 +24,8 @@ import (
 	"net/http"
 	"strings"
 
+	"time"
+
 	"github.com/ory/hydra/x"
 
 	"github.com/ory/x/errorsx"
@@ -66,8 +68,8 @@ func matchScopes(scopeStrategy fosite.ScopeStrategy, previousConsent []AcceptOAu
 	return nil
 }
 
-func createCsrfSession(w http.ResponseWriter, r *http.Request, conf x.CookieConfigProvider, store sessions.Store, name string, csrfValue string) error {
-	// Errors can be ignored here, because we always get a session session back. Error typically means that the
+func createCsrfSession(w http.ResponseWriter, r *http.Request, conf x.CookieConfigProvider, store sessions.Store, name string, csrfValue string, maxAge time.Duration) error {
+	// Errors can be ignored here, because we always get a session back. Error typically means that the
 	// session doesn't exist yet.
 	session, _ := store.Get(r, name)
 
@@ -81,12 +83,13 @@ func createCsrfSession(w http.ResponseWriter, r *http.Request, conf x.CookieConf
 	session.Options.Secure = conf.CookieSecure(r.Context())
 	session.Options.SameSite = sameSite
 	session.Options.Domain = conf.CookieDomain(r.Context())
+	session.Options.MaxAge = int(maxAge.Seconds())
 	if err := session.Save(r, w); err != nil {
 		return errorsx.WithStack(err)
 	}
 
 	if sameSite == http.SameSiteNoneMode && conf.CookieSameSiteLegacyWorkaround(r.Context()) {
-		return createCsrfSession(w, r, conf, store, legacyCsrfSessionName(name), csrfValue)
+		return createCsrfSession(w, r, conf, store, legacyCsrfSessionName(name), csrfValue, maxAge)
 	}
 
 	return nil

consent/helper_test.go

@@ -25,6 +25,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/golang/mock/gomock"
 
@@ -298,82 +299,95 @@ func TestCreateCsrfSession(t *testing.T) {
 		secure   bool
 		domain   string
 		sameSite http.SameSite
+		maxAge   int
 	}
 	for _, tc := range []struct {
 		name                     string
 		secure                   bool
 		domain                   string
 		sameSite                 http.SameSite
+		maxAge                   time.Duration
 		sameSiteLegacyWorkaround bool
 		expectedCookies          map[string]cookie
 	}{
 		{
 			name:                     "csrf_default",
 			secure:                   true,
 			sameSite:                 http.SameSiteDefaultMode,
+			maxAge:                   10 * time.Second,
 			sameSiteLegacyWorkaround: false,
 			expectedCookies: map[string]cookie{
 				"csrf_default": {
 					httpOnly: true,
 					secure:   true,
 					sameSite: 0, // see https://golang.org/doc/go1.16#net/http
+					maxAge:   10,
 				},
 			},
 		},
 		{
 			name:                     "csrf_lax_insecure",
 			secure:                   false,
 			sameSite:                 http.SameSiteLaxMode,
+			maxAge:                   20 * time.Second,
 			sameSiteLegacyWorkaround: false,
 			expectedCookies: map[string]cookie{
 				"csrf_lax_insecure": {
 					httpOnly: true,
 					secure:   false,
 					sameSite: http.SameSiteLaxMode,
+					maxAge:   20,
 				},
 			},
 		},
 		{
 			name:                     "csrf_none",
 			secure:                   true,
 			sameSite:                 http.SameSiteNoneMode,
+			maxAge:                   30 * time.Second,
 			sameSiteLegacyWorkaround: false,
 			expectedCookies: map[string]cookie{
 				"csrf_none": {
 					httpOnly: true,
 					secure:   true,
 					sameSite: http.SameSiteNoneMode,
+					maxAge:   30,
 				},
 			},
 		},
 		{
 			name:                     "csrf_none_fallback",
 			secure:                   true,
 			sameSite:                 http.SameSiteNoneMode,
+			maxAge:                   40 * time.Second,
 			sameSiteLegacyWorkaround: true,
 			expectedCookies: map[string]cookie{
 				"csrf_none_fallback": {
 					httpOnly: true,
 					secure:   true,
 					sameSite: http.SameSiteNoneMode,
+					maxAge:   40,
 				},
 				"csrf_none_fallback_legacy": {
 					httpOnly: true,
 					secure:   true,
 					sameSite: 0,
+					maxAge:   40,
 				},
 			},
 		},
 		{
 			name:                     "csrf_strict_fallback_ignored",
 			secure:                   true,
 			sameSite:                 http.SameSiteStrictMode,
+			maxAge:                   50 * time.Second,
 			sameSiteLegacyWorkaround: true,
 			expectedCookies: map[string]cookie{
 				"csrf_strict_fallback_ignored": {
 					httpOnly: true,
 					secure:   true,
 					sameSite: http.SameSiteStrictMode,
+					maxAge:   50,
 				},
 			},
 		},
@@ -406,7 +420,7 @@ func TestCreateCsrfSession(t *testing.T) {
 			config.EXPECT().CookieSecure(gomock.Any()).Return(tc.secure).AnyTimes()
 			config.EXPECT().CookieDomain(gomock.Any()).Return(tc.domain).AnyTimes()
 
-			err := createCsrfSession(rr, req, config, store, tc.name, "value")
+			err := createCsrfSession(rr, req, config, store, tc.name, "value", tc.maxAge)
 			assert.NoError(t, err)
 
 			cookies := make(map[string]cookie)
@@ -415,6 +429,7 @@ func TestCreateCsrfSession(t *testing.T) {
 					httpOnly: c.HttpOnly,
 					secure:   c.Secure,
 					sameSite: c.SameSite,
+					maxAge:   c.MaxAge,
 					domain:   c.Domain,
 				}
 			}

consent/strategy_default.go

@@ -22,12 +22,15 @@ package consent
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"net/url"
 	"strconv"
 	"strings"
 	"time"
 
+	"github.com/twmb/murmur3"
+
 	"github.com/ory/hydra/driver/config"
 
 	"github.com/ory/x/errorsx"
@@ -247,6 +250,7 @@ func (s *DefaultStrategy) forwardAuthenticationRequest(ctx context.Context, w ht
 	}
 
 	// Set the session
+	cl := sanitizeClientFromRequest(ar)
 	if err := s.r.ConsentManager().CreateLoginRequest(
 		r.Context(),
 		&LoginRequest{
@@ -257,7 +261,7 @@ func (s *DefaultStrategy) forwardAuthenticationRequest(ctx context.Context, w ht
 			RequestedScope:    []string(ar.GetRequestedScopes()),
 			RequestedAudience: []string(ar.GetRequestedAudience()),
 			Subject:           subject,
-			Client:            sanitizeClientFromRequest(ar),
+			Client:            cl,
 			RequestURL:        iu.String(),
 			AuthenticatedAt:   sqlxx.NullTime(authenticatedAt),
 			RequestedAt:       time.Now().Truncate(time.Second).UTC(),
@@ -274,7 +278,8 @@ func (s *DefaultStrategy) forwardAuthenticationRequest(ctx context.Context, w ht
 		return errorsx.WithStack(err)
 	}
 
-	if err := createCsrfSession(w, r, s.r.Config(), s.r.CookieStore(ctx), s.r.Config().CookieNameLoginCSRF(ctx), csrf); err != nil {
+	clientSpecificCookieNameLoginCSRF := fmt.Sprintf("%s_%d", s.r.Config().CookieNameLoginCSRF(ctx), murmur3.Sum32(cl.ID.Bytes()))
+	if err := createCsrfSession(w, r, s.r.Config(), s.r.CookieStore(ctx), clientSpecificCookieNameLoginCSRF, csrf, s.c.ConsentRequestMaxAge(ctx)); err != nil {
 		return errorsx.WithStack(err)
 	}
 
@@ -335,7 +340,8 @@ func (s *DefaultStrategy) verifyAuthentication(w http.ResponseWriter, r *http.Re
 		return nil, errorsx.WithStack(fosite.ErrRequestUnauthorized.WithHint("The login request has expired. Please try again."))
 	}
 
-	if err := validateCsrfSession(r, s.r.Config(), s.r.CookieStore(ctx), s.r.Config().CookieNameLoginCSRF(ctx), session.LoginRequest.CSRF); err != nil {
+	clientSpecificCookieNameLoginCSRF := fmt.Sprintf("%s_%d", s.r.Config().CookieNameLoginCSRF(ctx), murmur3.Sum32(session.LoginRequest.Client.ID.Bytes()))
+	if err := validateCsrfSession(r, s.r.Config(), s.r.CookieStore(ctx), clientSpecificCookieNameLoginCSRF, session.LoginRequest.CSRF); err != nil {
 		return nil, err
 	}
 
@@ -523,6 +529,7 @@ func (s *DefaultStrategy) forwardConsentRequest(ctx context.Context, w http.Resp
 	challenge := strings.Replace(uuid.New(), "-", "", -1)
 	csrf := strings.Replace(uuid.New(), "-", "", -1)
 
+	cl := sanitizeClientFromRequest(ar)
 	if err := s.r.ConsentManager().CreateConsentRequest(
 		r.Context(),
 		&OAuth2ConsentRequest{
@@ -535,7 +542,7 @@ func (s *DefaultStrategy) forwardConsentRequest(ctx context.Context, w http.Resp
 			RequestedScope:         []string(ar.GetRequestedScopes()),
 			RequestedAudience:      []string(ar.GetRequestedAudience()),
 			Subject:                as.Subject,
-			Client:                 sanitizeClientFromRequest(ar),
+			Client:                 cl,
 			RequestURL:             as.LoginRequest.RequestURL,
 			AuthenticatedAt:        as.AuthenticatedAt,
 			RequestedAt:            as.RequestedAt,
@@ -549,7 +556,8 @@ func (s *DefaultStrategy) forwardConsentRequest(ctx context.Context, w http.Resp
 		return errorsx.WithStack(err)
 	}
 
-	if err := createCsrfSession(w, r, s.r.Config(), s.r.CookieStore(ctx), s.r.Config().CookieNameConsentCSRF(ctx), csrf); err != nil {
+	clientSpecificCookieNameConsentCSRF := fmt.Sprintf("%s_%d", s.r.Config().CookieNameConsentCSRF(ctx), murmur3.Sum32(cl.ID.Bytes()))
+	if err := createCsrfSession(w, r, s.r.Config(), s.r.CookieStore(ctx), clientSpecificCookieNameConsentCSRF, csrf, s.c.ConsentRequestMaxAge(ctx)); err != nil {
 		return errorsx.WithStack(err)
 	}
 
@@ -584,7 +592,8 @@ func (s *DefaultStrategy) verifyConsent(ctx context.Context, w http.ResponseWrit
 		return nil, errorsx.WithStack(fosite.ErrServerError.WithHint("The authenticatedAt value was not set."))
 	}
 
-	if err := validateCsrfSession(r, s.r.Config(), s.r.CookieStore(ctx), s.r.Config().CookieNameConsentCSRF(ctx), session.ConsentRequest.CSRF); err != nil {
+	clientSpecificCookieNameConsentCSRF := fmt.Sprintf("%s_%d", s.r.Config().CookieNameConsentCSRF(ctx), murmur3.Sum32(session.ConsentRequest.Client.ID.Bytes()))
+	if err := validateCsrfSession(r, s.r.Config(), s.r.CookieStore(ctx), clientSpecificCookieNameConsentCSRF, session.ConsentRequest.CSRF); err != nil {
 		return nil, err
 	}
 

consent/strategy_oauth_test.go

@@ -13,6 +13,8 @@ import (
 
 	"github.com/ory/x/ioutilx"
 
+	"github.com/twmb/murmur3"
+
 	"golang.org/x/oauth2"
 
 	"github.com/ory/x/pointerx"
@@ -275,6 +277,89 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 		})
 	})
 
+	t.Run("case=should set client specific csrf cookie names", func(t *testing.T) {
+		subject := "subject-1"
+		consentRequestMaxAge := reg.Config().ConsentRequestMaxAge(ctx).Seconds()
+		c := createDefaultClient(t)
+		testhelpers.NewLoginConsentUI(t, reg.Config(),
+			acceptLoginHandler(t, subject, &hydra.AcceptOAuth2LoginRequest{
+				Remember: pointerx.Bool(true),
+			}),
+			acceptConsentHandler(t, &hydra.AcceptOAuth2ConsentRequest{
+				Remember:   pointerx.Bool(true),
+				GrantScope: []string{"openid"},
+				Session: &hydra.AcceptOAuth2ConsentRequestSession{
+					AccessToken: map[string]interface{}{"foo": "bar"},
+					IdToken:     map[string]interface{}{"bar": "baz"},
+				},
+			}))
+		testhelpers.NewLoginConsentUI(t, reg.Config(),
+			checkAndAcceptLoginHandler(t, adminClient, subject, func(t *testing.T, res *hydra.OAuth2LoginRequest, err error) hydra.AcceptOAuth2LoginRequest {
+				require.NoError(t, err)
+				assert.Empty(t, res.Subject)
+				assert.Empty(t, pointerx.StringR(res.Client.ClientSecret))
+				return hydra.AcceptOAuth2LoginRequest{
+					Subject: subject,
+					Context: map[string]interface{}{"foo": "bar"},
+				}
+			}),
+			checkAndAcceptConsentHandler(t, adminClient, func(t *testing.T, res *hydra.OAuth2ConsentRequest, err error) hydra.AcceptOAuth2ConsentRequest {
+				require.NoError(t, err)
+				assert.Equal(t, subject, *res.Subject)
+				assert.Empty(t, pointerx.StringR(res.Client.ClientSecret))
+				return hydra.AcceptOAuth2ConsentRequest{
+					Remember:   pointerx.Bool(true),
+					GrantScope: []string{"openid"},
+					Session: &hydra.AcceptOAuth2ConsentRequestSession{
+						AccessToken: map[string]interface{}{"foo": "bar"},
+						IdToken:     map[string]interface{}{"bar": "baz"},
+					},
+				}
+			}))
+		hc := &http.Client{
+			Jar:       testhelpers.NewEmptyCookieJar(t),
+			Transport: &http.Transport{},
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
+		_, oauthRes := makeOAuth2Request(t, reg, hc, c, url.Values{"redirect_uri": {c.RedirectURIs[0]}, "scope": {"openid"}})
+		assert.EqualValues(t, http.StatusFound, oauthRes.StatusCode)
+		loginChallengeRedirect, err := oauthRes.Location()
+		require.NoError(t, err)
+		defer oauthRes.Body.Close()
+		setCookieHeader := oauthRes.Header.Get("set-cookie")
+		assert.NotNil(t, setCookieHeader)
+
+		t.Run("login cookie client specific suffix is set", func(t *testing.T) {
+			assert.Regexp(t, fmt.Sprintf("ory_hydra_login_csrf_dev_%d=.*", murmur3.Sum32(c.ID.Bytes())), setCookieHeader)
+		})
+
+		t.Run("login cookie max age is set", func(t *testing.T) {
+			assert.Regexp(t, fmt.Sprintf("ory_hydra_login_csrf_dev_%d=.*Max-Age=%.0f;.*", murmur3.Sum32(c.ID.Bytes()), consentRequestMaxAge), setCookieHeader)
+		})
+
+		loginChallengeRes, err := hc.Get(loginChallengeRedirect.String())
+		require.NoError(t, err)
+		defer loginChallengeRes.Body.Close()
+
+		loginVerifierRedirect, err := loginChallengeRes.Location()
+		loginVerifierRes, err := hc.Get(loginVerifierRedirect.String())
+		require.NoError(t, err)
+		defer loginVerifierRes.Body.Close()
+		setCookieHeader = loginVerifierRes.Header.Values("set-cookie")[1]
+		assert.NotNil(t, setCookieHeader)
+
+		t.Run("consent cookie client specific suffix set", func(t *testing.T) {
+			assert.Regexp(t, fmt.Sprintf("ory_hydra_consent_csrf_dev_%d=.*", murmur3.Sum32(c.ID.Bytes())), setCookieHeader)
+		})
+
+		t.Run("consent cookie max age is set", func(t *testing.T) {
+			assert.Regexp(t, fmt.Sprintf("ory_hydra_consent_csrf_dev_%d=.*Max-Age=%.0f;.*", murmur3.Sum32(c.ID.Bytes()), consentRequestMaxAge), setCookieHeader)
+		})
+	})
+
 	t.Run("case=should pass and check if login context is set properly", func(t *testing.T) {
 		// This should pass because login was remembered and session id should be set and session context should also work
 		subject := "aeneas-rekkas"

go.mod

@@ -61,6 +61,7 @@ require (
 	github.com/tidwall/sjson v1.2.4
 	github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80
 	github.com/toqueteos/webbrowser v1.2.0
+	github.com/twmb/murmur3 v1.1.6
 	github.com/urfave/negroni v1.0.0
 	go.opentelemetry.io/otel v1.9.0
 	go.step.sm/crypto v0.16.2

go.sum

@@ -1534,6 +1534,8 @@ github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80/go.mod h1:iFy
 github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9rrstGQ=
 github.com/toqueteos/webbrowser v1.2.0/go.mod h1:XWoZq4cyp9WeUeak7w7LXRUQf1F1ATJMir8RTqb4ayM=
 github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
+github.com/twmb/murmur3 v1.1.6 h1:mqrRot1BRxm+Yct+vavLMou2/iJt0tNVTTC0QoIjaZg=
+github.com/twmb/murmur3 v1.1.6/go.mod h1:Qq/R7NUyOfr65zD+6Q5IHKsJLwP7exErjN6lyyq3OSQ=
 github.com/twitchtv/twirp v8.1.1+incompatible/go.mod h1:RRJoFSAmTEh2weEqWtpPE3vFK5YBhA6bqp2l1kfCC5A=
 github.com/uber/jaeger-client-go v2.30.0+incompatible h1:D6wyKGCecFaSRUpo8lCVbaOOb6ThwMmTEbhRwtKR97o=
 github.com/uber/jaeger-client-go v2.30.0+incompatible/go.mod h1:WVhlPFC8FDjOFMMWRy2pZqQJSXxYSwNYOkTr/Z6d3Kk=