diff --git a/README.md b/README.md index 0ef0b35..1f78177 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,44 @@ make format_check # confere Black e isort Os alvos aceitam `compose=`; o padrão é `local.yml`. Não publique arquivos de configuração de ambiente que contenham credenciais. +## API COUNTER e política de acesso + +As rotas GET sob `/r51/` oferecem descoberta, relatórios COUNTER R5.1 e +extensões, incluindo downloads. `COUNTER_API_ACCESS_MODE` seleciona: + +- **`public` (padrão):** sem `api_key`, quota anônima; com chave válida, quota da chave. +- **`key_required`:** toda leitura de dados exige `api_key` válido. + +Chave ausente quando obrigatória, vazia, inválida, revogada, expirada ou repetida +retorna HTTP 401 / COUNTER 2020, sem fallback anônimo. +`/r51/status` é sempre público, ignora chaves e não consome quota. +Swagger (`/r51/`) e schema (`/r51/schema/`) são públicos e descrevem o modo ativo. + +| Variável | Padrão | Uso | +|---|---|---| +| `COUNTER_API_ACCESS_MODE` | `public` | `public` ou `key_required`. | +| `COUNTER_API_ANONYMOUS_QUOTA` | `30` | Requisições por IP/janela. | +| `COUNTER_API_KEY_QUOTA` | `300` | Requisições por chave/janela. | +| `COUNTER_API_RATE_WINDOW_SECONDS` | `60` | Janela fixa em segundos, iniciada na primeira requisição. | +| `REDIS_URL` | Obrigatória | Redis compartilhado por todos os workers da API. | + +Use quotas e janela positivas. A quota é compartilhada entre rotas e formatos; +se esgotada, retorna 429/1020 com `Retry-After` em segundos. Falhas de Redis +retornam 503/1000 sem executar a consulta, sem fallback local. +Anônimos usam `REMOTE_ADDR`, não `X-Forwarded-For` ou `Forwarded`: proxy/NAT pode +compartilhar quota. Configure o IP somente via proxy confiável que remova headers +externos; instâncias independentes devem usar bancos Redis separados. + +Emita uma chave para o consumidor: +```bash +python manage.py issue_counter_api_key 'OCABr backend' +``` +Guarde a chave no backend, nunca no browser; envie `api_key` via HTTPS e não +registre query strings em logs, APM ou erros. A chave só é exibida uma vez. + +O acesso anônimo diverge da seção 8.2 do COUNTER R5.1; nenhum modo implica +certificação COUNTER. + ## Configuração das tarefas As opções de parsing aceitam listas separadas por vírgulas: diff --git a/config/settings/base.py b/config/settings/base.py index f1ee389..030f296 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -339,6 +339,10 @@ "root": {"level": "INFO", "handlers": ["console"]}, } +# Redis +# ------------------------------------------------------------------------------ +REDIS_URL = env.str("REDIS_URL") + # Celery # ------------------------------------------------------------------------------ if USE_TZ: @@ -468,9 +472,18 @@ "TITLE": "SciELO COUNTER API", "DESCRIPTION": ( "COUNTER Project Release 5.1 API with SciELO extensions. " + "Access mode is configured as public (optional api_key for a higher quota) " + "or key_required. Supplied invalid, empty, expired or revoked keys are " + "rejected, except on status, which is always public and unmetered. " + "Anonymous access diverges from COUNTER R5.1 section 8.2; this API " + "description does not assert COUNTER certification. " "Report availability follows completed months in OpenSearch." ), "VERSION": "5.1", + "POSTPROCESSING_HOOKS": [ + "drf_spectacular.hooks.postprocess_schema_enums", + "counter_api.openapi.apply_access_policy", + ], "SERVE_INCLUDE_SCHEMA": False, "SERVE_AUTHENTICATION": [], "SERVE_PERMISSIONS": ["rest_framework.permissions.AllowAny"], @@ -479,15 +492,6 @@ "displayOperationId": True, "persistAuthorization": False, }, - "APPEND_COMPONENTS": { - "securitySchemes": { - "CounterAPIKey": { - "type": "apiKey", - "in": "query", - "name": "api_key", - } - } - }, } # JWT @@ -539,6 +543,15 @@ COUNTER_CUSTOMER_ID = env("COUNTER_CUSTOMER_ID", default="0000000000000000") COUNTER_INSTITUTION_NAME = env("COUNTER_INSTITUTION_NAME", default="The World") COUNTER_CREATED_BY = env("COUNTER_CREATED_BY", default="SciELO") + +COUNTER_API_ACCESS_MODE = env.str("COUNTER_API_ACCESS_MODE", default="public") +COUNTER_API_ANONYMOUS_QUOTA = env.int("COUNTER_API_ANONYMOUS_QUOTA", default=30) +COUNTER_API_KEY_QUOTA = env.int("COUNTER_API_KEY_QUOTA", default=300) +COUNTER_API_RATE_WINDOW_SECONDS = env.int("COUNTER_API_RATE_WINDOW_SECONDS", default=60) + +if COUNTER_API_ACCESS_MODE not in {"public", "key_required"}: + raise ValueError("COUNTER_API_ACCESS_MODE must be public or key_required") + COUNTER_QUERY_BUDGET_SECONDS = env.int( "COUNTER_QUERY_BUDGET_SECONDS", default=25, diff --git a/counter_api/access.py b/counter_api/access.py index 0b610b2..e705997 100644 --- a/counter_api/access.py +++ b/counter_api/access.py @@ -1,3 +1,4 @@ +from django.conf import settings from rest_framework.renderers import JSONRenderer from rest_framework.response import Response from rest_framework.views import APIView @@ -5,21 +6,46 @@ from counter_api.exceptions import CounterAPIError from counter_api.models import APIKey from counter_api.negotiation import CounterContentNegotiation +from counter_api.rate_limit import RateLimitExceeded, check_rate_limit class CounterAPIView(APIView): + """Read-only COUNTER policy, independent of global JWT/admin access rules.""" + + access_exempt = False + authentication_classes = [] permission_classes = [] renderer_classes = [JSONRenderer] content_negotiation_class = CounterContentNegotiation http_method_names = ["get"] + throttle_classes = [] + + def initial(self, request, *args, **kwargs): + super().initial(request, *args, **kwargs) + if request.method != "GET" or self.access_exempt: + return + + supplied = "api_key" in request.query_params + key = None + if supplied: + values = request.query_params.getlist("api_key") + if len(values) == 1: + key = APIKey.authenticate(values[0]) + + if (supplied and key is None) or ( + not supplied and settings.COUNTER_API_ACCESS_MODE == "key_required" + ): + raise CounterAPIError(2020, "APIKey Invalid", 401) + check_rate_limit(request, key) -def get_authentication_error(request): - key = APIKey.authenticate(request.query_params.get("api_key")) - if key: - return None + def handle_exception(self, exc): + if isinstance(exc, CounterAPIError): + headers = {} + if isinstance(exc, RateLimitExceeded): + headers["Retry-After"] = str(exc.retry_after) - error = CounterAPIError(2020, "APIKey Invalid", 401) + return Response(exc.as_dict(), status=exc.status_code, headers=headers) - return Response(error.as_dict(), status=error.status_code) + return super().handle_exception(exc) diff --git a/counter_api/extensions/views.py b/counter_api/extensions/views.py index f5e0f97..08426c8 100644 --- a/counter_api/extensions/views.py +++ b/counter_api/extensions/views.py @@ -5,7 +5,7 @@ from drf_spectacular.utils import OpenApiParameter, extend_schema from rest_framework.response import Response -from counter_api.access import CounterAPIView, get_authentication_error +from counter_api.access import CounterAPIView from counter_api.availability import available_months from counter_api.constants import METRICS from counter_api.dates import parse_period @@ -29,7 +29,6 @@ from counter_api.extensions.ranking import RankingQuery from counter_api.openapi import ( BEGIN_DATE_PARAMETER, - COUNTER_AUTH, END_DATE_PARAMETER, EXCEL_CONTENT_TYPE, PLATFORM_PARAMETER, @@ -52,7 +51,7 @@ def _download_response(payload, kind, platform, output_format): class RankingView(CounterAPIView): @extend_schema( tags=["SciELO extensions"], - auth=COUNTER_AUTH, + auth=[], operation_id="extension_ranking", summary="Rank descendants of a collection, journal or book", parameters=[ @@ -80,10 +79,6 @@ class RankingView(CounterAPIView): }, ) def get(self, request): - authentication_error = get_authentication_error(request) - if authentication_error: - return authentication_error - try: output_format = request.query_params.get("format", "json") if output_format not in DOWNLOAD_FORMATS: @@ -113,17 +108,13 @@ def get(self, request): class CapabilitiesView(CounterAPIView): @extend_schema( tags=["SciELO extensions"], - auth=COUNTER_AUTH, + auth=[], operation_id="extension_capabilities", summary="Discover SciELO extension capabilities for a platform", parameters=[PLATFORM_PARAMETER], responses={200: CapabilitiesSerializer, 400: ExceptionSerializer}, ) def get(self, request): - authentication_error = get_authentication_error(request) - if authentication_error: - return authentication_error - try: platform = get_platform(request.query_params.get("platform")) months = available_months(platform) @@ -145,12 +136,12 @@ def get(self, request): "distribution_dimensions": list(DISTRIBUTION_CAPABILITIES), "country_language_granularity": "year", "other_granularity": "month", - "first_month_available": months[0].strftime("%Y-%m") - if months - else None, - "last_month_available": months[-1].strftime("%Y-%m") - if months - else None, + "first_month_available": ( + months[0].strftime("%Y-%m") if months else None + ), + "last_month_available": ( + months[-1].strftime("%Y-%m") if months else None + ), } ) @@ -158,7 +149,7 @@ def get(self, request): class DistributionView(CounterAPIView): @extend_schema( tags=["SciELO extensions"], - auth=COUNTER_AUTH, + auth=[], operation_id="extension_distribution", summary="Distribute accesses by one dimension or a supported pair", parameters=[ @@ -180,10 +171,6 @@ class DistributionView(CounterAPIView): }, ) def get(self, request): - authentication_error = get_authentication_error(request) - if authentication_error: - return authentication_error - try: output_format = request.query_params.get("format", "json") if output_format not in DOWNLOAD_FORMATS: diff --git a/counter_api/openapi.py b/counter_api/openapi.py index 9473acd..1bcc844 100644 --- a/counter_api/openapi.py +++ b/counter_api/openapi.py @@ -6,7 +6,6 @@ from counter_api.constants import REPORTS EXCEL_CONTENT_TYPE = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" -COUNTER_AUTH = [{"CounterAPIKey": []}] class ExceptionSerializer(serializers.Serializer): @@ -225,8 +224,74 @@ class CounterReportSerializer(serializers.Serializer): (200, EXCEL_CONTENT_TYPE): OpenApiTypes.BINARY, (200, "text/tab-separated-values"): OpenApiTypes.BINARY, 400: ExceptionSerializer, - 401: ExceptionSerializer, 403: ExceptionSerializer, 404: OpenApiResponse(description="Report unavailable for the platform."), 503: ExceptionSerializer, } + + +def apply_access_policy(result, generator, request, public): + """Describe the running mode without duplicating policy on every view.""" + required = settings.COUNTER_API_ACCESS_MODE == "key_required" + result["info"]["description"] += ( + f" Current access mode: {settings.COUNTER_API_ACCESS_MODE}. " + f"Fixed-window quotas: {settings.COUNTER_API_ANONYMOUS_QUOTA} anonymous / " + f"{settings.COUNTER_API_KEY_QUOTA} per key, across all data routes, per " + f"{settings.COUNTER_API_RATE_WINDOW_SECONDS} seconds. " + "Redis failures deny data access with HTTP 503 / COUNTER 1000." + ) + + result["components"].setdefault("securitySchemes", {})["CounterAPIKey"] = { + "type": "apiKey", + "in": "query", + "name": "api_key", + "description": ( + "Required in key_required mode; optional in public mode for the higher " + "quota. A supplied key must be nonempty, active and unexpired. " + "Use HTTPS and redact query strings from logs. Status ignores keys." + ), + } + + error_schema = { + "type": "object", + "required": ["Code", "Message"], + "properties": { + "Code": {"type": "integer"}, + "Message": {"type": "string"}, + "Data": {"type": "string"}, + }, + } + + for path, operations in result["paths"].items(): + if not path.startswith("/r51/") or "get" not in operations: + continue + + operation = operations["get"] + if path == "/r51/status": + operation["security"] = [] + operation["description"] = "Always public, ignores keys, no rate limit." + continue + + operation["security"] = [{"CounterAPIKey": []}] + if not required: + operation["security"].append({}) + + for code, description in ( + ("401", "COUNTER 2020: missing required key or supplied invalid key."), + ("429", "COUNTER 1020: rate quota exhausted. Retry after the window."), + ("503", "COUNTER 1000: service or shared rate limiter unavailable."), + ): + response = { + "description": description, + "content": {"application/json": {"schema": error_schema}}, + } + if code == "429": + response["headers"] = { + "Retry-After": { + "description": "Seconds until the fixed window expires.", + "schema": {"type": "integer", "minimum": 1}, + } + } + operation["responses"][code] = response + + return result diff --git a/counter_api/rate_limit.py b/counter_api/rate_limit.py new file mode 100644 index 0000000..13e01fe --- /dev/null +++ b/counter_api/rate_limit.py @@ -0,0 +1,58 @@ +"""Shared fixed windows, anchored at the first request, using Redis atomically.""" + +import hashlib +from functools import lru_cache + +from django.conf import settings +from redis import Redis +from redis.exceptions import RedisError + +from counter_api.exceptions import CounterAPIError, service_unavailable + +# Do not use Django's cache: production intentionally ignores cache exceptions. +WINDOW_SCRIPT = """ +local count = tonumber(redis.call('GET', KEYS[1]) or '0') +local ttl = redis.call('PTTL', KEYS[1]) +if count >= tonumber(ARGV[1]) and ttl >= 0 then + return {0, math.ceil(ttl / 1000)} +end +if ttl < 0 then + redis.call('SET', KEYS[1], 1, 'PX', ARGV[2] * 1000) +else + redis.call('INCR', KEYS[1]) +end +return {1, math.ceil(redis.call('PTTL', KEYS[1]) / 1000)} +""" + + +@lru_cache(maxsize=4) +def get_redis(url): + return Redis.from_url(url, socket_connect_timeout=1, socket_timeout=1) + + +class RateLimitExceeded(CounterAPIError): + def __init__(self, retry_after): + super().__init__(1020, "Client has made too many requests", 429) + self.retry_after = max(1, retry_after) + + +def check_rate_limit(request, api_key): + if api_key is not None: + identity = f"key:{api_key.digest}" + quota = settings.COUNTER_API_KEY_QUOTA + else: + identity = f"anonymous:{request.META.get('REMOTE_ADDR', '')}" + quota = settings.COUNTER_API_ANONYMOUS_QUOTA + + digest = hashlib.sha256(identity.encode()).hexdigest() + bucket = f"counter:rate:v1:{digest}" + + try: + allowed, retry_after = get_redis(settings.REDIS_URL).eval( + WINDOW_SCRIPT, 1, bucket, quota, settings.COUNTER_API_RATE_WINDOW_SECONDS + ) + except RedisError: + raise service_unavailable() from None + + if not allowed: + raise RateLimitExceeded(retry_after) diff --git a/counter_api/tests/conftest.py b/counter_api/tests/conftest.py new file mode 100644 index 0000000..38529ab --- /dev/null +++ b/counter_api/tests/conftest.py @@ -0,0 +1,11 @@ +from unittest.mock import patch + +import pytest + + +@pytest.fixture(autouse=True) +def counter_redis(): + """Isolate Redis; integration tests opt in with COUNTER_TEST_REDIS_URL.""" + with patch("counter_api.rate_limit.get_redis") as get_redis: + get_redis.return_value.eval.return_value = [1, 60] + yield get_redis.return_value diff --git a/counter_api/tests/test_access.py b/counter_api/tests/test_access.py new file mode 100644 index 0000000..36ef023 --- /dev/null +++ b/counter_api/tests/test_access.py @@ -0,0 +1,274 @@ +import os +import subprocess +import sys +import time +from concurrent.futures import ThreadPoolExecutor +from datetime import timedelta +from unittest.mock import patch +from uuid import uuid4 + +import pytest +from django.urls import reverse +from django.utils import timezone +from redis import Redis +from redis.exceptions import ConnectionError, ResponseError, TimeoutError + +from counter_api.models import APIKey +from counter_api.rate_limit import WINDOW_SCRIPT + +DATA_PATHS = ( + "/r51/members", + "/r51/platforms", + "/r51/reports", + "/r51/reports/tr", + "/r51/extensions/capabilities", + "/r51/extensions/ranking", + "/r51/extensions/distribution", +) + + +@pytest.mark.django_db +@pytest.mark.parametrize("mode", ["public", "key_required"]) +def test_invalid_keys_rejected_on_every_data_route( + client, settings, counter_redis, mode +): + settings.COUNTER_API_ACCESS_MODE = mode + revoked, revoked_value = APIKey.issue("revoked") + revoked.active = False + revoked.save() + _, expired = APIKey.issue("expired", timezone.now() - timedelta(seconds=1)) + + for path in DATA_PATHS: + for value in ("", "invalid", revoked_value, expired): + response = client.get(path, {"api_key": value}) + assert response.status_code == 401 + assert response.json() == {"Code": 2020, "Message": "APIKey Invalid"} + response = client.get(path, {"api_key": ["invalid", "other"]}) + assert response.status_code == 401 + + counter_redis.eval.assert_not_called() + assert not APIKey.objects.exclude(last_used_at=None).exists() + + +@pytest.mark.django_db +@pytest.mark.parametrize("mode", ["public", "key_required"]) +def test_mode_gate_and_admin_remain_separate(client, settings, mode, counter_redis): + settings.COUNTER_API_ACCESS_MODE = mode + key, value = APIKey.issue("client") + + for path in DATA_PATHS: + response = client.get(path) + assert (response.status_code == 401) == (mode == "key_required") + assert client.get("/r51/platforms", {"api_key": value}).status_code == 200 + key.refresh_from_db() + assert key.last_used_at is not None + + for name in ("admin:index", "wagtailadmin_home"): + for params in ({}, {"api_key": value}): + response = client.get(reverse(name), params) + assert response.status_code == 302 + assert "/login/" in response.url + + +@pytest.mark.django_db +@pytest.mark.parametrize("mode", ["public", "key_required"]) +def test_status_is_always_public_unmetered_and_ignores_keys( + client, settings, mode, counter_redis +): + settings.COUNTER_API_ACCESS_MODE = mode + counter_redis.eval.side_effect = ConnectionError("unavailable") + + with patch("counter_api.access.APIKey.authenticate") as authenticate: + for params in ({}, {"api_key": ""}, {"api_key": "invalid"}): + assert client.get("/r51/status", params).status_code == 200 + authenticate.assert_not_called() + + counter_redis.eval.assert_not_called() + assert client.get("/r51/").status_code == 200 + assert client.get("/r51/schema/").status_code == 200 + assert client.post("/r51/platforms").status_code == 405 + + +@pytest.mark.django_db +@pytest.mark.parametrize("mode", ["public", "key_required"]) +@pytest.mark.parametrize("failure", [ConnectionError, TimeoutError, ResponseError]) +def test_redis_failures_deny_data(client, settings, counter_redis, mode, failure): + settings.COUNTER_API_ACCESS_MODE = mode + _, value = APIKey.issue("client") + counter_redis.eval.side_effect = failure("secret redis connection details") + + for path in DATA_PATHS: + response = client.get(path, {"api_key": value}) + assert response.status_code == 503 + assert response.json() == {"Code": 1000, "Message": "Service Not Available"} + if mode == "public": + assert client.get("/r51/platforms").status_code == 503 + + +@pytest.mark.django_db +def test_distinct_shared_quotas_hashing_and_untrusted_headers( + client, settings, counter_redis +): + settings.COUNTER_API_ACCESS_MODE = "public" + settings.COUNTER_API_ANONYMOUS_QUOTA = 2 + settings.COUNTER_API_KEY_QUOTA = 4 + settings.COUNTER_API_RATE_WINDOW_SECONDS = 17 + counts = {} + + def window(script, number_keys, bucket, quota, seconds): + assert script == WINDOW_SCRIPT + assert number_keys == 1 + assert seconds == 17 + counts[bucket] = counts.get(bucket, 0) + 1 + return [int(counts[bucket] <= quota), 9] + + counter_redis.eval.side_effect = window + assert client.get("/r51/platforms", REMOTE_ADDR="192.0.2.1").status_code == 200 + assert client.get("/r51/members", REMOTE_ADDR="192.0.2.1").status_code == 200 + response = client.get( + "/r51/reports/tr", + REMOTE_ADDR="192.0.2.1", + HTTP_X_FORWARDED_FOR="198.51.100.1", + HTTP_FORWARDED="for=198.51.100.2", + ) + assert response.status_code == 429 + assert response.json()["Code"] == 1020 + assert response["Retry-After"] == "9" + assert len(counts) == 1 + + assert client.get("/r51/platforms", REMOTE_ADDR="192.0.2.2").status_code == 200 + _, value = APIKey.issue("client") + for address in ("192.0.2.1", "192.0.2.2", "192.0.2.3", "192.0.2.4"): + assert ( + client.get( + "/r51/platforms", {"api_key": value}, REMOTE_ADDR=address + ).status_code + == 200 + ) + response = client.get("/r51/members", {"api_key": value}) + assert response.status_code == 429 + + _, second_value = APIKey.issue("second-client") + assert client.get("/r51/platforms", {"api_key": second_value}).status_code == 200 + + for bucket in counts: + assert bucket.startswith("counter:rate:v1:") + assert len(bucket.rsplit(":", 1)[1]) == 64 + assert value not in bucket + assert "192.0.2" not in bucket + + +@pytest.mark.django_db +@pytest.mark.parametrize("mode", ["public", "key_required"]) +def test_schema_describes_running_mode(client, settings, mode): + settings.COUNTER_API_ACCESS_MODE = mode + schema = client.get("/r51/schema/", HTTP_ACCEPT="application/json").json() + assert f"Current access mode: {mode}" in schema["info"]["description"] + assert "certification" in schema["info"]["description"] + + for path, operations in schema["paths"].items(): + operation = operations["get"] + assert "api_key" not in [p["name"] for p in operation.get("parameters", [])] + + if path == "/r51/status": + assert operation["security"] == [] + assert "401" not in operation["responses"] + assert "429" not in operation["responses"] + else: + expected = [{"CounterAPIKey": []}] + if mode == "public": + expected.append({}) + assert operation["security"] == expected + assert {"401", "429", "503"} <= operation["responses"].keys() + assert "Retry-After" in operation["responses"]["429"]["headers"] + + +def test_invalid_mode_fails_startup(): + result = subprocess.run( + [sys.executable, "-c", "import config.settings.base"], + env={**os.environ, "COUNTER_API_ACCESS_MODE": "invalid"}, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode != 0 + assert ( + "ValueError: COUNTER_API_ACCESS_MODE must be public or key_required" + in result.stderr + ) + + +@pytest.mark.skipif( + not os.environ.get("COUNTER_TEST_REDIS_URL"), reason="requires disposable Redis" +) +def test_real_redis_lua_is_atomic_and_window_expires(): + # Only a random, owned bucket is touched; never flush Redis. + url = os.environ["COUNTER_TEST_REDIS_URL"] + clients = [Redis.from_url(url) for _ in range(4)] + bucket = f"counter:test:{uuid4().hex}" + + try: + + def attempt(index): + return clients[index % 4].eval(WINDOW_SCRIPT, 1, bucket, 7, 2) + + with ThreadPoolExecutor(max_workers=12) as pool: + results = list(pool.map(attempt, range(50))) + + assert sum(result[0] for result in results) == 7 + assert all(1 <= result[1] <= 2 for result in results) + assert clients[0].get(bucket) == b"7" + assert 0 < clients[0].pttl(bucket) <= 2000 + + time.sleep(2.05) + assert attempt(0) == [1, 2] + + finally: + clients[0].delete(bucket) + for redis in clients: + redis.close() + + +@pytest.mark.django_db +@pytest.mark.parametrize("mode", ["public", "key_required"]) +@pytest.mark.skipif( + not os.environ.get("COUNTER_TEST_REDIS_URL"), reason="requires disposable Redis" +) +def test_real_redis_http_quota_and_recovery(client, settings, mode): + settings.COUNTER_API_ACCESS_MODE = mode + settings.COUNTER_API_ANONYMOUS_QUOTA = 1 + settings.COUNTER_API_KEY_QUOTA = 2 + settings.COUNTER_API_RATE_WINDOW_SECONDS = 1 + _, value = APIKey.issue(uuid4().hex) + redis = Redis.from_url(os.environ["COUNTER_TEST_REDIS_URL"]) + buckets = set() + + def connection(url): + return redis + + original_eval = redis.eval + + def evaluate(script, number_keys, bucket, *args): + buckets.add(bucket) + return original_eval(script, number_keys, bucket, *args) + + try: + with patch("counter_api.rate_limit.get_redis", side_effect=connection): + with patch.object(redis, "eval", side_effect=evaluate): + for _ in range(2): + response = client.get("/r51/platforms", {"api_key": value}) + assert response.status_code == 200 + + response = client.get("/r51/members", {"api_key": value}) + assert response.status_code == 429 + assert response.json()["Code"] == 1020 + assert response["Retry-After"] == "1" + assert client.get("/r51/status").status_code == 200 + + time.sleep(1.05) + response = client.get("/r51/platforms", {"api_key": value}) + assert response.status_code == 200 + finally: + for bucket in buckets: + redis.delete(bucket) + redis.close() diff --git a/counter_api/tests/test_extension_views.py b/counter_api/tests/test_extension_views.py index 96b1d69..73d0828 100644 --- a/counter_api/tests/test_extension_views.py +++ b/counter_api/tests/test_extension_views.py @@ -1,10 +1,15 @@ +import csv from datetime import date +from io import BytesIO, StringIO from unittest.mock import patch from django.test import TestCase +from openpyxl import load_workbook from collection.models import Collection +from counter_api.exceptions import service_unavailable from counter_api.models import APIKey +from counter_api.openapi import EXCEL_CONTENT_TYPE class ExtensionViewTests(TestCase): @@ -23,7 +28,7 @@ def test_extension_capabilities_show_journal_relations(self, available_months): response = self.client.get( "/r51/extensions/capabilities", - {"api_key": self.api_key, "platform": "scl"}, + {"platform": "scl"}, ) self.assertEqual(response.status_code, 200) @@ -36,31 +41,123 @@ def test_extension_capabilities_show_journal_relations(self, available_months): ) self.assertNotIn("coverage", response.json()) + @patch("counter_api.extensions.views.DistributionQuery") @patch("counter_api.extensions.views.RankingQuery") - def test_extension_ranking_download_uses_same_result(self, query_class): - query_class.return_value.run.return_value = { - "groups": [ + def test_public_extensions_preserve_values_in_all_formats( + self, ranking_class, distribution_class + ): + cases = ( + ( + "ranking", + ranking_class, { - "dimensions": {}, - "items": [{"id": "article-1", "title": "Artigo", "count": 12}], - } - ] - } - response = self.client.get( - "/r51/extensions/ranking", - { - "api_key": self.api_key, - "platform": "scl", - "begin_date": "2025-08", - "end_date": "2025-08", - "parent_type": "journal", - "parent_id": "Print_ISSN:1234-5678", - "entity_type": "articles", - "format": "csv", - }, + "parent_type": "journal", + "parent_id": "Print_ISSN:1234-5678", + "entity_type": "articles", + }, + { + "groups": [ + { + "dimensions": {}, + "items": [ + {"id": "article-1", "title": "Artigo", "count": 12} + ], + } + ] + }, + ["Country", "Language", "Rank", "ID", "Title", "Count"], + [None, None, 1, "article-1", "Artigo", 12], + ), + ( + "distribution", + distribution_class, + {"entity_type": "collection", "entity_id": "scl", "dimension": "yop"}, + { + "dimension": "yop", + "total": 12, + "buckets": [{"dimensions": {"yop": 2024}, "count": 12}], + }, + ["Yop", "Count"], + [2024, 12], + ), ) + for kind, query_class, filters, payload, headings, row in cases: + query_class.return_value.run.return_value = payload + for key_state, key in ( + ("absent", None), + ("valid", self.api_key), + ): + for output_format in ("json", "csv", "xlsx"): + with self.subTest(kind=kind, key=key_state, format=output_format): + params = { + "platform": "scl", + "begin_date": "2024-01", + "end_date": "2024-01", + "format": output_format, + **filters, + } + if key is not None: + params["api_key"] = key + response = self.client.get(f"/r51/extensions/{kind}", params) + self.assertEqual(response.status_code, 200) + if output_format == "json": + self.assertEqual(response.json(), payload) + continue + content = b"".join(response.streaming_content) + if output_format == "csv": + self.assertEqual(response["Content-Type"], "text/csv") + rows = list(csv.reader(StringIO(content.decode("utf-8")))) + self.assertEqual( + rows, + [headings, ["" if v is None else str(v) for v in row]], + ) + else: + self.assertEqual( + response["Content-Type"], EXCEL_CONTENT_TYPE + ) + workbook = load_workbook(BytesIO(content), read_only=True) + self.assertEqual( + list(workbook.active.values), + [tuple(headings), tuple(row)], + ) + workbook.close() - lines = b"".join(response.streaming_content).decode("utf-8").splitlines() - self.assertEqual(response.status_code, 200) - self.assertEqual(response["Content-Type"], "text/csv") - self.assertEqual(lines[1], ",,1,article-1,Artigo,12") + @patch("counter_api.extensions.views.DistributionQuery") + @patch("counter_api.extensions.views.RankingQuery") + def test_public_extensions_do_not_turn_failures_into_empty_success( + self, ranking_class, distribution_class + ): + for kind, query_class in ( + ("ranking", ranking_class), + ("distribution", distribution_class), + ): + query_class.return_value.run.side_effect = service_unavailable() + for output_format in ("json", "csv", "xlsx"): + with self.subTest(kind=kind, format=output_format): + response = self.client.get( + f"/r51/extensions/{kind}", + { + "platform": "scl", + "begin_date": "2024-01", + "end_date": "2024-01", + "format": output_format, + }, + ) + self.assertEqual(response.status_code, 503) + self.assertEqual(response.json()["Code"], 1000) + + @patch("counter_api.extensions.views.DistributionQuery") + @patch("counter_api.extensions.views.RankingQuery") + def test_public_extensions_still_reject_current_and_future_months( + self, ranking_class, distribution_class + ): + for month in (date.today().strftime("%Y-%m"), "9999-12"): + for kind in ("ranking", "distribution"): + with self.subTest(kind=kind, month=month): + response = self.client.get( + f"/r51/extensions/{kind}", + {"platform": "scl", "begin_date": month, "end_date": month}, + ) + self.assertEqual(response.status_code, 400) + ranking_class.return_value.run.assert_not_called() + distribution_class.return_value.run.assert_not_called() diff --git a/counter_api/tests/test_http.py b/counter_api/tests/test_http.py index 0b8ed84..364f6ca 100644 --- a/counter_api/tests/test_http.py +++ b/counter_api/tests/test_http.py @@ -1,7 +1,7 @@ from datetime import date from unittest.mock import patch -from django.test import TestCase +from django.test import TestCase, override_settings from collection.models import Collection from counter_api.models import APIKey @@ -15,34 +15,52 @@ def setUp(self): collection_type="journals", is_active=True, ) - _key, self.api_key = APIKey.issue("tests") + self.key, self.api_key = APIKey.issue("tests") - def test_status_is_public_and_private_routes_require_api_key(self): - status = self.client.get("/r51/status") - reports = self.client.get("/r51/reports", {"platform": "scl"}) - - self.assertEqual(status.status_code, 200) - self.assertTrue(status.json()["Service_Active"]) - self.assertEqual(reports.status_code, 401) - self.assertEqual(reports.json()["Code"], 2020) - - def test_counter_api_ignores_global_jwt_authentication(self): - response = self.client.get( + @override_settings( + REST_FRAMEWORK={ + "DEFAULT_AUTHENTICATION_CLASSES": [ + "rest_framework_simplejwt.authentication.JWTAuthentication" + ], + "DEFAULT_PERMISSION_CLASSES": [ + "rest_framework.permissions.IsAuthenticated" + ], + "DEFAULT_SCHEMA_CLASS": "drf_spectacular.openapi.AutoSchema", + } + ) + @patch("counter_api.extensions.views.available_months") + @patch("counter_api.views.available_months") + def test_discovery_is_public_with_optional_valid_keys( + self, available_months, extension_months + ): + available_months.return_value = [date(2024, 1, 1)] + extension_months.return_value = [date(2024, 1, 1)] + paths = ( + "/r51/members", "/r51/platforms", - {"api_key": self.api_key}, - HTTP_AUTHORIZATION="Bearer invalid", + "/r51/reports", + "/r51/extensions/capabilities", ) - - self.assertEqual(response.status_code, 200) + for path in paths: + expected = self.client.get(path, {"platform": "scl"}) + self.assertEqual(expected.status_code, 200) + for key in (None, self.api_key): + with self.subTest(path=path, key_state=(key is None, key == "")): + params = {"platform": "scl"} + if key is not None: + params["api_key"] = key + response = self.client.get( + path, params, HTTP_AUTHORIZATION="Bearer invalid" + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), expected.json()) + self.key.refresh_from_db() + self.assertIsNotNone(self.key.last_used_at) @patch("counter_api.views.available_months") def test_report_catalog_uses_completed_months(self, available_months): available_months.return_value = [date(2024, 1, 1), date(2024, 8, 1)] - - response = self.client.get( - "/r51/reports", - {"api_key": self.api_key, "platform": "scl"}, - ) + response = self.client.get("/r51/reports", {"platform": "scl"}) self.assertEqual(response.status_code, 200) self.assertEqual( @@ -52,6 +70,31 @@ def test_report_catalog_uses_completed_months(self, available_months): self.assertEqual(response.json()[0]["First_Month_Available"], "2024-01") self.assertEqual(response.json()[0]["Last_Month_Available"], "2024-08") + def test_customer_validation_is_preserved_without_a_key(self): + for path in ("/r51/members", "/r51/reports", "/r51/reports/tr"): + with self.subTest(path=path): + response = self.client.get( + path, {"customer_id": "other-institution", "platform": "scl"} + ) + self.assertEqual(response.status_code, 403) + self.assertEqual(response.json()["Code"], 2010) + + def test_public_api_does_not_accept_writes(self): + for path in ( + "/r51/status", + "/r51/members", + "/r51/platforms", + "/r51/reports", + "/r51/reports/tr", + "/r51/extensions/capabilities", + "/r51/extensions/ranking", + "/r51/extensions/distribution", + ): + for method in ("post", "put", "patch", "delete"): + with self.subTest(path=path, method=method): + response = getattr(self.client, method)(path) + self.assertEqual(response.status_code, 405) + def test_swagger_and_schema_are_public_and_limited_to_counter(self): docs = self.client.get("/r51/") schema = self.client.get("/r51/schema/", HTTP_ACCEPT="application/json") @@ -73,7 +116,7 @@ def test_swagger_and_schema_are_public_and_limited_to_counter(self): "/r51/extensions/ranking", }, ) + self.assertFalse(payload.get("security")) self.assertEqual( - payload["components"]["securitySchemes"]["CounterAPIKey"], - {"type": "apiKey", "in": "query", "name": "api_key"}, + payload["components"]["securitySchemes"]["CounterAPIKey"]["name"], "api_key" ) diff --git a/counter_api/tests/test_report_views.py b/counter_api/tests/test_report_views.py index 6510938..1fb358d 100644 --- a/counter_api/tests/test_report_views.py +++ b/counter_api/tests/test_report_views.py @@ -6,6 +6,7 @@ from openpyxl import load_workbook from collection.models import Collection +from counter_api.exceptions import service_unavailable from counter_api.models import APIKey from counter_api.openapi import EXCEL_CONTENT_TYPE from counter_api.reports import build_report @@ -21,7 +22,6 @@ def setUp(self): collection_type="journals", is_active=True, ) - _key, self.api_key = APIKey.issue("tests") def test_title_report_omits_facts_without_source_metadata(self): records = [ @@ -132,7 +132,6 @@ def test_book_report_rejects_inconsistent_index_metrics( response = self.client.get( "/r51/reports/pr", { - "api_key": self.api_key, "platform": "books", "begin_date": "2025-08", "end_date": "2025-08", @@ -179,12 +178,22 @@ def test_report_uses_r51_shape_and_downloads_xlsx(self, query_class, split_perio ] query_class.return_value.deadline = float("inf") params = { - "api_key": self.api_key, "platform": "scl", "begin_date": "2024-01", "end_date": "2024-02", } + key, value = APIKey.issue("legacy-client") + for api_key in (None, value): + with self.subTest(with_key=api_key is not None): + request_params = dict(params) + if api_key is not None: + request_params["api_key"] = api_key + self.assert_report_formats(query_class, request_params) + key.refresh_from_db() + self.assertIsNotNone(key.last_used_at) + + def assert_report_formats(self, query_class, params): response = self.client.get("/r51/reports/tr_j3", params) payload = response.json() item = payload["Report_Items"][0] @@ -249,6 +258,7 @@ def test_report_uses_r51_shape_and_downloads_xlsx(self, query_class, split_perio self.assertEqual(workbook.active["G15"].value, "Print_ISSN") self.assertEqual(workbook.active["G16"].value, "1234-5678") self.assertNotIn("Begin_Date", workbook.active["B7"].value) + workbook.close() params["format"] = "tsv" response = self.client.get("/r51/reports/tr_j3", params) @@ -260,3 +270,20 @@ def test_report_uses_r51_shape_and_downloads_xlsx(self, query_class, split_perio self.assertNotIn("Metric_Type", lines[6]) self.assertEqual(lines[14].split("\t")[-2:], ["Jan-2024", "Feb-2024"]) self.assertEqual(lines[15].split("\t")[6], "1234-5678") + + @patch("counter_api.views.generate_report") + def test_public_report_failures_are_not_empty_successes(self, generate_report): + generate_report.side_effect = service_unavailable() + for output_format in ("json", "tsv", "xlsx"): + with self.subTest(format=output_format): + response = self.client.get( + "/r51/reports/tr", + { + "platform": "scl", + "begin_date": "2024-01", + "end_date": "2024-01", + "format": output_format, + }, + ) + self.assertEqual(response.status_code, 503) + self.assertEqual(response.json()["Code"], 1000) diff --git a/counter_api/views.py b/counter_api/views.py index 5d09011..5b9ae36 100644 --- a/counter_api/views.py +++ b/counter_api/views.py @@ -5,14 +5,13 @@ from drf_spectacular.utils import extend_schema from rest_framework.response import Response -from counter_api.access import CounterAPIView, get_authentication_error +from counter_api.access import CounterAPIView from counter_api.availability import available_months from counter_api.constants import PROPRIETARY_NAMESPACE, RELEASE, REPORTS from counter_api.dates import parse_period from counter_api.exceptions import CounterAPIError, service_unavailable from counter_api.generation import generate_report from counter_api.openapi import ( - COUNTER_AUTH, EXCEL_CONTENT_TYPE, MEMBER_PARAMETERS, REPORT_CATALOG_PARAMETERS, @@ -30,6 +29,8 @@ class StatusView(CounterAPIView): + access_exempt = True + @extend_schema( tags=["Service"], auth=[], @@ -53,21 +54,16 @@ def get(self, request): class MembersView(CounterAPIView): @extend_schema( tags=["Discovery"], - auth=COUNTER_AUTH, + auth=[], operation_id="counter_members", summary="List COUNTER customers", parameters=MEMBER_PARAMETERS, responses={ 200: MemberSerializer(many=True), - 401: ExceptionSerializer, 403: ExceptionSerializer, }, ) def get(self, request): - authentication_error = get_authentication_error(request) - if authentication_error: - return authentication_error - try: validate_customer(request.query_params) except CounterAPIError as error: @@ -91,42 +87,32 @@ def get(self, request): class PlatformsView(CounterAPIView): @extend_schema( tags=["Discovery"], - auth=COUNTER_AUTH, + auth=[], operation_id="counter_platforms", summary="List SciELO COUNTER platforms", responses={ 200: PlatformSerializer(many=True), - 401: ExceptionSerializer, }, ) def get(self, request): - authentication_error = get_authentication_error(request) - if authentication_error: - return authentication_error - return Response(list_platforms()) class ReportsView(CounterAPIView): @extend_schema( tags=["Discovery"], - auth=COUNTER_AUTH, + auth=[], operation_id="counter_reports", summary="List reports available for a platform", parameters=REPORT_CATALOG_PARAMETERS, responses={ 200: ReportDescriptorSerializer(many=True), 400: ExceptionSerializer, - 401: ExceptionSerializer, 403: ExceptionSerializer, 503: ExceptionSerializer, }, ) def get(self, request): - authentication_error = get_authentication_error(request) - if authentication_error: - return authentication_error - try: validate_customer(request.query_params) platform = get_platform(request.query_params.get("platform")) @@ -165,7 +151,7 @@ def get(self, request): class ReportView(CounterAPIView): @extend_schema( tags=["Reports"], - auth=COUNTER_AUTH, + auth=[], operation_id="counter_report", summary="Generate a COUNTER R5.1 report", description=( @@ -177,10 +163,6 @@ class ReportView(CounterAPIView): responses=REPORT_RESPONSES, ) def get(self, request, report_id): - authentication_error = get_authentication_error(request) - if authentication_error: - return authentication_error - try: validate_customer(request.query_params) platform = get_platform(request.query_params.get("platform"))