Fix cache invalidation bug for Forge key scope updates

app/api/routes/api_keys.py

@@ -13,7 +13,7 @@
     ForgeApiKeyResponse,
     ForgeApiKeyUpdate,
 )
-from app.core.cache import invalidate_provider_service_cache, invalidate_user_cache
+from app.core.cache import invalidate_provider_service_cache, invalidate_user_cache, invalidate_forge_scope_cache
 from app.core.database import get_db
 from app.core.security import generate_forge_api_key
 from app.models.forge_api_key import ForgeApiKey
@@ -137,6 +137,10 @@ def _update_api_key_internal(
     db.commit()
     db.refresh(db_api_key)
 
+    # Invalidate forge scope cache if the scope was updated
+    if api_key_update.allowed_provider_key_ids is not None:
+        invalidate_forge_scope_cache(db_api_key.key)
+
     response_data = db_api_key.__dict__.copy()
     response_data["allowed_provider_key_ids"] = [
         pk.id for pk in db_api_key.allowed_provider_keys
@@ -174,6 +178,7 @@ def _delete_api_key_internal(
     db.commit()
 
     invalidate_user_cache(key_to_invalidate)
+    invalidate_forge_scope_cache(key_to_invalidate)
     invalidate_provider_service_cache(current_user.id)
     return ForgeApiKeyResponse(**response_data)
 
@@ -195,6 +200,7 @@ def _regenerate_api_key_internal(
     # Invalidate caches for the old key
     old_key = db_api_key.key
     invalidate_user_cache(old_key)
+    invalidate_forge_scope_cache(old_key)
     invalidate_provider_service_cache(current_user.id)
 
     # Generate and set new key

app/core/async_cache.py

@@ -183,6 +183,29 @@ async def invalidate_user_cache_async(api_key: str) -> None:
     await async_user_cache.delete(f"user:{api_key}")
 
 
+async def invalidate_forge_scope_cache_async(api_key: str) -> None:
+    """Invalidate forge scope cache for a specific API key asynchronously.
+
+    Args:
+        api_key (str): The API key to invalidate cache for. Can include or exclude 'forge-' prefix.
+    """
+    if not api_key:
+        return
+
+    # The cache key format uses the API key WITHOUT the "forge-" prefix
+    # to match how it's set in get_user_by_api_key()
+    cache_key = api_key
+    if cache_key.startswith("forge-"):
+        cache_key = cache_key[6:]  # Remove "forge-" prefix to match cache setting format
+
+    await async_provider_service_cache.delete(f"forge_scope:{cache_key}")
+
+    if DEBUG_CACHE:
+        # Mask the API key for logging
+        masked_key = cache_key[:8] + "..." if len(cache_key) > 8 else cache_key
+        logger.debug(f"Cache: Invalidated forge scope cache for API key: {masked_key} (async)")
+
+
 async def invalidate_user_cache_by_id_async(user_id: int) -> None:
     """Invalidate all cache entries for a specific user ID asynchronously"""
     if not user_id:

app/core/cache.py

@@ -170,6 +170,29 @@ def invalidate_user_cache(api_key: str) -> None:
     user_cache.delete(f"user:{api_key}")
 
 
+def invalidate_forge_scope_cache(api_key: str) -> None:
+    """Invalidate forge scope cache for a specific API key.
+
+    Args:
+        api_key (str): The API key to invalidate cache for. Can include or exclude 'forge-' prefix.
+    """
+    if not api_key:
+        return
+
+    # The cache key format uses the API key WITHOUT the "forge-" prefix
+    # to match how it's set in get_user_by_api_key()
+    cache_key = api_key
+    if cache_key.startswith("forge-"):
+        cache_key = cache_key[6:]  # Remove "forge-" prefix to match cache setting format
+
+    provider_service_cache.delete(f"forge_scope:{cache_key}")
+
+    if DEBUG_CACHE:
+        # Mask the API key for logging
+        masked_key = cache_key[:8] + "..." if len(cache_key) > 8 else cache_key
+        logger.debug(f"Cache: Invalidated forge scope cache for API key: {masked_key}")
+
+
 # Provider service functions
 def get_cached_provider_service(user_id: int) -> Any:
     """Get a provider service from cache by user ID"""

tests/unit_tests/test_cache_invalidation.py

@@ -0,0 +1,117 @@
+"""
+Unit tests for cache invalidation behavior when Forge API key scope is updated.
+
+Tests the fix for issue #8: Newly added provider not reflected in allowed provider list for Forge key
+"""
+
+import asyncio
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from app.core.async_cache import invalidate_forge_scope_cache_async
+from app.core.cache import invalidate_forge_scope_cache
+
+
+class TestForgeKeyCacheInvalidation:
+    """Test cache invalidation for Forge API keys"""
+
+    def test_invalidate_forge_scope_cache_with_prefix(self):
+        """Test that cache invalidation works correctly with forge- prefix"""
+        # Mock the provider_service_cache
+        with patch('app.core.cache.provider_service_cache') as mock_cache:
+            # Test with full API key (including forge- prefix)
+            full_api_key = "forge-abc123def456"
+
+            invalidate_forge_scope_cache(full_api_key)
+
+            # Should strip the "forge-" prefix when creating cache key
+            expected_cache_key = "forge_scope:abc123def456"
+            mock_cache.delete.assert_called_once_with(expected_cache_key)
+
+    def test_invalidate_forge_scope_cache_without_prefix(self):
+        """Test that cache invalidation works correctly without forge- prefix"""
+        # Mock the provider_service_cache
+        with patch('app.core.cache.provider_service_cache') as mock_cache:
+            # Test with stripped API key (without forge- prefix)
+            stripped_api_key = "abc123def456"
+
+            invalidate_forge_scope_cache(stripped_api_key)
+
+            # Should use the key as-is when creating cache key
+            expected_cache_key = "forge_scope:abc123def456"
+            mock_cache.delete.assert_called_once_with(expected_cache_key)
+
+    def test_invalidate_forge_scope_cache_empty_key(self):
+        """Test that cache invalidation handles empty keys gracefully"""
+        # Mock the provider_service_cache
+        with patch('app.core.cache.provider_service_cache') as mock_cache:
+            # Test with empty API key
+            invalidate_forge_scope_cache("")
+
+            # Should not call delete for empty keys
+            mock_cache.delete.assert_not_called()
+
+    def test_invalidate_forge_scope_cache_none_key(self):
+        """Test that cache invalidation handles None keys gracefully"""
+        # Mock the provider_service_cache
+        with patch('app.core.cache.provider_service_cache') as mock_cache:
+            # Test with None API key
+            invalidate_forge_scope_cache(None)
+
+            # Should not call delete for None keys
+            mock_cache.delete.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_invalidate_forge_scope_cache_async_with_prefix(self):
+        """Test that async cache invalidation works correctly with forge- prefix"""
+        # Mock the async_provider_service_cache
+        with patch('app.core.async_cache.async_provider_service_cache') as mock_cache:
+            mock_cache.delete = AsyncMock()
+
+            # Test with full API key (including forge- prefix)
+            full_api_key = "forge-abc123def456"
+
+            await invalidate_forge_scope_cache_async(full_api_key)
+
+            # Should strip the "forge-" prefix when creating cache key
+            expected_cache_key = "forge_scope:abc123def456"
+            mock_cache.delete.assert_called_once_with(expected_cache_key)
+
+    @pytest.mark.asyncio
+    async def test_invalidate_forge_scope_cache_async_without_prefix(self):
+        """Test that async cache invalidation works correctly without forge- prefix"""
+        # Mock the async_provider_service_cache
+        with patch('app.core.async_cache.async_provider_service_cache') as mock_cache:
+            mock_cache.delete = AsyncMock()
+
+            # Test with stripped API key (without forge- prefix)
+            stripped_api_key = "abc123def456"
+
+            await invalidate_forge_scope_cache_async(stripped_api_key)
+
+            # Should use the key as-is when creating cache key
+            expected_cache_key = "forge_scope:abc123def456"
+            mock_cache.delete.assert_called_once_with(expected_cache_key)
+
+    def test_cache_key_format_consistency(self):
+        """Test that cache invalidation uses the same key format as cache setting"""
+        # This test verifies the fix for issue #8
+        # The bug was that cache was set with stripped key but invalidated with full key
+
+        with patch('app.core.cache.provider_service_cache') as mock_cache:
+            # Simulate the DB key format (with forge- prefix)
+            db_api_key = "forge-d8fc7c26e350771b28fe94b7"
+
+            # When we invalidate using the DB key
+            invalidate_forge_scope_cache(db_api_key)
+
+            # It should create the same cache key format used by get_user_by_api_key
+            # which strips the forge- prefix: api_key = api_key_from_header[6:]
+            stripped_key = db_api_key[6:]  # Remove "forge-" prefix
+            expected_cache_key = f"forge_scope:{stripped_key}"
+
+            mock_cache.delete.assert_called_once_with(expected_cache_key)
+
+            # Verify the exact cache key format
+            assert expected_cache_key == "forge_scope:d8fc7c26e350771b28fe94b7"