From 4b4e23229ac81166a6a523e133b4aac5a2b914ce Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 28 Feb 2025 12:32:30 -0500 Subject: [PATCH] Implement rate limiting on senstive tool shed APIs. --- .../dependencies/pinned-requirements.txt | 2 + lib/tool_shed/webapp/api2/users.py | 61 +++++++++++++------ lib/tool_shed/webapp/fast_app.py | 8 +++ pyproject.toml | 1 + 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/lib/galaxy/dependencies/pinned-requirements.txt b/lib/galaxy/dependencies/pinned-requirements.txt index 4ab8da70ac15..63b576ebef84 100644 --- a/lib/galaxy/dependencies/pinned-requirements.txt +++ b/lib/galaxy/dependencies/pinned-requirements.txt @@ -96,6 +96,7 @@ jsonschema-specifications==2024.10.1 kombu==5.4.2 lagom==2.7.5 legacy-cgi==2.6.2 ; python_full_version >= '3.13' +limits==4.0.1 lxml==5.3.1 mako==1.3.9 markdown==3.7 @@ -175,6 +176,7 @@ schema-salad==8.8.20250205075315 setuptools==75.8.0 setuptools-scm==5.0.2 six==1.17.0 +slowapi==0.1.9 sniffio==1.3.1 social-auth-core==4.5.6 sortedcontainers==2.4.0 diff --git a/lib/tool_shed/webapp/api2/users.py b/lib/tool_shed/webapp/api2/users.py index 54eee1b681cd..5cd01885a1d3 100644 --- a/lib/tool_shed/webapp/api2/users.py +++ b/lib/tool_shed/webapp/api2/users.py @@ -6,6 +6,7 @@ from fastapi import ( Body, + Request, Response, status, ) @@ -32,6 +33,7 @@ index, ) from tool_shed.structured_app import ToolShedApp +from tool_shed.webapp.fast_app import limiter from tool_shed.webapp.model import ( GalaxySession, User as SaUser, @@ -53,6 +55,8 @@ log = logging.getLogger(__name__) +SENSITIVE_API_REQUEST_LIMIT = "10/minute" + class UiRegisterRequest(BaseModel): email: str @@ -150,8 +154,9 @@ def show(self, trans: SessionRequestContext = DependsOnTrans, encoded_user_id: s summary="Return the user's API key", operation_id="users__get_or_create_api_key", ) + @limiter.limit(SENSITIVE_API_REQUEST_LIMIT) def get_or_create_api_key( - self, trans: SessionRequestContext = DependsOnTrans, encoded_user_id: str = UserIdPathParam + self, request: Request, trans: SessionRequestContext = DependsOnTrans, encoded_user_id: str = UserIdPathParam ) -> str: user = self._get_user(trans, encoded_user_id) return self.api_key_manager.get_or_create_api_key(user) @@ -161,8 +166,9 @@ def get_or_create_api_key( summary="Creates a new API key for the user", operation_id="users__create_api_key", ) + @limiter.limit(SENSITIVE_API_REQUEST_LIMIT) def create_api_key( - self, trans: SessionRequestContext = DependsOnTrans, encoded_user_id: str = UserIdPathParam + self, request: Request, trans: SessionRequestContext = DependsOnTrans, encoded_user_id: str = UserIdPathParam ) -> str: user = self._get_user(trans, encoded_user_id) return self.api_key_manager.create_api_key(user).key @@ -173,8 +179,10 @@ def create_api_key( status_code=status.HTTP_204_NO_CONTENT, operation_id="users__delete_api_key", ) + @limiter.limit(SENSITIVE_API_REQUEST_LIMIT) def delete_api_key( self, + request: Request, trans: SessionRequestContext = DependsOnTrans, encoded_user_id: str = UserIdPathParam, ): @@ -198,31 +206,35 @@ def _get_user(self, trans: SessionRequestContext, encoded_user_id: str): description="register a user", operation_id="users__internal_register", ) + @limiter.limit(SENSITIVE_API_REQUEST_LIMIT) def register( - self, trans: SessionRequestContext = DependsOnTrans, request: UiRegisterRequest = Body(...) + self, + request: Request, + trans: SessionRequestContext = DependsOnTrans, + register_request: UiRegisterRequest = Body(...), ) -> UiRegisterResponse: - honeypot_field = request.bear_field + honeypot_field = register_request.bear_field if honeypot_field != "": message = "You've been flagged as a possible bot. If you are not, please try registering again and fill the form out carefully." raise RequestParameterInvalidException(message) - username = request.username + username = register_request.username if username == "repos": raise RequestParameterInvalidException("Cannot create a user with the username 'repos'") - self.user_manager.create(email=request.email, username=username, password=request.password) + self.user_manager.create(email=register_request.email, username=username, password=register_request.password) if self.app.config.user_activation_on: - is_activation_sent = self.user_manager.send_activation_email(trans, request.email, username) + is_activation_sent = self.user_manager.send_activation_email(trans, register_request.email, username) if is_activation_sent: - return UiRegisterResponse(email=request.email, activation_sent=True) + return UiRegisterResponse(email=register_request.email, activation_sent=True) else: return UiRegisterResponse( - email=request.email, + email=register_request.email, activation_sent=False, activation_error=True, contact_email=self.app.config.error_email_to, ) else: - return UiRegisterResponse(email=request.email) + return UiRegisterResponse(email=register_request.email) @router.put( "/api_internal/change_password", @@ -230,11 +242,15 @@ def register( operation_id="users__internal_change_password", status_code=status.HTTP_204_NO_CONTENT, ) + @limiter.limit(SENSITIVE_API_REQUEST_LIMIT) def change_password( - self, trans: SessionRequestContext = DependsOnTrans, request: UiChangePasswordRequest = Body(...) + self, + request: Request, + trans: SessionRequestContext = DependsOnTrans, + change_request: UiChangePasswordRequest = Body(...), ): - password = request.password - current = request.current + password = change_request.password + current = change_request.current if trans.user is None: raise InsufficientPermissionsException("Must be logged into use this functionality") user_id = trans.user.id @@ -251,13 +267,14 @@ def change_password( description="login to web UI", operation_id="users__internal_login", ) + @limiter.limit(SENSITIVE_API_REQUEST_LIMIT) def internal_login( - self, trans: SessionRequestContext = DependsOnTrans, request: UiLoginRequest = Body(...) + self, request: Request, trans: SessionRequestContext = DependsOnTrans, login_request: UiLoginRequest = Body(...) ) -> UiLoginResponse: log.info(f"top of internal_login {trans.session_csrf_token}") - ensure_csrf_token(trans, request) - login = request.login - password = request.password + ensure_csrf_token(trans, login_request) + login = login_request.login + password = login_request.password user = self.user_manager.get_user_by_identity(login) if user is None: raise InsufficientPermissionsException(INVALID_LOGIN_OR_PASSWORD) @@ -279,11 +296,15 @@ def internal_login( description="logout of web UI", operation_id="users__internal_logout", ) + @limiter.limit(SENSITIVE_API_REQUEST_LIMIT) def internal_logout( - self, trans: SessionRequestContext = DependsOnTrans, request: UiLogoutRequest = Body(...) + self, + request: Request, + trans: SessionRequestContext = DependsOnTrans, + logout_request: UiLogoutRequest = Body(...), ) -> UiLogoutResponse: - ensure_csrf_token(trans, request) - handle_user_logout(trans, logout_all=request.logout_all) + ensure_csrf_token(trans, logout_request) + handle_user_logout(trans, logout_all=logout_request.logout_all) return UiLogoutResponse() diff --git a/lib/tool_shed/webapp/fast_app.py b/lib/tool_shed/webapp/fast_app.py index 50fe423a5f5d..1e2e49bc0ed4 100644 --- a/lib/tool_shed/webapp/fast_app.py +++ b/lib/tool_shed/webapp/fast_app.py @@ -18,6 +18,9 @@ RedirectResponse, ) from fastapi.staticfiles import StaticFiles +from slowapi import Limiter, _rate_limit_exceeded_handler +from slowapi.util import get_remote_address +from slowapi.errors import RateLimitExceeded from starlette_graphene3 import ( GraphQLApp, make_graphiql_handler, @@ -163,10 +166,15 @@ def mount_graphql(app: FastAPI, tool_shed_app: ToolShedApp): } +limiter = Limiter(key_func=get_remote_address) + + def initialize_fast_app(gx_webapp, tool_shed_app): app = get_fastapi_instance() add_exception_handler(app) add_request_id_middleware(app) + app.state.limiter = limiter + app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) # type: ignore[arg-type] from .config import SHED_API_VERSION def mount_static(directory: Path): diff --git a/pyproject.toml b/pyproject.toml index 31966ff3cfd0..5282ad46566c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,6 +91,7 @@ dependencies = [ "Routes", "s3fs>=2023.1.0", "schema-salad>=8.7.20240905150001", # Python 3.13 support + "slowapi", "social-auth-core>=4.5.0", # to drop dependency on abandoned python-jose "sortedcontainers", "SQLAlchemy>=2.0,<2.1,!=2.0.36", # https://github.com/sqlalchemy/sqlalchemy/issues/12019,