diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d4e4762..387476a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [5.17.2] - 2026-09-25 + +### Fixed + +- **Idle dashboard 429 Too Many Requests (#767)** — Global `RATELIMIT_DEFAULT` was too strict (`50 per hour` per IP) for background timer polling. Default is now `5000 per day;1000 per hour`, keyed per logged-in user (IP for anonymous). Polling endpoints (`/timer/status`, `/api/timer/status`, `/api/timer/heartbeat`, `/api/notifications`, `/service-worker.js`, `/offline`) are exempt. JSON `Accept` / API requests get a JSON 429 body; floating timer bar and idle pollers back off on 429 and skip while the tab is hidden. +- **OpenTelemetry export respects telemetry opt-in** — OTLP export to the baked-in shared Grafana Cloud tenant now requires the telemetry opt-in (`ENABLE_TELEMETRY` / admin toggle). Operator-owned backends configured via explicit `OTEL_EXPORTER_OTLP_ENDPOINT` + `OTEL_EXPORTER_OTLP_TOKEN` still export without the opt-in. Metric attributes are normalized (status class, environment) and `http.server.duration` histogram buckets are tightened. + +### Documentation + +- **Version** — Bumped `setup.py` to **5.17.2**; `VERSION` and desktop/browser-extension/mobile client versions aligned. + ## [5.17.1] - 2026-09-23 ### Fixed diff --git a/README.md b/README.md index 9cc7f2bd..4234af53 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,10 @@ TimeTracker has been continuously enhanced with powerful new features! Here's wh **Current version** is defined in `setup.py` (single source of truth). See [CHANGELOG.md](CHANGELOG.md) for versioned release history. +### ✨ Highlights of v5.17.2 + +**Patch (5.17.2):** **Idle dashboard no longer hits 429 (#767)** — higher per-user rate limit default, exempt timer/notification polling, and client back-off on 429. **OpenTelemetry opt-in** — export to the shared telemetry backend now requires the telemetry opt-in; operator-configured `OTEL_EXPORTER_OTLP_*` backends are unaffected. See [CHANGELOG.md](CHANGELOG.md#5172---2026-09-25). + ### ✨ Highlights of v5.17.1 **Patch (5.17.1):** **Health probes exempt from rate limiting** — `/_health`, `/_ready`, and the API health endpoints no longer return 429 under frequent liveness checks, fixing Render restart loops and 502s. See [CHANGELOG.md](CHANGELOG.md#5171---2026-09-23). diff --git a/VERSION b/VERSION index d3f30982..1c8b2cb5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -5.17.1 +5.17.2 diff --git a/app/__init__.py b/app/__init__.py index 54cd3cf8..11e3954f 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -45,6 +45,25 @@ def protect(self): return super().protect() +def _rate_limit_key(): + """Key rate limits per authenticated user; fall back to IP for anonymous. + + Logged-in users behind a shared NAT/proxy get separate buckets so one + colleague's dashboard polling cannot exhaust another's limit (Issue #767). + Unauthenticated routes (login, etc.) still key by IP. + """ + try: + from flask_login import current_user + + if current_user is not None and getattr(current_user, "is_authenticated", False): + user_id = getattr(current_user, "id", None) + if user_id is not None: + return f"user:{user_id}" + except Exception: + pass + return get_remote_address() + + # Initialize extensions db = SQLAlchemy() migrate = Migrate() @@ -52,7 +71,7 @@ def protect(self): socketio = SocketIO() babel = Babel() csrf = PathExemptCSRFProtect() -limiter = Limiter(key_func=get_remote_address, default_limits=[]) +limiter = Limiter(key_func=_rate_limit_key, default_limits=[]) oauth = OAuth() # Initialize Mail (will be configured in create_app) diff --git a/app/config.py b/app/config.py index 83fb45d0..d7a078f8 100644 --- a/app/config.py +++ b/app/config.py @@ -280,8 +280,9 @@ class Config: PERF_QUERY_PROFILE = os.getenv("PERF_QUERY_PROFILE", "false").lower() == "true" # Rate limiting - # Sensible default when unset; override via RATELIMIT_DEFAULT (semicolon/comma-separated). - RATELIMIT_DEFAULT = os.getenv("RATELIMIT_DEFAULT", "200 per day;50 per hour") + # Generous default for authenticated dashboards with background polling (Issue #767). + # Override via RATELIMIT_DEFAULT (semicolon/comma-separated). Keyed per user when logged in. + RATELIMIT_DEFAULT = os.getenv("RATELIMIT_DEFAULT", "5000 per day;1000 per hour") RATELIMIT_STORAGE_URI = os.getenv("RATELIMIT_STORAGE_URI", "memory://") # Redis configuration diff --git a/app/routes/api.py b/app/routes/api.py index b013b39a..b1aac426 100644 --- a/app/routes/api.py +++ b/app/routes/api.py @@ -317,6 +317,7 @@ def api_version_dismiss(): @api_bp.route("/api/timer/status") @login_required +@limiter.exempt @deprecated_session_api("/api/v1/timer/status") def timer_status(): """Get current timer status""" @@ -402,6 +403,7 @@ def timer_notes_suggestions(): @api_bp.route("/api/timer/heartbeat", methods=["POST"]) @login_required +@limiter.exempt @deprecated_session_api("/api/v1/timer/heartbeat") def api_timer_heartbeat(): """Record activity for the active timer (idle timeout safety net).""" @@ -2488,6 +2490,7 @@ def summary_today(): @api_bp.route("/api/notifications") @login_required +@limiter.exempt def api_smart_notifications(): """Smart in-app notification candidates (respects preferences, dismissals, caps).""" from app.services.notification_service import NotificationService diff --git a/app/routes/main.py b/app/routes/main.py index 13773ce1..71a22484 100644 --- a/app/routes/main.py +++ b/app/routes/main.py @@ -932,6 +932,7 @@ def manifest(): @main_bp.route("/offline") +@limiter.exempt def offline_page(): """Public offline fallback for PWA (no login required).""" resp = make_response(render_template("offline.html")) @@ -940,6 +941,7 @@ def offline_page(): @main_bp.route("/service-worker.js") +@limiter.exempt def service_worker(): """Site-scoped service worker; implementation lives in app/static/js/sw.js.""" return send_from_directory(current_app.static_folder, "js/sw.js", mimetype="application/javascript") diff --git a/app/routes/timer.py b/app/routes/timer.py index 2bb67e80..c882dfbf 100644 --- a/app/routes/timer.py +++ b/app/routes/timer.py @@ -7,7 +7,7 @@ from sqlalchemy import inspect, text from sqlalchemy.exc import ProgrammingError -from app import db, log_event, socketio, track_event +from app import db, limiter, log_event, socketio, track_event from app.constants import TimeEntrySource from app.models import Activity, Client, Project, Settings, Task, TimeEntry, User from app.services.client_service import ClientService @@ -890,6 +890,7 @@ def set_timer_start(): @timer_bp.route("/timer/status") @login_required +@limiter.exempt def timer_status(): """Get current timer status as JSON""" from app.models import Settings diff --git a/app/static/floating-timer-bar.js b/app/static/floating-timer-bar.js index 61fd361e..cf0a5490 100644 --- a/app/static/floating-timer-bar.js +++ b/app/static/floating-timer-bar.js @@ -6,6 +6,8 @@ 'use strict'; const POLL_INTERVAL_MS = 30000; + const FOCUS_REFETCH_THROTTLE_MS = 10000; + const DEFAULT_429_BACKOFF_MS = 60000; function syncFabDesktopHide(timerData) { try { @@ -29,6 +31,8 @@ this.switchLabel = 'Switch project'; this.projectsCache = null; this.switchPopover = null; + this.backoffUntil = 0; + this.lastFocusFetch = 0; this.init(); } @@ -44,7 +48,12 @@ this.render(); this.fetchStatus(); this.pollTimer = setInterval(() => this.fetchStatus(), POLL_INTERVAL_MS); - window.addEventListener('focus', () => this.fetchStatus()); + window.addEventListener('focus', () => { + const now = Date.now(); + if (now - this.lastFocusFetch < FOCUS_REFETCH_THROTTLE_MS) return; + this.lastFocusFetch = now; + this.fetchStatus(); + }); document.addEventListener('click', (evt) => { if (!this.switchPopover || this.switchPopover.classList.contains('hidden')) return; if (this.switchPopover.contains(evt.target)) return; @@ -151,8 +160,26 @@ } async fetchStatus() { + if (document.hidden) return; + if (Date.now() < this.backoffUntil) return; try { - const res = await fetch('/timer/status', { credentials: 'same-origin' }); + const res = await fetch('/timer/status', { + credentials: 'same-origin', + headers: { 'Accept': 'application/json' }, + }); + if (res.status === 429) { + const retryAfter = parseInt(res.headers.get('Retry-After'), 10); + const waitMs = (!isNaN(retryAfter) && retryAfter > 0) + ? retryAfter * 1000 + : DEFAULT_429_BACKOFF_MS; + this.backoffUntil = Date.now() + waitMs; + console.warn('FloatingTimerBar: rate limited, backing off', waitMs, 'ms'); + return; + } + if (!res.ok) { + console.warn('FloatingTimerBar: fetch status failed', res.status); + return; + } const data = await res.json(); if (data.active && data.timer) { this.timerData = data.timer; diff --git a/app/static/idle.js b/app/static/idle.js index 28da542f..2da5e842 100644 --- a/app/static/idle.js +++ b/app/static/idle.js @@ -376,6 +376,10 @@ } async function tick(){ + // Skip status polling while the tab is hidden to avoid burning rate-limit + // budget when the user is idle on another tab (Issue #767). Heartbeats still + // fire from markActive() / sendHeartbeat() when the user returns. + if (document.hidden) return; const active = await getTimer(); hasActiveTimer = !!active; if (!active) return; diff --git a/app/telemetry/otel_setup.py b/app/telemetry/otel_setup.py index 182b1191..0cd2962e 100644 --- a/app/telemetry/otel_setup.py +++ b/app/telemetry/otel_setup.py @@ -4,6 +4,11 @@ Initialization is gated on OTLP credentials (same sources as manual log export). Feature flags: ENABLE_TRACING, ENABLE_METRICS (default true when unset). +Export to the shared (baked-in) Grafana Cloud tenant additionally requires the +telemetry opt-in (``is_telemetry_enabled`` / admin toggle / ENABLE_TELEMETRY). +When the operator sets explicit ``OTEL_EXPORTER_OTLP_*`` env vars, export is +allowed without the opt-in (they own the backend). + Tests: Flask ``TESTING`` apps do not start network OTLP exporters (avoids background export threads and shutdown noise). Set ``OTEL_ENABLE_IN_TESTS=1`` for in-memory tracing/metrics without export. @@ -14,8 +19,9 @@ import atexit import logging import os +import time from contextlib import contextmanager -from typing import Any, Dict, Iterator, Optional, Tuple +from typing import Any, Dict, Iterator, Optional, Sequence, Tuple logger = logging.getLogger(__name__) @@ -37,12 +43,23 @@ _webhook_success: Any = None _webhook_failure: Any = None -# Test-only span exporter (set when OTEL_ENABLE_IN_TESTS=1) +# Test-only span/metric exporters (set when OTEL_ENABLE_IN_TESTS=1) _test_span_exporter: Any = None +_test_metric_reader: Any = None # Register atexit shutdown once; scoped_session / pytest can create many app lifetimes. _otel_atexit_registered = False +# True when OTLP endpoint+token came from explicit env vars (operator-owned backend). +_otlp_credentials_from_env = False + +# Cached result of _export_allowed(): (monotonic_ts, allowed) +_export_allowed_cache: Optional[Tuple[float, bool]] = None +_EXPORT_ALLOWED_TTL_S = 60.0 + +# Custom histogram boundaries for http.server.duration (seconds). +_HTTP_DURATION_BOUNDARIES = (0.05, 0.1, 0.25, 0.5, 1.0, 2.5, 5.0, 10.0) + def _env_bool(name: str, default: bool = True) -> bool: raw = os.getenv(name) @@ -65,32 +82,146 @@ def _app_version() -> str: def _metric_attrs() -> Dict[str, str]: - return {"environment": _deployment_environment(), "app_version": _app_version()} + # app_version lives on the resource as service.version (target_info in Mimir). + return {"environment": _deployment_environment()} + + +def _status_class(status_code: int) -> str: + if status_code >= 500: + return "5xx" + if status_code >= 400: + return "4xx" + if status_code >= 300: + return "3xx" + return "2xx" + + +def _normalize_otlp_base(endpoint: str) -> str: + base = endpoint.rstrip("/") + if base.endswith("/v1/logs"): + base = base[: -len("/v1/logs")] + elif base.endswith("/logs") and "/v1/" in base: + # tolerate .../otlp/v1/logs + idx = base.rfind("/v1/logs") + if idx != -1: + base = base[:idx] + return base def resolve_otlp_connection() -> Optional[Tuple[str, Dict[str, str]]]: """ Return (base_url, headers) for OTLP/HTTP exporters, or None if not configured. + + Explicit ``OTEL_EXPORTER_OTLP_ENDPOINT`` + ``OTEL_EXPORTER_OTLP_TOKEN`` env vars + take precedence over baked-in release defaults. Sets ``_otlp_credentials_from_env`` + accordingly so ``_export_allowed`` can skip the telemetry opt-in for operator-owned + backends. + Base URL has no trailing slash; traces/metrics append /v1/traces and /v1/metrics. """ + global _otlp_credentials_from_env + from app.config.analytics_defaults import get_analytics_config from app.telemetry.service import _build_otlp_auth_header + env_endpoint = (os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT") or "").strip() + env_token = (os.getenv("OTEL_EXPORTER_OTLP_TOKEN") or "").strip() + if env_endpoint and env_token: + _otlp_credentials_from_env = True + headers = {"Authorization": _build_otlp_auth_header(env_token)} + return _normalize_otlp_base(env_endpoint), headers + cfg = get_analytics_config() - endpoint = (cfg.get("otel_exporter_otlp_endpoint") or os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT") or "").strip() - token = (cfg.get("otel_exporter_otlp_token") or os.getenv("OTEL_EXPORTER_OTLP_TOKEN") or "").strip() + endpoint = (cfg.get("otel_exporter_otlp_endpoint") or "").strip() + token = (cfg.get("otel_exporter_otlp_token") or "").strip() if not endpoint or not token: + _otlp_credentials_from_env = False return None - base = endpoint.rstrip("/") - if base.endswith("/v1/logs"): - base = base[: -len("/v1/logs")] - elif base.endswith("/logs") and "/v1/" in base: - # tolerate .../otlp/v1/logs - idx = base.rfind("/v1/logs") - if idx != -1: - base = base[:idx] + + _otlp_credentials_from_env = False headers = {"Authorization": _build_otlp_auth_header(token)} - return base, headers + return _normalize_otlp_base(endpoint), headers + + +def invalidate_export_allowed_cache() -> None: + """Clear the cached opt-in decision (tests / after admin toggle).""" + global _export_allowed_cache + _export_allowed_cache = None + + +def _export_allowed() -> bool: + """ + Whether OTLP export (and metric recording) should proceed. + + Allowed when: + - Operator set explicit OTEL_EXPORTER_OTLP_* env credentials, or + - Telemetry opt-in is enabled (ENABLE_TELEMETRY / admin preference). + + Result is cached for ``_EXPORT_ALLOWED_TTL_S`` so the hot path stays cheap. + """ + global _export_allowed_cache + + now = time.monotonic() + cached = _export_allowed_cache + if cached is not None: + ts, val = cached + if now - ts < _EXPORT_ALLOWED_TTL_S: + return val + + if _otlp_credentials_from_env: + allowed = True + else: + try: + from app.utils.telemetry import is_telemetry_enabled + + allowed = bool(is_telemetry_enabled()) + except Exception: + allowed = False + + _export_allowed_cache = (now, allowed) + return allowed + + +class _OptInMetricExporter: + """Wraps a MetricExporter; no-ops export when telemetry opt-in is off.""" + + def __init__(self, inner: Any) -> None: + self._inner = inner + self._preferred_temporality = getattr(inner, "_preferred_temporality", None) + self._preferred_aggregation = getattr(inner, "_preferred_aggregation", None) + + def export(self, metrics_data: Any, timeout_millis: float = 10_000, **kwargs: Any) -> Any: + from opentelemetry.sdk.metrics.export import MetricExportResult + + if not _export_allowed(): + return MetricExportResult.SUCCESS + return self._inner.export(metrics_data, timeout_millis=timeout_millis, **kwargs) + + def force_flush(self, timeout_millis: float = 10_000) -> bool: + return self._inner.force_flush(timeout_millis=timeout_millis) + + def shutdown(self, timeout_millis: float = 30_000, **kwargs: Any) -> None: + return self._inner.shutdown(timeout_millis=timeout_millis, **kwargs) + + +class _OptInSpanExporter: + """Wraps a SpanExporter; no-ops export when telemetry opt-in is off.""" + + def __init__(self, inner: Any) -> None: + self._inner = inner + + def export(self, spans: Sequence[Any]) -> Any: + from opentelemetry.sdk.trace.export import SpanExportResult + + if not _export_allowed(): + return SpanExportResult.SUCCESS + return self._inner.export(spans) + + def shutdown(self) -> None: + return self._inner.shutdown() + + def force_flush(self, timeout_millis: int = 30000) -> bool: + return self._inner.force_flush(timeout_millis=timeout_millis) def install_id_attr() -> Dict[str, str]: @@ -168,27 +299,31 @@ def is_otel_metrics_active() -> bool: def record_http_server_metrics(method: str, route: str, status_code: int, duration_s: float) -> None: - if not _metrics_enabled or _http_duration is None: + if not _metrics_enabled or _http_duration is None or not _export_allowed(): return try: base = _metric_attrs() - attrs = { + method_attr = method or "UNKNOWN" + sc = _status_class(status_code) + # Histogram: method + status_class only (no route — high cardinality). + duration_attrs = {**base, "http.method": method_attr, "status_class": sc} + _http_duration.record(float(duration_s), duration_attrs) + # Counter: keep per-route for traffic volume. + request_attrs = { **base, - "http.method": method or "UNKNOWN", + "http.method": method_attr, "http.route": route or "unknown", + "status_class": sc, } - _http_duration.record(float(duration_s), attrs) - _http_requests.add(1, attrs) - if status_code >= 500: - _http_errors.add(1, {**attrs, "status_class": "5xx"}) - elif status_code >= 400: - _http_errors.add(1, {**attrs, "status_class": "4xx"}) + _http_requests.add(1, request_attrs) + if status_code >= 400: + _http_errors.add(1, {**base, "http.method": method_attr, "status_class": sc}) except Exception: pass def record_invoice_created() -> None: - if not _metrics_enabled or _invoice_created is None: + if not _metrics_enabled or _invoice_created is None or not _export_allowed(): return try: _invoice_created.add(1, _metric_attrs()) @@ -201,7 +336,7 @@ def record_invoice_duration_seconds(seconds: float, operation: str) -> None: timetracker.invoice.duration — use operation='pdf' for PDF generation latency, operation='create' for create/commit path duration. """ - if not _metrics_enabled or _invoice_duration is None: + if not _metrics_enabled or _invoice_duration is None or not _export_allowed(): return try: attrs = {**_metric_attrs(), "operation": operation} @@ -211,7 +346,7 @@ def record_invoice_duration_seconds(seconds: float, operation: str) -> None: def record_report_generated() -> None: - if not _metrics_enabled or _report_generated is None: + if not _metrics_enabled or _report_generated is None or not _export_allowed(): return try: _report_generated.add(1, _metric_attrs()) @@ -220,7 +355,7 @@ def record_report_generated() -> None: def record_export_duration_seconds(seconds: float, export_kind: str) -> None: - if not _metrics_enabled or _export_duration is None: + if not _metrics_enabled or _export_duration is None or not _export_allowed(): return try: attrs = {**_metric_attrs(), "export_kind": export_kind} @@ -230,7 +365,7 @@ def record_export_duration_seconds(seconds: float, export_kind: str) -> None: def record_background_job_outcome(job_id: str, success: bool) -> None: - if not _metrics_enabled: + if not _metrics_enabled or not _export_allowed(): return try: attrs = {**_metric_attrs(), "job_id": str(job_id)[:128]} @@ -243,7 +378,7 @@ def record_background_job_outcome(job_id: str, success: bool) -> None: def record_webhook_delivery(event_type: str, success: bool) -> None: - if not _metrics_enabled: + if not _metrics_enabled or not _export_allowed(): return try: et = (event_type or "unknown")[:128] @@ -276,6 +411,8 @@ def inject_traceparent_headers(response: Any) -> Any: def _active_timers_callback(options: Any) -> Any: from opentelemetry.metrics import Observation + if not _export_allowed(): + return app = _flask_app if app is None: yield Observation(0, _metric_attrs()) @@ -290,6 +427,26 @@ def _active_timers_callback(options: Any) -> Any: yield Observation(0, _metric_attrs()) +def _meter_provider_views() -> list: + from opentelemetry.sdk.metrics.view import ( + DropAggregation, + ExplicitBucketHistogramAggregation, + View, + ) + + return [ + # Keep Flask/SQLAlchemy *traces*; drop their high-cardinality *metrics* + # (http.host, http.target, connection pool labels, etc.). + View(meter_name="opentelemetry.instrumentation.flask", aggregation=DropAggregation()), + View(meter_name="opentelemetry.instrumentation.sqlalchemy", aggregation=DropAggregation()), + View( + instrument_name="http.server.duration", + meter_name="timetracker", + aggregation=ExplicitBucketHistogramAggregation(boundaries=list(_HTTP_DURATION_BOUNDARIES)), + ), + ] + + def _shutdown_providers() -> None: # OTLP exporters log on failure; during interpreter shutdown stderr may already be closed. _otel_loggers = ( @@ -345,15 +502,51 @@ def get_test_span_exporter() -> Any: return _test_span_exporter +def get_test_metric_reader() -> Any: + """Only populated when OTEL_ENABLE_IN_TESTS=1 during init.""" + return _test_metric_reader + + +def _force_reset_otel_globals() -> None: + """ + Clear OTel API set-once providers so a subsequent init_opentelemetry can replace them. + + The public set_*_provider APIs only allow a single call per process; pytest creates + many app lifetimes, so tests must reset the Once latch after shutdown. + """ + try: + from opentelemetry import metrics as metrics_api + from opentelemetry import trace as trace_api + from opentelemetry.util._once import Once + + trace_api._TRACER_PROVIDER = None # type: ignore[attr-defined] + trace_api._TRACER_PROVIDER_SET_ONCE = Once() # type: ignore[attr-defined] + + # Meter provider lives in opentelemetry.metrics._internal + from opentelemetry.metrics import _internal as metrics_internal + + metrics_internal._METER_PROVIDER = None # type: ignore[attr-defined] + metrics_internal._METER_PROVIDER_SET_ONCE = Once() # type: ignore[attr-defined] + # Keep metrics_api module alias in sync if it re-exports the Once. + if hasattr(metrics_api, "_METER_PROVIDER_SET_ONCE"): + metrics_api._METER_PROVIDER_SET_ONCE = metrics_internal._METER_PROVIDER_SET_ONCE # type: ignore[attr-defined] + if hasattr(metrics_api, "_METER_PROVIDER"): + metrics_api._METER_PROVIDER = None # type: ignore[attr-defined] + except Exception: + pass + + def reset_for_testing() -> None: """Tear down OTel globals so a new Flask app can call init_opentelemetry (pytest only).""" global _initialized, _tracing_enabled, _metrics_enabled, _flask_app global _http_duration, _http_requests, _http_errors global _invoice_created, _invoice_duration, _report_generated, _export_duration global _bg_job_success, _bg_job_failure, _webhook_success, _webhook_failure - global _test_span_exporter + global _test_span_exporter, _test_metric_reader + global _otlp_credentials_from_env if not _initialized: + invalidate_export_allowed_cache() return try: if _tracing_enabled: @@ -369,6 +562,7 @@ def reset_for_testing() -> None: _shutdown_providers() except Exception: pass + _force_reset_otel_globals() _initialized = False _tracing_enabled = False _metrics_enabled = False @@ -385,6 +579,9 @@ def reset_for_testing() -> None: _webhook_success = None _webhook_failure = None _test_span_exporter = None + _test_metric_reader = None + _otlp_credentials_from_env = False + invalidate_export_allowed_cache() def init_opentelemetry(app: Any) -> bool: @@ -396,7 +593,7 @@ def init_opentelemetry(app: Any) -> bool: global _http_duration, _http_requests, _http_errors global _invoice_created, _invoice_duration, _report_generated, _export_duration global _bg_job_success, _bg_job_failure, _webhook_success, _webhook_failure - global _test_span_exporter + global _test_span_exporter, _test_metric_reader if _initialized: return _tracing_enabled or _metrics_enabled @@ -448,6 +645,7 @@ def init_opentelemetry(app: Any) -> bool: trace_endpoint = f"{base.rstrip('/')}/v1/traces" metrics_endpoint = f"{base.rstrip('/')}/v1/metrics" + views = _meter_provider_views() if testing_memory: from opentelemetry.sdk.metrics import MeterProvider @@ -468,12 +666,15 @@ def init_opentelemetry(app: Any) -> bool: if enable_metrics_flag: reader = InMemoryMetricReader() - mp = MeterProvider(resource=resource, metric_readers=[reader]) + _test_metric_reader = reader + mp = MeterProvider(resource=resource, metric_readers=[reader], views=views) metrics_api.set_meter_provider(mp) _metrics_enabled = True else: _discard_reader = InMemoryMetricReader() - metrics_api.set_meter_provider(MeterProvider(resource=resource, metric_readers=[_discard_reader])) + metrics_api.set_meter_provider( + MeterProvider(resource=resource, metric_readers=[_discard_reader], views=views) + ) _metrics_enabled = False else: from opentelemetry.exporter.otlp.proto.http.metric_exporter import OTLPMetricExporter @@ -484,7 +685,7 @@ def init_opentelemetry(app: Any) -> bool: from opentelemetry.sdk.trace.export import BatchSpanProcessor if enable_trace_flag: - span_exp = OTLPSpanExporter(endpoint=trace_endpoint, headers=headers) + span_exp = _OptInSpanExporter(OTLPSpanExporter(endpoint=trace_endpoint, headers=headers)) tp = TracerProvider(resource=resource) tp.add_span_processor(BatchSpanProcessor(span_exp)) trace_api.set_tracer_provider(tp) @@ -495,19 +696,21 @@ def init_opentelemetry(app: Any) -> bool: if enable_metrics_flag: interval_ms = int(os.getenv("OTEL_METRICS_EXPORT_INTERVAL_MS", "60000")) - metric_exp = OTLPMetricExporter(endpoint=metrics_endpoint, headers=headers) + metric_exp = _OptInMetricExporter(OTLPMetricExporter(endpoint=metrics_endpoint, headers=headers)) # Both ``InMemoryMetricReader`` (used in the dev branch above) and # ``PeriodicExportingMetricReader`` are valid ``MetricReader``s; mypy # narrowed ``reader`` to the first one in the if/else. reader = PeriodicExportingMetricReader(metric_exp, export_interval_millis=interval_ms) # type: ignore[assignment] - mp = MeterProvider(resource=resource, metric_readers=[reader]) + mp = MeterProvider(resource=resource, metric_readers=[reader], views=views) metrics_api.set_meter_provider(mp) _metrics_enabled = True else: from opentelemetry.sdk.metrics.export import InMemoryMetricReader _discard_reader2 = InMemoryMetricReader() - metrics_api.set_meter_provider(MeterProvider(resource=resource, metric_readers=[_discard_reader2])) + metrics_api.set_meter_provider( + MeterProvider(resource=resource, metric_readers=[_discard_reader2], views=views) + ) _metrics_enabled = False if _metrics_enabled: @@ -594,8 +797,9 @@ def init_opentelemetry(app: Any) -> bool: _otel_atexit_registered = True _initialized = True logger.info( - "OpenTelemetry initialized tracing=%s metrics=%s", + "OpenTelemetry initialized tracing=%s metrics=%s otlp_from_env=%s", _tracing_enabled, _metrics_enabled, + _otlp_credentials_from_env, ) return True diff --git a/app/templates/main/dashboard.html b/app/templates/main/dashboard.html index 26bfa106..ca69385e 100644 --- a/app/templates/main/dashboard.html +++ b/app/templates/main/dashboard.html @@ -1372,7 +1372,11 @@

