-
Notifications
You must be signed in to change notification settings - Fork 0
[US-15.1 / OBT-250] Public creation requests for languages and projects #102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e13f5c9
a772bb1
9942f9e
eb57315
1e800ae
8efe0af
f49c5cf
6957740
1fb4730
3f76f27
7e6f58f
b41baea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| """merge sound_necklace main head into obt-250 | ||
|
|
||
| Revision ID: 00d369476436 | ||
| Revises: 20260710_0001, 20260717_0003 | ||
| Create Date: 2026-07-17 17:37:53.488698 | ||
|
|
||
| """ | ||
| from typing import Sequence, Union | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
|
|
||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision: str = '00d369476436' | ||
| down_revision: Union[str, None] = ('20260710_0001', '20260717_0003') | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
| depends_on: Union[str, Sequence[str], None] = None | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| pass | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| pass |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| """add public_requests table | ||
|
|
||
| Revision ID: 20260710_0001 | ||
| Revises: 20260609_0001 | ||
| Create Date: 2026-07-10 | ||
|
|
||
| """ | ||
|
|
||
| from collections.abc import Sequence | ||
|
|
||
| import sqlalchemy as sa | ||
| from alembic import op | ||
|
|
||
| revision: str = "20260710_0001" | ||
| down_revision: str | None = "20260609_0001" | ||
| branch_labels: str | Sequence[str] | None = None | ||
| depends_on: str | Sequence[str] | None = None | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| op.create_table( | ||
| "public_requests", | ||
| sa.Column("id", sa.String(length=36), nullable=False), | ||
| sa.Column("kind", sa.String(length=30), nullable=False), | ||
| sa.Column("status", sa.String(length=20), nullable=False, server_default="pending"), | ||
| sa.Column("requester_name", sa.String(length=200), nullable=False), | ||
| sa.Column("requester_email", sa.String(length=320), nullable=False), | ||
| sa.Column("name", sa.String(length=200), nullable=False), | ||
| sa.Column("code", sa.String(length=3), nullable=True), | ||
| sa.Column("description", sa.Text(), nullable=True), | ||
| sa.Column("language_id", sa.String(length=36), nullable=True), | ||
| sa.Column("new_language_name", sa.String(length=200), nullable=True), | ||
| sa.Column("new_language_code", sa.String(length=3), nullable=True), | ||
| sa.Column("reviewed_by", sa.String(length=36), nullable=True), | ||
| sa.Column("reviewed_at", sa.DateTime(timezone=True), nullable=True), | ||
| sa.Column("review_reason", sa.Text(), nullable=True), | ||
| sa.Column("created_entity_id", sa.String(length=36), nullable=True), | ||
| sa.Column( | ||
| "requested_at", | ||
| sa.DateTime(timezone=True), | ||
| server_default=sa.text("now()"), | ||
| nullable=False, | ||
| ), | ||
| sa.ForeignKeyConstraint(["language_id"], ["languages.id"], ondelete="SET NULL"), | ||
| sa.ForeignKeyConstraint(["reviewed_by"], ["users.id"], ondelete="SET NULL"), | ||
| sa.PrimaryKeyConstraint("id"), | ||
| ) | ||
| op.create_index("ix_public_requests_kind", "public_requests", ["kind"]) | ||
| op.create_index("ix_public_requests_status", "public_requests", ["status"]) | ||
| op.create_index("ix_public_requests_requester_email", "public_requests", ["requester_email"]) | ||
| op.create_index("ix_public_requests_code", "public_requests", ["code"]) | ||
|
Comment on lines
+21
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Enforce pending-language uniqueness atomically. The SELECT-based availability check cannot prevent concurrent conflicting submissions.
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| op.drop_index("ix_public_requests_code", table_name="public_requests") | ||
| op.drop_index("ix_public_requests_requester_email", table_name="public_requests") | ||
| op.drop_index("ix_public_requests_status", table_name="public_requests") | ||
| op.drop_index("ix_public_requests_kind", table_name="public_requests") | ||
| op.drop_table("public_requests") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| from fastapi import APIRouter, Depends, Request, status | ||
| from sqlalchemy.ext.asyncio import AsyncSession | ||
|
|
||
| from app.core.database import get_db | ||
| from app.core.rate_limit import limiter | ||
| from app.models.public_request import ( | ||
| PublicLanguageOption, | ||
| PublicLanguageRequestCreate, | ||
| PublicProjectRequestCreate, | ||
| PublicRequestResponse, | ||
| ) | ||
| from app.services import language_service, public_request_service | ||
|
|
||
| router = APIRouter() | ||
|
|
||
|
|
||
| @router.get("/languages", response_model=list[PublicLanguageOption]) | ||
| @limiter.limit("30/minute") | ||
| async def list_public_languages( | ||
| request: Request, | ||
| db: AsyncSession = Depends(get_db), | ||
| ) -> list[PublicLanguageOption]: | ||
| languages = await language_service.list_languages(db) | ||
| return [PublicLanguageOption.model_validate(lang) for lang in languages] | ||
|
|
||
|
|
||
| @router.post( | ||
| "/requests/language", | ||
| response_model=PublicRequestResponse, | ||
| status_code=status.HTTP_201_CREATED, | ||
| ) | ||
| @limiter.limit("5/minute;20/hour") | ||
| async def request_language_creation( | ||
| request: Request, | ||
| payload: PublicLanguageRequestCreate, | ||
| db: AsyncSession = Depends(get_db), | ||
| ) -> PublicRequestResponse: | ||
| await public_request_service.verify_recaptcha(payload.recaptcha_token) | ||
| created = await public_request_service.create_language_request( | ||
| db, | ||
| requester_name=payload.requester_name, | ||
| requester_email=payload.requester_email, | ||
| name=payload.name, | ||
| code=payload.code, | ||
| description=payload.description, | ||
| ) | ||
| return PublicRequestResponse.model_validate(created) | ||
|
|
||
|
|
||
| @router.post( | ||
| "/requests/project", | ||
| response_model=PublicRequestResponse, | ||
| status_code=status.HTTP_201_CREATED, | ||
| ) | ||
| @limiter.limit("5/minute;20/hour") | ||
| async def request_project_creation( | ||
| request: Request, | ||
| payload: PublicProjectRequestCreate, | ||
| db: AsyncSession = Depends(get_db), | ||
| ) -> PublicRequestResponse: | ||
| await public_request_service.verify_recaptcha(payload.recaptcha_token) | ||
| created = await public_request_service.create_project_request( | ||
| db, | ||
| requester_name=payload.requester_name, | ||
| requester_email=payload.requester_email, | ||
| name=payload.name, | ||
| language_id=payload.language_id, | ||
| description=payload.description, | ||
| new_language_name=payload.new_language_name, | ||
| new_language_code=payload.new_language_code, | ||
| ) | ||
| return PublicRequestResponse.model_validate(created) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| from fastapi import APIRouter, Depends, Query | ||
| from sqlalchemy.ext.asyncio import AsyncSession | ||
|
|
||
| from app.core.auth_middleware import require_platform_admin | ||
| from app.core.database import get_db | ||
| from app.db.models.auth import User | ||
| from app.models.public_request import PublicRequestAdminResponse, PublicRequestReview | ||
| from app.services import public_request_service | ||
|
|
||
| router = APIRouter() | ||
|
|
||
|
|
||
| @router.get("", response_model=list[PublicRequestAdminResponse]) | ||
| async def list_public_requests( | ||
| kind: str | None = Query(default=None), | ||
| status: str | None = Query(default=None), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| db: AsyncSession = Depends(get_db), | ||
| _: User = Depends(require_platform_admin), | ||
| ) -> list[PublicRequestAdminResponse]: | ||
| requests = await public_request_service.list_public_requests(db, kind=kind, status=status) | ||
| return [PublicRequestAdminResponse.model_validate(request) for request in requests] | ||
|
|
||
|
|
||
| @router.patch("/{request_id}/review", response_model=PublicRequestAdminResponse) | ||
| async def review_public_request( | ||
| request_id: str, | ||
| payload: PublicRequestReview, | ||
| db: AsyncSession = Depends(get_db), | ||
| actor: User = Depends(require_platform_admin), | ||
| ) -> PublicRequestAdminResponse: | ||
| request = await public_request_service.review_public_request( | ||
| db, actor, request_id, payload.status, payload.reason | ||
| ) | ||
| return PublicRequestAdminResponse.model_validate(request) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import uuid | ||
| from datetime import datetime | ||
|
|
||
| from sqlalchemy import DateTime, ForeignKey, String, Text | ||
| from sqlalchemy.orm import Mapped, mapped_column | ||
| from sqlalchemy.sql import func | ||
|
|
||
| from app.core.database import Base | ||
|
|
||
|
|
||
| class PublicRequest(Base): | ||
| __tablename__ = "public_requests" | ||
|
|
||
| id: Mapped[str] = mapped_column(String(36), primary_key=True, default=lambda: str(uuid.uuid4())) | ||
| kind: Mapped[str] = mapped_column(String(30), index=True) | ||
| status: Mapped[str] = mapped_column(String(20), default="pending", index=True) | ||
| requester_name: Mapped[str] = mapped_column(String(200)) | ||
| requester_email: Mapped[str] = mapped_column(String(320), index=True) | ||
| name: Mapped[str] = mapped_column(String(200)) | ||
| code: Mapped[str | None] = mapped_column(String(3), nullable=True, index=True) | ||
| description: Mapped[str | None] = mapped_column(Text, nullable=True) | ||
| language_id: Mapped[str | None] = mapped_column( | ||
| ForeignKey("languages.id", ondelete="SET NULL"), nullable=True | ||
| ) | ||
| new_language_name: Mapped[str | None] = mapped_column(String(200), nullable=True) | ||
| new_language_code: Mapped[str | None] = mapped_column(String(3), nullable=True) | ||
| reviewed_by: Mapped[str | None] = mapped_column( | ||
| ForeignKey("users.id", ondelete="SET NULL"), nullable=True | ||
| ) | ||
| reviewed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) | ||
| review_reason: Mapped[str | None] = mapped_column(Text, nullable=True) | ||
| created_entity_id: Mapped[str | None] = mapped_column(String(36), nullable=True) | ||
| requested_at: Mapped[datetime] = mapped_column( | ||
| DateTime(timezone=True), server_default=func.now() | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,8 @@ | |
| from app.api.platform import router as platform_router | ||
| from app.api.project_health import router as project_health_router | ||
| from app.api.projects import router as projects_router | ||
| from app.api.public import router as public_router | ||
| from app.api.public_requests import router as public_requests_router | ||
| from app.api.rag import router as rag_router | ||
| from app.api.roles import router as roles_router | ||
| from app.api.sound_necklace import router as sound_necklace_router | ||
|
|
@@ -133,6 +135,12 @@ def create_app() -> FastAPI: | |
| app.include_router(organizations_router, prefix="/api/organizations", tags=["organizations"]) | ||
| app.include_router(places_router, prefix="/api/places", tags=["places"]) | ||
| app.include_router(projects_router, prefix="/api/projects", tags=["projects"]) | ||
| app.include_router(public_router, prefix="/api/public", tags=["public"]) | ||
| app.include_router( | ||
| public_requests_router, | ||
| prefix="/api/public-requests", | ||
| tags=["public-requests"], | ||
| ) | ||
|
Comment on lines
+138
to
+143
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the relevant app entrypoint and surrounding middleware/router setup.
git ls-files app/main.py app | sed -n '1,120p'
echo '--- app/main.py outline ---'
ast-grep outline app/main.py --view expanded || true
echo '--- app/main.py lines 1-220 ---'
cat -n app/main.py | sed -n '1,220p'
echo '--- search for ProxyHeadersMiddleware and rate limiting ---'
rg -n "ProxyHeadersMiddleware|trusted_hosts|X-Forwarded-For|rate limit|limiter|slowapi|public_requests|public-router|public_requests_router" appRepository: shemaobt/tripod-api Length of output: 15426 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- app/api/public.py ---'
cat -n app/api/public.py | sed -n '1,220p'
echo '--- app/api/public_requests.py ---'
cat -n app/api/public_requests.py | sed -n '1,220p'
echo '--- app/core/rate_limit.py ---'
cat -n app/core/rate_limit.py | sed -n '1,120p'
echo '--- search for dependencies/auth on these routers ---'
rg -n "Depends\\(|require_|oauth2|Authorization|jwt|current_user|admin|auth" app/api/public.py app/api/public_requests.py app/services/public_request app/core/rate_limit.pyRepository: shemaobt/tripod-api Length of output: 5730 🌐 Web query:
💡 Result: Uvicorn's ProxyHeadersMiddleware is designed to process forwarded headers—specifically X-Forwarded-For and X-Forwarded-Proto—to update the request scope's client and scheme information [1][2]. By modifying the scope's client address, this middleware allows subsequent application components (like rate limiters) to see the actual original client IP rather than the proxy's IP [3][4]. Key aspects of its behavior include: Trusted Hosts Configuration: The middleware only processes these headers if the connecting IP (the immediate upstream proxy) is present in the trusted_hosts list [1][5]. When configured via --forwarded-allow-ips (or the ProxyHeadersMiddleware constructor), this list can include IP addresses, IP networks, or literal paths [6][2]. Behavior of trusted_hosts = "": Setting this to "" instructs Uvicorn to trust every incoming connection as a proxy [6][2]. If enabled, the middleware will automatically parse and use the X-Forwarded-For headers from any client [2]. This poses a significant security risk, as any client can then spoof their IP address by injecting arbitrary X-Forwarded-For headers, which the middleware will then trust and propagate [2][3][7]. It should only be used when the infrastructure guarantees that all incoming headers are stripped or sanitized by an external, trusted gateway before reaching Uvicorn [2]. X-Forwarded-For Parsing Logic: When a trusted host is detected, the middleware parses the X-Forwarded-For header [1][8]. It searches for the correct client IP, typically by iterating through the list of IPs in the header and returning the first one that is not considered a proxy (or the correct client address based on the configuration) [9][10]. If no trusted proxy is found, or if the header is missing, the request scope's client address remains unchanged [1][5]. Interaction with SlowAPI: SlowAPI's Citations:
Restrict
🤖 Prompt for AI Agents |
||
| app.include_router(phases_router, prefix="/api/phases", tags=["phases"]) | ||
| app.include_router(books_router, prefix="/api/books", tags=["books"]) | ||
| app.include_router(pericopes_router, prefix="/api/pericopes", tags=["pericopes"]) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| from datetime import datetime | ||
| from typing import Literal | ||
|
|
||
| from pydantic import BaseModel, EmailStr, Field, model_validator | ||
|
|
||
| LANGUAGE_CODE_PATTERN = r"^[A-Za-z]{3}$" | ||
|
|
||
|
|
||
| class PublicLanguageOption(BaseModel): | ||
| id: str | ||
| name: str | ||
| code: str | ||
|
|
||
| model_config = {"from_attributes": True} | ||
|
|
||
|
|
||
| class PublicLanguageRequestCreate(BaseModel): | ||
| requester_name: str = Field(min_length=1, max_length=200) | ||
| requester_email: EmailStr | ||
| name: str = Field(min_length=1, max_length=200) | ||
| code: str = Field(pattern=LANGUAGE_CODE_PATTERN) | ||
| description: str | None = Field(default=None, max_length=10000) | ||
| recaptcha_token: str | None = None | ||
|
|
||
|
|
||
| class PublicProjectRequestCreate(BaseModel): | ||
| requester_name: str = Field(min_length=1, max_length=200) | ||
| requester_email: EmailStr | ||
| name: str = Field(min_length=1, max_length=200) | ||
| description: str | None = Field(default=None, max_length=10000) | ||
| language_id: str | None = None | ||
| new_language_name: str | None = Field(default=None, max_length=200) | ||
| new_language_code: str | None = Field(default=None, pattern=LANGUAGE_CODE_PATTERN) | ||
| recaptcha_token: str | None = None | ||
|
|
||
| @model_validator(mode="after") | ||
| def _one_language_mode(self) -> "PublicProjectRequestCreate": | ||
| if self.language_id and (self.new_language_name or self.new_language_code): | ||
| raise ValueError("Provide an existing language or a new one, not both") | ||
| return self | ||
|
|
||
|
|
||
| class PublicRequestReview(BaseModel): | ||
| status: Literal["approved", "rejected"] | ||
| reason: str | None = Field(default=None, max_length=2000) | ||
|
|
||
|
|
||
| class PublicRequestResponse(BaseModel): | ||
| id: str | ||
| kind: str | ||
| status: str | ||
| requester_name: str | ||
| requester_email: str | ||
| name: str | ||
| code: str | None | ||
| description: str | None | ||
| language_id: str | None | ||
| new_language_name: str | None | ||
| new_language_code: str | None | ||
| requested_at: datetime | ||
|
|
||
| model_config = {"from_attributes": True} | ||
|
|
||
|
|
||
| class PublicRequestAdminResponse(PublicRequestResponse): | ||
| reviewed_by: str | None | ||
| reviewed_at: datetime | None | ||
| review_reason: str | None | ||
| created_entity_id: str | None |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| from app.services.public_request.create_language_request import create_language_request | ||
| from app.services.public_request.create_project_request import create_project_request | ||
| from app.services.public_request.ensure_language_available import ensure_language_available | ||
| from app.services.public_request.list_public_requests import list_public_requests | ||
| from app.services.public_request.review_public_request import review_public_request | ||
| from app.services.public_request.verify_recaptcha import verify_recaptcha | ||
|
|
||
| __all__ = [ | ||
| "create_language_request", | ||
| "create_project_request", | ||
| "ensure_language_available", | ||
| "list_public_requests", | ||
| "review_public_request", | ||
| "verify_recaptcha", | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20260709_0001already revises20260609_0001, and main is at20260714_0002now.alembic upgrade headwill fail with multiple heads on deploy, and the git rebase will not catch it because the files don't conflict. Could you re-point the down_revision to the current head and verify the chain?