fix(cached): cached service update (delete or update method) clear cache key

Author: Athroniaeth

src/fastapi_api_key/services/cached.py

@@ -13,7 +13,7 @@
 
 from fastapi_api_key import ApiKeyService
 from fastapi_api_key.domain.base import D
-from fastapi_api_key.domain.errors import KeyNotProvided
+from fastapi_api_key.domain.errors import KeyNotProvided, InvalidKey, InvalidScopes, KeyNotFound
 from fastapi_api_key.hasher.base import ApiKeyHasher
 from fastapi_api_key.repositories.base import AbstractApiKeyRepository
 from fastapi_api_key.services.base import DEFAULT_SEPARATOR
@@ -47,26 +47,87 @@ def __init__(
     def _get_cache_key(self, key_id: str) -> str:
         return f"{self.cache_prefix}:{key_id}"
 
-    @staticmethod
-    def _hash_api_key(api_key: str) -> str:
+    def _hash_api_key(self, entity: D) -> str:
         """Hash the API key to use as cache key (don't store raw keys) with SHA256 (faster that Bcrypt)."""
-        buffer = api_key.encode()
+        # buffer = api_key.encode()
+        # _, key_prefix, _ = api_key.partition(DEFAULT_SEPARATOR)
+        # buffer = key_prefix.encode()
+        # return hashlib.sha256(buffer).hexdigest()
+        key_id, key_hash = entity.key_id, entity.key_hash
+        buffer = f"{key_id}{key_hash}".encode()
         return hashlib.sha256(buffer).hexdigest()
 
+    async def _initialize_cache(self, entity: D) -> D:
+        """Helper to initialize cache for an entity, to use after any update of the entity."""
+        hash_api_key = self._hash_api_key(entity)
+        await self.cache.delete(hash_api_key)
+        return entity
+
+    async def update(self, entity: D) -> D:
+        # Delete cache entry on update (useful when changing scopes or disabling)
+        entity = await super().update(entity)
+        return await self._initialize_cache(entity)
+
+    async def delete_by_id(self, id_: str) -> bool:
+        result = await self._repo.get_by_id(id_)
+
+        if result is None:
+            raise KeyNotFound(f"API key with ID '{id_}' not found")
+
+        # Delete cache entry on delete
+        # Todo: Found more optimized way to do this (delete_by_id return directly entity, or delete method)
+        await super().delete_by_id(id_)
+        await self._initialize_cache(result)
+        return True
+
     async def verify_key(self, api_key: Optional[str] = None, required_scopes: Optional[List[str]] = None) -> D:
+        required_scopes = required_scopes or []
+
         if api_key is None:
             raise KeyNotProvided("Api key must be provided (not given)")
 
-        hash_api_key = self._hash_api_key(api_key)
+        if api_key.strip() == "":
+            raise KeyNotProvided("Api key must be provided (empty)")
+
+        # Get the key_id part from the plain key
+        global_prefix, key_id, key_secret = self._get_parts(api_key)
+
+        # Global key_id "ak" for "api key"
+        if global_prefix != self.global_prefix:
+            raise InvalidKey("Api key is invalid (wrong global prefix)")
+
+        # Search entity by a key_id (can't brute force hashes)
+        entity = await self.get_by_key_id(key_id)
+
+        hash_api_key = self._hash_api_key(entity)
         cached_entity = await self.cache.get(hash_api_key)
 
         if cached_entity:
             return cached_entity
 
-        entity = await super().verify_key(
-            api_key=api_key,
-            required_scopes=required_scopes,
-        )
+        # Check if the entity can be used for authentication
+        # and refresh last_used_at if verified
+        entity.ensure_can_authenticate()
 
-        await self.cache.set(hash_api_key, entity)
-        return entity
+        key_hash = entity.key_hash
+
+        if not key_secret:
+            raise InvalidKey("API key is invalid (empty secret)")
+
+        if not self._hasher.verify(key_hash, key_secret):
+            raise InvalidKey("API key is invalid (hash mismatch)")
+
+        if required_scopes:
+            missing_scopes = [scope for scope in required_scopes if scope not in entity.scopes]
+            missing_scopes_str = ", ".join(missing_scopes)
+            if missing_scopes:
+                raise InvalidScopes(f"API key is missing required scopes: {missing_scopes_str}")
+
+        entity.touch()
+        updated = await self._repo.update(entity)
+
+        if updated is None:
+            raise KeyNotFound(f"API key with ID '{entity.id_}' not found during touch update")
+
+        await self.cache.set(hash_api_key, updated)
+        return updated

tests/units/test_svc.py

@@ -474,22 +474,13 @@ async def test_verify_key_hashes_key_and_writes_to_cache_on_miss(
       2) super().verify_key -> returns a fake entity
       3) Cache.set is called with the SHA256 hash key and the returned entity
     """
-    # Arrange
-    api_key = "super-secret-key"
-    expected_hash = hashlib.sha256(api_key.encode()).hexdigest()
-    fake_entity = {"id": "abc123"}
-
     # Mock cache with async get/set
-    cache = MagicMock()
+    cache = AsyncMock()
     cache.get = AsyncMock(return_value=None)
     cache.set = AsyncMock()
 
-    # Patch the parent class verify_key to avoid touching real repo logic
-    super_verify_mock = AsyncMock(return_value=fake_entity)
-    monkeypatch.setattr(ApiKeyService, "verify_key", super_verify_mock)
-
     # Build service
-    repo_mock = MagicMock()  # not used directly since we patched super().verify_key
+    repo_mock = InMemoryApiKeyRepository()
     service = CachedApiKeyService(
         repo=repo_mock,
         cache=cache,
@@ -500,21 +491,15 @@ async def test_verify_key_hashes_key_and_writes_to_cache_on_miss(
         global_prefix="ak",
     )
 
-    # Act
-    result = await service.verify_key(api_key)
+    entity = ApiKey(name="test")
+    entity, api_key = await service.create(entity)
 
-    # Assert
-    assert result == fake_entity
+    expected_hash = hashlib.sha256(f"{entity.key_id}{entity.key_hash}".encode()).hexdigest()
+    await service.verify_key(api_key)
 
     # cache.get must be called with the hashed key only (never the raw key)
     cache.get.assert_awaited_once_with(expected_hash)
-    assert not any(call_args[0][0] == api_key for call_args in cache.get.await_args_list)
-
-    # super().verify_key must be called once on miss
-    super_verify_mock.assert_awaited_once_with(api_key=api_key, required_scopes=None)
-
-    # cache.set must store under the hashed key
-    cache.set.assert_awaited_once_with(expected_hash, fake_entity)
+    cache.set.assert_awaited_once_with(expected_hash, entity)
 
 
 @pytest.mark.asyncio
@@ -523,71 +508,32 @@ async def test_verify_key_returns_cached_when_present(
     fixed_salt_hasher: ApiKeyHasher,
 ):
     """If the cache already contains the entity, return it and do NOT call the repo/super."""
-    # Arrange
-    api_key = "cached-key"
-    expected_hash = hashlib.sha256(api_key.encode()).hexdigest()
-    cached_entity = {"id": "in-cache"}
+    entity = ApiKey(name="test")
 
-    cache = MagicMock()
-    cache.get = AsyncMock(return_value=cached_entity)
+    cache = AsyncMock()
+    cache.get = AsyncMock(return_value=entity)
     cache.set = AsyncMock()
 
     # Even if patched, it must NOT be called in this scenario
     super_verify_mock = AsyncMock()
     monkeypatch.setattr(ApiKeyService, "verify_key", super_verify_mock)
 
-    repo_mock = MagicMock()
+    repo_mock = InMemoryApiKeyRepository()
     service = CachedApiKeyService(
         repo=repo_mock,
         cache=cache,
         hasher=fixed_salt_hasher,
     )
 
-    # Act
-    result = await service.verify_key(api_key)
+    entity, api_key = await service.create(entity)
+    expected_hash = hashlib.sha256(f"{entity.key_id}{entity.key_hash}".encode()).hexdigest()
 
-    # Assert
-    assert result == cached_entity
+    await service.verify_key(api_key)
     cache.get.assert_awaited_once_with(expected_hash)
     super_verify_mock.assert_not_awaited()
     cache.set.assert_not_awaited()
 
 
-@pytest.mark.asyncio
-async def test_verify_key_calls_super_on_cache_miss(
-    monkeypatch: pytest.MonkeyPatch,
-    fixed_salt_hasher: ApiKeyHasher,
-):
-    """On cache miss, the service must call super().verify_key and then populate the cache."""
-    # Arrange
-    api_key = "needs-lookup"
-    expected_hash = hashlib.sha256(api_key.encode()).hexdigest()
-    entity_from_repo = {"id": "from-repo"}
-
-    cache = MagicMock()
-    cache.get = AsyncMock(return_value=None)  # miss
-    cache.set = AsyncMock()
-
-    super_verify_mock = AsyncMock(return_value=entity_from_repo)
-    monkeypatch.setattr(ApiKeyService, "verify_key", super_verify_mock)
-
-    repo_mock = MagicMock()
-    service = CachedApiKeyService(
-        repo=repo_mock,
-        cache=cache,
-        hasher=fixed_salt_hasher,
-    )
-
-    # Act
-    result = await service.verify_key(api_key)
-
-    # Assert
-    assert result == entity_from_repo
-    cache.get.assert_awaited_once_with(expected_hash)
-    super_verify_mock.assert_awaited_once_with(api_key="needs-lookup", required_scopes=None)
-    cache.set.assert_awaited_once_with(expected_hash, entity_from_repo)
-
-
 @pytest.mark.asyncio
 async def test_verify_key_raises_when_missing_key(fixed_salt_hasher: ApiKeyHasher):
     """If no API key is provided, a KeyNotProvided error must be raised."""