diff --git a/backend/internal/auditlogs/cleanup_job.go b/backend/internal/auditlogs/cleanup_job.go new file mode 100644 index 00000000..5ca8321d --- /dev/null +++ b/backend/internal/auditlogs/cleanup_job.go @@ -0,0 +1,65 @@ +package auditlogs + +import ( + "context" + "fmt" + "log/slog" + "time" + + "github.com/italypaleale/francis/builtin/cronjob" + "gorm.io/gorm" + + "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" +) + +const ( + // cleanupJobInterval is how often the audit log cleanup job runs + cleanupJobInterval = 24 * time.Hour + // cleanupJobJitter spreads each occurrence around its scheduled time, so the cleanup jobs don't all hit the database at once + cleanupJobJitter = 5 * time.Minute +) + +type cleanupJob struct { + db *gorm.DB + retentionDays int +} + +// newCleanupJob returns the cron job actor that deletes audit logs past the retention window +func newCleanupJob(db *gorm.DB, retentionDays int) (*cronjob.CronJob, error) { + job := &cleanupJob{ + db: db, + retentionDays: retentionDays, + } + + cronActor, err := cronjob.New( + "ClearAuditLogs", + cronjob.WithJob(job.clearAuditLogs), + cronjob.WithInterval(cleanupJobInterval), + cronjob.WithJitter(cleanupJobJitter), + // Also run right after the job is first registered, so rows that aged out while Pocket ID wasn't running are removed at startup + cronjob.WithImmediate(), + cronjob.WithLogger(slog.Default()), + ) + if err != nil { + return nil, fmt.Errorf("error creating audit log cleanup cron job: %w", err) + } + + return cronActor, nil +} + +// clearAuditLogs deletes audit logs older than the configured retention window +func (j *cleanupJob) clearAuditLogs(ctx context.Context) error { + cutoff := time.Now().AddDate(0, 0, -j.retentionDays) + + st := j.db. + WithContext(ctx). + Delete(&model.AuditLog{}, "created_at < ?", datatype.DateTime(cutoff)) + if st.Error != nil { + return fmt.Errorf("failed to delete old audit logs: %w", st.Error) + } + + slog.InfoContext(ctx, "Deleted old audit logs", slog.Int64("count", st.RowsAffected)) + + return nil +} diff --git a/backend/internal/auditlogs/cleanup_job_test.go b/backend/internal/auditlogs/cleanup_job_test.go new file mode 100644 index 00000000..fc236098 --- /dev/null +++ b/backend/internal/auditlogs/cleanup_job_test.go @@ -0,0 +1,80 @@ +package auditlogs + +import ( + "testing" + "time" + + "github.com/italypaleale/francis/host/local" + "github.com/stretchr/testify/require" + + "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" + testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" +) + +func TestModuleRegistersAuditLogCleanupCronJob(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + testutils.NewActorHostForTest(t, func(t *testing.T, host *local.Host) { + t.Helper() + _, err := New(Dependencies{ + DB: db, + Actors: host, + RetentionDays: 90, + }) + require.NoError(t, err) + }) +} + +func TestModuleRequiresActorHostForCleanupJob(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + _, err := New(Dependencies{DB: db, RetentionDays: 90}) + require.ErrorContains(t, err, "actor host is required") + + // With the cleanup disabled there is nothing to register, so the actor host is not needed + _, err = New(Dependencies{DB: db, RetentionDays: 90, CleanupDisabled: true}) + require.NoError(t, err) +} + +func TestAuditLogCleanupJobDeletesLogsPastRetention(t *testing.T) { + const retentionDays = 90 + + db := testutils.NewDatabaseForTest(t) + user := model.User{ + Base: model.Base{ID: "cleanup-job-user"}, + Username: "cleanup-job-user", + FirstName: "Cleanup", + LastName: "Job", + DisplayName: "Cleanup Job", + } + err := db.Create(&user).Error + require.NoError(t, err) + + err = db.Create(&model.AuditLog{Base: model.Base{ID: "log-old"}, Event: model.AuditLogEventSignIn, UserID: user.ID}).Error + require.NoError(t, err) + err = db.Create(&model.AuditLog{Base: model.Base{ID: "log-recent"}, Event: model.AuditLogEventSignIn, UserID: user.ID}).Error + require.NoError(t, err) + + // BeforeCreate stamps CreatedAt, so the log past the retention window is backdated directly + oldCreatedAt := datatype.DateTime(time.Now().AddDate(0, 0, -retentionDays-1)) + err = db.Model(&model.AuditLog{}).Where("id = ?", "log-old").Update("created_at", oldCreatedAt).Error + require.NoError(t, err) + + job := &cleanupJob{db: db, retentionDays: retentionDays} + err = job.clearAuditLogs(t.Context()) + require.NoError(t, err) + + var remaining []string + err = db.Model(&model.AuditLog{}).Pluck("id", &remaining).Error + require.NoError(t, err) + require.Equal(t, []string{"log-recent"}, remaining) +} + +func TestNewCleanupJobCreatesCronActor(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + cronActor, err := newCleanupJob(db, 90) + require.NoError(t, err) + require.Equal(t, "cronjob.ClearAuditLogs", cronActor.ActorType()) +} diff --git a/backend/internal/auditlogs/module.go b/backend/internal/auditlogs/module.go new file mode 100644 index 00000000..95f1a514 --- /dev/null +++ b/backend/internal/auditlogs/module.go @@ -0,0 +1,44 @@ +// Package auditlogs owns the background maintenance of the audit log table. +package auditlogs + +import ( + "errors" + "fmt" + + "github.com/italypaleale/francis/host/local" + "gorm.io/gorm" +) + +type Dependencies struct { + DB *gorm.DB + Actors *local.Host + + // RetentionDays is how long audit logs are kept before the cleanup job deletes them + RetentionDays int + + // CleanupDisabled skips registering the cleanup cron job, for example in tests + CleanupDisabled bool +} + +type Module struct{} + +func New(deps Dependencies) (*Module, error) { + // Register the cleanup job for audit logs past the retention window + if !deps.CleanupDisabled { + if deps.Actors == nil { + return nil, errors.New("actor host is required for the audit log cleanup cron job") + } + + cleanupJob, err := newCleanupJob(deps.DB, deps.RetentionDays) + if err != nil { + return nil, err + } + + err = deps.Actors.RegisterBuiltInActor(cleanupJob) + if err != nil { + return nil, fmt.Errorf("error registering audit log cleanup cron actor: %w", err) + } + } + + return &Module{}, nil +} diff --git a/backend/internal/bootstrap/bootstrap.go b/backend/internal/bootstrap/bootstrap.go index 56292873..c3635f13 100644 --- a/backend/internal/bootstrap/bootstrap.go +++ b/backend/internal/bootstrap/bootstrap.go @@ -112,7 +112,7 @@ func Bootstrap(ctx context.Context) error { // Register scheduled jobs, only in non-test mode if common.EnvConfig.AppEnv != "test" { - err = registerScheduledJobs(ctx, db, svc, scheduler) + err = registerScheduledJobs(ctx, svc, scheduler) if err != nil { return fmt.Errorf("failed to register scheduled jobs: %w", err) } diff --git a/backend/internal/bootstrap/scheduler_bootstrap.go b/backend/internal/bootstrap/scheduler_bootstrap.go index aa44218f..2abf2136 100644 --- a/backend/internal/bootstrap/scheduler_bootstrap.go +++ b/backend/internal/bootstrap/scheduler_bootstrap.go @@ -4,17 +4,11 @@ import ( "context" "fmt" - "gorm.io/gorm" - "github.com/pocket-id/pocket-id/backend/internal/job" ) -func registerScheduledJobs(ctx context.Context, db *gorm.DB, svc *services, scheduler *job.Scheduler) error { - err := scheduler.RegisterDbCleanupJobs(ctx, db) - if err != nil { - return fmt.Errorf("failed to register DB cleanup jobs in scheduler: %w", err) - } - err = scheduler.RegisterScimJobs(ctx, svc.scimService) +func registerScheduledJobs(ctx context.Context, svc *services, scheduler *job.Scheduler) error { + err := scheduler.RegisterScimJobs(ctx, svc.scimService) if err != nil { return fmt.Errorf("failed to register SCIM scheduler job: %w", err) } diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index d1570970..7c619c2d 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -9,6 +9,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/api" "github.com/pocket-id/pocket-id/backend/internal/apikey" "github.com/pocket-id/pocket-id/backend/internal/appconfig" + "github.com/pocket-id/pocket-id/backend/internal/auditlogs" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/devicelogin" "github.com/pocket-id/pocket-id/backend/internal/email" @@ -41,6 +42,7 @@ type services struct { fileStorage storage.FileStorage apiKeyModule *apikey.Module + auditLogsModule *auditlogs.Module deviceLoginModule *devicelogin.Module ldapSyncModule *ldapsync.Module oidcModule *oidc.Module @@ -92,6 +94,17 @@ func initServices( } svc.auditLogService = service.NewAuditLogService(db, svc.emailModule, svc.geoLiteModule, svc.appConfigService) + svc.auditLogsModule, err = auditlogs.New(auditlogs.Dependencies{ + DB: db, + Actors: actors, + RetentionDays: common.EnvConfig.AuditLogRetentionDays, + // Disable in test environment + CleanupDisabled: common.EnvConfig.AppEnv.IsTest(), + }) + if err != nil { + return nil, fmt.Errorf("failed to create audit logs module: %w", err) + } + svc.jwtService, err = service.NewJwtService(ctx, db, instanceID) if err != nil { return nil, fmt.Errorf("failed to create JWT service: %w", err) @@ -100,10 +113,13 @@ func initServices( svc.customClaimService = service.NewCustomClaimService(db) svc.webauthnModule, err = webauthn.New(webauthn.Dependencies{ DB: db, + Actors: actors, AppURL: common.EnvConfig.AppURL, Signer: svc.jwtService, AuditLog: svc.auditLogService, AppConfig: svc.appConfigService, + // Disable in test environment + CleanupDisabled: common.EnvConfig.AppEnv.IsTest(), }) if err != nil { return nil, fmt.Errorf("failed to create WebAuthn module: %w", err) @@ -128,6 +144,7 @@ func initServices( svc.oidcModule, err = oidc.New(ctx, oidc.Dependencies{ DB: db, + Actors: actors, HTTPClient: httpClient, GetCIMDURLAllowlist: svc.appConfigService.GetCIMDURLAllowlist, Config: oidc.Config{ @@ -141,6 +158,8 @@ func initServices( Reauth: svc.webauthnModule, AuditLog: svc.auditLogService, APIAccess: svc.apiModule, + // Disable in test environment + CleanupDisabled: common.EnvConfig.AppEnv.IsTest(), }) if err != nil { return nil, fmt.Errorf("failed to create OIDC module: %w", err) diff --git a/backend/internal/job/db_cleanup_job.go b/backend/internal/job/db_cleanup_job.go deleted file mode 100644 index a7e2f1e0..00000000 --- a/backend/internal/job/db_cleanup_job.go +++ /dev/null @@ -1,123 +0,0 @@ -package job - -import ( - "context" - "errors" - "fmt" - "log/slog" - "time" - - backoff "github.com/cenkalti/backoff/v5" - "gorm.io/gorm" - - "github.com/pocket-id/pocket-id/backend/internal/common" - "github.com/pocket-id/pocket-id/backend/internal/model" - datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" - "github.com/pocket-id/pocket-id/backend/internal/oidc" - "github.com/pocket-id/pocket-id/backend/internal/service" - "github.com/pocket-id/pocket-id/backend/internal/webauthn" -) - -func (s *Scheduler) RegisterDbCleanupJobs(ctx context.Context, db *gorm.DB) error { - jobs := &DbCleanupJobs{db: db} - - newBackOff := func() *backoff.ExponentialBackOff { - bo := backoff.NewExponentialBackOff() - bo.Multiplier = 4 - bo.RandomizationFactor = 0.1 - bo.InitialInterval = time.Second - bo.MaxInterval = 45 * time.Second - return bo - } - - // Use exponential backoff for each DB cleanup job so transient query failures are retried automatically rather than causing an immediate job failure - return errors.Join( - s.RegisterJob(ctx, "ClearWebauthnSessions", jobDefWithJitter(24*time.Hour), jobs.clearWebauthnSessions, service.RegisterJobOpts{RunImmediately: true, BackOff: newBackOff()}), - s.RegisterJob(ctx, "ClearOAuth2Sessions", jobDefWithJitter(24*time.Hour), jobs.clearOAuth2Sessions, service.RegisterJobOpts{RunImmediately: true, BackOff: newBackOff()}), - s.RegisterJob(ctx, "ClearOAuth2JTIs", jobDefWithJitter(24*time.Hour), jobs.clearOAuth2JTIs, service.RegisterJobOpts{RunImmediately: true, BackOff: newBackOff()}), - s.RegisterJob(ctx, "ClearInteractionSessions", jobDefWithJitter(24*time.Hour), jobs.clearInteractionSessions, service.RegisterJobOpts{RunImmediately: true, BackOff: newBackOff()}), - s.RegisterJob(ctx, "ClearReauthenticationTokens", jobDefWithJitter(24*time.Hour), jobs.clearReauthenticationTokens, service.RegisterJobOpts{RunImmediately: true, BackOff: newBackOff()}), - s.RegisterJob(ctx, "ClearAuditLogs", jobDefWithJitter(24*time.Hour), jobs.clearAuditLogs, service.RegisterJobOpts{RunImmediately: true, BackOff: newBackOff()}), - ) -} - -type DbCleanupJobs struct { - db *gorm.DB -} - -// clearWebauthnSessions deletes expired WebAuthn challenge sessions. -func (j *DbCleanupJobs) clearWebauthnSessions(ctx context.Context) error { - count, err := webauthn.CleanupExpiredSessions(ctx, j.db) - if err != nil { - return fmt.Errorf("failed to clean expired WebAuthn sessions: %w", err) - } - - slog.InfoContext(ctx, "Cleaned expired WebAuthn sessions", slog.Int64("count", count)) - - return nil -} - -// clearOAuth2Sessions deletes expired and invalidated OAuth2 sessions. -func (j *DbCleanupJobs) clearOAuth2Sessions(ctx context.Context) error { - count, err := oidc.CleanupExpiredOAuth2Sessions(ctx, j.db) - if err != nil { - return fmt.Errorf("failed to clean OAuth2 sessions: %w", err) - } - - slog.InfoContext(ctx, "Cleaned OAuth2 sessions", slog.Int64("count", count)) - - return nil -} - -// clearOAuth2JTIs deletes expired JWT IDs used for client assertion replay protection. -func (j *DbCleanupJobs) clearOAuth2JTIs(ctx context.Context) error { - count, err := oidc.CleanupExpiredClientAssertionJTIs(ctx, j.db) - if err != nil { - return fmt.Errorf("failed to clean OAuth2 client assertion JTIs: %w", err) - } - - slog.InfoContext(ctx, "Cleaned OAuth2 client assertion JTIs", slog.Int64("count", count)) - - return nil -} - -// clearInteractionSessions deletes abandoned OIDC interaction sessions. -func (j *DbCleanupJobs) clearInteractionSessions(ctx context.Context) error { - count, err := oidc.CleanupAbandonedInteractionSessions(ctx, j.db) - if err != nil { - return fmt.Errorf("failed to clean interaction sessions: %w", err) - } - - slog.InfoContext(ctx, "Cleaned interaction sessions", slog.Int64("count", count)) - - return nil -} - -// clearReauthenticationTokens deletes expired reauthentication tokens. -// What counts as expired is owned by the webauthn module. -func (j *DbCleanupJobs) clearReauthenticationTokens(ctx context.Context) error { - count, err := webauthn.CleanupExpiredReauthenticationTokens(ctx, j.db) - if err != nil { - return fmt.Errorf("failed to clean expired reauthentication tokens: %w", err) - } - - slog.InfoContext(ctx, "Cleaned expired reauthentication tokens", slog.Int64("count", count)) - - return nil -} - -// ClearAuditLogs deletes audit logs older than the configured retention window -func (j *DbCleanupJobs) clearAuditLogs(ctx context.Context) error { - cutoff := time.Now().AddDate(0, 0, -common.EnvConfig.AuditLogRetentionDays) - - st := j.db. - WithContext(ctx). - Delete(&model.AuditLog{}, "created_at < ?", datatype.DateTime(cutoff)) - if st.Error != nil { - return fmt.Errorf("failed to delete old audit logs: %w", st.Error) - } - - slog.InfoContext(ctx, "Deleted old audit logs", slog.Int64("count", st.RowsAffected)) - - return nil -} diff --git a/backend/internal/job/scheduler.go b/backend/internal/job/scheduler.go index 5985bb34..ad7f007e 100644 --- a/backend/internal/job/scheduler.go +++ b/backend/internal/job/scheduler.go @@ -163,9 +163,3 @@ func jobWithBackOff(job jobFn, bo backoff.BackOff) jobFn { return err } } - -func jobDefWithJitter(interval time.Duration) gocron.JobDefinition { - const jitter = 5 * time.Minute - - return gocron.DurationRandomJob(interval-jitter, interval+jitter) -} diff --git a/backend/internal/oidc/cleanup.go b/backend/internal/oidc/cleanup.go index 064a6429..c9bf5228 100644 --- a/backend/internal/oidc/cleanup.go +++ b/backend/internal/oidc/cleanup.go @@ -8,28 +8,28 @@ import ( "gorm.io/gorm" ) -// CleanupExpiredOAuth2Sessions deletes OAuth2 sessions whose tokens or codes have +// cleanupExpiredOAuth2Sessions deletes OAuth2 sessions whose tokens or codes have // expired. // // Invalidated-but-unexpired rows are intentionally KEPT until their original expiry: fosite relies on // finding the inactive row to detect a reuse and revoke the affected token family. -func CleanupExpiredOAuth2Sessions(ctx context.Context, db *gorm.DB) (int64, error) { +func cleanupExpiredOAuth2Sessions(ctx context.Context, db *gorm.DB) (int64, error) { st := db. WithContext(ctx). Delete(&OAuth2Session{}, "expires_at < ?", datatype.DateTime(time.Now())) return st.RowsAffected, st.Error } -// CleanupExpiredClientAssertionJTIs deletes expired JWT IDs used for client assertion replay protection. -func CleanupExpiredClientAssertionJTIs(ctx context.Context, db *gorm.DB) (int64, error) { +// cleanupExpiredClientAssertionJTIs deletes expired JWT IDs used for client assertion replay protection. +func cleanupExpiredClientAssertionJTIs(ctx context.Context, db *gorm.DB) (int64, error) { st := db. WithContext(ctx). Delete(&clientAssertionJTI{}, "expires_at < ?", datatype.DateTime(time.Now())) return st.RowsAffected, st.Error } -// CleanupAbandonedInteractionSessions removes interaction sessions that were never completed. -func CleanupAbandonedInteractionSessions(ctx context.Context, db *gorm.DB) (int64, error) { +// cleanupAbandonedInteractionSessions removes interaction sessions that were never completed. +func cleanupAbandonedInteractionSessions(ctx context.Context, db *gorm.DB) (int64, error) { st := db. WithContext(ctx). Delete(&InteractionSession{}, "created_at < ?", datatype.DateTime(time.Now().Add(-interactionSessionLifetime))) diff --git a/backend/internal/oidc/cleanup_job.go b/backend/internal/oidc/cleanup_job.go new file mode 100644 index 00000000..867504c5 --- /dev/null +++ b/backend/internal/oidc/cleanup_job.go @@ -0,0 +1,102 @@ +package oidc + +import ( + "context" + "fmt" + "log/slog" + "time" + + "github.com/italypaleale/francis/builtin/cronjob" + "gorm.io/gorm" +) + +const ( + // cleanupJobInterval is how often each of the OIDC cleanup jobs runs + cleanupJobInterval = 24 * time.Hour + // cleanupJobJitter spreads each occurrence around its scheduled time, so the cleanup jobs don't all hit the database at once + cleanupJobJitter = 5 * time.Minute +) + +type cleanupJobs struct { + db *gorm.DB +} + +// newCleanupJobs returns the cron job actors that delete expired OIDC rows from the database +func newCleanupJobs(db *gorm.DB) ([]*cronjob.CronJob, error) { + jobs := &cleanupJobs{db: db} + + // Create the built-in actor for the ClearOAuth2Sessions job + clearOAuth2Sessions, err := newCleanupJob("ClearOAuth2Sessions", jobs.clearOAuth2Sessions) + if err != nil { + return nil, err + } + + // Create the built-in actor for the ClearOAuth2JTIs job + clearOAuth2JTIs, err := newCleanupJob("ClearOAuth2JTIs", jobs.clearOAuth2JTIs) + if err != nil { + return nil, err + } + + // Create the built-in actor for the ClearInteractionSessions job + clearInteractionSessions, err := newCleanupJob("ClearInteractionSessions", jobs.clearInteractionSessions) + if err != nil { + return nil, err + } + + return []*cronjob.CronJob{clearOAuth2Sessions, clearOAuth2JTIs, clearInteractionSessions}, nil +} + +// newCleanupJob creates a cron job actor that runs one of the OIDC cleanups on a daily schedule +func newCleanupJob(name string, fn func(ctx context.Context) error) (*cronjob.CronJob, error) { + cronActor, err := cronjob.New( + name, + cronjob.WithJob(fn), + cronjob.WithInterval(cleanupJobInterval), + cronjob.WithJitter(cleanupJobJitter), + // Also run right after the job is first registered, so rows that expired while Pocket ID wasn't running are removed at startup + cronjob.WithImmediate(), + cronjob.WithLogger(slog.Default()), + ) + if err != nil { + return nil, fmt.Errorf("error creating %s cron job: %w", name, err) + } + + return cronActor, nil +} + +// clearOAuth2Sessions deletes expired OAuth2 sessions. +// Invalidated sessions are kept until their original expiry: see cleanupExpiredOAuth2Sessions. +func (j *cleanupJobs) clearOAuth2Sessions(ctx context.Context) error { + count, err := cleanupExpiredOAuth2Sessions(ctx, j.db) + if err != nil { + return fmt.Errorf("failed to clean OAuth2 sessions: %w", err) + } + + slog.InfoContext(ctx, "Cleaned OAuth2 sessions", slog.Int64("count", count)) + + return nil +} + +// clearOAuth2JTIs deletes expired JWT IDs used for client assertion replay protection. +func (j *cleanupJobs) clearOAuth2JTIs(ctx context.Context) error { + count, err := cleanupExpiredClientAssertionJTIs(ctx, j.db) + if err != nil { + return fmt.Errorf("failed to clean OAuth2 client assertion JTIs: %w", err) + } + + slog.InfoContext(ctx, "Cleaned OAuth2 client assertion JTIs", slog.Int64("count", count)) + + return nil +} + +// clearInteractionSessions deletes abandoned OIDC interaction sessions. +func (j *cleanupJobs) clearInteractionSessions(ctx context.Context) error { + count, err := cleanupAbandonedInteractionSessions(ctx, j.db) + if err != nil { + return fmt.Errorf("failed to clean interaction sessions: %w", err) + } + + slog.InfoContext(ctx, "Cleaned interaction sessions", slog.Int64("count", count)) + + return nil +} diff --git a/backend/internal/oidc/cleanup_job_test.go b/backend/internal/oidc/cleanup_job_test.go new file mode 100644 index 00000000..4cb97fef --- /dev/null +++ b/backend/internal/oidc/cleanup_job_test.go @@ -0,0 +1,102 @@ +package oidc + +import ( + "testing" + "time" + + "github.com/italypaleale/francis/host/local" + "github.com/stretchr/testify/require" + + "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" + testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" +) + +func TestNewCleanupJobsCreatesOneCronActorPerTable(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + jobs, err := newCleanupJobs(db) + require.NoError(t, err) + + actorTypes := make([]string, len(jobs)) + for i, j := range jobs { + actorTypes[i] = j.ActorType() + } + require.Equal(t, []string{ + "cronjob.ClearOAuth2Sessions", + "cronjob.ClearOAuth2JTIs", + "cronjob.ClearInteractionSessions", + }, actorTypes) + + // Every job must be registrable on an actor host + testutils.NewActorHostForTest(t, func(t *testing.T, host *local.Host) { + t.Helper() + for _, j := range jobs { + err := host.RegisterBuiltInActor(j) + require.NoError(t, err) + } + }) +} + +func TestOIDCCleanupJobsDeleteExpiredRows(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + err := db.Create(&model.OidcClient{Base: model.Base{ID: "cleanup-job-client"}, Name: "Cleanup Job Client"}).Error + require.NoError(t, err) + + var ( + past = datatype.DateTime(time.Now().Add(-time.Hour)) + future = datatype.DateTime(time.Now().Add(time.Hour)) + ) + + err = db.Create(&OAuth2Session{ + Base: model.Base{ID: "session-expired"}, Kind: "access_token", Key: "k-expired", RequestID: "r1", + ClientID: "cleanup-job-client", Active: true, RequestData: `{"client_id":"cleanup-job-client"}`, ExpiresAt: &past, + }).Error + require.NoError(t, err) + err = db.Create(&OAuth2Session{ + Base: model.Base{ID: "session-active"}, Kind: "access_token", Key: "k-active", RequestID: "r2", + ClientID: "cleanup-job-client", Active: true, RequestData: `{"client_id":"cleanup-job-client"}`, ExpiresAt: &future, + }).Error + require.NoError(t, err) + + err = db.Create(&clientAssertionJTI{Base: model.Base{ID: "jti-expired"}, JTI: "expired", ExpiresAt: past}).Error + require.NoError(t, err) + err = db.Create(&clientAssertionJTI{Base: model.Base{ID: "jti-active"}, JTI: "active", ExpiresAt: future}).Error + require.NoError(t, err) + + err = db.Create(&InteractionSession{ + Base: model.Base{ID: "interaction-abandoned"}, Scopes: datatype.StringList{"openid"}, + ClientID: "cleanup-job-client", RequestedAt: datatype.DateTime(time.Now()), Parameters: map[string]string{}, + }).Error + require.NoError(t, err) + err = db.Create(&InteractionSession{ + Base: model.Base{ID: "interaction-pending"}, Scopes: datatype.StringList{"openid"}, + ClientID: "cleanup-job-client", RequestedAt: datatype.DateTime(time.Now()), Parameters: map[string]string{}, + }).Error + require.NoError(t, err) + // BeforeCreate stamps CreatedAt, so the abandoned session is backdated past its lifetime directly + abandonedCreatedAt := datatype.DateTime(time.Now().Add(-interactionSessionLifetime - time.Minute)) + err = db.Model(&InteractionSession{}).Where("id = ?", "interaction-abandoned").Update("created_at", abandonedCreatedAt).Error + require.NoError(t, err) + + jobs := &cleanupJobs{db: db} + err = jobs.clearOAuth2Sessions(t.Context()) + require.NoError(t, err) + err = jobs.clearOAuth2JTIs(t.Context()) + require.NoError(t, err) + err = jobs.clearInteractionSessions(t.Context()) + require.NoError(t, err) + + var remaining []string + err = db.Model(&OAuth2Session{}).Pluck("id", &remaining).Error + require.NoError(t, err) + require.Equal(t, []string{"session-active"}, remaining) + + err = db.Model(&clientAssertionJTI{}).Pluck("id", &remaining).Error + require.NoError(t, err) + require.Equal(t, []string{"jti-active"}, remaining) + + err = db.Model(&InteractionSession{}).Pluck("id", &remaining).Error + require.NoError(t, err) + require.Equal(t, []string{"interaction-pending"}, remaining) +} diff --git a/backend/internal/oidc/cleanup_test.go b/backend/internal/oidc/cleanup_test.go index f71c9a3d..d057f0ce 100644 --- a/backend/internal/oidc/cleanup_test.go +++ b/backend/internal/oidc/cleanup_test.go @@ -17,7 +17,8 @@ import ( // past their expiry are removed. func TestCleanupExpiredOAuth2SessionsKeepsInvalidatedButUnexpiredSessions(t *testing.T) { db := testutils.NewDatabaseForTest(t) - require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "cleanup-client"}, Name: "Cleanup Client"}).Error) + err := db.Create(&model.OidcClient{Base: model.Base{ID: "cleanup-client"}, Name: "Cleanup Client"}).Error + require.NoError(t, err) future := datatype.DateTime(time.Now().Add(time.Hour)) @@ -27,14 +28,16 @@ func TestCleanupExpiredOAuth2SessionsKeepsInvalidatedButUnexpiredSessions(t *tes {Base: model.Base{ID: "active"}, Kind: "refresh_token", Key: "k-active", RequestID: "r3", ClientID: "cleanup-client", Active: true, RequestData: `{"client_id":"cleanup-client"}`, ExpiresAt: &future}, } for i := range rows { - require.NoError(t, db.Create(&rows[i]).Error) + err = db.Create(&rows[i]).Error + require.NoError(t, err) } - deleted, err := CleanupExpiredOAuth2Sessions(t.Context(), db) + deleted, err := cleanupExpiredOAuth2Sessions(t.Context(), db) require.NoError(t, err) require.Equal(t, int64(1), deleted) var remaining []string - require.NoError(t, db.Model(&OAuth2Session{}).Pluck("id", &remaining).Error) + err = db.Model(&OAuth2Session{}).Pluck("id", &remaining).Error + require.NoError(t, err) require.ElementsMatch(t, []string{"active", "rotated"}, remaining) } diff --git a/backend/internal/oidc/module.go b/backend/internal/oidc/module.go index cce7c5ed..9de3ef57 100644 --- a/backend/internal/oidc/module.go +++ b/backend/internal/oidc/module.go @@ -2,11 +2,13 @@ package oidc import ( "context" + "errors" "fmt" "net/http" "time" "github.com/gin-gonic/gin" + "github.com/italypaleale/francis/host/local" "github.com/lestrrat-go/jwx/v3/jwa" "github.com/pocket-id/pocket-id/backend/internal/model" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" @@ -40,6 +42,7 @@ type AuditLogger interface { type Dependencies struct { DB *gorm.DB + Actors *local.Host Config Config HTTPClient *http.Client @@ -50,6 +53,9 @@ type Dependencies struct { Reauth ReauthenticationTokenConsumer AuditLog AuditLogger APIAccess APIAccessProvider + + // CleanupDisabled skips registering the cron jobs that delete expired rows from the database, for example in tests + CleanupDisabled bool } type Module struct { @@ -94,6 +100,25 @@ func New(ctx context.Context, deps Dependencies) (*Module, error) { deviceService := newDeviceService(provider, store, provider.deviceStrategy, authorizationService, claimsService, deps.AuditLog, deps.DB) endSessionService := newEndSessionService(deps.DB, store, deps.Signer, deps.Config.BaseURL) + // Register the cleanup jobs for expired OIDC rows + if !deps.CleanupDisabled { + if deps.Actors == nil { + return nil, errors.New("actor host is required for the OIDC cleanup cron jobs") + } + + jobs, err := newCleanupJobs(deps.DB) + if err != nil { + return nil, err + } + + for _, cj := range jobs { + err = deps.Actors.RegisterBuiltInActor(cj) + if err != nil { + return nil, fmt.Errorf("error registering OIDC cleanup cron actor %q: %w", cj.ActorType(), err) + } + } + } + return &Module{ Preview: previewBuilder, diff --git a/backend/internal/webauthn/cleanup.go b/backend/internal/webauthn/cleanup.go index 64b63c9b..d1ce2689 100644 --- a/backend/internal/webauthn/cleanup.go +++ b/backend/internal/webauthn/cleanup.go @@ -9,18 +9,18 @@ import ( datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" ) -// CleanupExpiredSessions deletes WebAuthn sessions that have expired +// cleanupExpiredSessions deletes WebAuthn sessions that have expired // It returns the number of rows removed -func CleanupExpiredSessions(ctx context.Context, db *gorm.DB) (int64, error) { +func cleanupExpiredSessions(ctx context.Context, db *gorm.DB) (int64, error) { st := db. WithContext(ctx). Delete(&WebauthnSession{}, "expires_at < ?", datatype.DateTime(time.Now())) return st.RowsAffected, st.Error } -// CleanupExpiredReauthenticationTokens deletes reauthentication tokens that have expired +// cleanupExpiredReauthenticationTokens deletes reauthentication tokens that have expired // It returns the number of rows removed -func CleanupExpiredReauthenticationTokens(ctx context.Context, db *gorm.DB) (int64, error) { +func cleanupExpiredReauthenticationTokens(ctx context.Context, db *gorm.DB) (int64, error) { st := db. WithContext(ctx). Delete(&ReauthenticationToken{}, "expires_at < ?", datatype.DateTime(time.Now())) diff --git a/backend/internal/webauthn/cleanup_job.go b/backend/internal/webauthn/cleanup_job.go new file mode 100644 index 00000000..b16b4e07 --- /dev/null +++ b/backend/internal/webauthn/cleanup_job.go @@ -0,0 +1,83 @@ +package webauthn + +import ( + "context" + "fmt" + "log/slog" + "time" + + "github.com/italypaleale/francis/builtin/cronjob" + "gorm.io/gorm" +) + +const ( + // cleanupJobInterval is how often each of the WebAuthn cleanup jobs runs + cleanupJobInterval = 24 * time.Hour + // cleanupJobJitter spreads each occurrence around its scheduled time, so the cleanup jobs don't all hit the database at once + cleanupJobJitter = 5 * time.Minute +) + +type cleanupJobs struct { + db *gorm.DB +} + +// newCleanupJobs returns the cron job actors that delete expired WebAuthn rows from the database +func newCleanupJobs(db *gorm.DB) ([]*cronjob.CronJob, error) { + jobs := &cleanupJobs{db: db} + + // Create the built-in actor for the ClearWebauthnSessions job + clearWebauthnSessions, err := newCleanupJob("ClearWebauthnSessions", jobs.clearWebauthnSessions) + if err != nil { + return nil, err + } + + // Create the built-in actor for the ClearReauthenticationTokens job + clearReauthenticationTokens, err := newCleanupJob("ClearReauthenticationTokens", jobs.clearReauthenticationTokens) + if err != nil { + return nil, err + } + + return []*cronjob.CronJob{clearWebauthnSessions, clearReauthenticationTokens}, nil +} + +// newCleanupJob creates a cron job actor that runs one of the WebAuthn cleanups on a daily schedule +func newCleanupJob(name string, fn func(ctx context.Context) error) (*cronjob.CronJob, error) { + cronActor, err := cronjob.New( + name, + cronjob.WithJob(fn), + cronjob.WithInterval(cleanupJobInterval), + cronjob.WithJitter(cleanupJobJitter), + // Also run right after the job is first registered, so rows that expired while Pocket ID wasn't running are removed at startup + cronjob.WithImmediate(), + cronjob.WithLogger(slog.Default()), + ) + if err != nil { + return nil, fmt.Errorf("error creating %s cron job: %w", name, err) + } + + return cronActor, nil +} + +// clearWebauthnSessions deletes expired WebAuthn challenge sessions. +func (j *cleanupJobs) clearWebauthnSessions(ctx context.Context) error { + count, err := cleanupExpiredSessions(ctx, j.db) + if err != nil { + return fmt.Errorf("failed to clean expired WebAuthn sessions: %w", err) + } + + slog.InfoContext(ctx, "Cleaned expired WebAuthn sessions", slog.Int64("count", count)) + + return nil +} + +// clearReauthenticationTokens deletes expired reauthentication tokens. +func (j *cleanupJobs) clearReauthenticationTokens(ctx context.Context) error { + count, err := cleanupExpiredReauthenticationTokens(ctx, j.db) + if err != nil { + return fmt.Errorf("failed to clean expired reauthentication tokens: %w", err) + } + + slog.InfoContext(ctx, "Cleaned expired reauthentication tokens", slog.Int64("count", count)) + + return nil +} diff --git a/backend/internal/webauthn/cleanup_job_test.go b/backend/internal/webauthn/cleanup_job_test.go new file mode 100644 index 00000000..e73e30a3 --- /dev/null +++ b/backend/internal/webauthn/cleanup_job_test.go @@ -0,0 +1,81 @@ +package webauthn + +import ( + "testing" + "time" + + "github.com/italypaleale/francis/host/local" + "github.com/stretchr/testify/require" + + "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" + testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" +) + +func TestNewCleanupJobsCreatesOneCronActorPerTable(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + jobs, err := newCleanupJobs(db) + require.NoError(t, err) + + actorTypes := make([]string, len(jobs)) + for i, j := range jobs { + actorTypes[i] = j.ActorType() + } + require.Equal(t, []string{ + "cronjob.ClearWebauthnSessions", + "cronjob.ClearReauthenticationTokens", + }, actorTypes) + + // Every job must be registrable on an actor host + testutils.NewActorHostForTest(t, func(t *testing.T, host *local.Host) { + t.Helper() + for _, j := range jobs { + rErr := host.RegisterBuiltInActor(j) + require.NoError(t, rErr) + } + }) +} + +func TestWebauthnCleanupJobsDeleteExpiredRows(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + user := model.User{ + Base: model.Base{ID: "cleanup-job-user"}, + Username: "cleanup-job-user", + FirstName: "Cleanup", + LastName: "Job", + DisplayName: "Cleanup Job", + } + err := db.Create(&user).Error + require.NoError(t, err) + + var ( + past = datatype.DateTime(time.Now().Add(-time.Hour)) + future = datatype.DateTime(time.Now().Add(time.Hour)) + ) + + err = db.Create(&WebauthnSession{Base: model.Base{ID: "session-expired"}, Challenge: "c-expired", ExpiresAt: past}).Error + require.NoError(t, err) + err = db.Create(&WebauthnSession{Base: model.Base{ID: "session-active"}, Challenge: "c-active", ExpiresAt: future}).Error + require.NoError(t, err) + + err = db.Create(&ReauthenticationToken{Base: model.Base{ID: "token-expired"}, Token: "t-expired", ExpiresAt: past, UserID: user.ID}).Error + require.NoError(t, err) + err = db.Create(&ReauthenticationToken{Base: model.Base{ID: "token-active"}, Token: "t-active", ExpiresAt: future, UserID: user.ID}).Error + require.NoError(t, err) + + jobs := &cleanupJobs{db: db} + err = jobs.clearWebauthnSessions(t.Context()) + require.NoError(t, err) + err = jobs.clearReauthenticationTokens(t.Context()) + require.NoError(t, err) + + var remaining []string + err = db.Model(&WebauthnSession{}).Pluck("id", &remaining).Error + require.NoError(t, err) + require.Equal(t, []string{"session-active"}, remaining) + + err = db.Model(&ReauthenticationToken{}).Pluck("id", &remaining).Error + require.NoError(t, err) + require.Equal(t, []string{"token-active"}, remaining) +} diff --git a/backend/internal/webauthn/module.go b/backend/internal/webauthn/module.go index 300f8ac6..0d7b6479 100644 --- a/backend/internal/webauthn/module.go +++ b/backend/internal/webauthn/module.go @@ -2,9 +2,12 @@ package webauthn import ( "context" + "errors" + "fmt" "time" "github.com/gin-gonic/gin" + "github.com/italypaleale/francis/host/local" "github.com/lestrrat-go/jwx/v3/jwt" "gorm.io/gorm" @@ -26,11 +29,15 @@ type AuditLogger interface { type Dependencies struct { DB *gorm.DB + Actors *local.Host AppURL string Signer TokenService AuditLog AuditLogger AppConfig appconfig.AppConfigResolver + + // CleanupDisabled skips registering the cron jobs that delete expired rows from the database, for example in tests + CleanupDisabled bool } type Module struct { @@ -44,6 +51,25 @@ func New(deps Dependencies) (*Module, error) { return nil, err } + // Register the cleanup jobs for expired WebAuthn rows + if !deps.CleanupDisabled { + if deps.Actors == nil { + return nil, errors.New("actor host is required for the WebAuthn cleanup cron jobs") + } + + jobs, err := newCleanupJobs(deps.DB) + if err != nil { + return nil, err + } + + for _, cj := range jobs { + err = deps.Actors.RegisterBuiltInActor(cj) + if err != nil { + return nil, fmt.Errorf("error registering WebAuthn cleanup cron actor %q: %w", cj.ActorType(), err) + } + } + } + return &Module{ service: service, handler: newHandler(service, deps.AppConfig),