diff --git a/store/sqlite/sqlite.go b/store/sqlite/sqlite.go index 2791307..ebd5323 100644 --- a/store/sqlite/sqlite.go +++ b/store/sqlite/sqlite.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "sync" + "time" "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/bluesky-social/indigo/atproto/syntax" @@ -171,15 +172,43 @@ func (s *Store) DeleteAuthRequestInfo(ctx context.Context, state string) error { return err } -// CleanupExpiredRequests removes auth requests older than the given duration. -// Call this periodically (e.g. every 10 minutes) to prevent unbounded growth. -func (s *Store) CleanupExpiredRequests(ctx context.Context, olderThanMinutes int) error { +// CleanupExpiredRequests removes auth requests older than the given duration +// and returns the number of rows deleted. Call this periodically (e.g. every +// 10 minutes) to prevent unbounded growth of incomplete OAuth callback state. +func (s *Store) CleanupExpiredRequests(ctx context.Context, olderThan time.Duration) (int64, error) { s.mu.Lock() defer s.mu.Unlock() - _, err := s.db.ExecContext(ctx, + res, err := s.db.ExecContext(ctx, `DELETE FROM oauth_auth_requests WHERE created_at < datetime('now', ?)`, - fmt.Sprintf("-%d minutes", olderThanMinutes), + fmt.Sprintf("-%d seconds", int64(olderThan.Seconds())), ) - return err + if err != nil { + return 0, err + } + return res.RowsAffected() +} + +// CleanupExpiredSessions removes sessions whose updated_at is older than the +// given duration and returns the number of rows deleted. Use a cutoff at or +// above the upstream refresh-token lifetime (~90 days on bsky PDS) so live +// sessions are never pruned. +// +// Sessions saved by SaveSession use SQLite's CURRENT_TIMESTAMP format +// ("YYYY-MM-DD HH:MM:SS"). If callers have inserted rows with a different +// timestamp format (e.g. RFC3339Nano), the string comparison can mis-classify +// them — every SaveSession rewrites updated_at, so the inconsistency self- +// heals within a refresh cycle. +func (s *Store) CleanupExpiredSessions(ctx context.Context, olderThan time.Duration) (int64, error) { + s.mu.Lock() + defer s.mu.Unlock() + + res, err := s.db.ExecContext(ctx, + `DELETE FROM oauth_sessions WHERE updated_at < datetime('now', ?)`, + fmt.Sprintf("-%d seconds", int64(olderThan.Seconds())), + ) + if err != nil { + return 0, err + } + return res.RowsAffected() } diff --git a/store/sqlite/sqlite_test.go b/store/sqlite/sqlite_test.go index 844ef16..db72eb0 100644 --- a/store/sqlite/sqlite_test.go +++ b/store/sqlite/sqlite_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "testing" + "time" "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/bluesky-social/indigo/atproto/syntax" @@ -117,3 +118,77 @@ func TestAuthRequestCRUD(t *testing.T) { t.Fatal("expected error after delete") } } + +func TestCleanupExpiredRequests(t *testing.T) { + s := testStore(t) + ctx := context.Background() + + // Fresh row via the normal API (created_at = CURRENT_TIMESTAMP). + fresh := oauth.AuthRequestData{State: "fresh"} + if err := s.SaveAuthRequestInfo(ctx, fresh); err != nil { + t.Fatal(err) + } + + // Backdated row inserted directly so it predates the cutoff. + _, err := s.db.ExecContext(ctx, + `INSERT INTO oauth_auth_requests (state, data, created_at) VALUES (?, ?, datetime('now', '-1 hour'))`, + "stale", `{"State":"stale"}`, + ) + if err != nil { + t.Fatal(err) + } + + deleted, err := s.CleanupExpiredRequests(ctx, 10*time.Minute) + if err != nil { + t.Fatal(err) + } + if deleted != 1 { + t.Fatalf("expected 1 deleted, got %d", deleted) + } + + if _, err := s.GetAuthRequestInfo(ctx, "stale"); err == nil { + t.Fatal("expected stale auth request to be gone") + } + if _, err := s.GetAuthRequestInfo(ctx, "fresh"); err != nil { + t.Fatalf("fresh auth request should remain: %v", err) + } +} + +func TestCleanupExpiredSessions(t *testing.T) { + s := testStore(t) + ctx := context.Background() + did, _ := syntax.ParseDID("did:plc:test123") + + fresh := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "fresh", + HostURL: "https://pds.example.com", + } + if err := s.SaveSession(ctx, fresh); err != nil { + t.Fatal(err) + } + + // Backdated row. + _, err := s.db.ExecContext(ctx, ` + INSERT INTO oauth_sessions (did, session_id, data, updated_at) + VALUES (?, ?, ?, datetime('now', '-100 days')) + `, did.String(), "stale", `{"AccountDID":"did:plc:test123","SessionID":"stale"}`) + if err != nil { + t.Fatal(err) + } + + deleted, err := s.CleanupExpiredSessions(ctx, 90*24*time.Hour) + if err != nil { + t.Fatal(err) + } + if deleted != 1 { + t.Fatalf("expected 1 deleted, got %d", deleted) + } + + if _, err := s.GetSession(ctx, did, "stale"); err == nil { + t.Fatal("expected stale session to be gone") + } + if _, err := s.GetSession(ctx, did, "fresh"); err != nil { + t.Fatalf("fresh session should remain: %v", err) + } +}