feat: handle concurrent refreshes and improve graceful refreshing (#3895

) This patch improves Ory Hydra's ability to deal with refresh flows which, for example, concurrently refresh the same token. Furthermore, graceful token refresh has been improved to handle a variety of edge cases and scenarios. Additionally, serializability errors in CockroachDB are now correctly retried. See ory-corp/cloud#7311 Closes #3895
ory · Dec 17, 2024 · 0a6c966 · 0a6c966
1 parent 63736ba
commit 0a6c966
Show file tree

Hide file tree

Showing 57 changed files with 2,323 additions and 1,999 deletions.
diff --git a/.docker/Dockerfile-hsm b/.docker/Dockerfile-hsm
@@ -17,20 +17,32 @@ COPY . .
 
 ###############################
 
-FROM builder as build-hydra
+FROM builder AS build-hydra
 RUN go build -tags sqlite,hsm -o /usr/bin/hydra
 
 ###############################
 
-FROM builder as test-hsm
+FROM builder AS test-hsm
 ENV HSM_ENABLED=true
 ENV HSM_LIBRARY=/usr/lib/softhsm/libsofthsm2.so
 ENV HSM_TOKEN_LABEL=hydra
 ENV HSM_PIN=1234
 
-RUN apt-get -y install softhsm opensc &&\
-    pkcs11-tool --module "$HSM_LIBRARY" --slot 0 --init-token --so-pin 0000 --init-pin --pin "$HSM_PIN" --label "$HSM_TOKEN_LABEL" &&\
-    go test -p 1 -v -failfast -short -tags=sqlite,hsm ./...
+RUN apt-get -y install softhsm opensc
+RUN pkcs11-tool --module "$HSM_LIBRARY" --slot 0 --init-token --so-pin 0000 --init-pin --pin "$HSM_PIN" --label "$HSM_TOKEN_LABEL"
+RUN go test -p 1 -failfast -short -tags=sqlite,hsm ./...
+
+
+FROM builder AS test-refresh-hsm
+ENV HSM_ENABLED=true
+ENV HSM_LIBRARY=/usr/lib/softhsm/libsofthsm2.so
+ENV HSM_TOKEN_LABEL=hydra
+ENV HSM_PIN=1234
+ENV UPDATE_SNAPSHOTS=true
+
+RUN apt-get -y install softhsm opensc
+RUN pkcs11-tool --module "$HSM_LIBRARY" --slot 0 --init-token --so-pin 0000 --init-pin --pin "$HSM_PIN" --label "$HSM_TOKEN_LABEL"
+RUN go test -p 1 -failfast -short -tags=sqlite,hsm,refresh ./...
 
 ###############################
 

diff --git a/.schema/config.schema.json b/.schema/config.schema.json
@@ -1101,11 +1101,11 @@
           "examples": ["https://my-example.app/token-refresh-hook"],
           "oneOf": [
             {
-              "type": "string",
-              "format": "uri"
+              "$ref": "#/definitions/webhook_config"
             },
             {
-              "$ref": "#/definitions/webhook_config"
+              "type": "string",
+              "format": "uri"
             }
           ]
         },
@@ -1114,11 +1114,11 @@
           "examples": ["https://my-example.app/token-hook"],
           "oneOf": [
             {
-              "type": "string",
-              "format": "uri"
+              "$ref": "#/definitions/webhook_config"
             },
             {
-              "$ref": "#/definitions/webhook_config"
+              "type": "string",
+              "format": "uri"
             }
           ]
         }

diff --git a/Makefile b/Makefile
@@ -90,9 +90,10 @@ quicktest:
 quicktest-hsm:
 	DOCKER_BUILDKIT=1 DOCKER_CONTENT_TRUST=1 docker build --progress=plain -f .docker/Dockerfile-hsm --target test-hsm -t oryd/hydra:${IMAGE_TAG} --target test-hsm .
 
-.PHONY: refresh
-refresh:
+.PHONY: test-refresh
+test-refresh:
 	UPDATE_SNAPSHOTS=true go test -failfast -short -tags sqlite,sqlite_omit_load_extension ./...
+	DOCKER_BUILDKIT=1 DOCKER_CONTENT_TRUST=1 docker build --progress=plain -f .docker/Dockerfile-hsm --target test-refresh-hsm -t oryd/hydra:${IMAGE_TAG} --target test-refresh-hsm .
 
 authors:  # updates the AUTHORS file
 	curl https://raw.githubusercontent.com/ory/ci/master/authors/authors.sh | env PRODUCT="Ory Hydra" bash

