feat: handle concurrent refreshes and improve graceful refreshing

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.
ory · Dec 5, 2024 · 269fbb6 · 269fbb6
1 parent 4c13d5f
commit 269fbb6
Show file tree

Hide file tree

Showing 57 changed files with 2,311 additions and 1,715 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)