From b8f66dedaff935d16cc344cb7a5ecef6de8ef940 Mon Sep 17 00:00:00 2001 From: Valentin Porchet Date: Sat, 26 Oct 2024 15:56:27 +0200 Subject: [PATCH] feat: added configurable rate limiting system (#205) * feat: added configurable rate limiting system * feat: finished rate limit feature * fix: updated nginx template --- .env.dist | 7 +- Makefile | 6 +- README.md | 3 +- app/commands/check_and_update_cache.py | 6 +- app/commands/check_new_hero.py | 8 +- app/common/cache_manager.py | 16 +++ app/common/helpers.py | 73 +++--------- app/common/overfast_client.py | 112 ++++++++++++++++++ app/config.py | 21 +++- app/handlers/api_request_handler.py | 3 +- .../search_players_request_handler.py | 23 ++-- app/main.py | 42 ++++--- app/models/errors.py | 10 ++ app/parsers/generics/api_parser.py | 11 +- app/parsers/search_data_parser.py | 5 +- app/routers/gamemodes.py | 2 - build/nginx/Dockerfile | 5 +- build/nginx/entrypoint.sh | 7 ++ ...st-api.conf => overfast-api.conf.template} | 25 ++++ build/reverse-proxy/default.conf | 16 +++ build/reverse-proxy/nginx.conf | 29 +++++ docker-compose.yml | 19 +++ pyproject.toml | 2 +- tests/views/conftest.py | 3 - tests/views/test_gamemodes_route.py | 30 ----- tests/views/test_hero_routes.py | 45 +++++++ tests/views/test_heroes_route.py | 19 +++ tests/views/test_player_career_route.py | 20 ++++ tests/views/test_player_stats_route.py | 23 ++++ .../views/test_player_stats_summary_route.py | 22 ++++ tests/views/test_player_summary_view.py | 20 ++++ tests/views/test_roles_route.py | 21 ++++ tests/views/test_search_players_route.py | 20 ++++ 33 files changed, 525 insertions(+), 149 deletions(-) create mode 100644 app/common/overfast_client.py create mode 100644 build/nginx/entrypoint.sh rename build/nginx/{overfast-api.conf => overfast-api.conf.template} (65%) create mode 100644 build/reverse-proxy/default.conf create mode 100644 build/reverse-proxy/nginx.conf diff --git a/.env.dist b/.env.dist index b3898c8..54244d2 100644 --- a/.env.dist +++ b/.env.dist @@ -7,7 +7,12 @@ APP_BASE_URL=https://overfast-api.tekrop.fr LOG_LEVEL=info MAX_CONCURRENT_REQUESTS=5 STATUS_PAGE_URL= -TOO_MANY_REQUESTS_RESPONSE=false + +# Rate limiting +BLIZZARD_RATE_LIMIT_RETRY_AFTER=5 +RATE_LIMIT_PER_SECOND_PER_IP=10 +RATE_LIMIT_PER_IP_BURST=2 +MAX_CONNECTIONS_PER_IP=5 # Redis REDIS_CACHING_ENABLED=true diff --git a/Makefile b/Makefile index 973e227..ba2362a 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,11 @@ up: ## Build & run OverFastAPI application (production mode) down: ## Stop the app and remove containers @echo "Stopping OverFastAPI and cleaning containers..." - docker compose down -v --remove-orphans + docker compose --profile "*" down -v --remove-orphans + +up-testing: ## Run OverFastAPI application (testing mode) + @echo "Launching OverFastAPI (testing mode)..." + docker compose --profile testing up -d clean: down ## Clean up Docker environment @echo "Cleaning Docker environment..." diff --git a/README.md b/README.md index b00e99e..7ea0955 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ ## ✨ [Live instance](https://overfast-api.tekrop.fr) -The live instance is restricted to **30 req/s** per IP (a shared limit across all endpoints). If you require more, consider hosting your own instance on a server 👍 +The live instance is restricted with a rate limit around 10 req/s per IP (a shared limit across all endpoints). This limit may be adjusted as needed. If you require higher throughput, consider hosting your own instance on a server 👍 - Live instance (Redoc documentation) : https://overfast-api.tekrop.fr/ - Swagger UI : https://overfast-api.tekrop.fr/docs @@ -44,6 +44,7 @@ Then, execute the following commands to launch the dev server : ```shell make build # Build the images, needed for all further commands make start # Launch OverFast API (dev mode) +make testing # Launch OverFast API (testing mode with reverse proxy) ``` The dev server will be running on the port `8000`. You can use the `make down` command to stop and remove the containers. Feel free to type `make` or `make help` to access a comprehensive list of all available commands for your reference. diff --git a/app/commands/check_and_update_cache.py b/app/commands/check_and_update_cache.py index 375e5e0..9f4202e 100644 --- a/app/commands/check_and_update_cache.py +++ b/app/commands/check_and_update_cache.py @@ -4,13 +4,13 @@ import asyncio -import httpx from fastapi import HTTPException from app.common.cache_manager import CacheManager from app.common.exceptions import ParserBlizzardError, ParserParsingError -from app.common.helpers import overfast_client_settings, overfast_internal_error +from app.common.helpers import overfast_internal_error from app.common.logging import logger +from app.common.overfast_client import OverFastClient from app.config import settings from app.parsers.gamemodes_parser import GamemodesParser from app.parsers.generics.abstract_parser import AbstractParser @@ -112,7 +112,7 @@ async def main(): logger.info("Done ! Retrieved keys : {}", len(keys_to_update)) # Instanciate one HTTPX Client to use for all the updates - client = httpx.AsyncClient(**overfast_client_settings) + client = OverFastClient() tasks = [] for key in keys_to_update: diff --git a/app/commands/check_new_hero.py b/app/commands/check_new_hero.py index a8ce614..ceac260 100644 --- a/app/commands/check_new_hero.py +++ b/app/commands/check_new_hero.py @@ -5,17 +5,17 @@ import asyncio -import httpx from fastapi import HTTPException from app.common.enums import HeroKey -from app.common.helpers import overfast_client_settings, send_discord_webhook_message +from app.common.helpers import send_discord_webhook_message from app.common.logging import logger +from app.common.overfast_client import OverFastClient from app.config import settings from app.parsers.heroes_parser import HeroesParser -async def get_distant_hero_keys(client: httpx.AsyncClient) -> set[str]: +async def get_distant_hero_keys(client: OverFastClient) -> set[str]: """Get a set of Overwatch hero keys from the Blizzard heroes page""" heroes_parser = HeroesParser(client=client) @@ -42,7 +42,7 @@ async def main(): logger.info("OK ! Starting to check if a new hero is here...") # Instanciate one HTTPX Client to use for all the updates - client = httpx.AsyncClient(**overfast_client_settings) + client = OverFastClient() distant_hero_keys = await get_distant_hero_keys(client) local_hero_keys = get_local_hero_keys() diff --git a/app/common/cache_manager.py b/app/common/cache_manager.py index 606671a..46dd55d 100644 --- a/app/common/cache_manager.py +++ b/app/common/cache_manager.py @@ -192,3 +192,19 @@ def update_parser_cache_last_update(self, cache_key: str, expire: int) -> None: @redis_connection_handler def delete_keys(self, keys: Iterable[str]) -> None: self.redis_server.delete(*keys) + + @redis_connection_handler + def is_being_rate_limited(self) -> bool: + return self.redis_server.exists(settings.blizzard_rate_limit_key) + + @redis_connection_handler + def get_global_rate_limit_remaining_time(self) -> int: + return self.redis_server.ttl(settings.blizzard_rate_limit_key) + + @redis_connection_handler + def set_global_rate_limit(self) -> None: + self.redis_server.set( + settings.blizzard_rate_limit_key, + value=0, + ex=settings.blizzard_rate_limit_retry_after, + ) diff --git a/app/common/helpers.py b/app/common/helpers.py index 80a8f4b..27b17a5 100644 --- a/app/common/helpers.py +++ b/app/common/helpers.py @@ -9,10 +9,14 @@ from typing import TYPE_CHECKING, Any import httpx -from fastapi import HTTPException, Request, status +from fastapi import HTTPException, status from app.config import settings -from app.models.errors import BlizzardErrorMessage, InternalServerErrorMessage +from app.models.errors import ( + BlizzardErrorMessage, + InternalServerErrorMessage, + RateLimitErrorMessage, +) from .decorators import rate_limited from .logging import logger @@ -23,6 +27,19 @@ # Typical routes responses to return routes_responses = { + status.HTTP_429_TOO_MANY_REQUESTS: { + "model": RateLimitErrorMessage, + "description": "Rate Limit Error", + "headers": { + "Retry-After": { + "description": "Indicates how long to wait before making a new request", + "schema": { + "type": "string", + "example": "5", + }, + } + }, + }, status.HTTP_500_INTERNAL_SERVER_ERROR: { "model": InternalServerErrorMessage, "description": "Internal Server Error", @@ -32,10 +49,6 @@ "description": "Blizzard Server Error", }, } -if settings.too_many_requests_response: - routes_responses[status.HTTP_429_TOO_MANY_REQUESTS] = { - "description": "Rate Limit Error", - } # List of players used for testing players_ids = [ @@ -49,35 +62,6 @@ "JohnV1-1190", # Player without any title ingame ] -# httpx client settings -overfast_client_settings = { - "headers": { - "User-Agent": ( - f"OverFastAPI v{settings.app_version} - " - "https://github.com/TeKrop/overfast-api" - ), - "From": "valentin.porchet@proton.me", - }, - "http2": True, - "timeout": 10, - "follow_redirects": True, -} - - -async def overfast_request(client: httpx.AsyncClient, url: str) -> httpx.Response: - """Make an HTTP GET request with custom headers and retrieve the result""" - try: - logger.debug("Requesting {}...", url) - response = await client.get(url) - except httpx.TimeoutException as error: - raise blizzard_response_error( - status_code=0, - error="Blizzard took more than 10 seconds to respond, resulting in a timeout", - ) from error - else: - logger.debug("OverFast request done !") - return response - def overfast_internal_error(url: str, error: Exception) -> HTTPException: """Returns an Internal Server Error. Also log it and eventually send @@ -110,25 +94,6 @@ def overfast_internal_error(url: str, error: Exception) -> HTTPException: ) -def blizzard_response_error(status_code: int, error: str) -> HTTPException: - """Retrieve a generic error response when a Blizzard page doesn't load""" - logger.error( - "Received an error from Blizzard. HTTP {} : {}", - status_code, - error, - ) - - return HTTPException( - status_code=status.HTTP_504_GATEWAY_TIMEOUT, - detail=f"Couldn't get Blizzard page (HTTP {status_code} error) : {error}", - ) - - -def blizzard_response_error_from_request(req: Request) -> HTTPException: - """Alias for sending Blizzard error from a request directly""" - return blizzard_response_error(req.status_code, req.text) - - @rate_limited(max_calls=1, interval=1800) def send_discord_webhook_message(message: str) -> httpx.Response | None: """Helper method for sending a Discord webhook message. It's limited to diff --git a/app/common/overfast_client.py b/app/common/overfast_client.py new file mode 100644 index 0000000..5088279 --- /dev/null +++ b/app/common/overfast_client.py @@ -0,0 +1,112 @@ +import httpx +from fastapi import HTTPException, Request, status + +from app.config import settings + +from .cache_manager import CacheManager +from .helpers import send_discord_webhook_message +from .logging import logger +from .metaclasses import Singleton + + +class OverFastClient(metaclass=Singleton): + def __init__(self): + self.cache_manager = CacheManager() + self.client = httpx.AsyncClient( + headers={ + "User-Agent": ( + f"OverFastAPI v{settings.app_version} - " + "https://github.com/TeKrop/overfast-api" + ), + "From": "valentin.porchet@proton.me", + }, + http2=True, + timeout=10, + follow_redirects=True, + ) + + async def get(self, url: str) -> httpx.Response: + """Make an HTTP GET request with custom headers and retrieve the result""" + + # First, check if we're being rate limited + self._check_rate_limit() + + # Make the API call + try: + response = await self.client.get(url) + except httpx.TimeoutException as error: + raise self._blizzard_response_error( + status_code=0, + error="Blizzard took more than 10 seconds to respond, resulting in a timeout", + ) from error + + logger.debug("OverFast request done !") + + # Make sure we catch HTTP 403 from Blizzard when it happens, + # so we don't make any more call before some amount of time + if response.status_code == status.HTTP_403_FORBIDDEN: + raise self._blizzard_forbidden_error() + + return response + + async def aclose(self) -> None: + """Properly close HTTPX Async Client""" + await self.client.aclose() + + def _check_rate_limit(self) -> None: + """Make sure we're not being rate limited by Blizzard before making + any API call. Else, return an HTTP 429 with Retry-After header. + """ + if self.cache_manager.is_being_rate_limited(): + raise self._too_many_requests_response( + retry_after=self.cache_manager.get_global_rate_limit_remaining_time() + ) + + def blizzard_response_error_from_request(self, req: Request) -> HTTPException: + """Alias for sending Blizzard error from a request directly""" + return self._blizzard_response_error(req.status_code, req.text) + + @staticmethod + def _blizzard_response_error(status_code: int, error: str) -> HTTPException: + """Retrieve a generic error response when a Blizzard page doesn't load""" + logger.error( + "Received an error from Blizzard. HTTP {} : {}", + status_code, + error, + ) + + return HTTPException( + status_code=status.HTTP_504_GATEWAY_TIMEOUT, + detail=f"Couldn't get Blizzard page (HTTP {status_code} error) : {error}", + ) + + def _blizzard_forbidden_error(self) -> HTTPException: + """Retrieve a generic error response when Blizzard returns forbidden error. + Also prevent further calls to Blizzard for a given amount of time. + """ + + # We have to block future requests to Blizzard, cache the information on Redis + self.cache_manager.set_global_rate_limit() + + # If Discord Webhook configuration is enabled, send a message to the + # given channel using Discord Webhook URL + send_discord_webhook_message( + "Blizzard Rate Limit reached ! Blocking further calls for " + f"{settings.blizzard_rate_limit_retry_after} seconds..." + ) + + return self._too_many_requests_response( + retry_after=settings.blizzard_rate_limit_retry_after + ) + + @staticmethod + def _too_many_requests_response(retry_after: int) -> HTTPException: + """Generic method to return an HTTP 429 response with Retry-After header""" + return HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail=( + "API has been rate limited by Blizzard, please wait for " + f"{retry_after} seconds before retrying" + ), + headers={"Retry-After": str(retry_after)}, + ) diff --git a/app/config.py b/app/config.py index 3967715..10684ff 100644 --- a/app/config.py +++ b/app/config.py @@ -45,9 +45,24 @@ class Settings(BaseSettings): # Optional, status page URL if you have any to provide status_page_url: str | None = None - # Enable this option if you're using a reverse proxy to handle rate limits - # in order to indicate HTTP 429 as possible response from the API in doc - too_many_requests_response: bool = True + ############ + # RATE LIMITING + ############ + + # Redis key for Blizzard rate limit storage + blizzard_rate_limit_key: str = "blizzard-rate-limit" + + # Number of seconds before the user is authorized to make calls to Blizzard again + blizzard_rate_limit_retry_after: int = 5 + + # Global rate limit of requests per second per ip to apply on the API + rate_limit_per_second_per_ip: int = 10 + + # Global burst value to apply on rate limit before rejecting requests + rate_limit_per_ip_burst: int = 2 + + # Global maximum number of connection per ip + max_connections_per_ip: int = 5 ############ # REDIS CONFIGURATION diff --git a/app/handlers/api_request_handler.py b/app/handlers/api_request_handler.py index 99092e9..0cd808c 100644 --- a/app/handlers/api_request_handler.py +++ b/app/handlers/api_request_handler.py @@ -22,7 +22,6 @@ class APIRequestHandler(ABC): def __init__(self, request: Request): self.cache_key = CacheManager.get_cache_key_from_request(request) - self.overfast_client = request.app.overfast_client @property @abstractmethod @@ -52,7 +51,7 @@ async def process_request(self, **kwargs) -> dict: # Instanciate the parser, it will check if a Parser Cache is here. # If not, it will retrieve its associated Blizzard # page and use the kwargs to generate the appropriate URL - parser = parser_class(client=self.overfast_client, **kwargs) + parser = parser_class(**kwargs) # Do the parsing. Internally, it will check for Parser Cache # before doing a real parsing using BeautifulSoup diff --git a/app/handlers/search_players_request_handler.py b/app/handlers/search_players_request_handler.py index e4dfc64..f91af6e 100644 --- a/app/handlers/search_players_request_handler.py +++ b/app/handlers/search_players_request_handler.py @@ -7,11 +7,10 @@ from app.common.cache_manager import CacheManager from app.common.enums import Locale from app.common.helpers import ( - blizzard_response_error_from_request, get_player_title, - overfast_request, ) from app.common.logging import logger +from app.common.overfast_client import OverFastClient from app.config import settings from app.parsers.search_data_parser import NamecardParser, PortraitParser, TitleParser @@ -30,7 +29,7 @@ class SearchPlayersRequestHandler: def __init__(self, request: Request): self.cache_key = CacheManager.get_cache_key_from_request(request) - self.overfast_client = request.app.overfast_client + self.overfast_client = OverFastClient() async def process_request(self, **kwargs) -> dict: """Main method used to process the request from user and return final data. @@ -43,11 +42,9 @@ async def process_request(self, **kwargs) -> dict: """ # Request the data from Blizzard URL - req = await overfast_request( - client=self.overfast_client, url=self.get_blizzard_url(**kwargs) - ) + req = await self.overfast_client.get(url=self.get_blizzard_url(**kwargs)) if req.status_code != status.HTTP_200_OK: - raise blizzard_response_error_from_request(req) + raise self.overfast_client.blizzard_response_error_from_request(req) players = req.json() @@ -113,17 +110,11 @@ def get_blizzard_url(**kwargs) -> str: return f"{settings.blizzard_host}/{locale}{settings.search_account_path}/{kwargs.get('name')}/" def get_avatar_url(self, player: dict, player_id: str) -> str | None: - return PortraitParser( - client=self.overfast_client, player_id=player_id - ).retrieve_data_value(player) + return PortraitParser(player_id=player_id).retrieve_data_value(player) def get_namecard_url(self, player: dict, player_id: str) -> str | None: - return NamecardParser( - client=self.overfast_client, player_id=player_id - ).retrieve_data_value(player) + return NamecardParser(player_id=player_id).retrieve_data_value(player) def get_title(self, player: dict, player_id: str) -> str | None: - title = TitleParser( - client=self.overfast_client, player_id=player_id - ).retrieve_data_value(player) + title = TitleParser(player_id=player_id).retrieve_data_value(player) return get_player_title(title) diff --git a/app/main.py b/app/main.py index 56d19b0..13288a7 100644 --- a/app/main.py +++ b/app/main.py @@ -2,7 +2,6 @@ from contextlib import asynccontextmanager, suppress -import httpx from fastapi import FastAPI, Request from fastapi.openapi.docs import get_redoc_html, get_swagger_ui_html from fastapi.openapi.utils import get_openapi @@ -12,14 +11,14 @@ from .commands.update_search_data_cache import update_search_data_cache from .common.enums import RouteTag -from .common.helpers import overfast_client_settings from .common.logging import logger +from .common.overfast_client import OverFastClient from .config import settings from .routers import gamemodes, heroes, maps, players, roles @asynccontextmanager -async def lifespan(app: FastAPI): # pragma: no cover +async def lifespan(_: FastAPI): # pragma: no cover # Update search data list from Blizzard before starting up if settings.redis_caching_enabled: logger.info("Updating search data cache (avatars, namecards, titles)") @@ -28,12 +27,12 @@ async def lifespan(app: FastAPI): # pragma: no cover # Instanciate HTTPX Async Client logger.info("Instanciating HTTPX AsyncClient...") - app.overfast_client = httpx.AsyncClient(**overfast_client_settings) + overfast_client = OverFastClient() yield # Properly close HTTPX Async Client - await app.overfast_client.aclose() + await overfast_client.aclose() app = FastAPI(title="OverFast API", docs_url=None, redoc_url=None, lifespan=lifespan) @@ -43,23 +42,26 @@ async def lifespan(app: FastAPI): # pragma: no cover reverse proxy and **Redis** for caching. Its tailored caching mechanism significantly reduces calls to Blizzard pages, ensuring swift and precise data delivery to users. -{ -""" -This live instance is restricted to **30 req/s** per IP (a shared limit across all endpoints). -If you require more, consider hosting your own instance on a server 👍 +This live instance is configured with the following restrictions: +- Rate Limit per IP: **{settings.rate_limit_per_second_per_ip} requests/second** (burst capacity : +**{settings.rate_limit_per_ip_burst}**) +- Maximum connections per IP: **{settings.max_connections_per_ip}** +- Retry delay after Blizzard rate limiting: **{settings.blizzard_rate_limit_retry_after} seconds** + +This limit may be adjusted as needed. If you require higher throughput, consider +hosting your own instance on a server 👍 + +Swagger UI (useful for trying API calls) : {settings.app_base_url}/docs + +{f"Status page : {settings.status_page_url}" if settings.status_page_url else ""} """ -if settings.too_many_requests_response -else "" -} + +players_section_description = """Overwatch players data : summary, statistics, etc. In player career statistics, various conversions are applied for ease of use: - **Duration values** are converted to **seconds** (integer) - **Percent values** are represented as **integers**, omitting the percent symbol - Integer and float string representations are converted to their respective types - -Swagger UI (useful for trying API calls) : {settings.app_base_url}/docs - -{f"Status page : {settings.status_page_url}" if settings.status_page_url else ""} """ @@ -104,7 +106,7 @@ def custom_openapi(): # pragma: no cover }, { "name": RouteTag.PLAYERS, - "description": "Overwatch players data : summary, statistics, etc.", + "description": players_section_description, "externalDocs": { "description": "Blizzard profile pages, source of the information", "url": "https://overwatch.blizzard.com/en-us/search/", @@ -132,7 +134,11 @@ def custom_openapi(): # pragma: no cover @app.exception_handler(StarletteHTTPException) async def http_exception_handler(_: Request, exc: StarletteHTTPException): - return JSONResponse(content={"error": exc.detail}, status_code=exc.status_code) + return JSONResponse( + content={"error": exc.detail}, + status_code=exc.status_code, + headers=exc.headers, + ) # We need to override default Redoc page in order to be diff --git a/app/models/errors.py b/app/models/errors.py index 789cde1..c69f2f4 100644 --- a/app/models/errors.py +++ b/app/models/errors.py @@ -40,3 +40,13 @@ class HeroParserErrorMessage(BaseModel): description="Message describing the hero parser error", examples=["Hero not found or not released yet"], ) + + +class RateLimitErrorMessage(BaseModel): + error: str = Field( + ..., + description="Message describing the rate limit error and number of seconds before retrying", + examples=[ + "API has been rate limited by Blizzard, please wait for 5 seconds before retrying" + ], + ) diff --git a/app/parsers/generics/api_parser.py b/app/parsers/generics/api_parser.py index c34a1a4..26f4bc3 100644 --- a/app/parsers/generics/api_parser.py +++ b/app/parsers/generics/api_parser.py @@ -4,13 +4,12 @@ from functools import cached_property from typing import ClassVar -import httpx from bs4 import BeautifulSoup from fastapi import status from app.common.enums import Locale from app.common.exceptions import ParserParsingError -from app.common.helpers import blizzard_response_error_from_request, overfast_request +from app.common.overfast_client import OverFastClient from app.config import settings from .abstract_parser import AbstractParser @@ -24,9 +23,9 @@ class APIParser(AbstractParser): # List of valid HTTP codes when retrieving Blizzard pages valid_http_codes: ClassVar[list] = [status.HTTP_200_OK] - def __init__(self, client: httpx.AsyncClient, **kwargs): + def __init__(self, **kwargs): self.blizzard_url = self.get_blizzard_url(**kwargs) - self.overfast_client = client + self.overfast_client = OverFastClient() super().__init__(**kwargs) @property @@ -53,9 +52,9 @@ async def retrieve_and_parse_data(self) -> None: """Method used to retrieve data from Blizzard (HTML data), parsing it and storing it into self.data attribute. """ - req = await overfast_request(client=self.overfast_client, url=self.blizzard_url) + req = await self.overfast_client.get(url=self.blizzard_url) if req.status_code not in self.valid_http_codes: - raise blizzard_response_error_from_request(req) + raise self.overfast_client.blizzard_response_error_from_request(req) # Initialize BeautifulSoup object self.root_tag = BeautifulSoup(req.text, "lxml").body.find( diff --git a/app/parsers/search_data_parser.py b/app/parsers/search_data_parser.py index 6a6da65..8897aad 100644 --- a/app/parsers/search_data_parser.py +++ b/app/parsers/search_data_parser.py @@ -5,7 +5,6 @@ from app.commands.update_search_data_cache import retrieve_search_data from app.common.enums import SearchDataType from app.common.exceptions import ParserParsingError, SearchDataRetrievalError -from app.common.helpers import blizzard_response_error_from_request, overfast_request from app.common.logging import logger from app.config import settings @@ -32,9 +31,9 @@ async def retrieve_and_parse_data(self) -> None: """Method used to retrieve data from Blizzard (JSON data), parsing it and storing it into self.data attribute. """ - req = await overfast_request(client=self.overfast_client, url=self.blizzard_url) + req = await self.overfast_client.get(url=self.blizzard_url) if req.status_code not in self.valid_http_codes: - raise blizzard_response_error_from_request(req) + raise self.overfast_client.blizzard_response_error_from_request(req) self.players = req.json() diff --git a/app/routers/gamemodes.py b/app/routers/gamemodes.py index 0f19ee8..dd4a837 100644 --- a/app/routers/gamemodes.py +++ b/app/routers/gamemodes.py @@ -4,7 +4,6 @@ from app.common.decorators import validation_error_handler from app.common.enums import RouteTag -from app.common.helpers import routes_responses from app.handlers.list_gamemodes_request_handler import ListGamemodesRequestHandler from app.models.gamemodes import GamemodeDetails @@ -13,7 +12,6 @@ @router.get( "", - responses=routes_responses, tags=[RouteTag.GAMEMODES], summary="Get a list of gamemodes", description=( diff --git a/build/nginx/Dockerfile b/build/nginx/Dockerfile index 9ace234..c79ef00 100644 --- a/build/nginx/Dockerfile +++ b/build/nginx/Dockerfile @@ -32,7 +32,10 @@ RUN cd nginx-$NGINX_VERSION && \ # Copy specific configuration files COPY nginx.conf /etc/nginx/nginx.conf -COPY overfast-api.conf /etc/nginx/conf.d/default.conf +COPY overfast-api.conf.template /etc/nginx/conf.d/default.conf.template + +COPY entrypoint.sh /entrypoint.sh +RUN chmod +x /entrypoint.sh # Clean everything RUN rm -rf nginx-$NGINX_VERSION* && \ diff --git a/build/nginx/entrypoint.sh b/build/nginx/entrypoint.sh new file mode 100644 index 0000000..9f710bc --- /dev/null +++ b/build/nginx/entrypoint.sh @@ -0,0 +1,7 @@ +#!/bin/sh + +# Replace placeholders and generate config from template +envsubst '${RATE_LIMIT_PER_SECOND_PER_IP} ${RATE_LIMIT_PER_IP_BURST} ${MAX_CONNECTIONS_PER_IP}' < /etc/nginx/conf.d/default.conf.template > /etc/nginx/conf.d/default.conf + +# Launch nginx +nginx -g 'daemon off;' \ No newline at end of file diff --git a/build/nginx/overfast-api.conf b/build/nginx/overfast-api.conf.template similarity index 65% rename from build/nginx/overfast-api.conf rename to build/nginx/overfast-api.conf.template index 5f4b246..c96b452 100644 --- a/build/nginx/overfast-api.conf +++ b/build/nginx/overfast-api.conf.template @@ -1,3 +1,11 @@ +# Create connection zone to limit number of connections per IP/domain +limit_conn_zone $http_x_forwarded_for zone=ofconn:10m; +limit_conn_status 429; + +# Create request zone to introduce a rate limit per IP +limit_req_zone $http_x_forwarded_for zone=ofreq:10m rate=${RATE_LIMIT_PER_SECOND_PER_IP}r/s; +limit_req_status 429; + server { listen 80; @@ -11,6 +19,7 @@ server { expires 1d; add_header Cache-Control public; + add_header Access-Control-Allow-Origin "*" always; } # Favicon @@ -28,6 +37,13 @@ server { # Main route location / { + # Rate limiting instructions + limit_conn ofconn ${MAX_CONNECTIONS_PER_IP}; + limit_req zone=ofreq burst=${RATE_LIMIT_PER_IP_BURST} nodelay; + + # Handle HTTP 429 when triggered by nginx rate limit + error_page 429 = @limit_reached; + # Always authorize any origin as it's a public API add_header Access-Control-Allow-Origin "*" always; @@ -66,4 +82,13 @@ server { # Ensure HEAD requests are passed as GET proxy_method GET; } + + # nginx rate limit reached + location @limit_reached { + add_header Access-Control-Allow-Origin "*" always; + add_header Retry-After 1 always; + + default_type application/json; + return 429 '{"error": "API rate limit reached, please wait for 1 second before retrying"}'; + } } \ No newline at end of file diff --git a/build/reverse-proxy/default.conf b/build/reverse-proxy/default.conf new file mode 100644 index 0000000..70b9038 --- /dev/null +++ b/build/reverse-proxy/default.conf @@ -0,0 +1,16 @@ +server { + listen 80; + listen [::]:80; + server_name localhost; + + location / { + proxy_pass http://nginxbackend; + + # Ensure headers are forwarded + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_pass_request_headers on; + } +} diff --git a/build/reverse-proxy/nginx.conf b/build/reverse-proxy/nginx.conf new file mode 100644 index 0000000..38243bf --- /dev/null +++ b/build/reverse-proxy/nginx.conf @@ -0,0 +1,29 @@ +user nginx; +worker_processes auto; + +error_log /var/log/nginx/error.log notice; +pid /var/run/nginx.pid; + +events { + worker_connections 1024; +} + +http { + include /etc/nginx/mime.types; + default_type application/octet-stream; + + log_format main '$remote_addr - $remote_user [$time_local] "$request" ' + '$status $body_bytes_sent "$http_referer" ' + '"$http_user_agent" "$http_x_forwarded_for"'; + + access_log /var/log/nginx/access.log main; + + sendfile on; + keepalive_timeout 65; + + upstream nginxbackend { + server nginx:80; + } + + include /etc/nginx/conf.d/*.conf; +} \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml index e64b444..b6b192b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -54,6 +54,8 @@ services: context: ./build/nginx ports: - "${APP_PORT}:80" + entrypoint: ["/entrypoint.sh"] + env_file: ${APP_VOLUME_PATH:-.}/.env volumes: - static_volume:/static depends_on: @@ -66,5 +68,22 @@ services: interval: 5s timeout: 2s + reverse-proxy: + profiles: + - testing + image: nginx:alpine + ports: + - "8080:80" + volumes: + - ./build/reverse-proxy/default.conf:/etc/nginx/conf.d/default.conf + - ./build/reverse-proxy/nginx.conf:/etc/nginx/nginx.conf + depends_on: + nginx: + condition: service_started + healthcheck: + test: ["CMD-SHELL", "wget --spider --quiet http://localhost || exit 1"] + interval: 5s + timeout: 2s + volumes: static_volume: \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 51deb6b..1188fa0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "overfast-api" -version = "2.39.0" +version = "2.40.0" description = "Overwatch API giving data about heroes, maps, and players statistics." license = {file = "LICENSE"} authors = [ diff --git a/tests/views/conftest.py b/tests/views/conftest.py index a8572fe..cd70067 100644 --- a/tests/views/conftest.py +++ b/tests/views/conftest.py @@ -1,12 +1,9 @@ -import httpx import pytest from fastapi.testclient import TestClient -from app.common.helpers import overfast_client_settings from app.main import app @pytest.fixture(scope="session") def client() -> TestClient: - app.overfast_client = httpx.AsyncClient(**overfast_client_settings) return TestClient(app) diff --git a/tests/views/test_gamemodes_route.py b/tests/views/test_gamemodes_route.py index 773f8f6..ee42ff3 100644 --- a/tests/views/test_gamemodes_route.py +++ b/tests/views/test_gamemodes_route.py @@ -1,38 +1,8 @@ -from unittest.mock import Mock, patch - -import pytest from fastapi import status from fastapi.testclient import TestClient -@pytest.fixture(scope="module", autouse=True) -def _setup_gamemodes_test(home_html_data: str): - with patch( - "httpx.AsyncClient.get", - return_value=Mock(status_code=status.HTTP_200_OK, text=home_html_data), - ): - yield - - def test_get_gamemodes(client: TestClient, gamemodes_json_data: list): response = client.get("/gamemodes") assert response.status_code == status.HTTP_200_OK assert response.json() == gamemodes_json_data - - -def test_get_gamemodes_internal_error(client: TestClient): - with patch( - "app.handlers.list_gamemodes_request_handler." - "ListGamemodesRequestHandler.process_request", - return_value=[{"invalid_key": "invalid_value"}], - ): - response = client.get("/gamemodes") - assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR - assert response.json() == { - "error": ( - "An internal server error occurred during the process. The developer " - "received a notification, but don't hesitate to create a GitHub " - "issue if you want any news concerning the bug resolution : " - "https://github.com/TeKrop/overfast-api/issues" - ), - } diff --git a/tests/views/test_hero_routes.py b/tests/views/test_hero_routes.py index 81c1fe2..94cadac 100644 --- a/tests/views/test_hero_routes.py +++ b/tests/views/test_hero_routes.py @@ -6,6 +6,7 @@ from app.common.enums import HeroKey from app.common.helpers import read_csv_data_file +from app.config import settings @pytest.mark.parametrize( @@ -92,6 +93,25 @@ def test_get_hero_internal_error(client: TestClient): } +def test_get_hero_blizzard_forbidden_error(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get(f"/heroes/{HeroKey.ANA}") + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } + + @pytest.mark.parametrize( ("hero_name", "hero_html_data"), [(HeroKey.ANA, HeroKey.ANA)], @@ -159,3 +179,28 @@ def test_get_hero_no_hitpoints( response = client.get(f"/heroes/{hero_name}") assert response.status_code == status.HTTP_200_OK assert response.json()["hitpoints"] is None + + +def test_get_hero_blizzard_forbidden_error_and_caching(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock(status_code=status.HTTP_403_FORBIDDEN, text="403 Forbidden"), + ): + response1 = client.get(f"/heroes/{HeroKey.ANA}") + response2 = client.get(f"/heroes/{HeroKey.ANA}") + + assert ( + response1.status_code + == response2.status_code + == status.HTTP_429_TOO_MANY_REQUESTS + ) + assert ( + response1.json() + == response2.json() + == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } + ) diff --git a/tests/views/test_heroes_route.py b/tests/views/test_heroes_route.py index 788b379..2cc5e49 100644 --- a/tests/views/test_heroes_route.py +++ b/tests/views/test_heroes_route.py @@ -98,3 +98,22 @@ def test_get_heroes_internal_error(client: TestClient): "https://github.com/TeKrop/overfast-api/issues" ), } + + +def test_get_heroes_blizzard_forbidden_error(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get("/heroes") + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } diff --git a/tests/views/test_player_career_route.py b/tests/views/test_player_career_route.py index dcffde3..82d9972 100644 --- a/tests/views/test_player_career_route.py +++ b/tests/views/test_player_career_route.py @@ -7,6 +7,7 @@ from httpx import TimeoutException from app.common.helpers import players_ids +from app.config import settings @pytest.mark.parametrize( @@ -114,6 +115,25 @@ def test_get_player_career_internal_error(client: TestClient): } +def test_get_player_career_blizzard_forbidden_error(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get("/players/TeKrop-2217") + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } + + @pytest.mark.parametrize("player_html_data", ["Unknown-1234"], indirect=True) def test_get_player_parser_init_error(client: TestClient, player_html_data: str): with patch( diff --git a/tests/views/test_player_stats_route.py b/tests/views/test_player_stats_route.py index 305dfa5..76a3f2b 100644 --- a/tests/views/test_player_stats_route.py +++ b/tests/views/test_player_stats_route.py @@ -7,6 +7,7 @@ from httpx import TimeoutException from app.common.enums import HeroKeyCareerFilter, PlayerGamemode, PlayerPlatform +from app.config import settings platforms = {p.value for p in PlayerPlatform} gamemodes = {g.value for g in PlayerGamemode} @@ -162,6 +163,28 @@ def test_get_player_stats_blizzard_timeout(client: TestClient, uri: str): } +@pytest.mark.parametrize(("uri"), [("/stats"), ("/stats/career")]) +def test_get_player_stats_blizzard_forbidden_error(client: TestClient, uri: str): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get( + f"/players/TeKrop-2217{uri}?gamemode={PlayerGamemode.QUICKPLAY}", + ) + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } + + def test_get_player_stats_internal_error(client: TestClient): with patch( "app.handlers.get_player_career_request_handler.GetPlayerCareerRequestHandler.process_request", diff --git a/tests/views/test_player_stats_summary_route.py b/tests/views/test_player_stats_summary_route.py index 38cb334..ad6309c 100644 --- a/tests/views/test_player_stats_summary_route.py +++ b/tests/views/test_player_stats_summary_route.py @@ -7,6 +7,7 @@ from app.common.enums import PlayerGamemode, PlayerPlatform from app.common.helpers import read_json_file +from app.config import settings platforms = {p.value for p in PlayerPlatform} gamemodes = {g.value for g in PlayerGamemode} @@ -111,3 +112,24 @@ def test_get_player_stats_internal_error(client: TestClient): "https://github.com/TeKrop/overfast-api/issues" ), } + + +def test_get_player_stats_blizzard_forbidden_error(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get( + f"/players/TeKrop-2217/stats/summary?gamemode={PlayerGamemode.QUICKPLAY}", + ) + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } diff --git a/tests/views/test_player_summary_view.py b/tests/views/test_player_summary_view.py index 1746a57..69f8681 100644 --- a/tests/views/test_player_summary_view.py +++ b/tests/views/test_player_summary_view.py @@ -7,6 +7,7 @@ from httpx import TimeoutException from app.common.helpers import players_ids +from app.config import settings @pytest.mark.parametrize( @@ -107,3 +108,22 @@ def test_get_player_summary_internal_error(client: TestClient): "https://github.com/TeKrop/overfast-api/issues" ), } + + +def test_get_player_summary_blizzard_forbidden_error(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get("/players/TeKrop-2217/summary") + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } diff --git a/tests/views/test_roles_route.py b/tests/views/test_roles_route.py index d7770f4..5c141d3 100644 --- a/tests/views/test_roles_route.py +++ b/tests/views/test_roles_route.py @@ -4,6 +4,8 @@ from fastapi import status from fastapi.testclient import TestClient +from app.config import settings + @pytest.fixture(scope="module", autouse=True) def _setup_roles_test(home_html_data: str): @@ -60,3 +62,22 @@ def test_get_roles_internal_error(client: TestClient): "https://github.com/TeKrop/overfast-api/issues" ), } + + +def test_get_roles_blizzard_forbidden_error(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get("/roles") + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } diff --git a/tests/views/test_search_players_route.py b/tests/views/test_search_players_route.py index ba8e619..d366920 100644 --- a/tests/views/test_search_players_route.py +++ b/tests/views/test_search_players_route.py @@ -7,6 +7,7 @@ from httpx import TimeoutException from app.common.cache_manager import CacheManager +from app.config import settings @pytest.fixture(scope="module", autouse=True) @@ -87,6 +88,25 @@ def test_search_players_blizzard_timeout(client: TestClient): } +def test_get_roles_blizzard_forbidden_error(client: TestClient): + with patch( + "httpx.AsyncClient.get", + return_value=Mock( + status_code=status.HTTP_403_FORBIDDEN, + text="403 Forbidden", + ), + ): + response = client.get("/players?name=Player") + + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.json() == { + "error": ( + "API has been rate limited by Blizzard, please wait for " + f"{settings.blizzard_rate_limit_retry_after} seconds before retrying" + ) + } + + def test_search_players( client: TestClient, search_players_api_json_data: dict, search_data_json_data: dict ):