Skip to content

Commit

Permalink
Implement rate limiting on senstive tool shed APIs.
Browse files Browse the repository at this point in the history
  • Loading branch information
jmchilton committed Feb 28, 2025
1 parent fcbb830 commit 4b4e232
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 20 deletions.
2 changes: 2 additions & 0 deletions lib/galaxy/dependencies/pinned-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
61 changes: 41 additions & 20 deletions lib/tool_shed/webapp/api2/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from fastapi import (
Body,
Request,
Response,
status,
)
Expand All @@ -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,
Expand All @@ -53,6 +55,8 @@

log = logging.getLogger(__name__)

SENSITIVE_API_REQUEST_LIMIT = "10/minute"


class UiRegisterRequest(BaseModel):
email: str
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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,
):
Expand All @@ -198,43 +206,51 @@ 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",
description="reset a user",
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
Expand All @@ -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)
Expand All @@ -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()


Expand Down
8 changes: 8 additions & 0 deletions lib/tool_shed/webapp/fast_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 4b4e232

Please sign in to comment.