Skip to content

Commit 0469516

Browse files
committed
feat: add authorization monitoring metrics
Signed-off-by: Major Hayden <major@redhat.com>
1 parent ca125c4 commit 0469516

4 files changed

Lines changed: 187 additions & 30 deletions

File tree

src/authorization/middleware.py

Lines changed: 43 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Authorization middleware and decorators."""
22

3+
import time
34
from collections.abc import Callable
45
from functools import lru_cache, wraps
56
from typing import Any, Optional
@@ -18,6 +19,7 @@
1819
)
1920
from configuration import configuration
2021
from log import get_logger
22+
from metrics import recording
2123
from models.config import Action
2224
from models.responses import (
2325
ForbiddenResponse,
@@ -124,39 +126,50 @@ async def _perform_authorization_check(
124126
HTTPException: with 403 Forbidden if the resolved roles are not
125127
permitted to perform `action`.
126128
"""
127-
role_resolver, access_resolver = get_authorization_resolvers()
129+
start_time = time.monotonic()
130+
result = "error"
128131

129132
try:
130-
auth = kwargs["auth"]
131-
except KeyError as exc:
132-
logger.error(
133-
"Authorization only allowed on endpoints that accept "
134-
"'auth: Any = Depends(get_auth_dependency())'"
133+
role_resolver, access_resolver = get_authorization_resolvers()
134+
135+
try:
136+
auth = kwargs["auth"]
137+
except KeyError as exc:
138+
logger.error(
139+
"Authorization only allowed on endpoints that accept "
140+
"'auth: Any = Depends(get_auth_dependency())'"
141+
)
142+
response = InternalServerErrorResponse.generic()
143+
raise HTTPException(**response.model_dump()) from exc
144+
145+
# Everyone gets the everyone (aka *) role
146+
everyone_roles = {"*"}
147+
148+
user_roles = await role_resolver.resolve_roles(auth) | everyone_roles
149+
150+
if not access_resolver.check_access(action, user_roles):
151+
response = ForbiddenResponse.endpoint(user_id=auth[0])
152+
result = "denied"
153+
raise HTTPException(**response.model_dump())
154+
155+
authorized_actions = access_resolver.get_actions(user_roles)
156+
157+
req: Optional[Request] = None
158+
if "request" in kwargs and isinstance(kwargs["request"], Request):
159+
req = kwargs["request"]
160+
else:
161+
for arg in args:
162+
if isinstance(arg, Request):
163+
req = arg
164+
break
165+
if req is not None:
166+
req.state.authorized_actions = authorized_actions
167+
result = "success"
168+
finally:
169+
recording.record_authorization_check(action.value, result)
170+
recording.record_authorization_duration(
171+
action.value, result, time.monotonic() - start_time
135172
)
136-
response = InternalServerErrorResponse.generic()
137-
raise HTTPException(**response.model_dump()) from exc
138-
139-
# Everyone gets the everyone (aka *) role
140-
everyone_roles = {"*"}
141-
142-
user_roles = await role_resolver.resolve_roles(auth) | everyone_roles
143-
144-
if not access_resolver.check_access(action, user_roles):
145-
response = ForbiddenResponse.endpoint(user_id=auth[0])
146-
raise HTTPException(**response.model_dump())
147-
148-
authorized_actions = access_resolver.get_actions(user_roles)
149-
150-
req: Optional[Request] = None
151-
if "request" in kwargs and isinstance(kwargs["request"], Request):
152-
req = kwargs["request"]
153-
else:
154-
for arg in args:
155-
if isinstance(arg, Request):
156-
req = arg
157-
break
158-
if req is not None:
159-
req.state.authorized_actions = authorized_actions
160173

161174

162175
def authorize(action: Action) -> Callable:

src/metrics/__init__.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,17 @@
5555
"LLM tokens received",
5656
["provider", "model", "endpoint"],
5757
)
58+
59+
# Counter to track authorization checks by protected action.
60+
authorization_checks_total = Counter(
61+
"ls_authorization_checks_total",
62+
"Authorization checks",
63+
["action", "result"],
64+
)
65+
66+
# Histogram to measure authorization check latency by protected action.
67+
authorization_duration_seconds = Histogram(
68+
"ls_authorization_duration_seconds",
69+
"Authorization check duration",
70+
["action", "result"],
71+
)

