satellite/{console, db}: extend dbcleanup chore to delete expired API…

… keys Extended existing console db cleanup chore to also delete expired object browser API keys. Issue: #6854 Change-Id: Icc2b28d43092c884adb124a392695e076ad0b9b3
storj · Apr 19, 2024 · 49a2af6 · 49a2af6
1 parent 8dacb7b
commit 49a2af6
Show file tree

Hide file tree

Showing 6 changed files with 329 additions and 9 deletions.
diff --git a/satellite/console/apikeys.go b/satellite/console/apikeys.go
@@ -31,6 +31,8 @@ type APIKeys interface {
 	Update(ctx context.Context, key APIKeyInfo) error
 	// Delete deletes APIKeyInfo from store
 	Delete(ctx context.Context, id uuid.UUID) error
+	// DeleteExpiredByNamePrefix deletes expired APIKeyInfo from store by key name prefix
+	DeleteExpiredByNamePrefix(ctx context.Context, lifetime time.Duration, prefix string, asOfSystemTimeInterval time.Duration, pageSize int) error
 }
 
 // RESTKeys is an interface for rest key operations.

diff --git a/satellite/console/apikeys_test.go b/satellite/console/apikeys_test.go
@@ -6,6 +6,7 @@ package console_test
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -271,5 +272,96 @@ func TestApiKeysRepository(t *testing.T) {
 			assert.Equal(t, uint64(0), keys.TotalCount)
 			assert.Equal(t, 0, len(keys.APIKeys))
 		})
+
+		t.Run("DeleteExpiredByNamePrefix", func(t *testing.T) {
+			pr, err := projects.Insert(ctx, &console.Project{
+				Name: "ProjectName3",
+			})
+			assert.NotNil(t, pr)
+			assert.NoError(t, err)
+
+			secret, err := macaroon.NewSecret()
+			assert.NoError(t, err)
+
+			key, err := macaroon.NewAPIKey(secret)
+			assert.NoError(t, err)
+			key1, err := macaroon.NewAPIKey(secret)
+			assert.NoError(t, err)
+			key2, err := macaroon.NewAPIKey(secret)
+			assert.NoError(t, err)
+			key3, err := macaroon.NewAPIKey(secret)
+			assert.NoError(t, err)
+
+			prefix := "prefix"
+			now := time.Now()
+
+			keyInfo := console.APIKeyInfo{
+				Name:      "randomName",
+				ProjectID: pr.ID,
+				Secret:    secret,
+			}
+			keyInfo1 := console.APIKeyInfo{
+				Name:      prefix,
+				ProjectID: pr.ID,
+				Secret:    secret,
+			}
+			keyInfo2 := console.APIKeyInfo{
+				Name:      prefix + "test",
+				ProjectID: pr.ID,
+				Secret:    secret,
+			}
+			keyInfo3 := console.APIKeyInfo{
+				Name:      prefix + "test1",
+				ProjectID: pr.ID,
+				Secret:    secret,
+			}
+
+			createdKey, err := apikeys.Create(ctx, key.Head(), keyInfo)
+			assert.NoError(t, err)
+			assert.NotNil(t, createdKey)
+			createdKey1, err := apikeys.Create(ctx, key1.Head(), keyInfo1)
+			assert.NoError(t, err)
+			assert.NotNil(t, createdKey1)
+			createdKey2, err := apikeys.Create(ctx, key2.Head(), keyInfo2)
+			assert.NoError(t, err)
+			assert.NotNil(t, createdKey2)
+			createdKey3, err := apikeys.Create(ctx, key3.Head(), keyInfo3)
+			assert.NoError(t, err)
+			assert.NotNil(t, createdKey3)
+
+			query := "UPDATE api_keys SET created_at = $1 WHERE id = $2"
+
+			_, err = db.Testing().RawDB().ExecContext(ctx, query, now.Add(-24*time.Hour), createdKey1.ID)
+			assert.NoError(t, err)
+			_, err = db.Testing().RawDB().ExecContext(ctx, query, now.Add(-24*2*time.Hour), createdKey2.ID)
+			assert.NoError(t, err)
+			_, err = db.Testing().RawDB().ExecContext(ctx, query, now.Add(-24*3*time.Hour), createdKey3.ID)
+			assert.NoError(t, err)
+
+			cursor := console.APIKeyCursor{Page: 1, Limit: 10}
+			keys, err := apikeys.GetPagedByProjectID(ctx, pr.ID, cursor, "")
+			assert.NoError(t, err)
+			assert.NotNil(t, keys)
+			assert.Len(t, keys.APIKeys, 4)
+
+			// Even with a page size set to 1, 2 of the 4 keys must be deleted.
+			err = apikeys.DeleteExpiredByNamePrefix(ctx, time.Hour*47, prefix, 0, 1)
+			assert.NoError(t, err)
+
+			keys, err = apikeys.GetPagedByProjectID(ctx, pr.ID, cursor, "")
+			assert.NoError(t, err)
+			assert.NotNil(t, keys)
+			assert.Len(t, keys.APIKeys, 2)
+
+			// 1 of the 2 remaining keys has to be deleted because the only one doesn't have a prefix.
+			err = apikeys.DeleteExpiredByNamePrefix(ctx, time.Hour*23, prefix, 0, 1)
+			assert.NoError(t, err)
+
+			keys, err = apikeys.GetPagedByProjectID(ctx, pr.ID, cursor, "")
+			assert.NoError(t, err)
+			assert.NotNil(t, keys)
+			assert.Len(t, keys.APIKeys, 1)
+			assert.Equal(t, keyInfo.Name, keys.APIKeys[0].Name)
+		})
 	})
 }