diff --git a/aead/aead_test.go b/aead/aead_test.go
@@ -10,13 +10,14 @@ import (
 	"io"
 	"testing"
 
-	"github.com/ory/hydra/v2/aead"
-	"github.com/ory/hydra/v2/driver/config"
-	"github.com/ory/hydra/v2/internal"
+	"github.com/ory/hydra/v2/internal/testhelpers"
 
 	"github.com/pborman/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/ory/hydra/v2/aead"
+	"github.com/ory/hydra/v2/driver/config"
 )
 
 func secret(t *testing.T) string {
@@ -43,7 +44,7 @@ func TestAEAD(t *testing.T) {
 			t.Run("case=without-rotation", func(t *testing.T) {
 				t.Parallel()
 				ctx := context.Background()
-				c := internal.NewConfigurationWithDefaults()
+				c := testhelpers.NewConfigurationWithDefaults()
 				c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
 				a := NewCipher(c)
 
@@ -63,7 +64,7 @@ func TestAEAD(t *testing.T) {
 			t.Run("case=wrong-secret", func(t *testing.T) {
 				t.Parallel()
 				ctx := context.Background()
-				c := internal.NewConfigurationWithDefaults()
+				c := testhelpers.NewConfigurationWithDefaults()
 				c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
 				a := NewCipher(c)
 
@@ -78,7 +79,7 @@ func TestAEAD(t *testing.T) {
 			t.Run("case=with-rotation", func(t *testing.T) {
 				t.Parallel()
 				ctx := context.Background()
-				c := internal.NewConfigurationWithDefaults()
+				c := testhelpers.NewConfigurationWithDefaults()
 				old := secret(t)
 				c.MustSet(ctx, config.KeyGetSystemSecret, []string{old})
 				a := NewCipher(c)
@@ -106,7 +107,7 @@ func TestAEAD(t *testing.T) {
 			t.Run("case=with-rotation-wrong-secret", func(t *testing.T) {
 				t.Parallel()
 				ctx := context.Background()
-				c := internal.NewConfigurationWithDefaults()
+				c := testhelpers.NewConfigurationWithDefaults()
 				c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
 				a := NewCipher(c)
 
@@ -123,7 +124,7 @@ func TestAEAD(t *testing.T) {
 			t.Run("suite=with additional data", func(t *testing.T) {
 				t.Parallel()
 				ctx := context.Background()
-				c := internal.NewConfigurationWithDefaults()
+				c := testhelpers.NewConfigurationWithDefaults()
 				c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
 				a := NewCipher(c)
 

diff --git a/client/handler_test.go b/client/handler_test.go
@@ -35,7 +35,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/ory/hydra/v2/client"
-	"github.com/ory/hydra/v2/internal"
 )
 
 type responseSnapshot struct {
@@ -56,7 +55,7 @@ func getClientID(body string) string {
 
 func TestHandler(t *testing.T) {
 	ctx := context.Background()
-	reg := internal.NewMockedRegistry(t, &contextx.Default{})
+	reg := testhelpers.NewMockedRegistry(t, &contextx.Default{})
 	h := client.NewHandler(reg)
 	reg.WithContextualizer(&contextx.TestContextualizer{})
 

diff --git a/client/sdk_test.go b/client/sdk_test.go
@@ -9,6 +9,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/ory/hydra/v2/internal/testhelpers"
+
 	"github.com/ory/x/assertx"
 
 	"github.com/ory/x/ioutilx"
@@ -26,8 +28,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/ory/hydra/v2/internal"
-
 	hydra "github.com/ory/hydra-client-go/v2"
 	"github.com/ory/hydra/v2/client"
 )
@@ -63,11 +63,11 @@ var defaultIgnoreFields = []string{"client_id", "registration_access_token", "re
 
 func TestClientSDK(t *testing.T) {
 	ctx := context.Background()
-	conf := internal.NewConfigurationWithDefaults()
+	conf := testhelpers.NewConfigurationWithDefaults()
 	conf.MustSet(ctx, config.KeySubjectTypesSupported, []string{"public"})
 	conf.MustSet(ctx, config.KeyDefaultClientScope, []string{"foo", "bar"})
 	conf.MustSet(ctx, config.KeyPublicAllowDynamicRegistration, true)
-	r := internal.NewRegistryMemory(t, conf, &contextx.Static{C: conf.Source(ctx)})
+	r := testhelpers.NewRegistryMemory(t, conf, &contextx.Static{C: conf.Source(ctx)})
 
 	routerAdmin := x.NewRouterAdmin(conf.AdminURL)
 	routerPublic := x.NewRouterPublic()

diff --git a/client/validator_test.go b/client/validator_test.go
@@ -12,6 +12,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/ory/hydra/v2/internal/testhelpers"
+
 	"github.com/hashicorp/go-retryablehttp"
 
 	"github.com/ory/fosite"
@@ -24,17 +26,16 @@ import (
 
 	. "github.com/ory/hydra/v2/client"
 	"github.com/ory/hydra/v2/driver/config"
-	"github.com/ory/hydra/v2/internal"
 	"github.com/ory/hydra/v2/x"
 	"github.com/ory/x/contextx"
 )
 
 func TestValidate(t *testing.T) {
 	ctx := context.Background()
-	c := internal.NewConfigurationWithDefaults()
+	c := testhelpers.NewConfigurationWithDefaults()
 	c.MustSet(ctx, config.KeySubjectTypesSupported, []string{"pairwise", "public"})
 	c.MustSet(ctx, config.KeyDefaultClientScope, []string{"openid"})
-	reg := internal.NewRegistryMemory(t, c, &contextx.Static{C: c.Source(ctx)})
+	reg := testhelpers.NewRegistryMemory(t, c, &contextx.Static{C: c.Source(ctx)})
 	v := NewValidator(reg)
 
 	testCtx := context.TODO()
@@ -186,7 +187,7 @@ func (f *fakeHTTP) HTTPClient(ctx context.Context, opts ...httpx.ResilientOption
 }
 
 func TestValidateSectorIdentifierURL(t *testing.T) {
-	reg := internal.NewMockedRegistry(t, &contextx.Default{})
+	reg := testhelpers.NewMockedRegistry(t, &contextx.Default{})
 	var payload string
 
 	var h http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
@@ -268,8 +269,8 @@ const validJWKS = `
 
 func TestValidateIPRanges(t *testing.T) {
 	ctx := context.Background()
-	c := internal.NewConfigurationWithDefaults()
-	reg := internal.NewRegistryMemory(t, c, &contextx.Static{C: c.Source(ctx)})
+	c := testhelpers.NewConfigurationWithDefaults()
+	reg := testhelpers.NewRegistryMemory(t, c, &contextx.Static{C: c.Source(ctx)})
 
 	v := NewValidator(reg)
 	c.MustSet(ctx, config.KeyClientHTTPNoPrivateIPRanges, true)
@@ -287,10 +288,10 @@ func TestValidateIPRanges(t *testing.T) {
 
 func TestValidateDynamicRegistration(t *testing.T) {
 	ctx := context.Background()
-	c := internal.NewConfigurationWithDefaults()
+	c := testhelpers.NewConfigurationWithDefaults()
 	c.MustSet(ctx, config.KeySubjectTypesSupported, []string{"pairwise", "public"})
 	c.MustSet(ctx, config.KeyDefaultClientScope, []string{"openid"})
-	reg := internal.NewRegistryMemory(t, c, &contextx.Static{C: c.Source(ctx)})
+	reg := testhelpers.NewRegistryMemory(t, c, &contextx.Static{C: c.Source(ctx)})
 
 	testCtx := context.TODO()
 	v := NewValidator(reg)

diff --git a/cmd/cmd_helper_test.go b/cmd/cmd_helper_test.go
@@ -19,7 +19,6 @@ import (
 
 	"github.com/ory/hydra/v2/client"
 	"github.com/ory/hydra/v2/driver"
-	"github.com/ory/hydra/v2/internal"
 	"github.com/ory/hydra/v2/internal/testhelpers"
 	"github.com/ory/x/cmdx"
 	"github.com/ory/x/contextx"
@@ -40,7 +39,7 @@ func setupRoutes(t *testing.T, cmd *cobra.Command) (*httptest.Server, *httptest.
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
 
-	reg := internal.NewMockedRegistry(t, &contextx.Default{})
+	reg := testhelpers.NewMockedRegistry(t, &contextx.Default{})
 	public, admin := testhelpers.NewOAuth2Server(ctx, t, reg)
 
 	cmdx.RegisterHTTPClientFlags(cmd.Flags())

diff --git a/consent/handler_test.go b/consent/handler_test.go
@@ -13,13 +13,14 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ory/hydra/v2/internal/testhelpers"
+
 	"github.com/stretchr/testify/require"
 
 	hydra "github.com/ory/hydra-client-go/v2"
 	"github.com/ory/hydra/v2/client"
 	. "github.com/ory/hydra/v2/consent"
 	"github.com/ory/hydra/v2/flow"
-	"github.com/ory/hydra/v2/internal"
 	"github.com/ory/hydra/v2/x"
 	"github.com/ory/x/contextx"
 	"github.com/ory/x/pointerx"
@@ -42,8 +43,8 @@ func TestGetLogoutRequest(t *testing.T) {
 			challenge := "challenge" + key
 			requestURL := "http://192.0.2.1"
 
-			conf := internal.NewConfigurationWithDefaults()
-			reg := internal.NewRegistryMemory(t, conf, &contextx.Default{})
+			conf := testhelpers.NewConfigurationWithDefaults()
+			reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{})
 
 			if tc.exists {
 				cl := &client.Client{ID: "client" + key}
@@ -97,8 +98,8 @@ func TestGetLoginRequest(t *testing.T) {
 			challenge := "challenge" + key
 			requestURL := "http://192.0.2.1"
 
-			conf := internal.NewConfigurationWithDefaults()
-			reg := internal.NewRegistryMemory(t, conf, &contextx.Default{})
+			conf := testhelpers.NewConfigurationWithDefaults()
+			reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{})
 
 			if tc.exists {
 				cl := &client.Client{ID: "client" + key}
@@ -163,8 +164,8 @@ func TestGetConsentRequest(t *testing.T) {
 			challenge := "challenge" + key
 			requestURL := "http://192.0.2.1"
 
-			conf := internal.NewConfigurationWithDefaults()
-			reg := internal.NewRegistryMemory(t, conf, &contextx.Default{})
+			conf := testhelpers.NewConfigurationWithDefaults()
+			reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{})
 
 			if tc.exists {
 				cl := &client.Client{ID: "client" + key}
@@ -238,8 +239,8 @@ func TestGetLoginRequestWithDuplicateAccept(t *testing.T) {
 		challenge := "challenge"
 		requestURL := "http://192.0.2.1"
 
-		conf := internal.NewConfigurationWithDefaults()
-		reg := internal.NewRegistryMemory(t, conf, &contextx.Default{})
+		conf := testhelpers.NewConfigurationWithDefaults()
+		reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{})
 
 		cl := &client.Client{ID: "client"}
 		require.NoError(t, reg.ClientManager().CreateClient(ctx, cl))

diff --git a/consent/sdk_test.go b/consent/sdk_test.go
@@ -11,6 +11,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ory/hydra/v2/internal/testhelpers"
+
 	"github.com/ory/hydra/v2/consent/test"
 
 	hydra "github.com/ory/hydra-client-go/v2"
@@ -23,7 +25,6 @@ import (
 
 	. "github.com/ory/hydra/v2/consent"
 	"github.com/ory/hydra/v2/driver/config"
-	"github.com/ory/hydra/v2/internal"
 	"github.com/ory/hydra/v2/x"
 	"github.com/ory/x/contextx"
 )
@@ -35,10 +36,10 @@ func makeID(base string, network string, key string) string {
 func TestSDK(t *testing.T) {
 	ctx := context.Background()
 	network := "t1"
-	conf := internal.NewConfigurationWithDefaults()
+	conf := testhelpers.NewConfigurationWithDefaults()
 	conf.MustSet(ctx, config.KeyIssuerURL, "https://www.ory.sh")
 	conf.MustSet(ctx, config.KeyAccessTokenLifespan, time.Minute)
-	reg := internal.NewRegistryMemory(t, conf, &contextx.Default{})
+	reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{})
 
 	consentChallenge := func(f *Flow) string { return x.Must(f.ToConsentChallenge(ctx, reg)) }
 	consentVerifier := func(f *Flow) string { return x.Must(f.ToConsentVerifier(ctx, reg)) }

diff --git a/consent/strategy_logout_test.go b/consent/strategy_logout_test.go
@@ -28,7 +28,6 @@ import (
 	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"
 	"github.com/ory/hydra/v2/internal/testhelpers"
 	"github.com/ory/x/contextx"
 	"github.com/ory/x/ioutilx"
@@ -37,7 +36,7 @@ import (
 func TestLogoutFlows(t *testing.T) {
 	ctx := context.Background()
 	fakeKratos := kratos.NewFake()
-	reg := internal.NewMockedRegistry(t, &contextx.Default{})
+	reg := testhelpers.NewMockedRegistry(t, &contextx.Default{})
 	reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "opaque")
 	reg.Config().MustSet(ctx, config.KeyConsentRequestMaxAge, time.Hour)
 

diff --git a/consent/strategy_oauth_test.go b/consent/strategy_oauth_test.go
@@ -37,12 +37,11 @@ import (
 	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"
 )
 
 func TestStrategyLoginConsentNext(t *testing.T) {
 	ctx := context.Background()
-	reg := internal.NewMockedRegistry(t, &contextx.Default{})
+	reg := testhelpers.NewMockedRegistry(t, &contextx.Default{})
 	reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "opaque")
 	reg.Config().MustSet(ctx, config.KeyConsentRequestMaxAge, time.Hour)
 	reg.Config().MustSet(ctx, config.KeyConsentRequestMaxAge, time.Hour)