Skip to content

Commit f72c9ad

Browse files
fix(authz): skip cached checks for api key principals
1 parent 9a5b28f commit f72c9ad

4 files changed

Lines changed: 164 additions & 0 deletions

File tree

agentex/src/api/authentication_cache.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,12 @@ async def get_authorization_check(
319319
principal_context: Any,
320320
) -> bool | None:
321321
"""Get cached authorization check result."""
322+
if self._contains_api_key(principal_context):
323+
logger.debug(
324+
"Skipping authorization check cache lookup for API key principal"
325+
)
326+
return None
327+
322328
cache_key = self._create_authorization_cache_key(
323329
resource_type, resource_selector, operation, principal_context
324330
)
@@ -339,6 +345,12 @@ async def set_authorization_check(
339345
allowed: bool,
340346
) -> None:
341347
"""Cache authorization check result."""
348+
if self._contains_api_key(principal_context):
349+
logger.debug(
350+
"Skipping authorization check cache write for API key principal"
351+
)
352+
return
353+
342354
cache_key = self._create_authorization_cache_key(
343355
resource_type, resource_selector, operation, principal_context
344356
)
@@ -358,6 +370,11 @@ async def clear_all(self) -> None:
358370
await self.authorization_check_cache.clear()
359371
logger.info("All authentication and authorization caches cleared")
360372

373+
async def clear_authorization_checks(self) -> None:
374+
"""Clear cached authorization check results."""
375+
await self.authorization_check_cache.clear()
376+
logger.info("Authorization check cache cleared")
377+
361378
async def cleanup_expired(self) -> None:
362379
"""Remove expired entries from all caches."""
363380
await self.agent_identity_cache.remove_expired()

agentex/src/domain/services/authorization_service.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ def _bypass(self) -> bool:
3939
def is_enabled(self) -> bool:
4040
return self.enabled
4141

42+
async def _clear_authorization_cache(self) -> None:
43+
auth_cache = await get_auth_cache()
44+
await auth_cache.clear_authorization_checks()
45+
4246
async def grant(
4347
self, resource: AgentexResource, *, commit: bool = True, principal_context=...
4448
) -> None:
@@ -61,6 +65,7 @@ async def grant(
6165
resource,
6266
AuthorizedOperationType.create,
6367
)
68+
await self._clear_authorization_cache()
6469
return result
6570

