diff --git a/backend/app/core/metrics/__init__.py b/backend/app/core/metrics/__init__.py index 3f2d08b0..cbd09148 100644 --- a/backend/app/core/metrics/__init__.py +++ b/backend/app/core/metrics/__init__.py @@ -2,15 +2,11 @@ from app.core.metrics.connections import ConnectionMetrics from app.core.metrics.database import IdempotencyMetrics from app.core.metrics.events import EventMetrics -from app.core.metrics.rate_limit import RateLimitMetrics -from app.core.metrics.security import SecurityMetrics __all__ = [ "BaseMetrics", "ConnectionMetrics", "IdempotencyMetrics", "EventMetrics", - "RateLimitMetrics", - "SecurityMetrics", "setup_metrics", ] diff --git a/backend/app/core/metrics/rate_limit.py b/backend/app/core/metrics/rate_limit.py deleted file mode 100644 index a0059feb..00000000 --- a/backend/app/core/metrics/rate_limit.py +++ /dev/null @@ -1,44 +0,0 @@ -from app.core.metrics.base import BaseMetrics - - -class RateLimitMetrics(BaseMetrics): - """Metrics for rate limiting system.""" - - def _create_instruments(self) -> None: - self._requests = self._meter.create_counter( - name="rate_limit.requests.total", - description="Total number of rate limit checks", - unit="1", - ) - self._allowed = self._meter.create_counter( - name="rate_limit.allowed.total", - description="Number of allowed requests", - unit="1", - ) - self._rejected = self._meter.create_counter( - name="rate_limit.rejected.total", - description="Number of rejected requests", - unit="1", - ) - self._bypass = self._meter.create_counter( - name="rate_limit.bypass.total", - description="Number of bypassed rate limit checks", - unit="1", - ) - - def record_request(self, endpoint: str, authenticated: bool, algorithm: str) -> None: - """Record a rate limit check request.""" - attrs = {"endpoint": endpoint, "authenticated": str(authenticated).lower(), "algorithm": algorithm} - self._requests.add(1, attrs) - - def record_allowed(self, endpoint: str, group: str) -> None: - """Record an allowed request.""" - self._allowed.add(1, {"endpoint": endpoint, "group": group}) - - def record_rejected(self, endpoint: str, group: str) -> None: - """Record a rejected request.""" - self._rejected.add(1, {"endpoint": endpoint, "group": group}) - - def record_bypass(self, endpoint: str) -> None: - """Record a bypassed rate limit check.""" - self._bypass.add(1, {"endpoint": endpoint}) diff --git a/backend/app/core/metrics/security.py b/backend/app/core/metrics/security.py deleted file mode 100644 index e783de6e..00000000 --- a/backend/app/core/metrics/security.py +++ /dev/null @@ -1,113 +0,0 @@ -import time -from collections.abc import Generator -from contextlib import contextmanager - -from app.core.metrics.base import BaseMetrics - - -class SecurityMetrics(BaseMetrics): - """Metrics for security events.""" - - def _create_instruments(self) -> None: - # Authentication metrics - self.authentication_attempts = self._meter.create_counter( - name="authentication.attempts.total", description="Total number of authentication attempts", unit="1" - ) - - self.authentication_failures = self._meter.create_counter( - name="authentication.failures.total", description="Total number of failed authentications", unit="1" - ) - - self.authentication_duration = self._meter.create_histogram( - name="authentication.duration", description="Time taken for authentication in seconds", unit="s" - ) - - # Token metrics - self.token_validation_failures = self._meter.create_counter( - name="token.validation.failures.total", - description="Access tokens rejected while decoding, by reason", - unit="1", - ) - - # Authorization metrics - self.authorization_checks = self._meter.create_counter( - name="authorization.checks.total", description="Total number of authorization checks", unit="1" - ) - - self.authorization_denials = self._meter.create_counter( - name="authorization.denials.total", description="Total number of authorization denials", unit="1" - ) - - # CSRF protection metrics - self.csrf_validation_failures = self._meter.create_counter( - name="csrf.validation.failures.total", description="Total number of CSRF validation failures", unit="1" - ) - - # Password metrics - self.password_reset_requests = self._meter.create_counter( - name="password.reset.requests.total", description="Total number of password reset requests", unit="1" - ) - - self.weak_password_attempts = self._meter.create_counter( - name="weak.password.attempts.total", description="Total number of weak password attempts", unit="1" - ) - - # Brute force detection - self.brute_force_attempts = self._meter.create_counter( - name="brute.force.attempts.total", - description="Login attempts rejected because the account is locked", - unit="1", - ) - - self.accounts_locked = self._meter.create_counter( - name="accounts.locked.total", description="Total number of accounts locked due to security", unit="1" - ) - - @contextmanager - def track_authentication(self, method: str) -> Generator[None, None, None]: - start = time.monotonic() - success = False - try: - yield - success = True - finally: - self.authentication_attempts.add(1, attributes={"method": method, "success": str(success)}) - if not success: - self.authentication_failures.add(1, attributes={"method": method}) - self.authentication_duration.record(time.monotonic() - start, attributes={"method": method}) - - def record_token_validation_failure(self, reason: str) -> None: - self.token_validation_failures.add(1, attributes={"reason": reason}) - - def record_authorization_check( - self, resource: str, action: str, allowed: bool, user_role: str | None = None - ) -> None: - self.authorization_checks.add( - 1, - attributes={ - "resource": resource, - "action": action, - "allowed": str(allowed), - "user_role": user_role or "unknown", - }, - ) - - if not allowed: - self.authorization_denials.add( - 1, attributes={"resource": resource, "action": action, "user_role": user_role or "unknown"} - ) - - def record_csrf_validation_failure(self, reason: str) -> None: - self.csrf_validation_failures.add(1, attributes={"reason": reason}) - - def record_password_reset_request(self, method: str = "admin") -> None: - self.password_reset_requests.add(1, attributes={"method": method}) - - def record_weak_password_attempt(self, weakness_type: str) -> None: - self.weak_password_attempts.add(1, attributes={"weakness_type": weakness_type}) - - def record_brute_force_attempt(self) -> None: - self.brute_force_attempts.add(1) - - def record_account_locked(self, reason: str) -> None: - self.accounts_locked.add(1, attributes={"reason": reason}) diff --git a/backend/app/core/providers.py b/backend/app/core/providers.py index cd757089..405fe406 100644 --- a/backend/app/core/providers.py +++ b/backend/app/core/providers.py @@ -16,8 +16,6 @@ ConnectionMetrics, EventMetrics, IdempotencyMetrics, - RateLimitMetrics, - SecurityMetrics, ) from app.core.security import SecurityService from app.core.tracing import Tracer @@ -143,18 +141,18 @@ def get_rate_limit_service( self, redis_client: redis.Redis, settings: Settings, - rate_limit_metrics: RateLimitMetrics, + tracer: trace.Tracer, logger: structlog.stdlib.BoundLogger, ) -> RateLimitService: - return RateLimitService(redis_client, settings, rate_limit_metrics, logger) + return RateLimitService(redis_client, settings, tracer, logger) class CoreServicesProvider(Provider): scope = Scope.APP @provide - def get_security_service(self, settings: Settings, security_metrics: SecurityMetrics) -> SecurityService: - return SecurityService(settings, security_metrics) + def get_security_service(self, settings: Settings, tracer: trace.Tracer) -> SecurityService: + return SecurityService(settings, tracer) @provide def get_tracer( @@ -257,18 +255,10 @@ def get_event_metrics(self, settings: Settings) -> EventMetrics: def get_connection_metrics(self, settings: Settings) -> ConnectionMetrics: return ConnectionMetrics(settings) - @provide - def get_rate_limit_metrics(self, settings: Settings) -> RateLimitMetrics: - return RateLimitMetrics(settings) - @provide def get_idempotency_metrics(self, settings: Settings) -> IdempotencyMetrics: return IdempotencyMetrics(settings) - @provide - def get_security_metrics(self, settings: Settings) -> SecurityMetrics: - return SecurityMetrics(settings) - class RepositoryProvider(Provider): """Provides all repository instances. @@ -360,7 +350,7 @@ def get_auth_service( self, user_repository: UserRepository, security_service: SecurityService, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, logger: structlog.stdlib.BoundLogger, lockout_service: LoginLockoutService, runtime_settings: RuntimeSettingsLoader, @@ -370,7 +360,7 @@ def get_auth_service( return AuthService( user_repo=user_repository, security_service=security_service, - security_metrics=security_metrics, + tracer=tracer, logger=logger, lockout_service=lockout_service, runtime_settings=runtime_settings, @@ -560,7 +550,7 @@ def get_admin_user_service( execution_service: ExecutionService, rate_limit_service: RateLimitService, security_service: SecurityService, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, logger: structlog.stdlib.BoundLogger, ) -> AdminUserService: return AdminUserService( @@ -569,7 +559,7 @@ def get_admin_user_service( execution_service=execution_service, rate_limit_service=rate_limit_service, security_service=security_service, - security_metrics=security_metrics, + tracer=tracer, logger=logger, ) diff --git a/backend/app/core/security.py b/backend/app/core/security.py index 66790bb7..b6e25a82 100644 --- a/backend/app/core/security.py +++ b/backend/app/core/security.py @@ -6,11 +6,11 @@ import jwt from fastapi import Request from fastapi.security import OAuth2PasswordBearer +from opentelemetry import trace from pwdlib import PasswordHash from pwdlib.hashers.bcrypt import BcryptHasher from pydantic import TypeAdapter, ValidationError -from app.core.metrics import SecurityMetrics from app.domain.user import CSRFValidationError, InvalidCredentialsError from app.settings import Settings @@ -30,9 +30,9 @@ class AccessTokenClaims(SubjectClaims): class SecurityService: - def __init__(self, settings: Settings, security_metrics: SecurityMetrics) -> None: + def __init__(self, settings: Settings, tracer: trace.Tracer) -> None: self.settings = settings - self._security_metrics = security_metrics + self._tracer = tracer # --8<-- [start:password_hashing] self._password_hash = PasswordHash((BcryptHasher(rounds=self.settings.BCRYPT_ROUNDS),)) @@ -69,18 +69,18 @@ def decode_token(self, token: str, *, allow_expired: bool = False) -> str: options={"verify_exp": not allow_expired}, ) except jwt.ExpiredSignatureError: - self._security_metrics.record_token_validation_failure("expired") + self._tracer.start_span("auth.token_rejected", attributes={"auth.reason": "expired"}).end() raise InvalidCredentialsError() from None except jwt.InvalidSignatureError: - self._security_metrics.record_token_validation_failure("invalid_signature") + self._tracer.start_span("auth.token_rejected", attributes={"auth.reason": "invalid_signature"}).end() raise InvalidCredentialsError() from None except jwt.PyJWTError: - self._security_metrics.record_token_validation_failure("invalid") + self._tracer.start_span("auth.token_rejected", attributes={"auth.reason": "invalid"}).end() raise InvalidCredentialsError() from None try: claims = _subject_adapter.validate_python(payload) except ValidationError: - self._security_metrics.record_token_validation_failure("missing_subject") + self._tracer.start_span("auth.token_rejected", attributes={"auth.reason": "missing_subject"}).end() raise InvalidCredentialsError() from None return claims["sub"] @@ -175,15 +175,15 @@ def validate_csrf_from_request(self, request: Request) -> str: cookie_token = request.cookies.get("csrf_token", "") if not header_token: - self._security_metrics.record_csrf_validation_failure("missing_header") + self._tracer.start_span("auth.csrf_rejected", attributes={"auth.reason": "missing_header"}).end() raise CSRFValidationError("CSRF token missing from X-CSRF-Token header") if not self.validate_csrf_token(header_token, cookie_token): - self._security_metrics.record_csrf_validation_failure("token_mismatch") + self._tracer.start_span("auth.csrf_rejected", attributes={"auth.reason": "token_mismatch"}).end() raise CSRFValidationError("CSRF token invalid or does not match cookie") if not self._verify_csrf_signature(header_token, access_token): - self._security_metrics.record_csrf_validation_failure("invalid_signature") + self._tracer.start_span("auth.csrf_rejected", attributes={"auth.reason": "invalid_signature"}).end() raise CSRFValidationError("CSRF token signature invalid") return header_token diff --git a/backend/app/services/admin/admin_user_service.py b/backend/app/services/admin/admin_user_service.py index f6f0de6b..88969cc0 100644 --- a/backend/app/services/admin/admin_user_service.py +++ b/backend/app/services/admin/admin_user_service.py @@ -2,8 +2,8 @@ from datetime import datetime, timedelta, timezone import structlog +from opentelemetry import trace -from app.core.metrics import SecurityMetrics from app.core.security import SecurityService from app.db import UserRepository from app.domain.admin import AdminUserOverviewDomain, DerivedCountsDomain @@ -39,7 +39,7 @@ def __init__( execution_service: ExecutionService, rate_limit_service: RateLimitService, security_service: SecurityService, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, logger: structlog.stdlib.BoundLogger, ) -> None: self._users = user_repository @@ -47,7 +47,7 @@ def __init__( self._executions = execution_service self._rate_limits = rate_limit_service self._security = security_service - self._security_metrics = security_metrics + self._tracer = tracer self.logger = logger async def get_user_overview(self, user_id: str, hours: int = 24) -> AdminUserOverviewDomain: @@ -266,7 +266,7 @@ async def delete_user(self, *, admin_user_id: str, user_id: str, cascade: bool) async def reset_user_password(self, *, admin_user_id: str, user_id: str, new_password: str) -> None: self.logger.info("Admin resetting user password", admin_user_id=admin_user_id, target_user_id=user_id) - self._security_metrics.record_password_reset_request(method="admin") + self._tracer.start_span("auth.password_reset", attributes={"auth.method": "admin"}).end() hashed = self._security.get_password_hash(new_password) pr = PasswordReset(user_id=user_id, new_password=hashed) if not await self._users.reset_user_password(pr): diff --git a/backend/app/services/auth_service.py b/backend/app/services/auth_service.py index 68be201d..09b5b04b 100644 --- a/backend/app/services/auth_service.py +++ b/backend/app/services/auth_service.py @@ -3,8 +3,9 @@ import structlog from fastapi import Request +from opentelemetry import trace +from opentelemetry.util.types import AttributeValue -from app.core.metrics import SecurityMetrics from app.core.security import SecurityService from app.db import UserRepository from app.domain.enums import LoginMethod, UserRole @@ -43,7 +44,7 @@ def __init__( self, user_repo: UserRepository, security_service: SecurityService, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, logger: structlog.stdlib.BoundLogger, lockout_service: LoginLockoutService, runtime_settings: RuntimeSettingsLoader, @@ -52,7 +53,7 @@ def __init__( ): self.user_repo = user_repo self.security_service = security_service - self.security_metrics = security_metrics + self._tracer = tracer self.logger = logger self._lockout = lockout_service self._runtime_settings = runtime_settings @@ -84,12 +85,13 @@ async def get_current_user(self, request: Request) -> User: async def get_admin(self, request: Request) -> User: user = await self.get_current_user(request) - self.security_metrics.record_authorization_check( - "/admin", - request.method, - user.role == UserRole.ADMIN, - user_role=user.role, - ) + attributes: dict[str, AttributeValue] = { + "auth.resource": "/admin", + "auth.action": request.method, + "auth.allowed": user.role == UserRole.ADMIN, + "auth.user_role": user.role, + } + self._tracer.start_span("auth.authorization", attributes=attributes).end() if user.role != UserRole.ADMIN: self.logger.warning("Admin access denied", username=user.username, role=user.role) raise AdminAccessRequiredError(user.username) @@ -121,7 +123,7 @@ async def _fail_login( key=username, ) if locked: - self.security_metrics.record_account_locked("brute_force") + self._tracer.start_span("auth.account_locked", attributes={"auth.reason": "brute_force"}).end() raise AccountLockedError("Account locked due to too many failed attempts") raise InvalidCredentialsError() @@ -133,10 +135,11 @@ async def login( user_agent: str | None, ) -> LoginResult: if await self._lockout.check_locked(username): - self.security_metrics.record_brute_force_attempt() + self._tracer.start_span("auth.locked_login").end() raise AccountLockedError("Account temporarily locked due to too many failed attempts") - with self.security_metrics.track_authentication("login"): + login_attributes: dict[str, AttributeValue] = {"auth.method": "login", "auth.success": False} + with self._tracer.start_as_current_span("auth.login", attributes=login_attributes) as span: user = await self.user_repo.get_user(username) if not user: @@ -186,6 +189,7 @@ async def login( key=user.username, ) + span.set_attribute("auth.success", True) return LoginResult( username=user.username, role=user.role, @@ -205,7 +209,7 @@ async def register( effective = await self._runtime_settings.get_effective_settings() min_len = effective.password_min_length if len(password) < min_len: - self.security_metrics.record_weak_password_attempt("too_short") + self._tracer.start_span("auth.weak_password", attributes={"auth.weakness_type": "too_short"}).end() raise ValidationError(f"Password must be at least {min_len} characters") existing = await self.user_repo.get_user(username) diff --git a/backend/app/services/rate_limit_service.py b/backend/app/services/rate_limit_service.py index 93a44364..95bee6aa 100644 --- a/backend/app/services/rate_limit_service.py +++ b/backend/app/services/rate_limit_service.py @@ -9,9 +9,10 @@ import redis.asyncio as redis import structlog +from opentelemetry import trace +from opentelemetry.util.types import AttributeValue from pydantic import Json, TypeAdapter -from app.core.metrics import RateLimitMetrics from app.domain.rate_limit import ( EndpointUsageStats, RateLimitAlgorithm, @@ -34,20 +35,28 @@ def __init__( self, redis_client: redis.Redis, settings: Settings, - metrics: RateLimitMetrics, + tracer: trace.Tracer, logger: structlog.stdlib.BoundLogger, ): self.redis = redis_client self.settings = settings self.logger = logger self.prefix = settings.RATE_LIMIT_REDIS_PREFIX - self.metrics = metrics + self._tracer = tracer self._pattern_cache: dict[str, re.Pattern[str]] = {} # Patterns to match IDs and replace with * self._uuid_pattern = re.compile(r"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}") self._id_pattern = re.compile(r"/[0-9a-zA-Z]{20,}(?=/|$)") + def _report_request(self, endpoint: str, authenticated: bool, algorithm: str) -> None: + attributes: dict[str, AttributeValue] = { + "rate_limit.endpoint": endpoint, + "rate_limit.authenticated": authenticated, + "rate_limit.algorithm": algorithm, + } + self._tracer.start_span("rate_limit.request", attributes=attributes).end() + def _index_key(self, user_id: str) -> str: """Key of the Redis set that indexes all per-user rate limit state keys.""" return f"{self.prefix}index:{user_id}" @@ -88,9 +97,7 @@ async def check_rate_limit(self, user_id: str, endpoint: str) -> RateLimitStatus return await self._check(user_id, endpoint, await self._get_config()) except redis.RedisError as e: self.logger.error("Rate limit check failed, allowing the request", endpoint=endpoint, error=str(e)) - self.metrics.record_request( - self._normalize_endpoint(endpoint), not user_id.startswith("ip:"), "redis_error" - ) + self._report_request(self._normalize_endpoint(endpoint), not user_id.startswith("ip:"), "redis_error") return self._unlimited() async def _check(self, user_id: str, endpoint: str, config: RateLimitConfig) -> RateLimitStatus: @@ -102,21 +109,20 @@ async def _check(self, user_id: str, endpoint: str, config: RateLimitConfig) -> user_config = self._user_limit(config, user_id) if user_config.bypass_rate_limit: - self.metrics.record_bypass(normalized_endpoint) - self.metrics.record_request(normalized_endpoint, authenticated, "bypassed") + self._report_request(normalized_endpoint, authenticated, "bypassed") return self._unlimited() for rule in self._matching_rules(endpoint, user_config, config): return await self._apply_rule(user_id, endpoint, rule, user_config.global_multiplier) - self.metrics.record_request(normalized_endpoint, authenticated, "no_limit") + self._report_request(normalized_endpoint, authenticated, "no_limit") return self._unlimited() async def _apply_rule(self, user_id: str, endpoint: str, rule: RateLimitRule, multiplier: float) -> RateLimitStatus: normalized_endpoint = self._normalize_endpoint(endpoint) effective_limit = int(rule.requests * multiplier) - self.metrics.record_request(normalized_endpoint, not user_id.startswith("ip:"), rule.algorithm) + self._report_request(normalized_endpoint, not user_id.startswith("ip:"), rule.algorithm) if rule.algorithm == RateLimitAlgorithm.TOKEN_BUCKET: status = await self._check_token_bucket( @@ -125,10 +131,12 @@ async def _apply_rule(self, user_id: str, endpoint: str, rule: RateLimitRule, mu else: status = await self._check_sliding_window(user_id, endpoint, effective_limit, rule.window_seconds, rule) - if status.allowed: - self.metrics.record_allowed(normalized_endpoint, rule.group) - else: - self.metrics.record_rejected(normalized_endpoint, rule.group) + attributes: dict[str, AttributeValue] = { + "rate_limit.endpoint": normalized_endpoint, + "rate_limit.group": rule.group, + "rate_limit.allowed": status.allowed, + } + self._tracer.start_span("rate_limit.decision", attributes=attributes).end() return status diff --git a/backend/grafana/provisioning/dashboards/rate-limiting-dashboard.json b/backend/grafana/provisioning/dashboards/rate-limiting-dashboard.json index be01119b..63b3ae4e 100644 --- a/backend/grafana/provisioning/dashboards/rate-limiting-dashboard.json +++ b/backend/grafana/provisioning/dashboards/rate-limiting-dashboard.json @@ -364,13 +364,13 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum(rate(rate_limit_requests_total{endpoint!~\"/api/v1/result/.*\"}[5m])) by (endpoint)", - "legendFormat": "{{endpoint}}", + "expr": "sum(rate(rate_limit_requests_total{rate_limit_endpoint!~\"/api/v1/result/.*\"}[5m])) by (rate_limit_endpoint)", + "legendFormat": "{{rate_limit_endpoint}}", "refId": "A", "datasource": "Victoria Metrics" }, { - "expr": "sum(rate(rate_limit_requests_total{endpoint=~\"/api/v1/result/.*\"}[5m]))", + "expr": "sum(rate(rate_limit_requests_total{rate_limit_endpoint=~\"/api/v1/result/.*\"}[5m]))", "legendFormat": "/api/v1/result/*", "refId": "B", "datasource": "Victoria Metrics" @@ -451,8 +451,8 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum(rate(rate_limit_requests_total[5m])) by (authenticated)", - "legendFormat": "authenticated={{authenticated}}", + "expr": "sum(rate(rate_limit_requests_total[5m])) by (rate_limit_authenticated)", + "legendFormat": "authenticated={{rate_limit_authenticated}}", "refId": "A", "datasource": "Victoria Metrics" } @@ -535,8 +535,8 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum(increase(rate_limit_requests_total[1h])) by (algorithm)", - "legendFormat": "{{algorithm}}", + "expr": "sum(increase(rate_limit_requests_total[1h])) by (rate_limit_algorithm)", + "legendFormat": "{{rate_limit_algorithm}}", "refId": "A", "datasource": "Victoria Metrics" } @@ -616,14 +616,14 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum(rate(rate_limit_allowed_total[5m])) by (group)", - "legendFormat": "allowed: {{group}}", + "expr": "sum(rate(rate_limit_allowed_total[5m])) by (rate_limit_group)", + "legendFormat": "allowed: {{rate_limit_group}}", "refId": "A", "datasource": "Victoria Metrics" }, { - "expr": "sum(rate(rate_limit_rejected_total[5m])) by (group)", - "legendFormat": "rejected: {{group}}", + "expr": "sum(rate(rate_limit_rejected_total[5m])) by (rate_limit_group)", + "legendFormat": "rejected: {{rate_limit_group}}", "refId": "B", "datasource": "Victoria Metrics" } @@ -717,8 +717,8 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum(rate(rate_limit_bypass_total[5m])) by (endpoint)", - "legendFormat": "{{endpoint}}", + "expr": "sum(rate(rate_limit_bypass_total[5m])) by (rate_limit_endpoint)", + "legendFormat": "{{rate_limit_endpoint}}", "refId": "A", "datasource": "Victoria Metrics" } @@ -798,8 +798,8 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum(rate(rate_limit_rejected_total[5m])) by (endpoint)", - "legendFormat": "{{endpoint}}", + "expr": "sum(rate(rate_limit_rejected_total[5m])) by (rate_limit_endpoint)", + "legendFormat": "{{rate_limit_endpoint}}", "refId": "A", "datasource": "Victoria Metrics" } diff --git a/backend/grafana/provisioning/dashboards/security-auth.json b/backend/grafana/provisioning/dashboards/security-auth.json index 11e2286d..2f18defc 100644 --- a/backend/grafana/provisioning/dashboards/security-auth.json +++ b/backend/grafana/provisioning/dashboards/security-auth.json @@ -486,8 +486,8 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum by (reason) (rate(token_validation_failures_total[5m]))", - "legendFormat": "{{reason}}", + "expr": "sum by (auth_reason) (rate(token_validation_failures_total[5m]))", + "legendFormat": "{{auth_reason}}", "refId": "C", "datasource": "Victoria Metrics" } @@ -567,8 +567,8 @@ "pluginVersion": "8.3.3", "targets": [ { - "expr": "sum by (reason) (rate(csrf_validation_failures_total[5m]))", - "legendFormat": "{{reason}}", + "expr": "sum by (auth_reason) (rate(csrf_validation_failures_total[5m]))", + "legendFormat": "{{auth_reason}}", "refId": "B", "datasource": "Victoria Metrics" } diff --git a/backend/otel-collector-config.yaml b/backend/otel-collector-config.yaml index 3063e00b..8a14b9de 100644 --- a/backend/otel-collector-config.yaml +++ b/backend/otel-collector-config.yaml @@ -620,6 +620,164 @@ connectors: - span.name == "replay.report" gauge: value: Int(span.attributes["replay.queued_events"]) + - name: authentication.attempts + description: Password logins, by success + unit: "{attempt}" + attributes: + - key: auth.method + - key: auth.success + conditions: + - span.name == "auth.login" + sum: + value: "1" + monotonic: true + - name: authentication.failures + description: Password logins that failed + unit: "{attempt}" + attributes: + - key: auth.method + conditions: + - span.name == "auth.login" and span.attributes["auth.success"] == false + sum: + value: "1" + monotonic: true + - name: authentication.duration + description: Time to check a password login + unit: s + attributes: + - key: auth.method + conditions: + - span.name == "auth.login" + histogram: + buckets: [0.01, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5] + value: Seconds(span.end_time - span.start_time) + - name: token.validation.failures + description: Access tokens rejected, by reason + unit: "{token}" + attributes: + - key: auth.reason + conditions: + - span.name == "auth.token_rejected" + sum: + value: "1" + monotonic: true + - name: csrf.validation.failures + description: Requests rejected by the CSRF check, by reason + unit: "{request}" + attributes: + - key: auth.reason + conditions: + - span.name == "auth.csrf_rejected" + sum: + value: "1" + monotonic: true + - name: authorization.checks + description: Admin route checks, by result + unit: "{check}" + attributes: + - key: auth.resource + - key: auth.action + - key: auth.allowed + - key: auth.user_role + conditions: + - span.name == "auth.authorization" + sum: + value: "1" + monotonic: true + - name: authorization.denials + description: Admin route checks that denied access + unit: "{check}" + attributes: + - key: auth.resource + - key: auth.action + - key: auth.user_role + conditions: + - span.name == "auth.authorization" and span.attributes["auth.allowed"] == false + sum: + value: "1" + monotonic: true + - name: password.reset.requests + description: Password resets + unit: "{reset}" + attributes: + - key: auth.method + conditions: + - span.name == "auth.password_reset" + sum: + value: "1" + monotonic: true + - name: weak.password.attempts + description: Registrations refused for a weak password + unit: "{attempt}" + attributes: + - key: auth.weakness_type + conditions: + - span.name == "auth.weak_password" + sum: + value: "1" + monotonic: true + - name: brute.force.attempts + description: Logins against an account that is already locked + unit: "{attempt}" + conditions: + - span.name == "auth.locked_login" + sum: + value: "1" + monotonic: true + - name: accounts.locked + description: Accounts locked after too many failed logins + unit: "{account}" + attributes: + - key: auth.reason + conditions: + - span.name == "auth.account_locked" + sum: + value: "1" + monotonic: true + - name: rate_limit.requests + description: Requests the rate limiter checked, by algorithm + unit: "{request}" + attributes: + - key: rate_limit.endpoint + - key: rate_limit.authenticated + - key: rate_limit.algorithm + conditions: + - span.name == "rate_limit.request" + sum: + value: "1" + monotonic: true + - name: rate_limit.allowed + description: Requests a rate limit rule let through + unit: "{request}" + attributes: + - key: rate_limit.endpoint + - key: rate_limit.group + conditions: + - span.name == "rate_limit.decision" and span.attributes["rate_limit.allowed"] == true + sum: + value: "1" + monotonic: true + - name: rate_limit.rejected + description: Requests a rate limit rule rejected + unit: "{request}" + attributes: + - key: rate_limit.endpoint + - key: rate_limit.group + conditions: + - span.name == "rate_limit.decision" and span.attributes["rate_limit.allowed"] == false + sum: + value: "1" + monotonic: true + - name: rate_limit.bypass + description: Requests from users who bypass rate limits + unit: "{request}" + attributes: + - key: rate_limit.endpoint + conditions: + - span.name == "rate_limit.request" and span.attributes["rate_limit.algorithm"] == "bypassed" + sum: + value: "1" + monotonic: true processors: batch: diff --git a/backend/tests/e2e/conftest.py b/backend/tests/e2e/conftest.py index 82220b81..a623f552 100644 --- a/backend/tests/e2e/conftest.py +++ b/backend/tests/e2e/conftest.py @@ -13,7 +13,6 @@ from httpx2 import AsyncClient from starlette.types import ASGIApp, Message, Scope -from app.core.metrics import RateLimitMetrics from app.core.security import SecurityService from app.db.docs import EventDocument, NotificationDocument from app.db.docs.saga import SagaDocument @@ -265,11 +264,6 @@ async def admin_events_repository(scope: AsyncContainer) -> AdminEventsRepositor return await scope.get(AdminEventsRepository) -@pytest.fixture -async def rate_limit_metrics(scope: AsyncContainer) -> RateLimitMetrics: - return await scope.get(RateLimitMetrics) - - @pytest.fixture def exec_request() -> ExecutionRequest: """Simple python print execution.""" diff --git a/backend/tests/e2e/services/rate_limit/test_rate_limit_service.py b/backend/tests/e2e/services/rate_limit/test_rate_limit_service.py index 12ed4489..4eb59f4c 100644 --- a/backend/tests/e2e/services/rate_limit/test_rate_limit_service.py +++ b/backend/tests/e2e/services/rate_limit/test_rate_limit_service.py @@ -4,8 +4,8 @@ import pytest import redis.asyncio as redis import structlog +from opentelemetry import trace -from app.core.metrics import RateLimitMetrics from app.domain.rate_limit import ( EndpointGroup, RateLimitAlgorithm, @@ -20,12 +20,10 @@ @pytest.fixture -async def svc( - redis_client: redis.Redis, rate_limit_metrics: RateLimitMetrics, test_settings: Settings -) -> RateLimitService: +async def svc(redis_client: redis.Redis, test_settings: Settings) -> RateLimitService: """A service of its own, so a test's config and counters live under a prefix no other test reads.""" settings = test_settings.model_copy(update={"RATE_LIMIT_REDIS_PREFIX": f"rate_limit_test:{uuid4().hex}:"}) - return RateLimitService(redis_client, settings, rate_limit_metrics, structlog.get_logger("test")) + return RateLimitService(redis_client, settings, trace.NoOpTracer(), structlog.get_logger("test")) async def test_normalize_and_bypass_and_no_rule(svc: RateLimitService) -> None: @@ -177,7 +175,7 @@ async def test_sliding_window_pipeline_failure(test_settings: Settings) -> None: client.sadd = AsyncMock() client.pipeline = MagicMock() client.pipeline.return_value.execute = AsyncMock(side_effect=ConnectionError("Pipeline failed")) - svc = RateLimitService(client, test_settings, MagicMock(spec=RateLimitMetrics), structlog.get_logger("test")) + svc = RateLimitService(client, test_settings, trace.NoOpTracer(), structlog.get_logger("test")) rule = RateLimitRule( endpoint_pattern=r"^/api", group=EndpointGroup.API, @@ -211,7 +209,7 @@ async def test_token_bucket_invalid_data(svc: RateLimitService) -> None: async def test_update_config_serialization_error(test_settings: Settings) -> None: client = AsyncMock(spec=redis.Redis) client.set.side_effect = ValueError("Serialization failed") - svc = RateLimitService(client, test_settings, MagicMock(spec=RateLimitMetrics), structlog.get_logger("test")) + svc = RateLimitService(client, test_settings, trace.NoOpTracer(), structlog.get_logger("test")) with pytest.raises(ValueError): await svc.update_config(RateLimitConfig(default_rules=[])) @@ -230,7 +228,7 @@ async def test_user_without_override_gets_the_defaults(svc: RateLimitService) -> async def test_reset_user_limits_error(test_settings: Settings) -> None: client = AsyncMock(spec=redis.Redis) client.smembers.side_effect = ConnectionError("smembers failed") - svc = RateLimitService(client, test_settings, MagicMock(spec=RateLimitMetrics), structlog.get_logger("test")) + svc = RateLimitService(client, test_settings, trace.NoOpTracer(), structlog.get_logger("test")) with pytest.raises(ConnectionError): await svc.reset_user_limits("user") diff --git a/backend/tests/unit/conftest.py b/backend/tests/unit/conftest.py index 35f3e81d..4c1a4982 100644 --- a/backend/tests/unit/conftest.py +++ b/backend/tests/unit/conftest.py @@ -26,8 +26,6 @@ ConnectionMetrics, EventMetrics, IdempotencyMetrics, - RateLimitMetrics, - SecurityMetrics, ) from app.domain.enums import ExecutionStatus, QueuePriority from app.domain.execution import DomainExecution @@ -237,16 +235,6 @@ def event_metrics(test_settings: Settings) -> EventMetrics: return EventMetrics(test_settings) -@pytest.fixture -def rate_limit_metrics(test_settings: Settings) -> RateLimitMetrics: - return RateLimitMetrics(test_settings) - - -@pytest.fixture -def security_metrics(test_settings: Settings) -> SecurityMetrics: - return SecurityMetrics(test_settings) - - @pytest.fixture def db() -> NoReturn: raise RuntimeError("Unit tests should not access DB; use mocks, or move the test to tests/e2e") diff --git a/backend/tests/unit/core/metrics/test_replay_and_security_metrics.py b/backend/tests/unit/core/metrics/test_replay_and_security_metrics.py deleted file mode 100644 index 4a325a26..00000000 --- a/backend/tests/unit/core/metrics/test_replay_and_security_metrics.py +++ /dev/null @@ -1,22 +0,0 @@ -from unittest.mock import MagicMock - -import pytest - -from app.core.metrics import SecurityMetrics -from app.settings import Settings - -pytestmark = pytest.mark.unit - - -def test_track_authentication_counts_a_raised_login_as_failure(test_settings: Settings) -> None: - m = SecurityMetrics(test_settings) - m.authentication_attempts = MagicMock() - m.authentication_failures = MagicMock() - m.authentication_duration = MagicMock() - - with pytest.raises(ValueError), m.track_authentication("login"): - raise ValueError("bad password") - - m.authentication_attempts.add.assert_called_once_with(1, attributes={"method": "login", "success": "False"}) - m.authentication_failures.add.assert_called_once_with(1, attributes={"method": "login"}) - m.authentication_duration.record.assert_called_once() diff --git a/backend/tests/unit/core/test_csrf.py b/backend/tests/unit/core/test_csrf.py index 20da3522..cbcdea71 100644 --- a/backend/tests/unit/core/test_csrf.py +++ b/backend/tests/unit/core/test_csrf.py @@ -1,7 +1,7 @@ import pytest +from opentelemetry import trace from starlette.requests import Request -from app.core.metrics import SecurityMetrics from app.core.security import SecurityService from app.domain.user import CSRFValidationError from app.settings import Settings @@ -30,9 +30,9 @@ def make_request( class TestCSRFTokenGeneration: """Tests for CSRF token generation.""" - def test_generates_signed_token_format(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_generates_signed_token_format(self, test_settings: Settings, tracer: trace.Tracer) -> None: """CSRF token has nonce.signature format.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token = security.generate_csrf_token("session-abc") @@ -42,9 +42,9 @@ def test_generates_signed_token_format(self, test_settings: Settings, security_m assert len(nonce) > 0 assert len(signature) == 64 # sha256 hexdigest - def test_generates_unique_tokens(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_generates_unique_tokens(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Each CSRF token is unique (different nonce each time).""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) tokens = {security.generate_csrf_token("same-session") for _ in range(100)} @@ -53,10 +53,10 @@ def test_generates_unique_tokens(self, test_settings: Settings, security_metrics def test_different_sessions_produce_different_tokens( self, test_settings: Settings, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, ) -> None: """Tokens for different sessions differ even with same nonce derivation.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token_a = security.generate_csrf_token("session-a") token_b = security.generate_csrf_token("session-b") @@ -67,18 +67,18 @@ def test_different_sessions_produce_different_tokens( class TestCSRFTokenValidation: """Tests for CSRF token validation (double-submit check).""" - def test_validates_matching_tokens(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_validates_matching_tokens(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Matching CSRF tokens pass validation.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token = security.generate_csrf_token("session-1") result = security.validate_csrf_token(token, token) assert result is True - def test_rejects_mismatched_tokens(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_rejects_mismatched_tokens(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Mismatched CSRF tokens fail validation.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token1 = security.generate_csrf_token("session-1") token2 = security.generate_csrf_token("session-2") @@ -99,12 +99,12 @@ def test_rejects_mismatched_tokens(self, test_settings: Settings, security_metri def test_rejects_empty_tokens( self, test_settings: Settings, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, header_token: str, cookie_token: str, ) -> None: """Empty CSRF tokens fail validation.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) result = security.validate_csrf_token(header_token, cookie_token) @@ -114,29 +114,29 @@ def test_rejects_empty_tokens( class TestCSRFSignatureVerification: """Tests for CSRF HMAC signature verification.""" - def test_valid_signature_passes(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_valid_signature_passes(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Token verified against the same session_id succeeds.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token = security.generate_csrf_token("my-session") assert security._verify_csrf_signature(token, "my-session") is True - def test_wrong_session_rejected(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_wrong_session_rejected(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Token signed for session A fails verification against session B.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token = security.generate_csrf_token("session-a") assert security._verify_csrf_signature(token, "session-b") is False - def test_unsigned_token_rejected(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_unsigned_token_rejected(self, test_settings: Settings, tracer: trace.Tracer) -> None: """A plain random string without signature structure is rejected.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) assert security._verify_csrf_signature("plain-random-token", "session") is False - def test_tampered_signature_rejected(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_tampered_signature_rejected(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Modifying the signature portion causes rejection.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token = security.generate_csrf_token("session-x") nonce, sig = token.split(".", 1) @@ -151,23 +151,23 @@ class TestCSRFExemptPaths: def test_exempt_paths_includes_login_and_register( self, test_settings: Settings, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, ) -> None: """CSRF exempt paths include login and register.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) assert "/api/v1/auth/login" in security.CSRF_EXEMPT_PATHS assert "/api/v1/auth/register" in security.CSRF_EXEMPT_PATHS - def test_logout_is_not_exempt(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_logout_is_not_exempt(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Logout is NOT exempt from CSRF validation.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) assert "/api/v1/auth/logout" not in security.CSRF_EXEMPT_PATHS - def test_exempt_paths_is_frozenset(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_exempt_paths_is_frozenset(self, test_settings: Settings, tracer: trace.Tracer) -> None: """CSRF exempt paths is a frozenset (immutable).""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) assert isinstance(security.CSRF_EXEMPT_PATHS, frozenset) @@ -175,9 +175,9 @@ def test_exempt_paths_is_frozenset(self, test_settings: Settings, security_metri class TestCSRFRequestValidation: """Tests for CSRF validation from HTTP requests.""" - def test_skips_get_requests(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_skips_get_requests(self, test_settings: Settings, tracer: trace.Tracer) -> None: """GET requests skip CSRF validation.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) req = make_request("GET", "/api/v1/anything") assert security.validate_csrf_from_request(req) == "skip" @@ -185,10 +185,10 @@ def test_skips_get_requests(self, test_settings: Settings, security_metrics: Sec def test_missing_header_raises_when_authenticated( self, test_settings: Settings, - security_metrics: SecurityMetrics, + tracer: trace.Tracer, ) -> None: """Missing CSRF header raises error for authenticated POST.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) req = make_request( "POST", "/api/v1/items", @@ -198,9 +198,9 @@ def test_missing_header_raises_when_authenticated( with pytest.raises(CSRFValidationError): security.validate_csrf_from_request(req) - def test_valid_tokens_pass(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_valid_tokens_pass(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Valid signed CSRF tokens pass full request validation.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) access_token = "my-access-token-value" token = security.generate_csrf_token(access_token) req = make_request( @@ -212,9 +212,9 @@ def test_valid_tokens_pass(self, test_settings: Settings, security_metrics: Secu assert security.validate_csrf_from_request(req) == token - def test_forged_token_rejected(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_forged_token_rejected(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Unsigned token matching in header+cookie is rejected (signature check).""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) forged = "forged-random-value" req = make_request( "POST", @@ -226,9 +226,9 @@ def test_forged_token_rejected(self, test_settings: Settings, security_metrics: with pytest.raises(CSRFValidationError, match="signature invalid"): security.validate_csrf_from_request(req) - def test_wrong_session_token_rejected(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_wrong_session_token_rejected(self, test_settings: Settings, tracer: trace.Tracer) -> None: """Token signed for one session rejected when presented with different access_token.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) token = security.generate_csrf_token("session-A-jwt") req = make_request( "POST", @@ -240,9 +240,9 @@ def test_wrong_session_token_rejected(self, test_settings: Settings, security_me with pytest.raises(CSRFValidationError, match="signature invalid"): security.validate_csrf_from_request(req) - def test_logout_requires_csrf(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: + def test_logout_requires_csrf(self, test_settings: Settings, tracer: trace.Tracer) -> None: """POST to logout with authentication requires CSRF token.""" - security = SecurityService(test_settings, security_metrics) + security = SecurityService(test_settings, tracer) req = make_request( "POST", "/api/v1/auth/logout", diff --git a/backend/tests/unit/core/test_rate_limit_middleware.py b/backend/tests/unit/core/test_rate_limit_middleware.py index 5c07d27f..2b33d5aa 100644 --- a/backend/tests/unit/core/test_rate_limit_middleware.py +++ b/backend/tests/unit/core/test_rate_limit_middleware.py @@ -3,10 +3,10 @@ import pytest from dishka import AsyncContainer +from opentelemetry import trace from starlette.responses import PlainTextResponse from starlette.types import Message, Receive, Scope, Send -from app.core.metrics import SecurityMetrics from app.core.middlewares import RateLimitMiddleware from app.core.security import SecurityService from app.domain.rate_limit import RateLimitStatus @@ -80,19 +80,17 @@ def _middleware(service: AsyncMock, security: SecurityService, settings: Setting ], ) async def test_only_health_and_auth_routes_skip_rate_limiting( - test_settings: Settings, security_metrics: SecurityMetrics, path: str, expected_status: int, checked: bool + test_settings: Settings, tracer: trace.Tracer, path: str, expected_status: int, checked: bool ) -> None: service = _rate_limit_service(allowed=False) - middleware = _middleware(service, SecurityService(test_settings, security_metrics), test_settings) + middleware = _middleware(service, SecurityService(test_settings, tracer), test_settings) assert await _request(middleware, path) == expected_status assert service.check_rate_limit.await_count == int(checked) -async def test_a_valid_token_keys_the_request_by_user_id( - test_settings: Settings, security_metrics: SecurityMetrics -) -> None: - security = SecurityService(test_settings, security_metrics) +async def test_a_valid_token_keys_the_request_by_user_id(test_settings: Settings, tracer: trace.Tracer) -> None: + security = SecurityService(test_settings, tracer) token = security.create_access_token({"sub": "u", "user_id": "user-1"}, expires_delta=timedelta(minutes=5)) service = _rate_limit_service(allowed=True) @@ -104,10 +102,10 @@ async def test_a_valid_token_keys_the_request_by_user_id( @pytest.mark.parametrize("cookies", ["access_token=forged.token.value", ""], ids=["forged_token", "anonymous"]) async def test_other_requests_are_keyed_by_client_ip( - test_settings: Settings, security_metrics: SecurityMetrics, cookies: str + test_settings: Settings, tracer: trace.Tracer, cookies: str ) -> None: service = _rate_limit_service(allowed=True) - middleware = _middleware(service, SecurityService(test_settings, security_metrics), test_settings) + middleware = _middleware(service, SecurityService(test_settings, tracer), test_settings) assert await _request(middleware, "/api/v1/execute", cookies) == 200 service.check_rate_limit.assert_awaited_once_with(user_id="ip:203.0.113.7", endpoint="/api/v1/execute") diff --git a/backend/tests/unit/core/test_security.py b/backend/tests/unit/core/test_security.py index 961cce4a..bc4aae1d 100644 --- a/backend/tests/unit/core/test_security.py +++ b/backend/tests/unit/core/test_security.py @@ -1,12 +1,12 @@ import time from datetime import datetime, timedelta, timezone -from unittest.mock import MagicMock from uuid import uuid4 import jwt import pytest +from opentelemetry import trace +from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter -from app.core.metrics import SecurityMetrics from app.core.security import SecurityService from app.domain.enums import UserRole from app.domain.user import InvalidCredentialsError @@ -17,9 +17,9 @@ class TestPasswordHashing: """Test password hashing functionality.""" @pytest.fixture - def security_svc(self, test_settings: Settings, security_metrics: SecurityMetrics) -> SecurityService: + def security_svc(self, test_settings: Settings, tracer: trace.Tracer) -> SecurityService: """Create SecurityService instance.""" - return SecurityService(test_settings, security_metrics) + return SecurityService(test_settings, tracer) def test_password_hash_creates_different_hash(self, security_svc: SecurityService) -> None: """Test that password hashing creates unique hashes.""" @@ -74,9 +74,9 @@ class TestSecurityService: """Test SecurityService functionality.""" @pytest.fixture - def security_service(self, test_settings: Settings, security_metrics: SecurityMetrics) -> SecurityService: + def security_service(self, test_settings: Settings, tracer: trace.Tracer) -> SecurityService: """Create SecurityService instance using test settings.""" - return SecurityService(test_settings, security_metrics) + return SecurityService(test_settings, tracer) def test_create_access_token_basic(self, security_service: SecurityService) -> None: """Test basic access token creation.""" @@ -172,8 +172,8 @@ class TestDecodeToken: """Test SecurityService.decode_token method.""" @pytest.fixture - def security_service(self, test_settings: Settings, security_metrics: SecurityMetrics) -> SecurityService: - return SecurityService(test_settings, security_metrics) + def security_service(self, test_settings: Settings, tracer: trace.Tracer) -> SecurityService: + return SecurityService(test_settings, tracer) def test_valid_token_returns_username(self, security_service: SecurityService) -> None: token = security_service.create_access_token({"sub": "testuser"}, expires_delta=timedelta(minutes=15)) @@ -225,9 +225,10 @@ def test_rejected_token_records_reason( lifetime: timedelta, key: str | None, reason: str, + tracer: trace.Tracer, + span_exporter: InMemorySpanExporter, ) -> None: - metrics = MagicMock(spec=SecurityMetrics) - svc = SecurityService(test_settings, metrics) + svc = SecurityService(test_settings, tracer) token = ( "not.a.token" if claims is None @@ -241,22 +242,24 @@ def test_rejected_token_records_reason( with pytest.raises(InvalidCredentialsError): svc.decode_token(token) - metrics.record_token_validation_failure.assert_called_once_with(reason) + [span] = span_exporter.get_finished_spans() + assert (span.name, span.attributes) == ("auth.token_rejected", {"auth.reason": reason}) - def test_valid_token_records_no_failure(self, test_settings: Settings) -> None: - metrics = MagicMock(spec=SecurityMetrics) - svc = SecurityService(test_settings, metrics) + def test_valid_token_records_no_failure( + self, test_settings: Settings, tracer: trace.Tracer, span_exporter: InMemorySpanExporter + ) -> None: + svc = SecurityService(test_settings, tracer) svc.decode_token(svc.create_access_token({"sub": "u"}, expires_delta=timedelta(minutes=5))) - metrics.record_token_validation_failure.assert_not_called() + assert span_exporter.get_finished_spans() == () class TestDecodeUserId: """SecurityService.decode_user_id gives an id only for a valid token that carries one.""" - def test_valid_token_returns_user_id(self, test_settings: Settings, security_metrics: SecurityMetrics) -> None: - svc = SecurityService(test_settings, security_metrics) + def test_valid_token_returns_user_id(self, test_settings: Settings, tracer: trace.Tracer) -> None: + svc = SecurityService(test_settings, tracer) token = svc.create_access_token({"sub": "u", "user_id": "user-1"}, expires_delta=timedelta(minutes=5)) assert svc.decode_user_id(token) == "user-1" @@ -272,10 +275,15 @@ def test_valid_token_returns_user_id(self, test_settings: Settings, security_met ids=["expired", "wrong_signature", "no_user_id", "malformed"], ) def test_unusable_token_raises_without_counting_a_failure( - self, test_settings: Settings, claims: dict[str, object] | None, lifetime: timedelta, key: str | None + self, + test_settings: Settings, + tracer: trace.Tracer, + span_exporter: InMemorySpanExporter, + claims: dict[str, object] | None, + lifetime: timedelta, + key: str | None, ) -> None: - metrics = MagicMock(spec=SecurityMetrics) - svc = SecurityService(test_settings, metrics) + svc = SecurityService(test_settings, tracer) token = ( "not.a.token" if claims is None @@ -288,4 +296,4 @@ def test_unusable_token_raises_without_counting_a_failure( with pytest.raises(InvalidCredentialsError): svc.decode_user_id(token) - metrics.record_token_validation_failure.assert_not_called() + assert span_exporter.get_finished_spans() == () diff --git a/backend/tests/unit/services/test_auth_service_metrics.py b/backend/tests/unit/services/test_auth_service_metrics.py index 4d7db09e..d0fe021a 100644 --- a/backend/tests/unit/services/test_auth_service_metrics.py +++ b/backend/tests/unit/services/test_auth_service_metrics.py @@ -2,11 +2,13 @@ import pytest import structlog +from opentelemetry import trace +from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter -from app.core.metrics import SecurityMetrics from app.core.security import SecurityService from app.db import UserRepository from app.domain.exceptions import AccountLockedError +from app.domain.user import InvalidCredentialsError from app.events import UnifiedProducer from app.services.auth_service import AuthService from app.services.login_lockout import LoginLockoutService @@ -16,15 +18,11 @@ pytestmark = pytest.mark.unit -async def test_login_on_locked_account_counts_a_blocked_attempt(test_settings: Settings) -> None: - metrics = MagicMock(spec=SecurityMetrics) - lockout = AsyncMock(spec=LoginLockoutService) - lockout.check_locked.return_value = True - user_repo = AsyncMock(spec=UserRepository) - service = AuthService( +def _service(test_settings: Settings, tracer: trace.Tracer, lockout: AsyncMock, user_repo: AsyncMock) -> AuthService: + return AuthService( user_repo=user_repo, security_service=MagicMock(spec=SecurityService), - security_metrics=metrics, + tracer=tracer, logger=structlog.get_logger("test.auth_service_metrics"), lockout_service=lockout, runtime_settings=AsyncMock(spec=RuntimeSettingsLoader), @@ -32,9 +30,34 @@ async def test_login_on_locked_account_counts_a_blocked_attempt(test_settings: S settings=test_settings, ) + +async def test_login_on_locked_account_counts_a_blocked_attempt( + test_settings: Settings, tracer: trace.Tracer, span_exporter: InMemorySpanExporter +) -> None: + lockout = AsyncMock(spec=LoginLockoutService) + lockout.check_locked.return_value = True + user_repo = AsyncMock(spec=UserRepository) + service = _service(test_settings, tracer, lockout, user_repo) + with pytest.raises(AccountLockedError): await service.login("alice", "secret", ip_address="10.0.0.1", user_agent=None) - metrics.record_brute_force_attempt.assert_called_once_with() - metrics.track_authentication.assert_not_called() + assert [span.name for span in span_exporter.get_finished_spans()] == ["auth.locked_login"] user_repo.get_user.assert_not_awaited() + + +async def test_failed_login_reports_an_unsuccessful_attempt( + test_settings: Settings, tracer: trace.Tracer, span_exporter: InMemorySpanExporter +) -> None: + lockout = AsyncMock(spec=LoginLockoutService) + lockout.check_locked.return_value = False + lockout.record_failed_attempt.return_value = False + user_repo = AsyncMock(spec=UserRepository) + user_repo.get_user.return_value = None + service = _service(test_settings, tracer, lockout, user_repo) + + with pytest.raises(InvalidCredentialsError): + await service.login("alice", "secret", ip_address="10.0.0.1", user_agent=None) + + [span] = span_exporter.get_finished_spans() + assert (span.name, span.attributes) == ("auth.login", {"auth.method": "login", "auth.success": False}) diff --git a/backend/tests/unit/services/test_rate_limit_service.py b/backend/tests/unit/services/test_rate_limit_service.py index 7dcfd184..cee371e2 100644 --- a/backend/tests/unit/services/test_rate_limit_service.py +++ b/backend/tests/unit/services/test_rate_limit_service.py @@ -1,10 +1,11 @@ -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock import pytest import redis.asyncio as redis import structlog +from opentelemetry import trace +from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter -from app.core.metrics import RateLimitMetrics from app.services.rate_limit_service import RateLimitService from app.settings import Settings @@ -12,13 +13,22 @@ @pytest.mark.parametrize("user_id", ["user-1", "ip:203.0.113.7"]) -async def test_redis_failure_lets_the_request_through(test_settings: Settings, user_id: str) -> None: +async def test_redis_failure_lets_the_request_through( + test_settings: Settings, tracer: trace.Tracer, span_exporter: InMemorySpanExporter, user_id: str +) -> None: client = AsyncMock(spec=redis.Redis) client.get.side_effect = redis.ConnectionError("Redis is down") - metrics = MagicMock(spec=RateLimitMetrics) - service = RateLimitService(client, test_settings, metrics, structlog.get_logger("test")) + service = RateLimitService(client, test_settings, tracer, structlog.get_logger("test")) status = await service.check_rate_limit(user_id, "/api/v1/execute") assert status.allowed is True - metrics.record_request.assert_called_once_with("/api/v1/execute", not user_id.startswith("ip:"), "redis_error") + [span] = span_exporter.get_finished_spans() + assert (span.name, span.attributes) == ( + "rate_limit.request", + { + "rate_limit.endpoint": "/api/v1/execute", + "rate_limit.authenticated": not user_id.startswith("ip:"), + "rate_limit.algorithm": "redis_error", + }, + ) diff --git a/docs/operations/metrics-reference.md b/docs/operations/metrics-reference.md index 5167a16b..66d27dfa 100644 --- a/docs/operations/metrics-reference.md +++ b/docs/operations/metrics-reference.md @@ -24,7 +24,7 @@ The collector's metrics pipeline (`backend/otel-collector-config.yaml`) reads th ## Metrics made from spans -Some metrics have no instrument in the code. A service puts the facts on a span, and the collector's `signaltometrics` connector (alpha in collector v0.162.0) turns them into counters, histograms and gauges, defined under `connectors` in `backend/otel-collector-config.yaml`. The SDK sends every span (`TRACING_SAMPLING_RATE = 1.0`), and the traces pipeline gives each span to the connector before `tail_sampling` decides what Jaeger keeps, so the counts don't depend on sampling. The connector emits a point only when a matching span arrives. Sent straight to VictoriaMetrics, a counter that sees one event every half hour would get one point every half hour, and `increase` and `rate` stop finding the previous point once two points are more than about 8 to 10 minutes apart (measured on v1.153.0): `increase` then counts the whole total again, and `rate` returns nothing. So counters and histograms go another way. `deltatocumulative` adds up the connector's deltas and keeps each total through 24 hours without spans (its default is 5 minutes, after which an idle counter would start again from zero). The collector's `prometheus` exporter serves the totals on `otel-collector:8889`, and VictoriaMetrics scrapes it every 15 seconds (`backend/victoria-metrics/scrape.yaml`, with `honor_labels` so each service's `job` and `instance` stay). A series the exporter hasn't updated for 24 hours drops out until its next span. Gauges still go by remote write, so a gauge goes stale when the job that reports it on a timer stops. Panels wrap counter rates in `or vector(0)` so a series that hasn't started yet reads 0. The execution, pod, queue, dead letter queue, notification and replay metrics work this way so far ([#389](https://github.com/HardMax71/Integr8sCode/issues/389)). +Some metrics have no instrument in the code. A service puts the facts on a span, and the collector's `signaltometrics` connector (alpha in collector v0.162.0) turns them into counters, histograms and gauges, defined under `connectors` in `backend/otel-collector-config.yaml`. The SDK sends every span (`TRACING_SAMPLING_RATE = 1.0`), and the traces pipeline gives each span to the connector before `tail_sampling` decides what Jaeger keeps, so the counts don't depend on sampling. The connector emits a point only when a matching span arrives. Sent straight to VictoriaMetrics, a counter that sees one event every half hour would get one point every half hour, and `increase` and `rate` stop finding the previous point once two points are more than about 8 to 10 minutes apart (measured on v1.153.0): `increase` then counts the whole total again, and `rate` returns nothing. So counters and histograms go another way. `deltatocumulative` adds up the connector's deltas and keeps each total through 24 hours without spans (its default is 5 minutes, after which an idle counter would start again from zero). The collector's `prometheus` exporter serves the totals on `otel-collector:8889`, and VictoriaMetrics scrapes it every 15 seconds (`backend/victoria-metrics/scrape.yaml`, with `honor_labels` so each service's `job` and `instance` stay). A series the exporter hasn't updated for 24 hours drops out until its next span. Gauges still go by remote write, so a gauge goes stale when the job that reports it on a timer stops. Panels wrap counter rates in `or vector(0)` so a series that hasn't started yet reads 0. The execution, pod, queue, dead letter queue, notification, replay, security and rate limit metrics work this way so far ([#389](https://github.com/HardMax71/Integr8sCode/issues/389)). ## Names and labels in VictoriaMetrics @@ -156,30 +156,32 @@ The status snapshot a stream sends first comes from the database, not Redis, and ### Authentication and authorization (backend) -| Metric | Type | Labels | Recorded in | -|--------|------|--------|-------------| -| `authentication_attempts_total` | counter | method, success | `AuthService.login` through `track_authentication`; method is `login` | -| `authentication_failures_total` | counter | method | the same | -| `authentication_duration_seconds` | histogram | method | the same | -| `accounts_locked_total` | counter | reason: brute_force | `AuthService`, when a failed login triggers the lockout | -| `brute_force_attempts_total` | counter | none | `AuthService.login`, a login against an account that is already locked | -| `weak_password_attempts_total` | counter | weakness_type: too_short | `AuthService.register` | -| `token_validation_failures_total` | counter | reason: expired, invalid_signature, invalid, missing_subject | `SecurityService.decode_token` | -| `csrf_validation_failures_total` | counter | reason: missing_header, token_mismatch, invalid_signature | `SecurityService.validate_csrf_from_request` | -| `authorization_checks_total` | counter | resource, action, allowed, user_role | `AuthService.get_admin`, on every admin route | -| `authorization_denials_total` | counter | resource, action, user_role | the same | -| `password_reset_requests_total` | counter | method: admin | `AdminUserService` | +Each login runs in an `auth.login` span; the other events leave a span named in the table where the code used to record the metric. + +| Metric | Type | Labels | Recorded from | +|--------|------|--------|---------------| +| `authentication_attempts_total` | counter | auth_method, auth_success | span `auth.login` from `AuthService.login`; the method is `login` | +| `authentication_failures_total` | counter | auth_method | the same span with `auth.success` false | +| `authentication_duration_seconds` | histogram | auth_method | the length of that span | +| `accounts_locked_total` | counter | auth_reason: brute_force | span `auth.account_locked`, when a failed login triggers the lockout | +| `brute_force_attempts_total` | counter | none | span `auth.locked_login`, a login against an account that is already locked | +| `weak_password_attempts_total` | counter | auth_weakness_type: too_short | span `auth.weak_password` from `AuthService.register` | +| `token_validation_failures_total` | counter | auth_reason: expired, invalid_signature, invalid, missing_subject | span `auth.token_rejected` from `SecurityService.decode_token` | +| `csrf_validation_failures_total` | counter | auth_reason: missing_header, token_mismatch, invalid_signature | span `auth.csrf_rejected` from `SecurityService.validate_csrf_from_request` | +| `authorization_checks_total` | counter | auth_resource, auth_action, auth_allowed, auth_user_role | span `auth.authorization` from `AuthService.get_admin`, on every admin route | +| `authorization_denials_total` | counter | auth_resource, auth_action, auth_user_role | the same span with `auth.allowed` false | +| `password_reset_requests_total` | counter | auth_method: admin | span `auth.password_reset` from `AdminUserService` | ### Rate limiting (backend) -`RateLimitService.check_rate_limit`, called from `RateLimitMiddleware`. `endpoint` is the path with UUIDs and any segment of 20 or more letters and digits replaced by `*`. Shorter IDs such as `/42` stay as they are. +`RateLimitService.check_rate_limit`, called from `RateLimitMiddleware`, leaves a `rate_limit.request` span for each check and a `rate_limit.decision` span when a rule applies. `rate_limit_endpoint` is the path with UUIDs and any segment of 20 or more letters and digits replaced by `*`. Shorter IDs such as `/42` stay as they are. -| Metric | Type | Labels | Notes | -|--------|------|--------|-------| -| `rate_limit_requests_total` | counter | endpoint, authenticated, algorithm | the matched rule's algorithm, or `bypassed` and `no_limit` | -| `rate_limit_allowed_total` | counter | endpoint, group | | -| `rate_limit_rejected_total` | counter | endpoint, group | | -| `rate_limit_bypass_total` | counter | endpoint | users with `bypass_rate_limit` | +| Metric | Type | Labels | Recorded from | +|--------|------|--------|---------------| +| `rate_limit_requests_total` | counter | rate_limit_endpoint, rate_limit_authenticated, rate_limit_algorithm | `rate_limit.request`: the matched rule's algorithm, or `bypassed`, `no_limit` and `redis_error` | +| `rate_limit_allowed_total` | counter | rate_limit_endpoint, rate_limit_group | `rate_limit.decision` with `rate_limit.allowed` true | +| `rate_limit_rejected_total` | counter | rate_limit_endpoint, rate_limit_group | the same span with `rate_limit.allowed` false | +| `rate_limit_bypass_total` | counter | rate_limit_endpoint | `rate_limit.request` with algorithm `bypassed`: users with `bypass_rate_limit` | ### Dead letter queue @@ -246,7 +248,7 @@ sum(rate(rate_limit_rejected_total[5m])) / sum(rate(rate_limit_requests_total[5m sum by (messaging_destination_name) (rate(messaging_publish_messages_message_total[5m])) # rejected access tokens by reason -sum by (reason) (rate(token_validation_failures_total[5m])) +sum by (auth_reason) (rate(token_validation_failures_total[5m])) # MongoDB cache hit ratio sum(rate(mongodb_cache_operations_total{type="hit"}[5m])) / sum(rate(mongodb_cache_operations_total[5m]))