feat: flush inactive/expired login and consent requests (#2381)

This patch resolves various table growth issues caused by expired/inactive login and consent flows never being purged from the database.

You may now use the new `hydra janitor` command to remove access & refresh tokens and login & consent requests which are no longer valid or used. The command follows the `notAfter` safe-guard approach to ensure records needed to be kept are not deleted.

To learn more, please use `hydra help janitor`.

This patch phases out the `/oauth2/flush` endpoint as the janitor is better suited for background tasks, is easier to run in a targeted fashion (e.g. as a singleton job), and does not cause HTTP timeouts.

Closes #1574

cmd/cli/handler.go

@@ -37,6 +37,7 @@ type Handler struct {
 	Introspection *IntrospectionHandler
 	Token         *TokenHandler
 	Migration     *MigrateHandler
+	Janitor       *JanitorHandler
 }
 
 func Remote(cmd *cobra.Command) string {
@@ -63,5 +64,6 @@ func NewHandler() *Handler {
 		Introspection: newIntrospectionHandler(),
 		Token:         newTokenHandler(),
 		Migration:     newMigrateHandler(),
+		Janitor:       NewJanitorHandler(),
 	}
 }

cmd/cli/handler_janitor.go

@@ -0,0 +1,164 @@
+package cli
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/ory/hydra/persistence"
+
+	"github.com/pkg/errors"
+
+	"github.com/ory/x/flagx"
+
+	"github.com/spf13/cobra"
+
+	"github.com/ory/hydra/driver"
+	"github.com/ory/hydra/driver/config"
+	"github.com/ory/x/configx"
+	"github.com/ory/x/errorsx"
+)
+
+const (
+	KeepIfYounger          = "keep-if-younger"
+	AccessLifespan         = "access-lifespan"
+	RefreshLifespan        = "refresh-lifespan"
+	ConsentRequestLifespan = "consent-request-lifespan"
+	OnlyTokens             = "tokens"
+	OnlyRequests           = "requests"
+	ReadFromEnv            = "read-from-env"
+	Config                 = "config"
+)
+
+type JanitorHandler struct{}
+
+func NewJanitorHandler() *JanitorHandler {
+	return &JanitorHandler{}
+}
+
+func (_ *JanitorHandler) Args(cmd *cobra.Command, args []string) error {
+	if len(args) == 0 &&
+		!flagx.MustGetBool(cmd, ReadFromEnv) &&
+		len(flagx.MustGetStringSlice(cmd, Config)) == 0 {
+
+		fmt.Printf("%s\n", cmd.UsageString())
+		return fmt.Errorf("%s\n%s\n%s\n",
+			"A DSN is required as a positional argument when not passing any of the following flags:",
+			"- Using the environment variable with flag -e, --read-from-env",
+			"- Using the config file with flag -c, --config")
+	}
+
+	if !flagx.MustGetBool(cmd, OnlyTokens) && !flagx.MustGetBool(cmd, OnlyRequests) {
+		return fmt.Errorf("%s\n%s\n", cmd.UsageString(),
+			"Janitor requires either --tokens or --requests or both to be set")
+	}
+
+	return nil
+}
+
+func (_ *JanitorHandler) RunE(cmd *cobra.Command, args []string) error {
+	return purge(cmd, args)
+}
+
+func purge(cmd *cobra.Command, args []string) error {
+	var d driver.Registry
+
+	co := []configx.OptionModifier{
+		configx.WithFlags(cmd.Flags()),
+		configx.SkipValidation(),
+	}
+
+	keys := map[string]string{
+		AccessLifespan:         config.KeyAccessTokenLifespan,
+		RefreshLifespan:        config.KeyRefreshTokenLifespan,
+		ConsentRequestLifespan: config.KeyConsentRequestMaxAge,
+	}
+
+	for k, v := range keys {
+		if x := flagx.MustGetDuration(cmd, k); x > 0 {
+			co = append(co, configx.WithValue(v, x))
+		}
+	}
+
+	notAfter := time.Now()
+
+	if keepYounger := flagx.MustGetDuration(cmd, KeepIfYounger); keepYounger > 0 {
+		notAfter = notAfter.Add(-keepYounger)
+	}
+
+	if !flagx.MustGetBool(cmd, ReadFromEnv) && len(flagx.MustGetStringSlice(cmd, Config)) == 0 {
+		co = append(co, configx.WithValue(config.KeyDSN, args[0]))
+	}
+
+	do := []driver.OptionsModifier{
+		driver.DisableValidation(),
+		driver.DisablePreloading(),
+		driver.WithOptions(co...),
+	}
+
+	d = driver.New(cmd.Context(), do...)
+
+	if len(d.Config().DSN()) == 0 {
+		return fmt.Errorf("%s\n%s\n%s\n", cmd.UsageString(),
+			"When using flag -e, environment variable DSN must be set.",
+			"When using flag -c, the dsn property should be set.")
+	}
+
+	if err := d.Init(cmd.Context()); err != nil {
+		return fmt.Errorf("%s\n%s\n", cmd.UsageString(),
+			"Janitor can only be executed against a SQL-compatible driver but DSN is not a SQL source.")
+	}
+
+	p := d.Persister()
+
+	var routineFlags []string
+
+	if flagx.MustGetBool(cmd, OnlyTokens) {
+		routineFlags = append(routineFlags, OnlyTokens)
+	}
+
+	if flagx.MustGetBool(cmd, OnlyRequests) {
+		routineFlags = append(routineFlags, OnlyRequests)
+	}
+
+	return cleanupRun(cmd.Context(), notAfter, addRoutine(p, routineFlags...)...)
+}
+
+func addRoutine(p persistence.Persister, names ...string) []cleanupRoutine {
+	var routines []cleanupRoutine
+	for _, n := range names {
+		switch n {
+		case OnlyTokens:
+			routines = append(routines, cleanup(p.FlushInactiveAccessTokens, "access tokens"))
+			routines = append(routines, cleanup(p.FlushInactiveRefreshTokens, "refresh tokens"))
+		case OnlyRequests:
+			routines = append(routines, cleanup(p.FlushInactiveLoginConsentRequests, "login-consent requests"))
+		}
+	}
+	return routines
+}
+
+type cleanupRoutine func(ctx context.Context, notAfter time.Time) error
+
+func cleanup(cr cleanupRoutine, routineName string) cleanupRoutine {
+	return func(ctx context.Context, notAfter time.Time) error {
+		if err := cr(ctx, notAfter); err != nil {
+			return errors.Wrap(errorsx.WithStack(err), fmt.Sprintf("Could not cleanup inactive %s", routineName))
+		}
+		fmt.Printf("Successfully completed Janitor run on %s\n", routineName)
+		return nil
+	}
+}
+
+func cleanupRun(ctx context.Context, notAfter time.Time, routines ...cleanupRoutine) error {
+	if len(routines) == 0 {
+		return errors.New("clean up run received 0 routines")
+	}
+
+	for _, r := range routines {
+		if err := r(ctx, notAfter); err != nil {
+			return err
+		}
+	}
+	return nil
+}

