Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 52 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
7f64e1a
feat: flush inactive/expired login and consent requests causing datab…
Benehiko Mar 4, 2021
f547918
fix: minor semantic changes
Benehiko Mar 5, 2021
6cb6fde
Merge branch 'master' into db-growth
Benehiko Mar 8, 2021
8db0764
refactor: change flush api to janitor cli command
Benehiko Mar 9, 2021
1cf7de9
fix: flush login/consent request query
Benehiko Mar 9, 2021
dc3e6e4
Merge branch 'master' into db-growth
Benehiko Mar 9, 2021
a02a8b1
fix: janitor cli
Benehiko Mar 9, 2021
0ec2032
fix: janitor cli
Benehiko Mar 9, 2021
0148442
fix: consent request db purge
Benehiko Mar 9, 2021
f8f33b0
perf: improve login/consent request flush queries
Benehiko Mar 10, 2021
2eb15d7
fix: janitor query for mysql
Benehiko Mar 10, 2021
5efe980
test: add login/consent timeout flush
Benehiko Mar 11, 2021
d71e17e
test: add login/consent reject flush
Benehiko Mar 11, 2021
e8ef7b6
Merge branch 'master' into db-growth
Benehiko Mar 11, 2021
e40751b
test: fix login/consent reject flush
Benehiko Mar 11, 2021
e19ada5
test: fix janitor purge test
Benehiko Mar 11, 2021
ee7b2b2
fix: code format
Benehiko Mar 11, 2021
b76c3d4
fix: janitor cli flags
Benehiko Mar 11, 2021
17b5d17
test: add login/consent timeout & reject dockertest
Benehiko Mar 12, 2021
0f6d12e
fix: add query in transaction & minor cli improvements
Benehiko Mar 12, 2021
fc19692
test: add janitor notAfter dockertest
Benehiko Mar 15, 2021
a70a7c4
Merge branch 'master' into db-growth
Benehiko Mar 15, 2021
25a12c3
refactor: janitor cli commands
Benehiko Mar 15, 2021
ee490d2
test: refactor janitor cli & add persister test
Benehiko Mar 16, 2021
3c2078c
refactor: janitor cli
Benehiko Mar 17, 2021
b38277b
refactor: janitor tests
Benehiko Mar 17, 2021
52d8981
fix: janitor cli random test failure
Benehiko Mar 18, 2021
626b1b8
refactor: flush handler tests
Benehiko Mar 18, 2021
5336c92
Merge branch 'master' into db-growth
Benehiko Mar 19, 2021
e91a763
refactor: minor changes to janitor
Benehiko Mar 19, 2021
bbf4b71
refactor: registry init sqlite check
Benehiko Mar 22, 2021
361b1d9
Merge branch 'master' into db-growth
Benehiko Mar 22, 2021
393d22a
refactor: registry base health routes
Benehiko Mar 22, 2021
769c615
refactor: registry base health routes
Benehiko Mar 22, 2021
e59b73c
Merge remote-tracking branch 'origin/db-growth' into db-growth
Benehiko Mar 22, 2021
a925f3d
Merge remote-tracking branch 'origin/db-growth' into db-growth
Benehiko Mar 22, 2021
88fdf9e
Merge remote-tracking branch 'origin/db-growth' into db-growth
Benehiko Mar 22, 2021
2857c56
refactor: janitor cli
Benehiko Mar 23, 2021
d97043f
fix: merge conflict
Benehiko Mar 23, 2021
a11677f
refactor: janitor cli
Benehiko Mar 24, 2021
01ad3f3
refactor: adopt cli factory pattern
aeneasr Mar 24, 2021
837ef59
refactor: janitor cli
Benehiko Mar 24, 2021
1e62537
refactor: flush handler
Benehiko Mar 24, 2021
4b93e65
refactor: janitor test
Benehiko Mar 24, 2021
34e3bd8
fix: merge conflict
Benehiko Mar 24, 2021
7553d32
fix: merge conflict
Benehiko Mar 24, 2021
a1294ef
refactor: flush login-consent query
Benehiko Mar 24, 2021
86fd229
refactor: cli command structure
Benehiko Mar 24, 2021
84e9930
fix: merge conflict
Benehiko Mar 24, 2021
bd34b24
refactor: flush login-consent query
Benehiko Mar 24, 2021
f108517
Merge branch 'master' into db-growth
Benehiko Mar 24, 2021
e12d41c
Merge branch 'master' into db-growth
aeneasr Mar 24, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/oidc-conformity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
fetch-depth: 2
- uses: actions/setup-go@v2
with:
go-version: '^1.16.1
go-version: '^1.16.1'
- name: Start service
run: ./test/conformance/start.sh
- name: Run tests
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ test-resetdb: node_modules
docker rm -f hydra_test_database_cockroach || true
docker run --rm --name hydra_test_database_mysql -p 3444:3306 -e MYSQL_ROOT_PASSWORD=secret -d mysql:5.7
docker run --rm --name hydra_test_database_postgres -p 3445:5432 -e POSTGRES_PASSWORD=secret -e POSTGRES_DB=postgres -d postgres:9.6
docker run --rm --name hydra_test_database_cockroach -p 3446:26257 -d cockroachdb/cockroach:v2.1.6 start --insecure
docker run --rm --name hydra_test_database_cockroach -p 3446:26257 -d cockroachdb/cockroach:v20.2.5 start-single-node --insecure

