diff --git a/app/api/routes/api_keys.py b/app/api/routes/api_keys.py index 8be9d6a..b6b8c9b 100644 --- a/app/api/routes/api_keys.py +++ b/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 diff --git a/app/core/async_cache.py b/app/core/async_cache.py index 43a88f0..c08e222 100644 --- a/app/core/async_cache.py +++ b/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: diff --git a/app/core/cache.py b/app/core/cache.py index b02913f..01e4d15 100644 --- a/app/core/cache.py +++ b/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""" diff --git a/tests/unit_tests/test_cache_invalidation.py b/tests/unit_tests/test_cache_invalidation.py new file mode 100644 index 0000000..6b6ac59 --- /dev/null +++ b/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" \ No newline at end of file