6671
async def revoke(
@@ -87,6 +92,7 @@ async def revoke(
8792
logger.info(
8893
f"Revoked {AuthorizedOperationType.delete} permission on {resource.type}:{resource.selector}"
8994
)
95+
await self._clear_authorization_cache()
9096
return result
9197

9298
async def check(
@@ -214,6 +220,7 @@ async def register_resource(
214220
f"{parent.type}:{parent.selector}" if parent is not None else None,
215221
)
216222
await self.gateway.register_resource(effective_principal, resource, parent)
223+
await self._clear_authorization_cache()
217224

218225
async def deregister_resource(
219226
self,
@@ -237,6 +244,7 @@ async def deregister_resource(
237244
resource.selector,
238245
)
239246
await self.gateway.deregister_resource(effective_principal, resource)
247+
await self._clear_authorization_cache()
240248

241249

242250
DAuthorizationService = Annotated[AuthorizationService, Depends(AuthorizationService)]

agentex/tests/unit/api/test_authentication_cache_metrics.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,53 @@ async def test_auth_gateway_response_with_api_key_principal_is_not_cached():
102102
await cache.set_auth_gateway_response(headers, principal)
103103

104104
assert await cache.get_auth_gateway_response(headers) is None
105+
106+
107+
@pytest.mark.unit
108+
@pytest.mark.asyncio
109+
async def test_authorization_check_with_api_key_principal_is_not_cached():
110+
cache = AuthenticationCache()
111+
principal = {"user_id": "user-1", "account_id": "acct-1", "api_key": "secret-key"}
112+
113+
await cache.set_authorization_check(
114+
resource_type="agent",
115+
resource_selector="agent-1",
116+
operation="execute",
117+
principal_context=principal,
118+
allowed=True,
119+
)
120+
121+
assert (
122+
await cache.get_authorization_check(
123+
resource_type="agent",
124+
resource_selector="agent-1",
125+
operation="execute",
126+
principal_context=principal,
127+
)
128+
is None
129+
)
130+
131+
132+
@pytest.mark.unit
133+
@pytest.mark.asyncio
134+
async def test_authorization_check_without_api_key_principal_is_cached():
135+
cache = AuthenticationCache()
136+
principal = {"user_id": "user-1", "account_id": "acct-1"}
137+
138+
await cache.set_authorization_check(
139+
resource_type="agent",
140+
resource_selector="agent-1",
141+
operation="read",
142+
principal_context=principal,
143+
allowed=True,
144+
)
145+
146+
assert (
147+
await cache.get_authorization_check(
148+
resource_type="agent",
149+
resource_selector="agent-1",
150+
operation="read",
151+
principal_context=principal,
152+
)
153+
is True
154+
)
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
from types import SimpleNamespace
2+
from unittest.mock import AsyncMock
3+
4+
import pytest
5+
from src.api.authentication_cache import reset_auth_cache
6+
from src.api.schemas.authorization_types import AgentexResource, AuthorizedOperationType
7+
from src.domain.services.authorization_service import AuthorizationService
8+
9+
10+
def _request_with_principal(principal_context):
11+
return SimpleNamespace(
12+
state=SimpleNamespace(
13+
principal_context=principal_context,
14+
agent_identity=None,
15+
)
16+
)
17+
18+
19+
def _service(principal_context, gateway):
20+
return AuthorizationService(
21+
enabled=True,
22+
gateway=gateway,
23+
request=_request_with_principal(principal_context),
24+
)
25+
26+
27+
@pytest.mark.unit
28+
@pytest.mark.asyncio
29+
async def test_api_key_principal_authorization_check_calls_gateway_each_time():
30+
await reset_auth_cache()
31+
try:
32+
gateway = AsyncMock()
33+
gateway.check.return_value = True
34+
service = _service(
35+
{"user_id": "user-1", "account_id": "acct-1", "api_key": "secret-key"},
36+
gateway,
37+
)
38+
resource = AgentexResource.agent("agent-1")
39+
40+
assert await service.check(resource, AuthorizedOperationType.execute) is True
41+
assert await service.check(resource, AuthorizedOperationType.execute) is True
42+
43+
assert gateway.check.await_count == 2
44+
finally:
45+
await reset_auth_cache()
46+
47+
48+
@pytest.mark.unit
49+
@pytest.mark.asyncio
50+
async def test_non_api_key_principal_authorization_check_uses_cache():
51+
await reset_auth_cache()
52+
try:
53+
gateway = AsyncMock()
54+
gateway.check.return_value = True
55+
service = _service({"user_id": "user-1", "account_id": "acct-1"}, gateway)
56+
resource = AgentexResource.agent("agent-1")
57+
58+
assert await service.check(resource, AuthorizedOperationType.read) is True
59+
assert await service.check(resource, AuthorizedOperationType.read) is True
60+
61+
assert gateway.check.await_count == 1
62+
finally:
63+
await reset_auth_cache()
64+
65+
66+
@pytest.mark.unit
67+
@pytest.mark.asyncio
68+
@pytest.mark.parametrize(
69+
"mutation",
70+
["grant", "revoke", "register_resource", "deregister_resource"],
71+
)
72+
async def test_authorization_mutations_clear_cached_authorization_checks(mutation):
73+
await reset_auth_cache()
74+
try:
75+
gateway = AsyncMock()
76+
gateway.check.return_value = True
77+
service = _service({"user_id": "user-1", "account_id": "acct-1"}, gateway)
78+
resource = AgentexResource.agent("agent-1")
79+
80+
assert await service.check(resource, AuthorizedOperationType.read) is True
81+
assert await service.check(resource, AuthorizedOperationType.read) is True
82+
assert gateway.check.await_count == 1
83+
84+
await getattr(service, mutation)(resource)
85+
86+
assert await service.check(resource, AuthorizedOperationType.read) is True
87+
assert gateway.check.await_count == 2
88+
finally:
89+
await reset_auth_cache()

0 commit comments

Comments
 (0)