# Runs tests in short mode, without database adapters
.PHONY: docker
Expand Down
2 changes: 2 additions & 0 deletions cmd/cli/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type Handler struct {
Introspection *IntrospectionHandler
Token *TokenHandler
Migration *MigrateHandler
Janitor *JanitorHandler
}

func Remote(cmd *cobra.Command) string {
Expand All @@ -63,5 +64,6 @@ func NewHandler() *Handler {
Introspection: newIntrospectionHandler(),
Token: newTokenHandler(),
Migration: newMigrateHandler(),
Janitor: newJanitorHandler(),
}
}
114 changes: 114 additions & 0 deletions cmd/cli/handler_janitor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package cli

import (
"context"
"fmt"
"time"

"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"
)

type JanitorHandler struct{}

func newJanitorHandler() *JanitorHandler {
return &JanitorHandler{}
}

func (j *JanitorHandler) Purge(cmd *cobra.Command, args []string) error {
var d driver.Registry

co := []configx.OptionModifier{
configx.WithFlags(cmd.Flags()),
configx.SkipValidation(),
}

keys := map[string]string{
"access-lifespan": config.KeyAccessTokenLifespan,
Benehiko marked this conversation as resolved.
Show resolved Hide resolved
"refresh-lifespan": config.KeyRefreshTokenLifespan,
"consent-request-lifespan": config.KeyConsentRequestMaxAge,
}

for k, v := range keys {
if x := flagx.MustGetString(cmd, k); x != "" {
if xp, err := time.ParseDuration(x); err == nil {
co = append(co, configx.WithValue(v, xp))
}
}
}

notAfter := time.Now()

if keepYounger := flagx.MustGetString(cmd, "keep-if-younger"); keepYounger != "" {
Benehiko marked this conversation as resolved.
Show resolved Hide resolved
if keepYoungerDuration, err := time.ParseDuration(keepYounger); err == nil {
Benehiko marked this conversation as resolved.
Show resolved Hide resolved
notAfter = notAfter.Add(-keepYoungerDuration)
}
}

if !flagx.MustGetBool(cmd, "read-from-env") && len(flagx.MustGetStringSlice(cmd, "config")) == 0 {
Benehiko marked this conversation as resolved.
Show resolved Hide resolved
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 routines []cleanupRoutine

if flagx.MustGetBool(cmd, "tokens") {
routines = append(routines, cleanup(p.FlushInactiveAccessTokens, "access tokens"))
routines = append(routines, cleanup(p.FlushInactiveRefreshTokens, "refresh tokens"))
}

if flagx.MustGetBool(cmd, "requests") {
routines = append(routines, cleanup(p.FlushInactiveLoginConsentRequests, "login-consent requests"))
}

return cleanupRun(cmd.Context(), notAfter, 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 {
for _, r := range routines {
Benehiko marked this conversation as resolved.
Show resolved Hide resolved
if err := r(ctx, notAfter); err != nil {
return err
}
}
return nil
}
247 changes: 247 additions & 0 deletions cmd/cli/handler_janitor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
package cli

import (
"context"
"fmt"
"testing"
"time"

"github.com/spf13/cobra"

"github.com/ory/hydra/internal/testhelpers"
"github.com/ory/x/cmdx"
"github.com/ory/x/configx"
"github.com/ory/x/flagx"

"github.com/stretchr/testify/require"
)

var (
keepIfYounger = "keep-if-younger"
accessLifespan = "access-lifespan"
refreshLifespan = "refresh-lifespan"
consentRequestLifespan = "consent-request-lifespan"
onlyTokens = "tokens"
onlyRequests = "requests"
)

func newJanitorCmd() *cobra.Command {
janitor := newJanitorHandler()
JanitorCmd := &cobra.Command{
Use: "janitor",
RunE: janitor.Purge,
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 &&
!flagx.MustGetBool(cmd, "read-from-env") &&
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)) || (flagx.MustGetBool(cmd, onlyTokens) && flagx.MustGetBool(cmd, onlyRequests)) {
Benehiko marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("%s\n%s\n", cmd.UsageString(),
"Janitor requires either --tokens or --requests to be set")
}

return nil
},
}
JanitorCmd.Flags().String(keepIfYounger, "", "Keep database records that are younger than a specified duration e.g. 1s, 1m, 1h.")
JanitorCmd.Flags().String(accessLifespan, "", "Set the access token lifespan e.g. 1s, 1m, 1h.")
JanitorCmd.Flags().String(refreshLifespan, "", "Set the refresh token lifespan e.g. 1s, 1m, 1h.")
JanitorCmd.Flags().String(consentRequestLifespan, "", "Set the login/consent request lifespan e.g. 1s, 1m, 1h")
JanitorCmd.Flags().Bool(onlyRequests, false, "This will only run the cleanup on requests and will skip token cleanup.")
JanitorCmd.Flags().Bool(onlyTokens, false, "This will only run the cleanup on tokens and will skip requests cleanup.")

JanitorCmd.Flags().BoolP("read-from-env", "e", false, "If set, reads the database connection string from the environment variable DSN or config file key dsn.")
configx.RegisterFlags(JanitorCmd.PersistentFlags())
return JanitorCmd
}

