Improve caching by adding user ids and roles to hash.

This commit is contained in:
2025-11-22 17:02:16 +00:00
parent 28223b40da
commit 39dc7b49cf
11 changed files with 198 additions and 43 deletions
+2
View File
@@ -3,3 +3,5 @@ test.sh
banned.json*
dist/
coverage.out
CLAUDE.md
graphql-monitoring-proxy
+26 -6
View File
@@ -155,6 +155,7 @@ You can still use the non-prefixed environment variables in the spirit of the ba
| `CACHE_TTL` | The cache TTL | `60` |
| `CACHE_MAX_MEMORY_SIZE` | Maximum memory size for cache in MB | `100` |
| `CACHE_MAX_ENTRIES` | Maximum number of entries in cache | `10000` |
| `CACHE_PER_USER_DISABLED` | **⚠️ SECURITY**: Disable per-user cache isolation | `false` (**DO NOT** set to `true` in multi-user apps) |
| `ENABLE_REDIS_CACHE` | Enable distributed Redis cache | `false` |
| `CACHE_REDIS_URL` | URL to redis server / cluster endpoint | `localhost:6379` |
| `CACHE_REDIS_PASSWORD` | Redis connection password | `` |
@@ -347,19 +348,38 @@ The admin dashboard (`/admin`) provides:
The cache engine is enabled in the background by default, using no additional resources.
You can then start using the cache by setting the `ENABLE_GLOBAL_CACHE` or `ENABLE_REDIS_CACHE` environment variable to `true` - which will enable the cache for all queries without introspection. You can leave the global cache disabled and enable the cache for specific queries by adding the `@cached` directive to the query.
**Important**: The cache key is calculated from the **entire request body**, which includes both the GraphQL query and variables. This means:
**Important**: The cache key is calculated from the **request body + user context (user ID and role)**. This means:
- Identical queries with different variables are cached separately
- Identical queries with different variable values get their own cache entries
- This ensures correct caching behavior for parameterized queries
- **Identical queries from different users are cached separately** (security isolation)
- **Identical queries with different roles are cached separately** (prevents privilege escalation)
- This ensures correct caching behavior and prevents data leakage between users
**🔒 Security Update (v0.27.0+)**: Cache keys now include user context by default to prevent security vulnerabilities where users could see each other's cached data. This is enabled by default and should NOT be disabled in multi-user applications.
Example:
```graphql
# These two requests will have DIFFERENT cache keys:
# These requests will have DIFFERENT cache keys:
# Different variables
query GetUser($id: ID!) { user(id: $id) { name } }
variables: { "id": "123" }
variables: { "id": "123" } // Cache key: MD5(body + user:alice + role:user)
query GetUser($id: ID!) { user(id: $id) { name } }
variables: { "id": "456" }
variables: { "id": "456" } // Cache key: MD5(body + user:alice + role:user)
# Different users (SECURITY: prevents data leakage)
query GetMyProfile { me { email } }
Authorization: Bearer token_for_alice // Cache key: MD5(body + user:alice + role:user)
query GetMyProfile { me { email } }
Authorization: Bearer token_for_bob // Cache key: MD5(body + user:bob + role:user)
# Different roles (SECURITY: prevents privilege escalation)
query GetData { data { value } }
Authorization: Bearer token_admin // Cache key: MD5(body + user:alice + role:admin)
query GetData { data { value } }
Authorization: Bearer token_user // Cache key: MD5(body + user:alice + role:user)
```
In the case of the `@cached` you can add additional parameters to the directive which will set the cache for specific queries to the provided time.
+6 -4
View File
@@ -215,11 +215,13 @@ func TestAdminDashboard_GetCacheStats(t *testing.T) {
CacheMaxMemorySize int
CacheMaxEntries int
GraphQLQueryCacheSize int
PerUserCacheDisabled bool
}{
CacheEnable: true,
CacheTTL: 60,
CacheMaxMemorySize: 100,
CacheMaxEntries: 10000,
CacheEnable: true,
CacheTTL: 60,
CacheMaxMemorySize: 100,
CacheMaxEntries: 10000,
PerUserCacheDisabled: false,
},
}
+42 -4
View File
@@ -3,6 +3,7 @@ package libpack_cache
import (
"bytes"
"compress/gzip"
"fmt"
"io"
"sync/atomic"
"time"
@@ -27,7 +28,9 @@ type CacheConfig struct {
MaxMemorySize int64 `json:"max_memory_size"` // Maximum memory size in bytes
MaxEntries int64 `json:"max_entries"` // Maximum number of entries
}
TTL int `json:"ttl"`
TTL int `json:"ttl"`
IncludeUserContext bool `json:"include_user_context"` // Include user ID and role in cache key
PerUserCacheDisabled bool `json:"per_user_cache_disabled"` // Disable per-user caching (backward compatibility)
}
type CacheStats struct {
@@ -52,10 +55,14 @@ var (
config *CacheConfig
)
// CalculateHash generates an MD5 hash from the request body.
// CalculateHash generates an MD5 hash from the request body and optionally user context.
// For GraphQL requests, this includes both the query and variables,
// ensuring that identical queries with different variables are cached separately.
//
// SECURITY FIX: This function now includes user ID and role in the cache key by default
// to prevent data leakage between authenticated users. Set CACHE_PER_USER_DISABLED=true
// to revert to the old behavior (NOT RECOMMENDED for multi-user applications).
//
// Example GraphQL request body:
//
// {
@@ -63,8 +70,39 @@ var (
// "variables": { "id": "123" }
// }
//
// Different variable values will produce different cache keys.
func CalculateHash(c *fiber.Ctx) string {
// With user context enabled (default):
// - Same query, same variables, same user → same cache key
// - Same query, same variables, different user → different cache key
//
// Different variable values will always produce different cache keys.
func CalculateHash(c *fiber.Ctx, userID string, userRole string) string {
cacheKeyData := string(c.Body())
// Include user context in cache key (default behavior for security)
// Only skip if explicitly disabled via configuration (backward compatibility)
if config != nil && !config.PerUserCacheDisabled {
// Normalize empty user values to prevent cache key collisions
if userID == "" {
userID = "-"
}
if userRole == "" {
userRole = "-"
}
// Append user context to ensure cache isolation between users
cacheKeyData = fmt.Sprintf("%s|uid:%s|role:%s", cacheKeyData, userID, userRole)
}
return strutil.Md5(cacheKeyData)
}
// CalculateHashLegacy generates a cache hash using only the request body (DEPRECATED).
// This function exists for backward compatibility only and should NOT be used
// in production multi-user applications as it creates a security vulnerability
// where users can see each other's cached data.
//
// Deprecated: Use CalculateHash with user context instead.
func CalculateHashLegacy(c *fiber.Ctx) string {
return strutil.Md5(c.Body())
}
+78 -16
View File
@@ -20,7 +20,7 @@ func (suite *Tests) Test_CalculateHash() {
// Test with empty body
suite.Run("empty body", func() {
ctx.Request().SetBody([]byte(""))
hash := CalculateHash(ctx)
hash := CalculateHash(ctx, "user1", "admin")
assert.NotEmpty(hash)
assert.Equal(32, len(hash)) // MD5 hash is 32 characters
})
@@ -28,7 +28,7 @@ func (suite *Tests) Test_CalculateHash() {
// Test with non-empty body
suite.Run("non-empty body", func() {
ctx.Request().SetBody([]byte("test body"))
hash := CalculateHash(ctx)
hash := CalculateHash(ctx, "user1", "admin")
assert.NotEmpty(hash)
assert.Equal(32, len(hash))
})
@@ -36,10 +36,10 @@ func (suite *Tests) Test_CalculateHash() {
// Test with different bodies produce different hashes
suite.Run("different bodies", func() {
ctx.Request().SetBody([]byte("body1"))
hash1 := CalculateHash(ctx)
hash1 := CalculateHash(ctx, "user1", "admin")
ctx.Request().SetBody([]byte("body2"))
hash2 := CalculateHash(ctx)
hash2 := CalculateHash(ctx, "user1", "admin")
assert.NotEqual(hash1, hash2)
})
@@ -51,10 +51,10 @@ func (suite *Tests) Test_CalculateHash() {
query2 := []byte(`{"query":"query GetUser($id: ID!) { user(id: $id) { name } }","variables":{"id":"456"}}`)
ctx.Request().SetBody(query1)
hash1 := CalculateHash(ctx)
hash1 := CalculateHash(ctx, "user1", "admin")
ctx.Request().SetBody(query2)
hash2 := CalculateHash(ctx)
hash2 := CalculateHash(ctx, "user1", "admin")
assert.NotEqual(hash1, hash2, "Different variables should produce different cache keys")
})
@@ -66,13 +66,83 @@ func (suite *Tests) Test_CalculateHash() {
query2 := []byte(`{"query":"query GetUsers { users { name } }","variables":{}}`)
ctx.Request().SetBody(query1)
hash1 := CalculateHash(ctx)
hash1 := CalculateHash(ctx, "user1", "admin")
ctx.Request().SetBody(query2)
hash2 := CalculateHash(ctx)
hash2 := CalculateHash(ctx, "user1", "admin")
assert.NotEqual(hash1, hash2, "Query with and without variables object should produce different cache keys")
})
// SECURITY TEST: Different users should get different cache keys
suite.Run("different users produce different cache keys", func() {
// Same query, same variables, but different users - CRITICAL SECURITY TEST
query := []byte(`{"query":"query GetMyProfile { me { id email } }"}`)
ctx.Request().SetBody(query)
hash1 := CalculateHash(ctx, "user1", "admin")
hash2 := CalculateHash(ctx, "user2", "user")
assert.NotEqual(hash1, hash2, "Different users MUST produce different cache keys to prevent data leakage")
})
// SECURITY TEST: Same user should get same cache key
suite.Run("same user produces same cache key", func() {
// Same query, same user
query := []byte(`{"query":"query GetMyProfile { me { id email } }"}`)
ctx.Request().SetBody(query)
hash1 := CalculateHash(ctx, "user1", "admin")
hash2 := CalculateHash(ctx, "user1", "admin")
assert.Equal(hash1, hash2, "Same user should get same cache key for cache effectiveness")
})
// SECURITY TEST: Different roles should get different cache keys
suite.Run("different roles produce different cache keys", func() {
// Same query, same user ID, but different roles
query := []byte(`{"query":"query GetData { data { value } }"}`)
ctx.Request().SetBody(query)
hash1 := CalculateHash(ctx, "user1", "admin")
hash2 := CalculateHash(ctx, "user1", "user")
assert.NotEqual(hash1, hash2, "Different roles MUST produce different cache keys to prevent privilege escalation")
})
// SECURITY TEST: Empty user context should be normalized
suite.Run("empty user context is normalized", func() {
query := []byte(`{"query":"query GetPublic { public { data } }"}`)
ctx.Request().SetBody(query)
// Empty strings should be normalized to "-"
hash1 := CalculateHash(ctx, "", "")
hash2 := CalculateHash(ctx, "-", "-")
assert.Equal(hash1, hash2, "Empty user context should be normalized to prevent cache key collisions")
})
// BACKWARD COMPATIBILITY TEST: Legacy mode without user context
suite.Run("legacy mode without user context", func() {
// Setup config with per-user cache disabled
oldConfig := config
config = &CacheConfig{
Logger: libpack_logger.New(),
Client: libpack_cache_memory.New(5 * time.Minute),
TTL: 60,
PerUserCacheDisabled: true, // Disable per-user caching
}
defer func() { config = oldConfig }()
query := []byte(`{"query":"query GetData { data { value } }"}`)
ctx.Request().SetBody(query)
// In legacy mode, different users should get the SAME cache key (backward compatibility)
hash1 := CalculateHash(ctx, "user1", "admin")
hash2 := CalculateHash(ctx, "user2", "user")
assert.Equal(hash1, hash2, "With per-user cache disabled, all users get same cache key (backward compatibility)")
})
}
func (suite *Tests) Test_CacheDelete() {
@@ -112,8 +182,6 @@ func (suite *Tests) Test_CacheDelete() {
suite.Run("uninitialized cache", func() {
// Save current config
oldConfig := config
// Set config to nil
config = nil
// This should not cause any errors
@@ -156,8 +224,6 @@ func (suite *Tests) Test_CacheStoreWithTTL() {
suite.Run("uninitialized cache", func() {
// Save current config
oldConfig := config
// Set config to nil
config = nil
// This should not cause any errors
@@ -194,8 +260,6 @@ func (suite *Tests) Test_CacheGetQueries() {
suite.Run("uninitialized cache", func() {
// Save current config
oldConfig := config
// Set config to nil
config = nil
// This should return 0
@@ -280,8 +344,6 @@ func (suite *Tests) Test_GetCacheStats() {
suite.Run("uninitialized cache", func() {
// Save current config
oldConfig := config
// Set config to nil
config = nil
// This should return empty stats
+6 -4
View File
@@ -33,8 +33,9 @@ func (suite *CircuitBreakerTestSuite) TestCircuitBreakerCacheFallback() {
ctx := app.AcquireCtx(requestCtx)
defer app.ReleaseCtx(ctx)
// Calculate the cache key that would be used
cacheKey := libpack_cache.CalculateHash(ctx)
// Calculate the cache key that would be used (with default user context since no auth headers)
// extractUserInfo() returns ("-", "-") when no auth is present
cacheKey := libpack_cache.CalculateHash(ctx, "-", "-")
// Add a test response to the cache
cachedResponse := []byte(`{"data":{"test":"cached-response"}}`)
@@ -158,8 +159,9 @@ func (suite *CircuitBreakerTestSuite) TestCacheDisabledFallback() {
ctx := app.AcquireCtx(requestCtx)
defer app.ReleaseCtx(ctx)
// Calculate cache key and store a response
cacheKey := libpack_cache.CalculateHash(ctx)
// Calculate cache key and store a response (with default user context since no auth headers)
// extractUserInfo() returns ("-", "-") when no auth is present
cacheKey := libpack_cache.CalculateHash(ctx, "-", "-")
cachedResponse := []byte(`{"data":{"test":"cached-response"}}`)
libpack_cache.CacheStore(cacheKey, cachedResponse)
+2 -2
View File
@@ -8,7 +8,6 @@ import (
"time"
"github.com/gofiber/fiber/v2"
"github.com/gookit/goutil/strutil"
libpack_cache "github.com/lukaszraczylo/graphql-monitoring-proxy/cache"
libpack_monitoring "github.com/lukaszraczylo/graphql-monitoring-proxy/monitoring"
"github.com/sony/gobreaker"
@@ -115,7 +114,8 @@ func (suite *Tests) TestCachingAndCircuitBreakerInteraction() {
suite.Equal(responseBody, firstResponseBody, "Response body should match server response")
// Calculate hash the same way the system does, before releasing context
cacheKey := strutil.Md5(ctx.Body())
// Use default user context ("-", "-") since no auth headers are set in this test
cacheKey := libpack_cache.CalculateHash(ctx, "-", "-")
// Store in cache directly for test
libpack_cache.CacheStore(cacheKey, []byte(responseBody))
+24 -2
View File
@@ -133,6 +133,27 @@ func parseConfig() {
c.Cache.CacheMaxEntries = getDetailsFromEnv("CACHE_MAX_ENTRIES", 10000) // Default 10000 entries
// GraphQL query parsing cache - auto-calculate based on CPU cores if not set
c.Cache.GraphQLQueryCacheSize = getDetailsFromEnv("GRAPHQL_QUERY_CACHE_SIZE", runtime.GOMAXPROCS(0)*250)
// SECURITY: Per-user cache isolation (enabled by default for security)
// Set CACHE_PER_USER_DISABLED=true ONLY if you have a single-user application
// or understand the security implications of shared cache across users
c.Cache.PerUserCacheDisabled = getDetailsFromEnv("CACHE_PER_USER_DISABLED", false)
// Log warning if per-user caching is disabled
if c.Cache.PerUserCacheDisabled {
defer func() {
if c.Logger != nil {
c.Logger.Warning(&libpack_logging.LogMessage{
Message: "⚠️ Per-user cache isolation is DISABLED - Users may see each other's cached data!",
Pairs: map[string]interface{}{
"security_risk": "CRITICAL - Do not use in multi-user applications",
"recommendation": "Remove CACHE_PER_USER_DISABLED or set it to false",
},
})
}
}()
}
// Redis cache
c.Cache.CacheRedisEnable = getDetailsFromEnv("ENABLE_REDIS_CACHE", false)
c.Cache.CacheRedisURL = getDetailsFromEnv("CACHE_REDIS_URL", "localhost:6379")
@@ -355,8 +376,9 @@ func parseConfig() {
// Initialize cache if enabled
if cfg.Cache.CacheEnable || cfg.Cache.CacheRedisEnable {
cacheConfig := &libpack_cache.CacheConfig{
Logger: cfg.Logger,
TTL: cfg.Cache.CacheTTL,
Logger: cfg.Logger,
TTL: cfg.Cache.CacheTTL,
PerUserCacheDisabled: cfg.Cache.PerUserCacheDisabled,
}
// Redis cache configurations
if cfg.Cache.CacheRedisEnable {
+5 -2
View File
@@ -332,8 +332,11 @@ func performProxyRequest(c *fiber.Ctx, proxyURL string) error {
return performProxyRequestWithRetries(c, proxyURL)
}
// Calculate cache key for potential fallback
cacheKey := libpack_cache.CalculateHash(c)
// Extract user context for cache key (needed for circuit breaker fallback)
userID, userRole := extractUserInfo(c)
// Calculate cache key for potential fallback - includes user context for security
cacheKey := libpack_cache.CalculateHash(c, userID, userRole)
// Execute request through circuit breaker
_, err := cb.Execute(func() (interface{}, error) {
+5 -2
View File
@@ -327,8 +327,11 @@ func extractUserInfo(c *fiber.Ctx) (string, string) {
// handleCaching manages the caching logic for GraphQL requests
func handleCaching(c *fiber.Ctx, parsedResult *parseGraphQLQueryResult, userID string) (bool, error) {
// Calculate query hash for cache key
calculatedQueryHash := libpack_cache.CalculateHash(c)
// Extract user role for cache key (in addition to userID already passed)
_, userRole := extractUserInfo(c)
// Calculate query hash for cache key - now includes user context for security
calculatedQueryHash := libpack_cache.CalculateHash(c, userID, userRole)
// Set cache time from header or default
if parsedResult.cacheTime == 0 {
+2 -1
View File
@@ -44,7 +44,8 @@ type config struct {
CacheRedisEnable bool
CacheMaxMemorySize int
CacheMaxEntries int
GraphQLQueryCacheSize int // Max number of parsed GraphQL queries to cache
GraphQLQueryCacheSize int // Max number of parsed GraphQL queries to cache
PerUserCacheDisabled bool // Disable per-user cache isolation (SECURITY RISK - not recommended)
}
Client struct {
GQLClient *graphql.BaseClient