src/metrics/recording.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,30 @@ def record_llm_token_usage(
109109
)
110110
except (AttributeError, TypeError, ValueError):
111111
logger.warning("Failed to update token metrics", exc_info=True)
112+
113+
114+
def record_authorization_check(action: str, result: str) -> None:
115+
"""Record one authorization check.
116+
117+
Args:
118+
action: Protected action name.
119+
result: Bounded result label, such as ``success`` or ``denied``.
120+
"""
121+
try:
122+
metrics.authorization_checks_total.labels(action, result).inc()
123+
except (AttributeError, TypeError, ValueError):
124+
logger.warning("Failed to update authorization metric", exc_info=True)
125+
126+
127+
def record_authorization_duration(action: str, result: str, duration: float) -> None:
128+
"""Record authorization check duration.
129+
130+
Args:
131+
action: Protected action name.
132+
result: Bounded result label, such as ``success`` or ``denied``.
133+
duration: Authorization check duration in seconds.
134+
"""
135+
try:
136+
metrics.authorization_duration_seconds.labels(action, result).observe(duration)
137+
except (AttributeError, TypeError, ValueError):
138+
logger.warning("Failed to update authorization duration metric", exc_info=True)

tests/unit/metrics/test_recording.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,38 @@
11
"""Unit tests for Prometheus metric recording helpers."""
22

3+
from collections.abc import Callable
4+
from dataclasses import dataclass
5+
from typing import Any
6+
7+
import pytest
38
from pytest_mock import MockerFixture
49

510
from metrics import recording
611

712

13+
@dataclass(frozen=True)
14+
class CounterRecorderCase:
15+
"""Expected behavior for a counter-style metric recorder."""
16+
17+
metric_path: str
18+
recorder: Callable[..., None]
19+
args: tuple[object, ...]
20+
labels: tuple[object, ...]
21+
warning_message: str
22+
23+
24+
@dataclass(frozen=True)
25+
class HistogramRecorderCase:
26+
"""Expected behavior for a histogram-style metric recorder."""
27+
28+
metric_path: str
29+
recorder: Callable[..., None]
30+
args: tuple[object, ...]
31+
labels: tuple[object, ...]
32+
duration: float
33+
warning_message: str
34+
35+
836
def test_measure_response_duration_records_timer(mocker: MockerFixture) -> None:
937
"""Test that response duration measurement uses the path label timer."""
1038
mock_timer = mocker.MagicMock()
@@ -159,3 +187,78 @@ def test_record_llm_token_usage_logs_metric_errors(mocker: MockerFixture) -> Non
159187
mock_logger.warning.assert_called_once_with(
160188
"Failed to update token metrics", exc_info=True
161189
)
190+
191+
192+
@pytest.fixture(name="recording_logger")
193+
def recording_logger_fixture(mocker: MockerFixture) -> Any:
194+
"""Patch the metric recording logger for failure assertions."""
195+
return mocker.patch("metrics.recording.logger")
196+
197+
198+
@pytest.mark.parametrize(
199+
"case",
200+
[
201+
CounterRecorderCase(
202+
metric_path="metrics.recording.metrics.authorization_checks_total",
203+
recorder=recording.record_authorization_check,
204+
args=("responses", "success"),
205+
labels=("responses", "success"),
206+
warning_message="Failed to update authorization metric",
207+
),
208+
],
209+
)
210+
def test_counter_recorders_update_metrics_and_log_errors(
211+
mocker: MockerFixture,
212+
recording_logger: Any,
213+
case: CounterRecorderCase,
214+
) -> None:
215+
"""Test new single-counter helpers with shared success and failure coverage."""
216+
mock_metric = mocker.patch(case.metric_path)
217+
218+
case.recorder(*case.args)
219+
220+
mock_metric.labels.assert_called_once_with(*case.labels)
221+
mock_metric.labels.return_value.inc.assert_called_once()
222+
223+
mock_metric.reset_mock()
224+
mock_metric.labels.return_value.inc.side_effect = AttributeError("missing")
225+
case.recorder(*case.args)
226+
227+
recording_logger.warning.assert_called_once_with(
228+
case.warning_message, exc_info=True
229+
)
230+
231+
232+
@pytest.mark.parametrize(
233+
"case",
234+
[
235+
HistogramRecorderCase(
236+
metric_path="metrics.recording.metrics.authorization_duration_seconds",
237+
recorder=recording.record_authorization_duration,
238+
args=("responses", "success", 0.25),
239+
labels=("responses", "success"),
240+
duration=0.25,
241+
warning_message="Failed to update authorization duration metric",
242+
),
243+
],
244+
)
245+
def test_histogram_recorders_observe_metrics_and_log_errors(
246+
mocker: MockerFixture,
247+
recording_logger: Any,
248+
case: HistogramRecorderCase,
249+
) -> None:
250+
"""Test new histogram helpers with shared success and failure coverage."""
251+
mock_metric = mocker.patch(case.metric_path)
252+
253+
case.recorder(*case.args)
254+
255+
mock_metric.labels.assert_called_once_with(*case.labels)
256+
mock_metric.labels.return_value.observe.assert_called_once_with(case.duration)
257+
258+
mock_metric.reset_mock()
259+
mock_metric.labels.return_value.observe.side_effect = TypeError("bad")
260+
case.recorder(*case.args)
261+
262+
recording_logger.warning.assert_called_once_with(
263+
case.warning_message, exc_info=True
264+
)

0 commit comments

Comments
 (0)