chore(code): reduce the code duplication EE-7278 (#11969)

api/apikey/apikey_test.go

@@ -3,7 +3,6 @@ package apikey
 import (
 	"testing"
 
-	"github.com/portainer/portainer/api/internal/securecookie"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -34,7 +33,7 @@ func Test_generateRandomKey(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := securecookie.GenerateRandomKey(tt.wantLenth)
+			got := GenerateRandomKey(tt.wantLenth)
 			is.Equal(tt.wantLenth, len(got))
 		})
 	}
@@ -42,7 +41,7 @@ func Test_generateRandomKey(t *testing.T) {
 	t.Run("Generated keys are unique", func(t *testing.T) {
 		keys := make(map[string]bool)
 		for i := 0; i < 100; i++ {
-			key := securecookie.GenerateRandomKey(8)
+			key := GenerateRandomKey(8)
 			_, ok := keys[string(key)]
 			is.False(ok)
 			keys[string(key)] = true

api/apikey/cache.go

@@ -1,69 +1,79 @@
 package apikey
 
 import (
-	lru "github.com/hashicorp/golang-lru"
 	portainer "github.com/portainer/portainer/api"
+
+	lru "github.com/hashicorp/golang-lru"
 )
 
-const defaultAPIKeyCacheSize = 1024
+const DefaultAPIKeyCacheSize = 1024
 
 // entry is a tuple containing the user and API key associated to an API key digest
-type entry struct {
-	user   portainer.User
+type entry[T any] struct {
+	user   T
 	apiKey portainer.APIKey
 }
 
-// apiKeyCache is a concurrency-safe, in-memory cache which primarily exists for to reduce database roundtrips.
+type UserCompareFn[T any] func(T, portainer.UserID) bool
+
+// ApiKeyCache is a concurrency-safe, in-memory cache which primarily exists for to reduce database roundtrips.
 // We store the api-key digest (keys) and the associated user and key-data (values) in the cache.
 // This is required because HTTP requests will contain only the api-key digest in the x-api-key request header;
 // digest value must be mapped to a portainer user (and respective key data) for validation.
 // This cache is used to avoid multiple database queries to retrieve these user/key associated to the digest.
-type apiKeyCache struct {
+type ApiKeyCache[T any] struct {
 	// cache type [string]entry cache (key: string(digest), value: user/key entry)
 	// note: []byte keys are not supported by golang-lru Cache
-	cache *lru.Cache
+	cache     *lru.Cache
+	userCmpFn UserCompareFn[T]
 }
 
 // NewAPIKeyCache creates a new cache for API keys
-func NewAPIKeyCache(cacheSize int) *apiKeyCache {
+func NewAPIKeyCache[T any](cacheSize int, userCompareFn UserCompareFn[T]) *ApiKeyCache[T] {
 	cache, _ := lru.New(cacheSize)
-	return &apiKeyCache{cache: cache}
+
+	return &ApiKeyCache[T]{cache: cache, userCmpFn: userCompareFn}
 }
 
 // Get returns the user/key associated to an api-key's digest
 // This is required because HTTP requests will contain the digest of the API key in header,
 // the digest value must be mapped to a portainer user.
-func (c *apiKeyCache) Get(digest string) (portainer.User, portainer.APIKey, bool) {
+func (c *ApiKeyCache[T]) Get(digest string) (T, portainer.APIKey, bool) {
 	val, ok := c.cache.Get(digest)
 	if !ok {
-		return portainer.User{}, portainer.APIKey{}, false
+		var t T
+
+		return t, portainer.APIKey{}, false
 	}
-	tuple := val.(entry)
+
+	tuple := val.(entry[T])
 
 	return tuple.user, tuple.apiKey, true
 }
 
 // Set persists a user/key entry to the cache
-func (c *apiKeyCache) Set(digest string, user portainer.User, apiKey portainer.APIKey) {
-	c.cache.Add(digest, entry{
+func (c *ApiKeyCache[T]) Set(digest string, user T, apiKey portainer.APIKey) {
+	c.cache.Add(digest, entry[T]{
 		user:   user,
 		apiKey: apiKey,
 	})
 }
 
 // Delete evicts a digest's user/key entry key from the cache
-func (c *apiKeyCache) Delete(digest string) {
+func (c *ApiKeyCache[T]) Delete(digest string) {
 	c.cache.Remove(digest)
 }
 
 // InvalidateUserKeyCache loops through all the api-keys associated to a user and removes them from the cache
-func (c *apiKeyCache) InvalidateUserKeyCache(userId portainer.UserID) bool {
+func (c *ApiKeyCache[T]) InvalidateUserKeyCache(userId portainer.UserID) bool {
 	present := false
+
 	for _, k := range c.cache.Keys() {
 		user, _, _ := c.Get(k.(string))
-		if user.ID == userId {
+		if c.userCmpFn(user, userId) {
 			present = c.cache.Remove(k)
 		}
 	}
+
 	return present
 }

api/apikey/cache_test.go

@@ -10,11 +10,11 @@ import (
 func Test_apiKeyCacheGet(t *testing.T) {
 	is := assert.New(t)
 
-	keyCache := NewAPIKeyCache(10)
+	keyCache := NewAPIKeyCache(10, compareUser)
 
 	// pre-populate cache
-	keyCache.cache.Add(string("foo"), entry{user: portainer.User{}, apiKey: portainer.APIKey{}})
-	keyCache.cache.Add(string(""), entry{user: portainer.User{}, apiKey: portainer.APIKey{}})
+	keyCache.cache.Add(string("foo"), entry[portainer.User]{user: portainer.User{}, apiKey: portainer.APIKey{}})
+	keyCache.cache.Add(string(""), entry[portainer.User]{user: portainer.User{}, apiKey: portainer.APIKey{}})
 
 	tests := []struct {
 		digest string
@@ -45,7 +45,7 @@ func Test_apiKeyCacheGet(t *testing.T) {
 func Test_apiKeyCacheSet(t *testing.T) {
 	is := assert.New(t)
 
-	keyCache := NewAPIKeyCache(10)
+	keyCache := NewAPIKeyCache(10, compareUser)
 
 	// pre-populate cache
 	keyCache.Set("bar", portainer.User{ID: 2}, portainer.APIKey{})
@@ -57,23 +57,23 @@ func Test_apiKeyCacheSet(t *testing.T) {
 	val, ok := keyCache.cache.Get(string("bar"))
 	is.True(ok)
 
-	tuple := val.(entry)
+	tuple := val.(entry[portainer.User])
 	is.Equal(portainer.User{ID: 2}, tuple.user)
 
 	val, ok = keyCache.cache.Get(string("foo"))
 	is.True(ok)
 
-	tuple = val.(entry)
+	tuple = val.(entry[portainer.User])
 	is.Equal(portainer.User{ID: 3}, tuple.user)
 }
 
 func Test_apiKeyCacheDelete(t *testing.T) {
 	is := assert.New(t)
 
-	keyCache := NewAPIKeyCache(10)
+	keyCache := NewAPIKeyCache(10, compareUser)
 
 	t.Run("Delete an existing entry", func(t *testing.T) {
-		keyCache.cache.Add(string("foo"), entry{user: portainer.User{ID: 1}, apiKey: portainer.APIKey{}})
+		keyCache.cache.Add(string("foo"), entry[portainer.User]{user: portainer.User{ID: 1}, apiKey: portainer.APIKey{}})
 		keyCache.Delete("foo")
 
 		_, ok := keyCache.cache.Get(string("foo"))
@@ -128,7 +128,7 @@ func Test_apiKeyCacheLRU(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			keyCache := NewAPIKeyCache(test.cacheLen)
+			keyCache := NewAPIKeyCache(test.cacheLen, compareUser)
 
 			for _, key := range test.key {
 				keyCache.Set(key, portainer.User{ID: 1}, portainer.APIKey{})
@@ -150,10 +150,10 @@ func Test_apiKeyCacheLRU(t *testing.T) {
 func Test_apiKeyCacheInvalidateUserKeyCache(t *testing.T) {
 	is := assert.New(t)
 
-	keyCache := NewAPIKeyCache(10)
+	keyCache := NewAPIKeyCache(10, compareUser)
 
 	t.Run("Removes users keys from cache", func(t *testing.T) {
-		keyCache.cache.Add(string("foo"), entry{user: portainer.User{ID: 1}, apiKey: portainer.APIKey{}})
+		keyCache.cache.Add(string("foo"), entry[portainer.User]{user: portainer.User{ID: 1}, apiKey: portainer.APIKey{}})
 
 		ok := keyCache.InvalidateUserKeyCache(1)
 		is.True(ok)
@@ -163,8 +163,8 @@ func Test_apiKeyCacheInvalidateUserKeyCache(t *testing.T) {
 	})
 
 	t.Run("Does not affect other keys", func(t *testing.T) {
-		keyCache.cache.Add(string("foo"), entry{user: portainer.User{ID: 1}, apiKey: portainer.APIKey{}})
-		keyCache.cache.Add(string("bar"), entry{user: portainer.User{ID: 2}, apiKey: portainer.APIKey{}})
+		keyCache.cache.Add(string("foo"), entry[portainer.User]{user: portainer.User{ID: 1}, apiKey: portainer.APIKey{}})
+		keyCache.cache.Add(string("bar"), entry[portainer.User]{user: portainer.User{ID: 2}, apiKey: portainer.APIKey{}})
 
 		ok := keyCache.InvalidateUserKeyCache(1)
 		is.True(ok)

api/apikey/service.go

@@ -1,14 +1,15 @@
 package apikey
 
 import (
+	"crypto/rand"
 	"crypto/sha256"
 	"encoding/base64"
 	"fmt"
+	"io"
 	"time"
 
 	portainer "github.com/portainer/portainer/api"
 	"github.com/portainer/portainer/api/dataservices"
-	"github.com/portainer/portainer/api/internal/securecookie"
 
 	"github.com/pkg/errors"
 )
@@ -20,30 +21,45 @@ var ErrInvalidAPIKey = errors.New("Invalid API key")
 type apiKeyService struct {
 	apiKeyRepository dataservices.APIKeyRepository
 	userRepository   dataservices.UserService
-	cache            *apiKeyCache
+	cache            *ApiKeyCache[portainer.User]
+}
+
+// GenerateRandomKey generates a random key of specified length
+// source: https://github.com/gorilla/securecookie/blob/master/securecookie.go#L515
+func GenerateRandomKey(length int) []byte {
+	k := make([]byte, length)
+	if _, err := io.ReadFull(rand.Reader, k); err != nil {
+		return nil
+	}
+
+	return k
+}
+
+func compareUser(u portainer.User, id portainer.UserID) bool {
+	return u.ID == id
 }
 
 func NewAPIKeyService(apiKeyRepository dataservices.APIKeyRepository, userRepository dataservices.UserService) *apiKeyService {
 	return &apiKeyService{
 		apiKeyRepository: apiKeyRepository,
 		userRepository:   userRepository,
-		cache:            NewAPIKeyCache(defaultAPIKeyCacheSize),
+		cache:            NewAPIKeyCache(DefaultAPIKeyCacheSize, compareUser),
 	}
 }
 
 // HashRaw computes a hash digest of provided raw API key.
 func (a *apiKeyService) HashRaw(rawKey string) string {
 	hashDigest := sha256.Sum256([]byte(rawKey))
+
 	return base64.StdEncoding.EncodeToString(hashDigest[:])
 }
 
 // GenerateApiKey generates a raw API key for a user (for one-time display).
 // The generated API key is stored in the cache and database.
 func (a *apiKeyService) GenerateApiKey(user portainer.User, description string) (string, *portainer.APIKey, error) {
-	randKey := securecookie.GenerateRandomKey(32)
+	randKey := GenerateRandomKey(32)
 	encodedRawAPIKey := base64.StdEncoding.EncodeToString(randKey)
 	prefixedAPIKey := portainerAPIKeyPrefix + encodedRawAPIKey
-
 	hashDigest := a.HashRaw(prefixedAPIKey)
 
 	apiKey := &portainer.APIKey{
@@ -54,8 +70,7 @@ func (a *apiKeyService) GenerateApiKey(user portainer.User, description string)
 		Digest:      hashDigest,
 	}
 
-	err := a.apiKeyRepository.Create(apiKey)
-	if err != nil {
+	if err := a.apiKeyRepository.Create(apiKey); err != nil {
 		return "", nil, errors.Wrap(err, "Unable to create API key")
 	}
 
@@ -78,7 +93,6 @@ func (a *apiKeyService) GetAPIKeys(userID portainer.UserID) ([]portainer.APIKey,
 // GetDigestUserAndKey returns the user and api-key associated to a specified hash digest.
 // A cache lookup is performed first; if the user/api-key is not found in the cache, respective database lookups are performed.
 func (a *apiKeyService) GetDigestUserAndKey(digest string) (portainer.User, portainer.APIKey, error) {
-	// get api key from cache if possible
 	cachedUser, cachedKey, ok := a.cache.Get(digest)
 	if ok {
 		return cachedUser, cachedKey, nil
@@ -106,20 +120,21 @@ func (a *apiKeyService) UpdateAPIKey(apiKey *portainer.APIKey) error {
 	if err != nil {
 		return errors.Wrap(err, "Unable to retrieve API key")
 	}
+
 	a.cache.Set(apiKey.Digest, user, *apiKey)
+
 	return a.apiKeyRepository.Update(apiKey.ID, apiKey)
 }
 
 // DeleteAPIKey deletes an API key and removes the digest/api-key entry from the cache.
 func (a *apiKeyService) DeleteAPIKey(apiKeyID portainer.APIKeyID) error {
-	// get api-key digest to remove from cache
 	apiKey, err := a.apiKeyRepository.Read(apiKeyID)
 	if err != nil {
 		return errors.Wrap(err, fmt.Sprintf("Unable to retrieve API key: %d", apiKeyID))
 	}
 
-	// delete the user/api-key from cache
 	a.cache.Delete(apiKey.Digest)
+
 	return a.apiKeyRepository.Delete(apiKeyID)
 }