cmd/cli/handler_janitor_test.go

@@ -0,0 +1,212 @@
+package cli_test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/ory/hydra/cmd"
+
+	"github.com/spf13/cobra"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/ory/hydra/cmd/cli"
+	"github.com/ory/hydra/internal/testhelpers"
+	"github.com/ory/x/cmdx"
+)
+
+func newJanitorCmd() *cobra.Command {
+	return cmd.NewRootCmd()
+}
+
+func TestJanitorHandler_PurgeTokenNotAfter(t *testing.T) {
+	ctx := context.Background()
+	testCycles := testhelpers.NewConsentJanitorTestHelper("").GetNotAfterTestCycles()
+
+	require.True(t, len(testCycles) > 0)
+
+	for k, v := range testCycles {
+		t.Run(fmt.Sprintf("case=%s", k), func(t *testing.T) {
+			jt := testhelpers.NewConsentJanitorTestHelper(t.Name())
+			reg, err := jt.GetRegistry(ctx, k)
+			require.NoError(t, err)
+
+			// setup test
+			t.Run("step=setup-access", jt.AccessTokenNotAfterSetup(ctx, reg.ClientManager(), reg.OAuth2Storage()))
+			t.Run("step=setup-refresh", jt.RefreshTokenNotAfterSetup(ctx, reg.ClientManager(), reg.OAuth2Storage()))
+
+			// run the cleanup routine
+			t.Run("step=cleanup", func(t *testing.T) {
+				cmdx.ExecNoErr(t, newJanitorCmd(),
+					"janitor",
+					fmt.Sprintf("--%s=%s", cli.KeepIfYounger, v.String()),
+					fmt.Sprintf("--%s=%s", cli.AccessLifespan, jt.GetAccessTokenLifespan().String()),
+					fmt.Sprintf("--%s=%s", cli.RefreshLifespan, jt.GetRefreshTokenLifespan().String()),
+					fmt.Sprintf("--%s", cli.OnlyTokens),
+					jt.GetDSN(),
+				)
+			})
+
+			// validate test
+			notAfter := time.Now().Round(time.Second).Add(-v)
+			t.Run("step=validate-access", jt.AccessTokenNotAfterValidate(ctx, notAfter, reg.OAuth2Storage()))
+			t.Run("step=validate-refresh", jt.RefreshTokenNotAfterValidate(ctx, notAfter, reg.OAuth2Storage()))
+		})
+	}
+}
+
+func TestJanitorHandler_PurgeLoginConsentNotAfter(t *testing.T) {
+	ctx := context.Background()
+
+	testCycles := testhelpers.NewConsentJanitorTestHelper("").GetNotAfterTestCycles()
+
+	for k, v := range testCycles {
+		jt := testhelpers.NewConsentJanitorTestHelper(k)
+		reg, err := jt.GetRegistry(ctx, k)
+		require.NoError(t, err)
+
+		t.Run(fmt.Sprintf("case=%s", k), func(t *testing.T) {
+			// Setup the test
+			t.Run("step=setup", jt.LoginConsentNotAfterSetup(ctx, reg.ConsentManager(), reg.ClientManager()))
+			// Run the cleanup routine
+			t.Run("step=cleanup", func(t *testing.T) {
+				cmdx.ExecNoErr(t, newJanitorCmd(),
+					"janitor",
+					fmt.Sprintf("--%s=%s", cli.KeepIfYounger, v.String()),
+					fmt.Sprintf("--%s=%s", cli.ConsentRequestLifespan, jt.GetConsentRequestLifespan().String()),
+					fmt.Sprintf("--%s", cli.OnlyRequests),
+					jt.GetDSN(),
+				)
+			})
+
+			notAfter := time.Now().Round(time.Second).Add(-v)
+			consentLifespan := time.Now().Round(time.Second).Add(-jt.GetConsentRequestLifespan())
+			t.Run("step=validate", jt.LoginConsentNotAfterValidate(ctx, notAfter, consentLifespan, reg.ConsentManager()))
+		})
+	}
+
+}
+
+func TestJanitorHandler_PurgeLoginConsent(t *testing.T) {
+	/*
+		Login and Consent also needs to be purged on two conditions besides the KeyConsentRequestMaxAge and notAfter time
+		- when a login/consent request was never completed (timed out)
+		- when a login/consent request was rejected
+	*/
+
+	t.Run("case=login-consent-timeout", func(t *testing.T) {
+		t.Run("case=login-timeout", func(t *testing.T) {
+			ctx := context.Background()
+			jt := testhelpers.NewConsentJanitorTestHelper(t.Name())
+			reg, err := jt.GetRegistry(ctx, t.Name())
+			require.NoError(t, err)
+
+			// setup
+			t.Run("step=setup", jt.LoginTimeoutSetup(ctx, reg.ConsentManager(), reg.ClientManager()))
+
+			// cleanup
+			t.Run("step=cleanup", func(t *testing.T) {
+				cmdx.ExecNoErr(t, newJanitorCmd(),
+					"janitor",
+					fmt.Sprintf("--%s", cli.OnlyRequests),
+					jt.GetDSN(),
+				)
+			})
+
+			t.Run("step=validate", jt.LoginTimeoutValidate(ctx, reg.ConsentManager()))
+
+		})
+
+		t.Run("case=consent-timeout", func(t *testing.T) {
+			ctx := context.Background()
+			jt := testhelpers.NewConsentJanitorTestHelper(t.Name())
+			reg, err := jt.GetRegistry(ctx, t.Name())
+			require.NoError(t, err)
+
+			// setup
+			t.Run("step=setup", jt.ConsentTimeoutSetup(ctx, reg.ConsentManager(), reg.ClientManager()))
+
+			// run cleanup
+			t.Run("step=cleanup", func(t *testing.T) {
+				cmdx.ExecNoErr(t, newJanitorCmd(),
+					"janitor",
+					fmt.Sprintf("--%s", cli.OnlyRequests),
+					jt.GetDSN(),
+				)
+			})
+
+			// validate
+			t.Run("step=validate", jt.ConsentTimeoutValidate(ctx, reg.ConsentManager()))
+		})
+
+	})
+
+	t.Run("case=login-consent-rejection", func(t *testing.T) {
+		ctx := context.Background()
+
+		t.Run("case=login-rejection", func(t *testing.T) {
+			jt := testhelpers.NewConsentJanitorTestHelper(t.Name())
+			reg, err := jt.GetRegistry(ctx, t.Name())
+			require.NoError(t, err)
+
+			// setup
+			t.Run("step=setup", jt.LoginRejectionSetup(ctx, reg.ConsentManager(), reg.ClientManager()))
+
+			// cleanup
+			t.Run("step=cleanup", func(t *testing.T) {
+				cmdx.ExecNoErr(t, newJanitorCmd(),
+					"janitor",
+					fmt.Sprintf("--%s", cli.OnlyRequests),
+					jt.GetDSN(),
+				)
+			})
+
+			// validate
+			t.Run("step=validate", jt.LoginRejectionValidate(ctx, reg.ConsentManager()))
+		})
+
+		t.Run("case=consent-rejection", func(t *testing.T) {
+			jt := testhelpers.NewConsentJanitorTestHelper(t.Name())
+			reg, err := jt.GetRegistry(ctx, t.Name())
+			require.NoError(t, err)
+
+			// setup
+			t.Run("step=setup", jt.ConsentRejectionSetup(ctx, reg.ConsentManager(), reg.ClientManager()))
+
+			// cleanup
+			t.Run("step=cleanup", func(t *testing.T) {
+				cmdx.ExecNoErr(t, newJanitorCmd(),
+					"janitor",
+					fmt.Sprintf("--%s", cli.OnlyRequests),
+					jt.GetDSN(),
+				)
+			})
+
+			// validate
+			t.Run("step=validate", jt.ConsentRejectionValidate(ctx, reg.ConsentManager()))
+		})
+
+	})
+
+}
+
+func TestJanitorHandler_Arguments(t *testing.T) {
+	cmdx.ExecNoErr(t, cmd.NewRootCmd(),
+		"janitor",
+		fmt.Sprintf("--%s", cli.OnlyRequests),
+		"memory",
+	)
+	cmdx.ExecNoErr(t, cmd.NewRootCmd(),
+		"janitor",
+		fmt.Sprintf("--%s", cli.OnlyTokens),
+		"memory",
+	)
+
+	_, _, err := cmdx.ExecCtx(context.Background(), cmd.NewRootCmd(), nil,
+		"janitor",
+		"memory")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "Janitor requires either --tokens or --requests or both to be set")
+}