feat(cache): implement hashed API key storage and retrieval

Adds a new hashed key storage mechanism for API keys in the cache. Replaces direct mapping to API keys with composite keys based on organizationId and name. Implements searching of API keys using hash comparisons for improved security. Updates related tests to ensure correct functionality and validate the hashing. Also, adds support for a new dependency `golang.org/x/crypto`.
2025-11-20 22:11:17 +01:00
parent 1e2236dc9e
commit 4468903535
11 changed files with 357 additions and 69 deletions
@@ -3,9 +3,72 @@ package hash
 import (
 	"crypto/sha256"
 	"encoding/base64"
+
+	"golang.org/x/crypto/bcrypt"
 )

+// String creates a SHA256 hash of a string (legacy, for non-sensitive data)
 func String(s string) string {
 	encoded := sha256.New().Sum([]byte(s))
 	return base64.StdEncoding.EncodeToString(encoded)
 }
+
+// APIKey hashes an API key using bcrypt for secure storage
+// Cost of 12 provides a good balance between security and performance
+func APIKey(key string) (string, error) {
+	hash, err := bcrypt.GenerateFromPassword([]byte(key), 12)
+	if err != nil {
+		return "", err
+	}
+	return string(hash), nil
+}
+
+// CompareAPIKey compares a plaintext API key with a hash
+// Supports both bcrypt (new) and SHA256 (legacy) hashes for backwards compatibility
+// Returns true if they match, false otherwise
+//
+// Migration Strategy:
+// Old API keys stored with SHA256 will continue to work. To upgrade them to bcrypt:
+// 1. Keys are automatically upgraded when users re-authenticate (if implemented)
+// 2. Or, run a one-time migration using MigrateAPIKeyHash when convenient
+func CompareAPIKey(hashedKey, plainKey string) bool {
+	// Bcrypt hashes start with $2a$, $2b$, or $2y$
+	// If the hash starts with $2, it's a bcrypt hash
+	if len(hashedKey) > 2 && hashedKey[0] == '$' && hashedKey[1] == '2' {
+		// New bcrypt hash
+		err := bcrypt.CompareHashAndPassword([]byte(hashedKey), []byte(plainKey))
+		return err == nil
+	}
+
+	// Legacy SHA256 hash - compare using the old method
+	legacyHash := String(plainKey)
+	return hashedKey == legacyHash
+}
+
+// IsLegacyHash returns true if the hash is a legacy SHA256 hash (not bcrypt)
+func IsLegacyHash(hashedKey string) bool {
+	return len(hashedKey) <= 2 || hashedKey[0] != '$' || hashedKey[1] != '2'
+}
+
+// MigrateAPIKeyHash can be used to upgrade a legacy SHA256 hash to bcrypt
+// This is useful for one-time migrations of existing keys
+// Returns the new bcrypt hash if the key is legacy, otherwise returns the original
+func MigrateAPIKeyHash(currentHash, plainKey string) (string, bool, error) {
+	// If already bcrypt, no migration needed
+	if !IsLegacyHash(currentHash) {
+		return currentHash, false, nil
+	}
+
+	// Verify the legacy hash is correct before migrating
+	if !CompareAPIKey(currentHash, plainKey) {
+		return "", false, nil // Invalid key, don't migrate
+	}
+
+	// Generate new bcrypt hash
+	newHash, err := APIKey(plainKey)
+	if err != nil {
+		return "", false, err
+	}
+
+	return newHash, true, nil
+}
@@ -0,0 +1,169 @@
+package hash
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAPIKey(t *testing.T) {
+	key := "test_api_key_12345" // gitleaks:allow
+
+	hash1, err := APIKey(key)
+	require.NoError(t, err)
+	assert.NotEmpty(t, hash1)
+	assert.NotEqual(t, key, hash1, "Hash should not equal plaintext")
+
+	// Bcrypt hashes should start with $2
+	assert.True(t, strings.HasPrefix(hash1, "$2"), "Should be a bcrypt hash")
+
+	// Same key should produce different hashes (due to salt)
+	hash2, err := APIKey(key)
+	require.NoError(t, err)
+	assert.NotEqual(t, hash1, hash2, "Bcrypt should produce different hashes with different salts")
+}
+
+func TestCompareAPIKey_Bcrypt(t *testing.T) {
+	key := "test_api_key_12345" // gitleaks:allow
+
+	hash, err := APIKey(key)
+	require.NoError(t, err)
+
+	// Correct key should match
+	assert.True(t, CompareAPIKey(hash, key))
+
+	// Wrong key should not match
+	assert.False(t, CompareAPIKey(hash, "wrong_key"))
+}
+
+func TestCompareAPIKey_Legacy(t *testing.T) {
+	key := "test_api_key_12345" // gitleaks:allow
+
+	// Create a legacy SHA256 hash
+	legacyHash := String(key)
+
+	// Should still work with legacy hashes
+	assert.True(t, CompareAPIKey(legacyHash, key))
+
+	// Wrong key should not match
+	assert.False(t, CompareAPIKey(legacyHash, "wrong_key"))
+}
+
+func TestCompareAPIKey_BackwardCompatibility(t *testing.T) {
+	tests := []struct {
+		name     string
+		hashFunc func(string) string
+		expectOK bool
+	}{
+		{
+			name: "bcrypt hash",
+			hashFunc: func(k string) string {
+				h, _ := APIKey(k)
+				return h
+			},
+			expectOK: true,
+		},
+		{
+			name: "legacy SHA256 hash",
+			hashFunc: func(k string) string {
+				return String(k)
+			},
+			expectOK: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			key := "test_key_123"
+			hash := tt.hashFunc(key)
+
+			result := CompareAPIKey(hash, key)
+			assert.Equal(t, tt.expectOK, result)
+		})
+	}
+}
+
+func TestString(t *testing.T) {
+	// Test that String function still works (for non-sensitive data)
+	input := "test_string"
+	hash1 := String(input)
+	hash2 := String(input)
+
+	// SHA256 should be deterministic
+	assert.Equal(t, hash1, hash2)
+	assert.NotEmpty(t, hash1)
+	assert.NotEqual(t, input, hash1)
+}
+
+func TestIsLegacyHash(t *testing.T) {
+	tests := []struct {
+		name     string
+		hash     string
+		isLegacy bool
+	}{
+		{
+			name:     "bcrypt hash",
+			hash:     "$2a$12$abcdefghijklmnopqrstuv",
+			isLegacy: false,
+		},
+		{
+			name:     "SHA256 hash",
+			hash:     "dXNfYWtfMTIzNDU2Nzg5MDEyMzQ1NuOwxEKY",
+			isLegacy: true,
+		},
+		{
+			name:     "empty string",
+			hash:     "",
+			isLegacy: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.isLegacy, IsLegacyHash(tt.hash))
+		})
+	}
+}
+
+func TestMigrateAPIKeyHash(t *testing.T) {
+	plainKey := "test_api_key_123"
+
+	t.Run("migrate legacy hash", func(t *testing.T) {
+		// Create a legacy SHA256 hash
+		legacyHash := String(plainKey)
+
+		// Migrate it
+		newHash, migrated, err := MigrateAPIKeyHash(legacyHash, plainKey)
+		require.NoError(t, err)
+		assert.True(t, migrated, "Should indicate migration occurred")
+		assert.NotEqual(t, legacyHash, newHash, "New hash should differ from legacy")
+		assert.True(t, strings.HasPrefix(newHash, "$2"), "New hash should be bcrypt")
+
+		// Verify new hash works
+		assert.True(t, CompareAPIKey(newHash, plainKey))
+	})
+
+	t.Run("no migration needed for bcrypt", func(t *testing.T) {
+		// Create a bcrypt hash
+		bcryptHash, err := APIKey(plainKey)
+		require.NoError(t, err)
+
+		// Try to migrate it
+		newHash, migrated, err := MigrateAPIKeyHash(bcryptHash, plainKey)
+		require.NoError(t, err)
+		assert.False(t, migrated, "Should not migrate bcrypt hash")
+		assert.Equal(t, bcryptHash, newHash, "Hash should remain unchanged")
+	})
+
+	t.Run("invalid key does not migrate", func(t *testing.T) {
+		legacyHash := String("correct_key")
+
+		// Try to migrate with wrong plaintext
+		newHash, migrated, err := MigrateAPIKeyHash(legacyHash, "wrong_key")
+		require.NoError(t, err)
+		assert.False(t, migrated, "Should not migrate invalid key")
+		assert.Empty(t, newHash, "Should return empty for invalid key")
+	})
+}