func TestJanitorHandler_PurgeTokenNotAfter(t *testing.T) {
ctx := context.Background()
testCycles := testhelpers.NewConsentJanitorTestHelper("").GetNotAfterTestCycles()

Benehiko marked this conversation as resolved.
Show resolved Hide resolved
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(),
fmt.Sprintf("--%s=%s", keepIfYounger, v.String()),
fmt.Sprintf("--%s=%s", accessLifespan, jt.GetAccessTokenLifespan().String()),
fmt.Sprintf("--%s=%s", refreshLifespan, jt.GetRefreshTokenLifespan().String()),
fmt.Sprintf("--%s", 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(),
fmt.Sprintf("--%s=%s", keepIfYounger, v.String()),
fmt.Sprintf("--%s=%s", consentRequestLifespan, jt.GetConsentRequestLifespan().String()),
fmt.Sprintf("--%s", 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(),
fmt.Sprintf("--%s", 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(),
fmt.Sprintf("--%s", 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(),
fmt.Sprintf("--%s", 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(),
fmt.Sprintf("--%s", onlyRequests),
jt.GetDSN(),
)
})

// validate
t.Run("step=validate", jt.ConsentRejectionValidate(ctx, reg.ConsentManager()))
})

})

}

/*
// TODO: this throws a panic like error instead of a pass on an expected error
func TestJanitorHandler_Arguments(t *testing.T) {
Benehiko marked this conversation as resolved.
Show resolved Hide resolved
cmdx.ExecNoErr(t, newJanitorCmd(),
fmt.Sprintf("--%s", onlyRequests),
"memory",
)
cmdx.ExecNoErr(t, newJanitorCmd(),
fmt.Sprintf("--%s", onlyTokens),
"memory",
)
cmdx.ExecExpectedErr(t, newJanitorCmd(),
fmt.Sprintf("--%s", onlyRequests),
fmt.Sprintf("--%s", onlyTokens),
"memory",
)
cmdx.ExecExpectedErr(t, newJanitorCmd(),
"memory",
)
}*/
Loading