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

fix: add cleanup statement for anonymous users #1497

Merged
merged 4 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 1 addition & 7 deletions internal/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,7 @@ func NewAPIWithVersion(ctx context.Context, globalConfig *conf.GlobalConfigurati
r.Use(recoverer)

if globalConfig.DB.CleanupEnabled {
cleanup := &models.Cleanup{
SessionTimebox: globalConfig.Sessions.Timebox,
SessionInactivityTimeout: globalConfig.Sessions.InactivityTimeout,
}

cleanup.Setup()

cleanup := models.NewCleanup(globalConfig)
r.UseBypass(api.databaseCleanup(cleanup))
}

Expand Down
27 changes: 18 additions & 9 deletions internal/models/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,19 @@ package models
import (
"fmt"
"sync/atomic"
"time"

"github.com/sirupsen/logrus"
"go.opentelemetry.io/otel/attribute"
metricglobal "go.opentelemetry.io/otel/metric/global"
metricinstrument "go.opentelemetry.io/otel/metric/instrument"
otelasyncint64instrument "go.opentelemetry.io/otel/metric/instrument/asyncint64"

"github.com/supabase/auth/internal/conf"
"github.com/supabase/auth/internal/observability"
"github.com/supabase/auth/internal/storage"
)

type Cleanup struct {
SessionTimebox *time.Duration
SessionInactivityTimeout *time.Duration

cleanupStatements []string

// cleanupNext holds an atomically incrementing value that determines which of
Expand All @@ -30,14 +27,17 @@ type Cleanup struct {
cleanupAffectedRows otelasyncint64instrument.Counter
}

func (c *Cleanup) Setup() {
func NewCleanup(config *conf.GlobalConfiguration) *Cleanup {
tableUsers := User{}.TableName()
tableRefreshTokens := RefreshToken{}.TableName()
tableSessions := Session{}.TableName()
tableRelayStates := SAMLRelayState{}.TableName()
tableFlowStates := FlowState{}.TableName()
tableMFAChallenges := Challenge{}.TableName()
tableMFAFactors := Factor{}.TableName()

c := &Cleanup{}

// These statements intentionally use SELECT ... FOR UPDATE SKIP LOCKED
// as this makes sure that only rows that are not being used in another
// transaction are deleted. These deletes are thus very quick and
Expand All @@ -55,14 +55,21 @@ func (c *Cleanup) Setup() {
fmt.Sprintf("delete from %q where id in (select id from %q where created_at < now() - interval '24 hours' and status = 'unverified' limit 100 for update skip locked);", tableMFAFactors, tableMFAFactors),
)

if c.SessionTimebox != nil {
timeboxSeconds := int((*c.SessionTimebox).Seconds())
if config.External.AnonymousUsers.Enabled {
// delete anonymous users older than 30 days
c.cleanupStatements = append(c.cleanupStatements,
fmt.Sprintf("delete from %q where id in (select id from %q where created_at < now() - interval '30 days' and is_anonymous is true limit 100 for update skip locked);", tableUsers, tableUsers),
)
}

if config.Sessions.Timebox != nil {
timeboxSeconds := int((*config.Sessions.Timebox).Seconds())

c.cleanupStatements = append(c.cleanupStatements, fmt.Sprintf("delete from %q where id in (select id from %q where created_at + interval '%d seconds' < now() - interval '24 hours' limit 100 for update skip locked);", tableSessions, tableSessions, timeboxSeconds))
}

if c.SessionInactivityTimeout != nil {
inactivitySeconds := int((*c.SessionInactivityTimeout).Seconds())
if config.Sessions.InactivityTimeout != nil {
inactivitySeconds := int((*config.Sessions.InactivityTimeout).Seconds())

// delete sessions with a refreshed_at column
c.cleanupStatements = append(c.cleanupStatements, fmt.Sprintf("delete from %q where id in (select id from %q where refreshed_at is not null and refreshed_at + interval '%d seconds' < now() - interval '24 hours' limit 100 for update skip locked);", tableSessions, tableSessions, inactivitySeconds))
Expand All @@ -81,6 +88,8 @@ func (c *Cleanup) Setup() {
}

c.cleanupAffectedRows = cleanupAffectedRows

return c
}

// Cleanup removes stale entities in the database. You can call it on each
Expand Down
14 changes: 6 additions & 8 deletions internal/models/cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ func TestCleanup(t *testing.T) {
conn, err := test.SetupDBConnection(globalConfig)
require.NoError(t, err)

sessionTimebox := 10 * time.Second
sessionInactivityTimeout := 5 * time.Second
timebox := 10 * time.Second
inactivityTimeout := 5 * time.Second
globalConfig.Sessions.Timebox = &timebox
globalConfig.Sessions.InactivityTimeout = &inactivityTimeout
globalConfig.External.AnonymousUsers.Enabled = true

cleanup := &Cleanup{
SessionTimebox: &sessionTimebox,
SessionInactivityTimeout: &sessionInactivityTimeout,
}

cleanup.Setup()
cleanup := NewCleanup(globalConfig)

for i := 0; i < 100; i += 1 {
_, err := cleanup.Clean(conn)
Expand Down
Loading