diff --git a/satellite/console/dbcleanup/chore.go b/satellite/console/dbcleanup/chore.go
@@ -29,19 +29,21 @@ type Config struct {
 
 // Chore periodically removes unwanted records from the satellite console database.
 type Chore struct {
-	log    *zap.Logger
-	db     console.DB
-	Loop   *sync2.Cycle
-	config Config
+	log           *zap.Logger
+	db            console.DB
+	Loop          *sync2.Cycle
+	config        Config
+	consoleConfig console.Config
 }
 
 // NewChore creates a new console DB cleanup chore.
-func NewChore(log *zap.Logger, db console.DB, config Config) *Chore {
+func NewChore(log *zap.Logger, db console.DB, config Config, consoleConfig console.Config) *Chore {
 	return &Chore{
-		log:    log,
-		db:     db,
-		config: config,
-		Loop:   sync2.NewCycle(config.Interval),
+		log:           log,
+		db:            db,
+		config:        config,
+		consoleConfig: consoleConfig,
+		Loop:          sync2.NewCycle(config.Interval),
 	}
 }
 
@@ -60,6 +62,11 @@ func (chore *Chore) Run(ctx context.Context) (err error) {
 			chore.log.Error("Error deleting expired webapp sessions", zap.Error(err))
 		}
 
+		err = chore.db.APIKeys().DeleteExpiredByNamePrefix(ctx, chore.consoleConfig.ObjectBrowserKeyLifetime, chore.consoleConfig.ObjectBrowserKeyNamePrefix, chore.config.AsOfSystemTimeInterval, chore.config.PageSize)
+		if err != nil {
+			chore.log.Error("Error deleting expired API keys", zap.Error(err))
+		}
+
 		return nil
 	})
 }