{{ _('En // Soft refresh dashboard timer hero when timer stops externally (idle detection, floating bar) document.addEventListener('tt:timer-status-changed', async function() { try { - var res = await fetch('/timer/status', { credentials: 'same-origin' }); + var res = await fetch('/timer/status', { + credentials: 'same-origin', + headers: { 'Accept': 'application/json' }, + }); + if (!res.ok) return; var data = await res.json(); var hero = document.querySelector('[data-timer-start]'); if (!data.active && hero) { diff --git a/app/utils/error_handlers.py b/app/utils/error_handlers.py index d7c37571..45ba956d 100644 --- a/app/utils/error_handlers.py +++ b/app/utils/error_handlers.py @@ -6,7 +6,7 @@ import sys from typing import Any, Dict, Optional -from flask import current_app, jsonify, request +from flask import current_app, make_response, render_template, request from marshmallow import ValidationError from sqlalchemy.exc import IntegrityError, SQLAlchemyError from werkzeug.exceptions import HTTPException @@ -67,6 +67,51 @@ def unprocessable_entity(error): return error_response(message="Unprocessable entity", error_code="unprocessable_entity", status_code=422) return error, 422 + @app.errorhandler(429) + def too_many_requests(error): + """Handle 429 Too Many Requests — JSON for API/fetch clients (Issue #767).""" + wants_json = ( + request.is_json + or request.path.startswith("/api/") + or bool(request.headers.get("X-Requested-With")) + or "application/json" in (request.headers.get("Accept") or "") + ) + retry_after = None + try: + if hasattr(error, "get_response"): + retry_after = error.get_response().headers.get("Retry-After") + except Exception: + retry_after = None + if not retry_after and getattr(error, "response", None) is not None: + try: + retry_after = error.response.headers.get("Retry-After") + except Exception: + pass + + message = ( + str(error.description) + if getattr(error, "description", None) + else "Too many requests. Please wait a moment and try again." + ) + + if wants_json: + response, status = error_response(message=message, error_code="rate_limited", status_code=429) + if retry_after: + response.headers["Retry-After"] = retry_after + return response, status + + html_resp = make_response( + render_template( + "errors/generic.html", + error=error, + error_info={"title": "Too Many Requests", "message": message}, + ), + 429, + ) + if retry_after: + html_resp.headers["Retry-After"] = retry_after + return html_resp + @app.errorhandler(ValidationError) def handle_marshmallow_validation_error(error): """Handle Marshmallow validation errors""" diff --git a/browser-extension/manifest.json b/browser-extension/manifest.json index 07883715..f82c42ec 100644 --- a/browser-extension/manifest.json +++ b/browser-extension/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 3, "name": "TimeTracker Timer", - "version": "5.17.1", + "version": "5.17.2", "description": "Start and stop TimeTracker timers from your browser toolbar.", "permissions": [ "storage", diff --git a/browser-extension/package.json b/browser-extension/package.json index 6c848f74..2e2b0d5a 100644 --- a/browser-extension/package.json +++ b/browser-extension/package.json @@ -1,7 +1,7 @@ { "name": "timetracker-browser-extension", "private": true, - "version": "5.17.1", + "version": "5.17.2", "description": "TimeTracker Chromium extension (Manifest V3)", "type": "module", "scripts": { diff --git a/desktop/package.json b/desktop/package.json index 2b6730d7..0bbe250c 100644 --- a/desktop/package.json +++ b/desktop/package.json @@ -1,6 +1,6 @@ { "name": "timetracker-desktop", - "version": "5.17.1", + "version": "5.17.2", "description": "TimeTracker desktop app for Windows, Linux, and macOS", "main": "src/main/main.js", "scripts": { diff --git a/docs/GAP_ROADMAP.md b/docs/GAP_ROADMAP.md index 25df770f..a685bc2d 100644 --- a/docs/GAP_ROADMAP.md +++ b/docs/GAP_ROADMAP.md @@ -76,8 +76,8 @@ See implementation commits and CHANGELOG Unreleased section for other Phase 1 it **Versions** -- Source of truth: `setup.py` (`5.17.1`); mirror `VERSION` at repo root. -- Clients aligned: `desktop/package.json`, `browser-extension/package.json` + `manifest.json`, `mobile/pubspec.yaml` (`5.17.1+1`). +- Source of truth: `setup.py` (`5.17.2`); mirror `VERSION` at repo root. +- Clients aligned: `desktop/package.json`, `browser-extension/package.json` + `manifest.json`, `mobile/pubspec.yaml` (`5.17.2+1`). - Desktop sidebar label via Vite `__APP_VERSION__` (`desktop/vite.config.mjs` reads `desktop/package.json`). **API login + 2FA** diff --git a/env.example b/env.example index 88948a5b..39ec6592 100644 --- a/env.example +++ b/env.example @@ -45,7 +45,8 @@ IDLE_TIMEOUT_MINUTES=30 IDLE_UNANSWERED_ACTION=review # Global Flask-Limiter defaults (semicolon or comma separated). -# RATELIMIT_DEFAULT=200 per day;50 per hour +# Defaults to "5000 per day;1000 per hour" when unset; keyed per logged-in user (else IP). +# RATELIMIT_DEFAULT=5000 per day;1000 per hour # Optional: Firebase Cloud Messaging for mobile idle "Still working?" wake-up (Issue #722). # Path to a service-account JSON file, or the raw JSON string. @@ -261,6 +262,9 @@ LOG_FILE=/data/logs/timetracker.log # # OpenTelemetry SDK (traces + metrics to OTLP; uses same endpoint/token as above) # When unset, tracing and metrics default to enabled if OTLP credentials are present. +# Server-side export to the shared (baked-in) Grafana Cloud tenant also requires the +# telemetry opt-in below (ENABLE_TELEMETRY / admin dashboard). Setting your own +# OTEL_EXPORTER_OTLP_* env vars allows export without that opt-in (you own the backend). # ENABLE_TRACING=true # ENABLE_METRICS=true # Optional: OTLP metrics export interval in milliseconds (default 60000) @@ -271,7 +275,8 @@ LOG_FILE=/data/logs/timetracker.log # Telemetry (optional, opt-in, anonymous) # Sends anonymous installation data to Grafana OTLP (version/platform/install heartbeat) -# Requires OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_EXPORTER_OTLP_TOKEN +# Also gates server-side OTel traces/metrics when using baked-in OTLP credentials. +# Requires OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_EXPORTER_OTLP_TOKEN (or baked-in defaults) # Default: false (disabled) # See docs/privacy.md for details # ENABLE_TELEMETRY=true diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index 6acc3fe8..402dfc57 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -1,7 +1,7 @@ name: timetracker_mobile description: TimeTracker mobile app for Android and iOS publish_to: 'none' -version: 5.17.1+1 +version: 5.17.2+1 environment: sdk: '>=3.0.0 <4.0.0' diff --git a/setup.py b/setup.py index 9aedc05c..7205874f 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ setup( name='timetracker', - version='5.17.1', + version='5.17.2', packages=find_packages(), include_package_data=True, package_data={ diff --git a/tests/test_health_ratelimit_exempt.py b/tests/test_health_ratelimit_exempt.py index be0fdb19..c0abbab9 100644 --- a/tests/test_health_ratelimit_exempt.py +++ b/tests/test_health_ratelimit_exempt.py @@ -1,36 +1,60 @@ """Health/readiness probes must not be subject to the default rate limit. Render (and similar platforms) probe /_health every few seconds from a fixed -IP. With RATELIMIT_DEFAULT of "50 per hour", those probes would otherwise -return 429 after ~4 minutes and trigger unhealthy restarts / 502s. +IP. Without exemption those probes would return 429 and trigger unhealthy +restarts / 502s. """ import pytest +from flask_limiter.wrappers import LimitGroup + +from app import limiter + + +def _force_strict_default(app, limit="5 per hour"): + """Temporarily tighten the global default so exemption tests are meaningful.""" + app.config["RATELIMIT_DEFAULT"] = limit + limiter._default_limits = [p.strip() for p in limit.replace(",", ";").split(";") if p.strip()] + limiter.limit_manager.set_default_limits( + [ + LimitGroup( + limit_provider=limit, + key_function=limiter._key_func, + ) + ] + ) + try: + limiter.reset() + except Exception: + pass @pytest.mark.routes def test_health_check_exempt_from_default_rate_limit(app, client): - """/_health must stay 200 even past the default 50-per-hour limit.""" - assert "50 per hour" in (app.config.get("RATELIMIT_DEFAULT") or "") + """/_health must stay 200 even past a strict default limit.""" + assert app.config.get("RATELIMIT_DEFAULT") + _force_strict_default(app, "5 per hour") - for i in range(55): + for i in range(20): response = client.get("/_health") assert response.status_code == 200, f"request {i + 1} got {response.status_code}" assert response.get_json()["status"] == "healthy" @pytest.mark.routes -def test_ready_check_exempt_from_default_rate_limit(client): +def test_ready_check_exempt_from_default_rate_limit(app, client): """/_ready must also be exempt so readiness probes are not rate-limited.""" - for i in range(55): + _force_strict_default(app, "5 per hour") + for i in range(20): response = client.get("/_ready") assert response.status_code == 200, f"request {i + 1} got {response.status_code}" @pytest.mark.routes -def test_api_health_endpoints_exempt_from_default_rate_limit(client): +def test_api_health_endpoints_exempt_from_default_rate_limit(app, client): """API health endpoints used by monitors/frontend probes stay available.""" - for i in range(55): + _force_strict_default(app, "5 per hour") + for i in range(20): r1 = client.get("/api/health") r2 = client.get("/api/v1/health") assert r1.status_code == 200, f"/api/health request {i + 1} got {r1.status_code}" diff --git a/tests/test_otel_integration.py b/tests/test_otel_integration.py index 43e6fc6b..b2dc71fa 100644 --- a/tests/test_otel_integration.py +++ b/tests/test_otel_integration.py @@ -1,7 +1,7 @@ """OpenTelemetry tracing, metrics hooks, and OTLP log correlation tests.""" import uuid -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -10,6 +10,10 @@ def otel_app(app_config, monkeypatch, tmp_path): """Flask app with in-memory OTel export (no network).""" monkeypatch.setenv("OTEL_ENABLE_IN_TESTS", "1") + # Avoid host env OTLP credentials making _export_allowed always true. + monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) + monkeypatch.delenv("OTEL_EXPORTER_OTLP_TOKEN", raising=False) + monkeypatch.delenv("ENABLE_TELEMETRY", raising=False) from app.telemetry.otel_setup import reset_for_testing @@ -34,6 +38,48 @@ def otel_client(otel_app): return otel_app.test_client() +@pytest.fixture +def otel_app_metrics_opted_in(app_config, monkeypatch, tmp_path): + """In-memory OTel app with telemetry opt-in so metric recording proceeds.""" + monkeypatch.setenv("OTEL_ENABLE_IN_TESTS", "1") + monkeypatch.setenv("ENABLE_TELEMETRY", "true") + monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) + monkeypatch.delenv("OTEL_EXPORTER_OTLP_TOKEN", raising=False) + + from app.telemetry.otel_setup import invalidate_export_allowed_cache, reset_for_testing + + reset_for_testing() + invalidate_export_allowed_cache() + + unique_db_path = tmp_path / f"otel_m_{uuid.uuid4().hex}.sqlite" + config = dict(app_config) + config["SQLALCHEMY_DATABASE_URI"] = f"sqlite:///{unique_db_path}" + + from app import create_app, db + + application = create_app(config) + with application.app_context(): + import app.models # noqa: F401 + + db.create_all() + return application + + +def _collect_metrics(reader): + """Return list of (scope_name, metric_name, attributes_dict, data_point) tuples.""" + out = [] + data = reader.get_metrics_data() + if data is None: + return out + for rm in data.resource_metrics: + for sm in rm.scope_metrics: + scope = sm.scope.name + for metric in sm.metrics: + for pt in metric.data.data_points: + out.append((scope, metric.name, dict(pt.attributes), pt)) + return out + + def test_health_request_emits_span(otel_client): resp = otel_client.get("/_health") assert resp.status_code == 200 @@ -75,3 +121,193 @@ def test_http_server_metrics_record_does_not_raise_when_otel_inactive(otel_app): reset_for_testing() record_http_server_metrics("GET", "/_health", 200, 0.01) + + +def test_http_metrics_not_recorded_when_opted_out(otel_app, monkeypatch): + """Without telemetry opt-in, record_* must not emit series.""" + monkeypatch.delenv("ENABLE_TELEMETRY", raising=False) + from app.telemetry import otel_setup + from app.telemetry.otel_setup import ( + get_test_metric_reader, + invalidate_export_allowed_cache, + record_http_server_metrics, + ) + + # Ensure baked (non-env) credentials path for the gate. + otel_setup._otlp_credentials_from_env = False + invalidate_export_allowed_cache() + + reader = get_test_metric_reader() + assert reader is not None + record_http_server_metrics("GET", "/_health", 200, 0.01) + collected = _collect_metrics(reader) + http_names = {name for _, name, _, _ in collected if name.startswith("http.server")} + assert http_names == set() + + +def test_http_metrics_recorded_when_opted_in(otel_app_metrics_opted_in): + from app.telemetry.otel_setup import ( + _HTTP_DURATION_BOUNDARIES, + get_test_metric_reader, + record_http_server_metrics, + ) + + reader = get_test_metric_reader() + assert reader is not None + record_http_server_metrics("GET", "/_health", 200, 0.12) + collected = _collect_metrics(reader) + + duration_pts = [(attrs, pt) for scope, name, attrs, pt in collected if name == "http.server.duration"] + request_pts = [(attrs, pt) for scope, name, attrs, pt in collected if name == "http.server.requests"] + assert len(duration_pts) == 1 + assert len(request_pts) == 1 + + dur_attrs, dur_pt = duration_pts[0] + req_attrs, _ = request_pts[0] + + # Histogram: method + status_class only (no route). + assert dur_attrs.get("http.method") == "GET" + assert dur_attrs.get("status_class") == "2xx" + assert "http.route" not in dur_attrs + assert "app_version" not in dur_attrs + assert list(dur_pt.explicit_bounds) == list(_HTTP_DURATION_BOUNDARIES) + + # Counter: keeps route + status_class. + assert req_attrs.get("http.route") == "/_health" + assert req_attrs.get("status_class") == "2xx" + assert "app_version" not in req_attrs + + +def test_http_errors_have_no_route(otel_app_metrics_opted_in): + from app.telemetry.otel_setup import get_test_metric_reader, record_http_server_metrics + + reader = get_test_metric_reader() + record_http_server_metrics("POST", "/login", 404, 0.02) + collected = _collect_metrics(reader) + error_pts = [attrs for _, name, attrs, _ in collected if name == "http.server.errors"] + assert len(error_pts) == 1 + assert error_pts[0].get("status_class") == "4xx" + assert "http.route" not in error_pts[0] + assert "app_version" not in error_pts[0] + + +def test_flask_instrumentation_metrics_dropped(otel_app_metrics_opted_in): + """FlaskInstrumentor metrics must be dropped via View; traces still work.""" + from app.telemetry.otel_setup import get_test_metric_reader, get_test_span_exporter + + client = otel_app_metrics_opted_in.test_client() + resp = client.get("/_health") + assert resp.status_code == 200 + + # Traces still emitted. + spans = get_test_span_exporter().get_finished_spans() + assert len(spans) >= 1 + + reader = get_test_metric_reader() + collected = _collect_metrics(reader) + flask_scopes = {scope for scope, _, _, _ in collected if scope.startswith("opentelemetry.instrumentation")} + assert flask_scopes == set() + + +def test_export_allowed_true_with_env_credentials(monkeypatch): + from app.telemetry import otel_setup + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "https://example.com/otlp") + monkeypatch.setenv("OTEL_EXPORTER_OTLP_TOKEN", "secret-token") + monkeypatch.delenv("ENABLE_TELEMETRY", raising=False) + + with patch("app.telemetry.service._build_otlp_auth_header", return_value="Basic x"): + with patch( + "app.config.analytics_defaults.get_analytics_config", + return_value={"otel_exporter_otlp_endpoint": "", "otel_exporter_otlp_token": ""}, + ): + conn = otel_setup.resolve_otlp_connection() + + assert conn is not None + assert otel_setup._otlp_credentials_from_env is True + otel_setup.invalidate_export_allowed_cache() + assert otel_setup._export_allowed() is True + + +def test_export_allowed_false_without_optin_baked_creds(monkeypatch): + from app.telemetry import otel_setup + + monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) + monkeypatch.delenv("OTEL_EXPORTER_OTLP_TOKEN", raising=False) + monkeypatch.delenv("ENABLE_TELEMETRY", raising=False) + + with patch( + "app.config.analytics_defaults.get_analytics_config", + return_value={ + "otel_exporter_otlp_endpoint": "https://baked.example/otlp", + "otel_exporter_otlp_token": "baked-token", + }, + ): + with patch("app.telemetry.service._build_otlp_auth_header", return_value="Basic y"): + conn = otel_setup.resolve_otlp_connection() + + assert conn is not None + assert otel_setup._otlp_credentials_from_env is False + otel_setup.invalidate_export_allowed_cache() + with patch("app.utils.telemetry.is_telemetry_enabled", return_value=False): + assert otel_setup._export_allowed() is False + + +def test_env_credentials_take_precedence_over_baked(monkeypatch): + from app.telemetry import otel_setup + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "https://env.example/otlp") + monkeypatch.setenv("OTEL_EXPORTER_OTLP_TOKEN", "env-token") + + with patch( + "app.config.analytics_defaults.get_analytics_config", + return_value={ + "otel_exporter_otlp_endpoint": "https://baked.example/otlp", + "otel_exporter_otlp_token": "baked-token", + }, + ): + with patch("app.telemetry.service._build_otlp_auth_header", side_effect=lambda t: f"Auth {t}"): + base, headers = otel_setup.resolve_otlp_connection() + + assert base == "https://env.example/otlp" + assert headers["Authorization"] == "Auth env-token" + assert otel_setup._otlp_credentials_from_env is True + + +def test_optin_metric_exporter_skips_when_disallowed(): + from opentelemetry.sdk.metrics.export import MetricExportResult + + from app.telemetry.otel_setup import _OptInMetricExporter, invalidate_export_allowed_cache + + inner = MagicMock() + inner.export.return_value = MetricExportResult.SUCCESS + inner._preferred_temporality = None + inner._preferred_aggregation = None + wrapper = _OptInMetricExporter(inner) + + invalidate_export_allowed_cache() + with patch("app.telemetry.otel_setup._export_allowed", return_value=False): + result = wrapper.export(MagicMock()) + assert result == MetricExportResult.SUCCESS + inner.export.assert_not_called() + + invalidate_export_allowed_cache() + with patch("app.telemetry.otel_setup._export_allowed", return_value=True): + wrapper.export(MagicMock()) + inner.export.assert_called_once() + + +def test_optin_span_exporter_skips_when_disallowed(): + from opentelemetry.sdk.trace.export import SpanExportResult + + from app.telemetry.otel_setup import _OptInSpanExporter, invalidate_export_allowed_cache + + inner = MagicMock() + inner.export.return_value = SpanExportResult.SUCCESS + wrapper = _OptInSpanExporter(inner) + + invalidate_export_allowed_cache() + with patch("app.telemetry.otel_setup._export_allowed", return_value=False): + result = wrapper.export([]) + assert result == SpanExportResult.SUCCESS + inner.export.assert_not_called() diff --git a/tests/test_polling_ratelimit_exempt.py b/tests/test_polling_ratelimit_exempt.py new file mode 100644 index 00000000..8c2fcc3b --- /dev/null +++ b/tests/test_polling_ratelimit_exempt.py @@ -0,0 +1,121 @@ +"""Background polling endpoints must not burn the global rate-limit budget (Issue #767). + +An idle dashboard polls /timer/status every 30s and /api/timer/status every 60s. +With the previous default of 50/hour per IP, that alone caused 429s within minutes. +""" + +import pytest +from flask_limiter.wrappers import LimitGroup + +from app import db, limiter +from app.models import User + + +def _force_strict_default(app, limit="5 per hour"): + app.config["RATELIMIT_DEFAULT"] = limit + limiter._default_limits = [p.strip() for p in limit.replace(",", ";").split(";") if p.strip()] + limiter.limit_manager.set_default_limits( + [ + LimitGroup( + limit_provider=limit, + key_function=limiter._key_func, + ) + ] + ) + try: + limiter.reset() + except Exception: + pass + + +@pytest.mark.routes +def test_timer_status_polling_exempt_from_default_rate_limit(app, authenticated_client): + """Authenticated /timer/status stays 200 past a strict global default.""" + _force_strict_default(app, "5 per hour") + for i in range(20): + response = authenticated_client.get( + "/timer/status", + headers={"Accept": "application/json"}, + ) + assert response.status_code == 200, f"request {i + 1} got {response.status_code}" + + +@pytest.mark.routes +def test_api_timer_status_polling_exempt_from_default_rate_limit(app, authenticated_client): + """Authenticated /api/timer/status stays 200 past a strict global default.""" + _force_strict_default(app, "5 per hour") + for i in range(20): + response = authenticated_client.get( + "/api/timer/status", + headers={"Accept": "application/json"}, + ) + assert response.status_code == 200, f"request {i + 1} got {response.status_code}" + + +@pytest.mark.routes +def test_api_notifications_exempt_from_default_rate_limit(app, authenticated_client): + """Idle.js polls /api/notifications; must not be rate-limited by the default.""" + _force_strict_default(app, "5 per hour") + for i in range(20): + response = authenticated_client.get( + "/api/notifications", + headers={"Accept": "application/json"}, + ) + assert response.status_code == 200, f"request {i + 1} got {response.status_code}" + + +@pytest.mark.routes +def test_service_worker_exempt_from_default_rate_limit(app, client): + """PWA service-worker updates must not return 429 under a strict default.""" + _force_strict_default(app, "5 per hour") + for i in range(20): + response = client.get("/service-worker.js") + assert response.status_code == 200, f"request {i + 1} got {response.status_code}" + + +@pytest.mark.routes +def test_rate_limit_json_429_for_accept_json(app, client): + """Fetch/XHR clients prefer JSON; 429 must not be an HTML error page.""" + _force_strict_default(app, "3 per hour") + + last = None + for _ in range(10): + last = client.get("/about", headers={"Accept": "application/json"}) + if last.status_code == 429: + break + + assert last is not None + assert last.status_code == 429 + assert last.is_json, f"expected JSON 429, got content-type={last.content_type!r}" + body = last.get_json() + assert body.get("error_code") == "rate_limited" + assert body.get("success") is False + + +@pytest.mark.routes +def test_rate_limit_keyed_per_authenticated_user(app, client, user): + """Two logged-in users behind the same IP get separate rate-limit buckets.""" + _force_strict_default(app, "3 per hour") + + other = User(username="otheruser767", role="user", email="other767@example.com") + other.set_password("password123") + db.session.add(other) + db.session.commit() + + # Exhaust user1's budget on a non-exempt page (same client IP throughout) + client.post("/login", data={"username": user.username, "password": "password123"}, follow_redirects=True) + for _ in range(10): + if client.get("/about").status_code == 429: + break + else: + pytest.fail("expected user1 to hit the rate limit on /about") + + # Switch to other user without clearing limiter storage — separate key => still 200 + client.get("/logout", follow_redirects=True) + client.post( + "/login", + data={"username": other.username, "password": "password123"}, + follow_redirects=True, + ) + r = client.get("/about") + assert r.status_code == 200, f"other user should have a fresh bucket, got {r.status_code}"