feat: enhance error handling and configuration in backend
- Added ORCID_REDIRECT_URI to docker-compose for OAuth callback. - Refactored CORS and trusted hosts settings in configuration for better clarity. - Introduced a new function to validate publication IDs and provide explicit error messages for researcher IDs. - Updated rate limiting strategy to simplify configuration. - Improved security headers middleware to safely remove sensitive headers.
This commit is contained in:
@@ -63,6 +63,27 @@ def _validate_pub_ids(pub_ids: List[UUID]) -> List[UUID]:
|
|||||||
return pub_ids
|
return pub_ids
|
||||||
|
|
||||||
|
|
||||||
|
def _raise_clear_error_if_researcher_id_was_used(db: Session, pub_ids: List[UUID]) -> None:
|
||||||
|
"""
|
||||||
|
Si el cliente envía por error el UUID de un investigador al endpoint
|
||||||
|
de publicaciones, devolvemos un mensaje explícito para guiar el uso.
|
||||||
|
"""
|
||||||
|
if len(pub_ids) != 1:
|
||||||
|
return
|
||||||
|
|
||||||
|
researcher = db.query(Researcher).filter(Researcher.id == pub_ids[0]).first()
|
||||||
|
if researcher:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=400,
|
||||||
|
detail=(
|
||||||
|
"The provided UUID belongs to a researcher, not a publication. "
|
||||||
|
"Use publication IDs for this endpoint, or call "
|
||||||
|
f"/api/export/sword/researcher/{researcher.orcid_id} "
|
||||||
|
f"(or /api/export/zip/researcher/{researcher.orcid_id})."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------
|
# ---------------------------------------------------------
|
||||||
# ENDPOINT 1: SWORD múltiples publicaciones
|
# ENDPOINT 1: SWORD múltiples publicaciones
|
||||||
# ---------------------------------------------------------
|
# ---------------------------------------------------------
|
||||||
@@ -81,6 +102,7 @@ async def export_multiple_sword(
|
|||||||
|
|
||||||
pubs = db.query(Publication).filter(Publication.id.in_(pub_ids)).all()
|
pubs = db.query(Publication).filter(Publication.id.in_(pub_ids)).all()
|
||||||
if not pubs:
|
if not pubs:
|
||||||
|
_raise_clear_error_if_researcher_id_was_used(db, pub_ids)
|
||||||
raise HTTPException(status_code=404, detail="No publications found")
|
raise HTTPException(status_code=404, detail="No publications found")
|
||||||
|
|
||||||
researcher = db.query(Researcher).filter_by(id=pubs[0].researcher_id).first()
|
researcher = db.query(Researcher).filter_by(id=pubs[0].researcher_id).first()
|
||||||
@@ -142,6 +164,7 @@ async def export_multiple_zip(
|
|||||||
|
|
||||||
pubs = db.query(Publication).filter(Publication.id.in_(pub_ids)).all()
|
pubs = db.query(Publication).filter(Publication.id.in_(pub_ids)).all()
|
||||||
if not pubs:
|
if not pubs:
|
||||||
|
_raise_clear_error_if_researcher_id_was_used(db, pub_ids)
|
||||||
raise HTTPException(status_code=404, detail="No publications found")
|
raise HTTPException(status_code=404, detail="No publications found")
|
||||||
|
|
||||||
researcher = db.query(Researcher).filter_by(id=pubs[0].researcher_id).first()
|
researcher = db.query(Researcher).filter_by(id=pubs[0].researcher_id).first()
|
||||||
|
|||||||
@@ -17,7 +17,7 @@ from app.schema.researcher import (
|
|||||||
ResearcherStatsSchema,
|
ResearcherStatsSchema,
|
||||||
ResearcherWithPublicationsSchema,
|
ResearcherWithPublicationsSchema,
|
||||||
)
|
)
|
||||||
from app.security.jwt import get_current_researcher, get_optional_current_researcher
|
from app.security.jwt import get_optional_current_researcher
|
||||||
from app.services.normalizer import PublicationNormalizer
|
from app.services.normalizer import PublicationNormalizer
|
||||||
from app.services.orcid_client import get_display_name, get_work_detail, get_works_summary
|
from app.services.orcid_client import get_display_name, get_work_detail, get_works_summary
|
||||||
from app.utils.orcid_validator import ORCID_PATTERN, is_valid_orcid
|
from app.utils.orcid_validator import ORCID_PATTERN, is_valid_orcid
|
||||||
@@ -184,22 +184,13 @@ def build_search_response(orcid_id: str, db: Session, current: Researcher | None
|
|||||||
# ENDPOINT 1: SEARCH + SYNC
|
# ENDPOINT 1: SEARCH + SYNC
|
||||||
# ---------------------------------------------------------
|
# ---------------------------------------------------------
|
||||||
|
|
||||||
def _search_rate_limit(request: Request) -> str:
|
|
||||||
"""
|
|
||||||
Aplica un límite distinto si el usuario está autenticado.
|
|
||||||
Como SlowAPI evalúa el decorador antes de las dependencias, devolvemos
|
|
||||||
el límite más restrictivo y subimos sólo si hay token (state.researcher).
|
|
||||||
"""
|
|
||||||
researcher = getattr(request.state, "researcher", None)
|
|
||||||
return settings.RATE_LIMIT_SEARCH_AUTH if researcher else settings.RATE_LIMIT_SEARCH_ANON
|
|
||||||
|
|
||||||
|
|
||||||
@router.post(
|
@router.post(
|
||||||
"/search",
|
"/search",
|
||||||
response_model=ResearcherBatchSearchResponseSchema,
|
response_model=ResearcherBatchSearchResponseSchema,
|
||||||
response_model_exclude_none=True,
|
response_model_exclude_none=True,
|
||||||
)
|
)
|
||||||
@limiter.limit(_search_rate_limit)
|
@limiter.limit(settings.RATE_LIMIT_SEARCH_ANON)
|
||||||
def search_and_sync_researchers(
|
def search_and_sync_researchers(
|
||||||
request: Request,
|
request: Request,
|
||||||
payload: ResearcherBatchSearchRequestSchema,
|
payload: ResearcherBatchSearchRequestSchema,
|
||||||
@@ -261,7 +252,7 @@ def sync_researcher(
|
|||||||
request: Request,
|
request: Request,
|
||||||
orcid_id: str = Path(min_length=19, max_length=19, pattern=ORCID_PATTERN),
|
orcid_id: str = Path(min_length=19, max_length=19, pattern=ORCID_PATTERN),
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
current: Researcher = Depends(get_current_researcher),
|
current: Researcher | None = Depends(get_optional_current_researcher),
|
||||||
):
|
):
|
||||||
if not is_valid_orcid(orcid_id):
|
if not is_valid_orcid(orcid_id):
|
||||||
raise HTTPException(status_code=400, detail="Invalid ORCID iD")
|
raise HTTPException(status_code=400, detail="Invalid ORCID iD")
|
||||||
|
|||||||
+18
-17
@@ -71,9 +71,9 @@ class Settings(BaseSettings):
|
|||||||
ORCID_OAUTH_STATE_COOKIE: str = "orcid_oauth_state"
|
ORCID_OAUTH_STATE_COOKIE: str = "orcid_oauth_state"
|
||||||
ORCID_OAUTH_STATE_TTL_SECONDS: int = 600
|
ORCID_OAUTH_STATE_TTL_SECONDS: int = 600
|
||||||
|
|
||||||
CORS_ALLOWED_ORIGINS: List[str] = Field(default_factory=list)
|
CORS_ALLOWED_ORIGINS: str = ""
|
||||||
|
|
||||||
TRUSTED_HOSTS: List[str] = Field(default_factory=lambda: ["*"])
|
TRUSTED_HOSTS: str = "*"
|
||||||
|
|
||||||
RATE_LIMIT_DEFAULT: str = "60/minute"
|
RATE_LIMIT_DEFAULT: str = "60/minute"
|
||||||
RATE_LIMIT_AUTH: str = "10/minute"
|
RATE_LIMIT_AUTH: str = "10/minute"
|
||||||
@@ -92,19 +92,11 @@ class Settings(BaseSettings):
|
|||||||
SECURITY_HSTS_INCLUDE_SUBDOMAINS: bool = True
|
SECURITY_HSTS_INCLUDE_SUBDOMAINS: bool = True
|
||||||
SECURITY_HSTS_PRELOAD: bool = False
|
SECURITY_HSTS_PRELOAD: bool = False
|
||||||
|
|
||||||
@field_validator("CORS_ALLOWED_ORIGINS", mode="before")
|
|
||||||
@classmethod
|
|
||||||
def _parse_cors(cls, v):
|
|
||||||
return _split_csv(v)
|
|
||||||
|
|
||||||
@field_validator("TRUSTED_HOSTS", mode="before")
|
|
||||||
@classmethod
|
|
||||||
def _parse_trusted_hosts(cls, v):
|
|
||||||
parsed = _split_csv(v) if not isinstance(v, list) else v
|
|
||||||
return parsed or ["*"]
|
|
||||||
|
|
||||||
@model_validator(mode="after")
|
@model_validator(mode="after")
|
||||||
def _validate_security(self) -> "Settings":
|
def _validate_security(self) -> "Settings":
|
||||||
|
cors_origins = self.cors_allowed_origins
|
||||||
|
trusted_hosts = self.trusted_hosts
|
||||||
|
|
||||||
if self.ENVIRONMENT == "production":
|
if self.ENVIRONMENT == "production":
|
||||||
weak = {"change_me", "changeme", "secret", "password", ""}
|
weak = {"change_me", "changeme", "secret", "password", ""}
|
||||||
if self.JWT_SECRET.strip().lower() in weak:
|
if self.JWT_SECRET.strip().lower() in weak:
|
||||||
@@ -116,11 +108,11 @@ class Settings(BaseSettings):
|
|||||||
raise ValueError(
|
raise ValueError(
|
||||||
"JWT_SECRET debe tener al menos 32 caracteres en producción."
|
"JWT_SECRET debe tener al menos 32 caracteres en producción."
|
||||||
)
|
)
|
||||||
if "*" in self.CORS_ALLOWED_ORIGINS:
|
if "*" in cors_origins:
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
"CORS_ALLOWED_ORIGINS no puede contener '*' en producción."
|
"CORS_ALLOWED_ORIGINS no puede contener '*' en producción."
|
||||||
)
|
)
|
||||||
if not self.CORS_ALLOWED_ORIGINS:
|
if not cors_origins:
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
"CORS_ALLOWED_ORIGINS debe definirse explícitamente en producción."
|
"CORS_ALLOWED_ORIGINS debe definirse explícitamente en producción."
|
||||||
)
|
)
|
||||||
@@ -128,12 +120,12 @@ class Settings(BaseSettings):
|
|||||||
raise ValueError(
|
raise ValueError(
|
||||||
"API_KEY_VALUE debe tener al menos 24 caracteres en producción."
|
"API_KEY_VALUE debe tener al menos 24 caracteres en producción."
|
||||||
)
|
)
|
||||||
if self.TRUSTED_HOSTS == ["*"]:
|
if trusted_hosts == ["*"]:
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
"TRUSTED_HOSTS debe definirse explícitamente en producción."
|
"TRUSTED_HOSTS debe definirse explícitamente en producción."
|
||||||
)
|
)
|
||||||
|
|
||||||
for origin in self.CORS_ALLOWED_ORIGINS:
|
for origin in cors_origins:
|
||||||
parsed = urlparse(origin)
|
parsed = urlparse(origin)
|
||||||
if parsed.scheme not in {"http", "https"} or not parsed.netloc:
|
if parsed.scheme not in {"http", "https"} or not parsed.netloc:
|
||||||
raise ValueError(f"Origen CORS inválido: {origin!r}")
|
raise ValueError(f"Origen CORS inválido: {origin!r}")
|
||||||
@@ -144,6 +136,15 @@ class Settings(BaseSettings):
|
|||||||
def is_production(self) -> bool:
|
def is_production(self) -> bool:
|
||||||
return self.ENVIRONMENT == "production"
|
return self.ENVIRONMENT == "production"
|
||||||
|
|
||||||
|
@property
|
||||||
|
def cors_allowed_origins(self) -> List[str]:
|
||||||
|
return _split_csv(self.CORS_ALLOWED_ORIGINS)
|
||||||
|
|
||||||
|
@property
|
||||||
|
def trusted_hosts(self) -> List[str]:
|
||||||
|
parsed = _split_csv(self.TRUSTED_HOSTS)
|
||||||
|
return parsed or ["*"]
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def docs_url(self) -> str | None:
|
def docs_url(self) -> str | None:
|
||||||
return "/docs" if self.DOCS_ENABLED else None
|
return "/docs" if self.DOCS_ENABLED else None
|
||||||
|
|||||||
@@ -39,8 +39,8 @@ def _build_limiter() -> Limiter:
|
|||||||
key_func=_key_func,
|
key_func=_key_func,
|
||||||
default_limits=[settings.RATE_LIMIT_DEFAULT],
|
default_limits=[settings.RATE_LIMIT_DEFAULT],
|
||||||
storage_uri=storage_uri,
|
storage_uri=storage_uri,
|
||||||
headers_enabled=True,
|
headers_enabled=False,
|
||||||
strategy="fixed-window-elastic-expiry",
|
strategy="fixed-window",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -1,19 +1,3 @@
|
|||||||
"""
|
|
||||||
Middleware de cabeceras de seguridad HTTP.
|
|
||||||
|
|
||||||
Aplica un perfil seguro por defecto:
|
|
||||||
- Strict-Transport-Security (HSTS) — fuerza HTTPS en navegadores compatibles.
|
|
||||||
- X-Content-Type-Options: nosniff
|
|
||||||
- X-Frame-Options: DENY (clickjacking)
|
|
||||||
- Referrer-Policy: strict-origin-when-cross-origin
|
|
||||||
- Permissions-Policy: bloquea APIs sensibles por defecto
|
|
||||||
- Cross-Origin-Opener-Policy / Resource-Policy: aislamiento del navegador
|
|
||||||
- Content-Security-Policy laxa para Swagger/OpenAPI (CDN), restrictiva para el resto.
|
|
||||||
|
|
||||||
NOTA: El frontend SPA tiene su propia CSP en su servidor. Aquí
|
|
||||||
endurecemos lo que sirve el backend (JSON, XML, ZIP, /docs, /redoc, etc.).
|
|
||||||
"""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from starlette.middleware.base import BaseHTTPMiddleware
|
from starlette.middleware.base import BaseHTTPMiddleware
|
||||||
@@ -82,7 +66,10 @@ class SecurityHeadersMiddleware(BaseHTTPMiddleware):
|
|||||||
hsts += "; preload"
|
hsts += "; preload"
|
||||||
response.headers.setdefault("Strict-Transport-Security", hsts)
|
response.headers.setdefault("Strict-Transport-Security", hsts)
|
||||||
|
|
||||||
response.headers.pop("Server", None)
|
# `MutableHeaders` no implementa `.pop()`. Eliminamos de forma segura.
|
||||||
response.headers.pop("X-Powered-By", None)
|
if "server" in response.headers:
|
||||||
|
del response.headers["server"]
|
||||||
|
if "x-powered-by" in response.headers:
|
||||||
|
del response.headers["x-powered-by"]
|
||||||
|
|
||||||
return response
|
return response
|
||||||
|
|||||||
+2
-2
@@ -72,7 +72,7 @@ app.add_middleware(
|
|||||||
|
|
||||||
app.add_middleware(
|
app.add_middleware(
|
||||||
CORSMiddleware,
|
CORSMiddleware,
|
||||||
allow_origins=settings.CORS_ALLOWED_ORIGINS,
|
allow_origins=settings.cors_allowed_origins,
|
||||||
allow_credentials=True,
|
allow_credentials=True,
|
||||||
allow_methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"],
|
allow_methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"],
|
||||||
allow_headers=[
|
allow_headers=[
|
||||||
@@ -89,7 +89,7 @@ app.add_middleware(
|
|||||||
|
|
||||||
app.add_middleware(
|
app.add_middleware(
|
||||||
TrustedHostMiddleware,
|
TrustedHostMiddleware,
|
||||||
allowed_hosts=settings.TRUSTED_HOSTS,
|
allowed_hosts=settings.trusted_hosts,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ services:
|
|||||||
environment:
|
environment:
|
||||||
DATABASE_URL: postgresql://postgres:postgres@db:5432/orcid_db
|
DATABASE_URL: postgresql://postgres:postgres@db:5432/orcid_db
|
||||||
REDIS_URL: redis://redis:6379/0
|
REDIS_URL: redis://redis:6379/0
|
||||||
|
ORCID_REDIRECT_URI: https://jargon-supreme-palpable.ngrok-free.dev/callback
|
||||||
depends_on:
|
depends_on:
|
||||||
db:
|
db:
|
||||||
condition: service_healthy
|
condition: service_healthy
|
||||||
|
|||||||
Reference in New Issue
Block a user