diff --git a/satellite/console/dbcleanup/chore_test.go b/satellite/console/dbcleanup/chore_test.go
@@ -0,0 +1,128 @@
+// Copyright (C) 2024 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package dbcleanup_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap/zaptest"
+
+	"storj.io/common/macaroon"
+	"storj.io/common/testcontext"
+	"storj.io/storj/private/testplanet"
+	"storj.io/storj/satellite/console"
+	"storj.io/storj/satellite/console/dbcleanup"
+)
+
+func TestChore(t *testing.T) {
+	testplanet.Run(t, testplanet.Config{
+		SatelliteCount: 1, StorageNodeCount: 0, UplinkCount: 0,
+	}, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) {
+		sat := planet.Satellites[0]
+		db := sat.DB
+		cfg := sat.Config
+
+		user1, err := sat.AddUser(ctx, console.CreateUser{
+			FullName: "Test User",
+			Email:    "example1@mail.test",
+		}, 1)
+		require.NoError(t, err)
+
+		user2, err := sat.AddUser(ctx, console.CreateUser{
+			FullName: "Test User",
+			Email:    "example2@mail.test",
+		}, 1)
+		require.NoError(t, err)
+
+		pr1, err := sat.AddProject(ctx, user1.ID, "Test Project")
+		require.NoError(t, err)
+		pr2, err := sat.AddProject(ctx, user2.ID, "Test Project")
+		require.NoError(t, err)
+
+		chore := dbcleanup.NewChore(zaptest.NewLogger(t), db.Console(), cfg.ConsoleDBCleanup, cfg.Console.Config)
+		ctx.Go(func() error {
+			return chore.Run(ctx)
+		})
+		defer ctx.Check(chore.Close)
+
+		t.Run("delete expired keys", func(t *testing.T) {
+			chore.Loop.Pause()
+
+			secret, err := macaroon.NewSecret()
+			require.NoError(t, err)
+
+			key1, err := macaroon.NewAPIKey(secret)
+			require.NoError(t, err)
+			key2, err := macaroon.NewAPIKey(secret)
+			require.NoError(t, err)
+			key3, err := macaroon.NewAPIKey(secret)
+			require.NoError(t, err)
+
+			now := time.Now()
+
+			keyInfo1 := console.APIKeyInfo{
+				Name:      cfg.Console.ObjectBrowserKeyNamePrefix,
+				ProjectID: pr1.ID,
+				Secret:    secret,
+			}
+			keyInfo2 := console.APIKeyInfo{
+				Name:      cfg.Console.ObjectBrowserKeyNamePrefix,
+				ProjectID: pr2.ID,
+				Secret:    secret,
+			}
+			keyInfo3 := console.APIKeyInfo{
+				Name:      "randomName",
+				ProjectID: pr2.ID,
+				Secret:    secret,
+			}
+
+			createdKey1, err := db.Console().APIKeys().Create(ctx, key1.Head(), keyInfo1)
+			require.NoError(t, err)
+			require.NotNil(t, createdKey1)
+			createdKey2, err := db.Console().APIKeys().Create(ctx, key2.Head(), keyInfo2)
+			require.NoError(t, err)
+			require.NotNil(t, createdKey2)
+			createdKey3, err := db.Console().APIKeys().Create(ctx, key3.Head(), keyInfo3)
+			require.NoError(t, err)
+			require.NotNil(t, createdKey3)
+
+			query := "UPDATE api_keys SET created_at = $1 WHERE id = $2"
+			createdAt := now.Add(-cfg.Console.ObjectBrowserKeyLifetime).Add(-time.Hour)
+
+			_, err = db.Testing().RawDB().ExecContext(ctx, query, createdAt, createdKey1.ID)
+			require.NoError(t, err)
+			_, err = db.Testing().RawDB().ExecContext(ctx, query, createdAt, createdKey3.ID)
+			require.NoError(t, err)
+
+			chore.Loop.TriggerWait()
+			chore.Loop.Pause()
+
+			// Expired key is removed.
+			createdKey1, err = db.Console().APIKeys().Get(ctx, createdKey1.ID)
+			require.Error(t, err)
+			require.Nil(t, createdKey1)
+
+			// Non-expired key and expired but not-prefixed key are both present.
+			cursor := console.APIKeyCursor{Page: 1, Limit: 10}
+
+			page, err := db.Console().APIKeys().GetPagedByProjectID(ctx, pr2.ID, cursor, "")
+			require.NoError(t, err)
+			require.NotNil(t, page)
+			require.Len(t, page.APIKeys, 2)
+
+			_, err = db.Testing().RawDB().ExecContext(ctx, query, createdAt, createdKey2.ID)
+			require.NoError(t, err)
+
+			chore.Loop.TriggerWait()
+			chore.Loop.Pause()
+
+			// Second expired key is removed.
+			createdKey2, err = db.Console().APIKeys().Get(ctx, createdKey2.ID)
+			require.Error(t, err)
+			require.Nil(t, createdKey2)
+		})
+	})
+}
diff --git a/satellite/core.go b/satellite/core.go
@@ -585,6 +585,7 @@ func New(log *zap.Logger, full *identity.FullIdentity, db DB,
 			peer.Log.Named("console.dbcleanup:chore"),
 			peer.DB.Console(),
 			config.ConsoleDBCleanup,
+			config.Console.Config,
 		)
 
 		peer.Services.Add(lifecycle.Item{