diff --git a/cmd/cli/handler_janitor.go b/cmd/cli/handler_janitor.go index bec0401fe36..d18f01c4d18 100644 --- a/cmd/cli/handler_janitor.go +++ b/cmd/cli/handler_janitor.go @@ -33,6 +33,7 @@ const ( ConsentRequestLifespan = "consent-request-lifespan" OnlyTokens = "tokens" OnlyRequests = "requests" + OnlyLoginSessions = "login-sessions" OnlyGrants = "grants" ReadFromEnv = "read-from-env" Config = "config" @@ -64,9 +65,12 @@ func (_ *JanitorHandler) Args(cmd *cobra.Command, args []string) error { "- Using the config file with flag -c, --config") } - if !flagx.MustGetBool(cmd, OnlyTokens) && !flagx.MustGetBool(cmd, OnlyRequests) && !flagx.MustGetBool(cmd, OnlyGrants) { + if !flagx.MustGetBool(cmd, OnlyTokens) && + !flagx.MustGetBool(cmd, OnlyRequests) && + !flagx.MustGetBool(cmd, OnlyGrants) && + !flagx.MustGetBool(cmd, OnlyLoginSessions) { return fmt.Errorf("%s\n%s\n", cmd.UsageString(), - "Janitor requires at least one of --tokens, --requests or --grants to be set") + "Janitor requires at least one of --tokens, --requests, --grants or --login-sessions to be set") } limit := flagx.MustGetInt(cmd, Limit) @@ -154,6 +158,10 @@ func purge(cmd *cobra.Command, args []string, sl *servicelocatorx.Options, dOpts routineFlags = append(routineFlags, OnlyGrants) } + if flagx.MustGetBool(cmd, OnlyLoginSessions) { + routineFlags = append(routineFlags, OnlyLoginSessions) + } + return cleanupRun(cmd.Context(), notAfter, limit, batchSize, addRoutine(p, routineFlags...)...) } @@ -168,6 +176,8 @@ func addRoutine(p persistence.Persister, names ...string) []cleanupRoutine { routines = append(routines, cleanup(p.FlushInactiveLoginConsentRequests, "login-consent requests")) case OnlyGrants: routines = append(routines, cleanup(p.FlushInactiveGrants, "grants")) + case OnlyLoginSessions: + routines = append(routines, cleanup(p.FlushInactiveLoginSessions, "login sessions")) } } return routines diff --git a/cmd/cli/handler_janitor_test.go b/cmd/cli/handler_janitor_test.go index c99440c23b1..92247523791 100644 --- a/cmd/cli/handler_janitor_test.go +++ b/cmd/cli/handler_janitor_test.go @@ -92,6 +92,95 @@ func TestJanitorHandler_PurgeLoginConsentNotAfter(t *testing.T) { } +func TestJanitorHandler_PurgeLoginSessions(t *testing.T) { + t.Run("case=cleanup-login-session", func(t *testing.T) { + t.Run("case=clean session without consent-request", func(t *testing.T) { + ctx := context.Background() + jt := testhelpers.NewConsentJanitorTestHelper(t.Name()) + reg, err := jt.GetRegistry(ctx, t.Name()) + require.NoError(t, err) + + const sessionID = "session_id" + + // setup + sessionHelper := testhelpers.NewJanitorSessionTestHelper(reg, t.Name()) + sessionHelper.CreateLoginSession(t, ctx, sessionID) + + // cleanup + t.Run("step=cleanup", func(t *testing.T) { + cmdx.ExecNoErr(t, newJanitorCmd(), + "janitor", + fmt.Sprintf("--%s", cli.OnlyLoginSessions), + jt.GetDSN(), + ) + }) + + // validate + sessionHelper.ValidateSessionNotExist(t, ctx, sessionID) + }) + + t.Run("case=clean sessions with consent-request", func(t *testing.T) { + ctx := context.Background() + jt := testhelpers.NewConsentJanitorTestHelper(t.Name()) + reg, err := jt.GetRegistry(ctx, t.Name()) + require.NoError(t, err) + + const ( + sessionID = "session_id_1" + ) + + //setup + sessionHelper := testhelpers.NewJanitorSessionTestHelper(reg, t.Name()) + sessionHelper.CreateLoginSession(t, ctx, sessionID) + sessionHelper.CreateEnvironmentForSession(t, ctx, sessionID) + + // cleanup + t.Run("step=cleanup", func(t *testing.T) { + cmdx.ExecNoErr(t, newJanitorCmd(), + "janitor", + fmt.Sprintf("--%s", cli.OnlyLoginSessions), + jt.GetDSN(), + ) + }) + + // validate + sessionHelper.ValidateSessionExist(t, ctx, sessionID) + }) + + t.Run("case=alive session and flush session", func(t *testing.T) { + ctx := context.Background() + jt := testhelpers.NewConsentJanitorTestHelper(t.Name()) + reg, err := jt.GetRegistry(ctx, t.Name()) + require.NoError(t, err) + + const ( + aliveSessionID = "session_id_1" + flushSessionID = "session_id_flush" + ) + + //setup + sessionHelper := testhelpers.NewJanitorSessionTestHelper(reg, t.Name()) + sessionHelper.CreateLoginSession(t, ctx, aliveSessionID) + sessionHelper.CreateEnvironmentForSession(t, ctx, aliveSessionID) + sessionHelper.CreateLoginSession(t, ctx, flushSessionID) + + // cleanup + t.Run("step=cleanup", func(t *testing.T) { + cmdx.ExecNoErr(t, newJanitorCmd(), + "janitor", + fmt.Sprintf("--%s", cli.OnlyLoginSessions), + jt.GetDSN(), + ) + }) + + // validate + sessionHelper.ValidateSessionExist(t, ctx, aliveSessionID) + sessionHelper.ValidateSessionNotExist(t, ctx, flushSessionID) + }) + + }) +} + func TestJanitorHandler_PurgeLoginConsent(t *testing.T) { /* Login and Consent also needs to be purged on two conditions besides the KeyConsentRequestMaxAge and notAfter time @@ -214,7 +303,7 @@ func TestJanitorHandler_Arguments(t *testing.T) { "janitor", "memory") require.Error(t, err) - require.Contains(t, err.Error(), "Janitor requires at least one of --tokens, --requests or --grants to be set") + require.Contains(t, err.Error(), "Janitor requires at least one of --tokens, --requests, --grants or --login-sessions to be set") cmdx.ExecNoErr(t, cmd.NewRootCmd(nil, nil, nil), "janitor", diff --git a/cmd/janitor.go b/cmd/janitor.go index 31ffd4e7a63..87b83d44971 100644 --- a/cmd/janitor.go +++ b/cmd/janitor.go @@ -71,9 +71,10 @@ Janitor can be used in several ways. cmd.Flags().Duration(cli.AccessLifespan, 0, "Set the access token lifespan e.g. 1s, 1m, 1h.") cmd.Flags().Duration(cli.RefreshLifespan, 0, "Set the refresh token lifespan e.g. 1s, 1m, 1h.") cmd.Flags().Duration(cli.ConsentRequestLifespan, 0, "Set the login/consent request lifespan e.g. 1s, 1m, 1h") - cmd.Flags().Bool(cli.OnlyRequests, false, "This will only run the cleanup on requests and will skip token and trust relationships cleanup.") - cmd.Flags().Bool(cli.OnlyTokens, false, "This will only run the cleanup on tokens and will skip requests and trust relationships cleanup.") - cmd.Flags().Bool(cli.OnlyGrants, false, "This will only run the cleanup on trust relationships and will skip requests and token cleanup.") + cmd.Flags().Bool(cli.OnlyRequests, false, "This will only run the cleanup on requests and will skip token, login sessions and trust relationships cleanup.") + cmd.Flags().Bool(cli.OnlyTokens, false, "This will only run the cleanup on tokens and will skip requests, login sessions and trust relationships cleanup.") + cmd.Flags().Bool(cli.OnlyGrants, false, "This will only run the cleanup on trust relationships and will skip requests, login sessions and token cleanup.") + cmd.Flags().Bool(cli.OnlyLoginSessions, false, "This will only run the cleanup on login sessions and will skip requests, trust relationship and token cleanup.") cmd.Flags().BoolP(cli.ReadFromEnv, "e", false, "If set, reads the database connection string from the environment variable DSN or config file key dsn.") configx.RegisterFlags(cmd.PersistentFlags()) return cmd diff --git a/internal/testhelpers/janitor_session_test_helper.go b/internal/testhelpers/janitor_session_test_helper.go new file mode 100644 index 00000000000..23e74cadbae --- /dev/null +++ b/internal/testhelpers/janitor_session_test_helper.go @@ -0,0 +1,93 @@ +package testhelpers + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/ory/fosite" + + "github.com/stretchr/testify/require" + + "github.com/ory/hydra/client" + "github.com/ory/hydra/consent" + "github.com/ory/hydra/driver" + "github.com/ory/x/sqlxx" +) + +type JanitorSessionTestHelper struct { + uniqueName string + reg driver.Registry +} + +func NewJanitorSessionTestHelper(reg driver.Registry, uniqueName string) *JanitorSessionTestHelper { + return &JanitorSessionTestHelper{reg: reg, uniqueName: uniqueName} +} + +func (h *JanitorSessionTestHelper) CreateEnvironmentForSession(t *testing.T, ctx context.Context, id string) { + clientDTO := h.CreateClient(t, ctx) + h.CreateLoginRequest(t, ctx, clientDTO, id) + h.CreateConsentRequest(t, ctx, id) +} + +func (h *JanitorSessionTestHelper) ValidateSessionExist(t *testing.T, ctx context.Context, id string) { + session, err := h.reg.ConsentManager().GetRememberedLoginSession(ctx, id) + require.NoError(t, err) + require.NotNil(t, session) + require.NotZero(t, session.ID) +} + +func (h *JanitorSessionTestHelper) ValidateSessionNotExist(t *testing.T, ctx context.Context, id string) { + session, err := h.reg.ConsentManager().GetRememberedLoginSession(ctx, id) + require.Error(t, err) + rpcErr := fosite.ErrorToRFC6749Error(err) + require.Equal(t, fosite.ErrNotFound.StatusCode(), rpcErr.StatusCode()) + require.Nil(t, session) +} + +func (h *JanitorSessionTestHelper) CreateClient(t *testing.T, ctx context.Context) *client.Client { + clientReq := &client.Client{ + OutfacingID: fmt.Sprintf("%s_flush-login-consent-1", h.uniqueName), + RedirectURIs: []string{"http://redirect"}, + } + require.NoError(t, h.reg.ClientManager().CreateClient(ctx, clientReq)) + return clientReq +} + +func (h *JanitorSessionTestHelper) CreateLoginSession(t *testing.T, ctx context.Context, sessionID string) { + require.NoError(t, h.reg.ConsentManager().CreateLoginSession(ctx, &consent.LoginSession{ + ID: sessionID, + Subject: "sub", + Remember: true, + })) +} + +func (h *JanitorSessionTestHelper) CreateLoginRequest(t *testing.T, ctx context.Context, clientDTO *client.Client, sessionID string) { + require.NoError(t, h.reg.ConsentManager().CreateLoginRequest(ctx, &consent.LoginRequest{ + ID: fmt.Sprintf("%s_flush-login-1", h.uniqueName), + RequestedScope: []string{"foo", "bar"}, + Subject: fmt.Sprintf("%s_flush-login-1", h.uniqueName), + Client: clientDTO, + RequestURL: "http://redirect", + RequestedAt: time.Now().Round(time.Second), + AuthenticatedAt: sqlxx.NullTime(time.Now().Round(time.Second)), + Verifier: fmt.Sprintf("%s_flush-login-1", h.uniqueName), + SessionID: sqlxx.NullString(sessionID), + })) +} + +func (h *JanitorSessionTestHelper) CreateConsentRequest(t *testing.T, ctx context.Context, sessionID string) { + require.NoError(t, h.reg.ConsentManager().CreateConsentRequest(ctx, &consent.ConsentRequest{ + ID: fmt.Sprintf("%s_flush-consent-1", h.uniqueName), + RequestedScope: []string{"foo", "bar"}, + Subject: fmt.Sprintf("%s_flush-consent-1", h.uniqueName), + OpenIDConnectContext: nil, + ClientID: fmt.Sprintf("%s_flush-login-consent-1", h.uniqueName), + RequestURL: "http://redirect", + LoginChallenge: sqlxx.NullString(fmt.Sprintf("%s_flush-login-1", h.uniqueName)), + RequestedAt: time.Now().Round(time.Second), + Verifier: fmt.Sprintf("%s_flush-consent-1", h.uniqueName), + LoginSessionID: sqlxx.NullString(sessionID), + })) +} diff --git a/persistence/sql/persister_consent.go b/persistence/sql/persister_consent.go index d99d18a78d0..0c025113c2b 100644 --- a/persistence/sql/persister_consent.go +++ b/persistence/sql/persister_consent.go @@ -11,11 +11,6 @@ import ( "time" "github.com/gobuffalo/pop/v6" - - "github.com/ory/x/sqlxx" - - "github.com/ory/x/errorsx" - "github.com/pkg/errors" "github.com/ory/fosite" @@ -23,7 +18,9 @@ import ( "github.com/ory/hydra/consent" "github.com/ory/hydra/flow" "github.com/ory/hydra/x" + "github.com/ory/x/errorsx" "github.com/ory/x/sqlcon" + "github.com/ory/x/sqlxx" ) var _ consent.Manager = &Persister{} @@ -611,6 +608,44 @@ func (p *Persister) VerifyAndInvalidateLogoutRequest(ctx context.Context, verifi }) } +func (p *Persister) FlushInactiveLoginSessions(ctx context.Context, _ time.Time, limit, _ int) error { + return p.transaction(ctx, func(ctx context.Context, c *pop.Connection) error { + // "hydra_oauth2_authentication_request" + var lr consent.LoginRequest + + // "hydra_oauth2_consent_request" + var cr consent.ConsentRequest + + // "hydra_oauth2_authentication_session" + var ls consent.LoginSession + query := fmt.Sprintf(` + DELETE FROM %[1]s WHERE id in + (SELECT id + FROM %[1]s + WHERE NOT EXISTS + ( + SELECT NULL + FROM %[2]s + WHERE %[2]s.login_session_id = %[1]s.id + ) + AND NOT EXISTS + ( + SELECT NULL + FROM %[3]s + WHERE %[3]s.login_session_id = %[1]s.id + ) + LIMIT %[4]d) + `, + (&ls).TableName(), + (&lr).TableName(), + (&cr).TableName(), + limit, + ) + + return p.Connection(ctx).RawQuery(query).Exec() + }) +} + func (p *Persister) FlushInactiveLoginConsentRequests(ctx context.Context, notAfter time.Time, limit int, batchSize int) error { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.FlushInactiveLoginConsentRequests") defer span.End() diff --git a/x/fosite_storer.go b/x/fosite_storer.go index 1afec037710..aefec6e3594 100644 --- a/x/fosite_storer.go +++ b/x/fosite_storer.go @@ -38,6 +38,8 @@ type FositeStorer interface { FlushInactiveRefreshTokens(ctx context.Context, notAfter time.Time, limit int, batchSize int) error + FlushInactiveLoginSessions(ctx context.Context, notAfter time.Time, limit, batchSize int) error + // DeleteOpenIDConnectSession deletes an OpenID Connect session. // This is duplicated from Ory Fosite to help against deprecation linting errors. DeleteOpenIDConnectSession(ctx context.Context, authorizeCode string) error