diff --git a/backend/src/modules/rate_limit/routes.py b/backend/src/modules/rate_limit/routes.py index 3556282f..a043e241 100644 --- a/backend/src/modules/rate_limit/routes.py +++ b/backend/src/modules/rate_limit/routes.py @@ -32,11 +32,15 @@ Results are paginated to handle systems with many rate limit configurations. """, - responses={401: {"description": "Not authenticated"}}, + responses={ + 401: {"description": "Not authenticated"}, + 403: {"description": "Not a superuser"}, + }, response_description="A paginated list of rate limits with their configuration details", ) async def get_rate_limits( db: AsyncSessionDep, + _: CurrentSuperUserDep, rate_limit_service: RateLimitServiceDep, page: int = 1, items_per_page: int = 10, @@ -76,12 +80,17 @@ async def get_rate_limits( Rate limit names are typically in the format of `path:limit:period`. """, - responses={401: {"description": "Not authenticated"}, 404: {"description": "Rate limit not found"}}, + responses={ + 401: {"description": "Not authenticated"}, + 403: {"description": "Not a superuser"}, + 404: {"description": "Rate limit not found"}, + }, response_description="Detailed configuration of the requested rate limit", ) async def get_rate_limit( name: str, db: AsyncSessionDep, + _: CurrentSuperUserDep, rate_limit_service: RateLimitServiceDep, ) -> dict[str, Any] | None: """ diff --git a/backend/src/modules/tier/routes.py b/backend/src/modules/tier/routes.py index 9613b96c..8bd5dbdc 100644 --- a/backend/src/modules/tier/routes.py +++ b/backend/src/modules/tier/routes.py @@ -4,7 +4,7 @@ from fastcrud import PaginatedListResponse, compute_offset, paginated_response from ...infrastructure.auth.http_exceptions import NotFoundException -from ...infrastructure.dependencies import AsyncSessionDep +from ...infrastructure.dependencies import AsyncSessionDep, CurrentUserDep from ..common.exceptions import TierNotFoundError from ..common.utils.error_handler import handle_exception from .dependencies import TierServiceDep @@ -13,14 +13,20 @@ router = APIRouter(tags=["Tiers"]) -@router.get("/", response_model=PaginatedListResponse[TierRead], summary="List tiers") +@router.get( + "/", + response_model=PaginatedListResponse[TierRead], + summary="List tiers", + responses={401: {"description": "Not authenticated"}}, +) async def get_tiers( db: AsyncSessionDep, + _: CurrentUserDep, tier_service: TierServiceDep, page: int = 1, items_per_page: int = 10, ) -> dict: - """Paginated list of tiers.""" + """Paginated list of tiers (authenticated).""" try: tiers_data = await tier_service.get_all( db=db, @@ -35,13 +41,22 @@ async def get_tiers( raise HTTPException(status_code=500, detail="An unexpected error occurred") -@router.get("/{name}", response_model=TierRead, summary="Get a tier by name") +@router.get( + "/{name}", + response_model=TierRead, + summary="Get a tier by name", + responses={ + 401: {"description": "Not authenticated"}, + 404: {"description": "Tier not found"}, + }, +) async def get_tier_by_name( name: str, db: AsyncSessionDep, + _: CurrentUserDep, tier_service: TierServiceDep, ) -> dict[str, Any]: - """Get a tier by name.""" + """Get a tier by name (authenticated).""" try: return await tier_service.get_by_name(name, db) except TierNotFoundError: diff --git a/backend/src/modules/user/routes.py b/backend/src/modules/user/routes.py index e9265cf4..e890ad27 100644 --- a/backend/src/modules/user/routes.py +++ b/backend/src/modules/user/routes.py @@ -13,6 +13,7 @@ from .dependencies import UserServiceDep from .schemas import ( UserCreate, + UserProfileRead, UserRead, UserTierUpdate, UserUpdate, @@ -117,22 +118,23 @@ async def get_current_user_profile( @router.get( "/{username}", - response_model=UserRead, + response_model=UserProfileRead, summary="Get User Profile by Username", description=""" - Retrieves a user's profile information by their unique username. + Retrieves a user's public profile by their unique username. - This endpoint can be used to look up any active user in the system by their - username. It returns the same profile data structure as other user - endpoints but does not include sensitive information. + Any signed-in user can look up any other active user. The response carries + the display fields only - no email address - so the lookup can't be used to + collect addresses. Read your own full record from `/users/me`. Note that usernames are case-sensitive in lookup operations. """, - responses={404: {"description": "User not found"}}, + responses={401: {"description": "Not authenticated"}, 404: {"description": "User not found"}}, response_description="The requested user's profile data", ) async def get_user_by_username( username: str, + _: CurrentUserDep, db: AsyncSessionDep, user_service: UserServiceDep, ) -> dict[str, Any]: diff --git a/backend/src/modules/user/schemas.py b/backend/src/modules/user/schemas.py index 2a1754fc..35bb3664 100644 --- a/backend/src/modules/user/schemas.py +++ b/backend/src/modules/user/schemas.py @@ -43,6 +43,24 @@ class User(TimestampSchema, UserBase, PersistentDeletion): oauth_updated_at: datetime | None = None +class UserProfileRead(BaseModel): + """Another user's profile: the fields any signed-in user may see. + + No email address, so looking someone up by username can't be used to collect + addresses. The owner reads their own record through ``/users/me``, and a + superuser through the list and active-and-inactive endpoints. + """ + + id: int + name: Annotated[str, Field(min_length=2, max_length=30, examples=["User Userson"])] + username: Annotated[ + str, + Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$", examples=["userson"]), + ] + profile_image_url: str + tier_id: int | None = None + + class UserRead(BaseModel): """Schema for reading user data, excludes sensitive information.""" diff --git a/backend/tests/integration/api/v1/test_read_endpoint_auth.py b/backend/tests/integration/api/v1/test_read_endpoint_auth.py new file mode 100644 index 00000000..94ac270b --- /dev/null +++ b/backend/tests/integration/api/v1/test_read_endpoint_auth.py @@ -0,0 +1,68 @@ +"""Who may read the tier and rate-limit endpoints.""" + +import pytest +from httpx import AsyncClient + +pytestmark = pytest.mark.asyncio + +ANONYMOUS_READS = [ + "/api/v1/users/", + "/api/v1/tiers/", + "/api/v1/rate-limits/", +] + + +@pytest.mark.parametrize("path", ANONYMOUS_READS) +async def test_read_endpoints_reject_anonymous_callers(client: AsyncClient, path: str): + """None of the collection reads answer without a session.""" + response = await client.get(path) + + assert response.status_code == 401 + + +async def test_named_reads_reject_anonymous_callers(client: AsyncClient, test_tier: dict): + """The by-name lookups are gated too, before the row is even looked up.""" + assert (await client.get(f"/api/v1/tiers/{test_tier['name']}")).status_code == 401 + assert (await client.get("/api/v1/rate-limits/anything")).status_code == 401 + + +async def test_tiers_are_readable_by_any_signed_in_user(auth_client: AsyncClient, test_tier: dict): + """Tiers describe what a plan offers, so any signed-in user may read them.""" + listing = await auth_client.get("/api/v1/tiers/") + named = await auth_client.get(f"/api/v1/tiers/{test_tier['name']}") + + assert listing.status_code == 200 + assert named.status_code == 200 + assert named.json()["name"] == test_tier["name"] + + +async def test_rate_limit_configuration_is_superuser_only(auth_client: AsyncClient): + """Rate-limit rows are operational configuration, not user-facing data.""" + assert (await auth_client.get("/api/v1/rate-limits/")).status_code == 403 + assert (await auth_client.get("/api/v1/rate-limits/anything")).status_code == 403 + + +async def test_rate_limits_are_readable_by_a_superuser(superuser_auth_client: AsyncClient): + """A superuser reads the same rows that PATCH and DELETE already required one for.""" + response = await superuser_auth_client.get("/api/v1/rate-limits/") + + assert response.status_code == 200 + assert "data" in response.json() + + +@pytest.mark.parametrize( + ("path", "method", "expected"), + [ + ("/api/v1/users/{username}", "get", {"401", "404"}), + ("/api/v1/tiers/", "get", {"401"}), + ("/api/v1/tiers/{name}", "get", {"401", "404"}), + ("/api/v1/rate-limits/", "get", {"401", "403"}), + ("/api/v1/rate-limits/{name}", "get", {"401", "403", "404"}), + ], +) +async def test_openapi_advertises_the_gate(client: AsyncClient, path: str, method: str, expected: set[str]): + """A client generated from the schema must know these can be refused.""" + schema = (await client.get("/openapi.json")).json() + operation = schema["paths"][path][method] + + assert expected <= set(operation["responses"]) diff --git a/backend/tests/integration/api/v1/users/test_read.py b/backend/tests/integration/api/v1/users/test_read.py index f192d573..7aa1c560 100644 --- a/backend/tests/integration/api/v1/users/test_read.py +++ b/backend/tests/integration/api/v1/users/test_read.py @@ -20,8 +20,8 @@ async def test_get_user_by_username_success(auth_client: AsyncClient, db_session data = response.json() assert data["username"] == username assert "id" in data - assert "email" in data assert "name" in data + assert "email" not in data async def test_get_user_by_username_not_found(auth_client: AsyncClient, db_session: AsyncSession): @@ -99,3 +99,25 @@ async def test_get_user_rate_limits(auth_client: AsyncClient, db_session: AsyncS assert response.status_code == 200 data = response.json() assert "rate_limits" in data + + +async def test_get_user_by_username_requires_authentication(client: AsyncClient, test_user: dict): + """An anonymous caller can't look anyone up by username.""" + response = await client.get(f"/api/v1/users/{test_user['username']}") + + assert response.status_code == 401 + + +async def test_profile_lookup_never_carries_an_email(auth_client: AsyncClient, test_user_2: dict, db_session: AsyncSession): + """Looking someone else up returns display fields, never their address. + + Registration is open, so anything this endpoint returns is readable by anyone + willing to sign up; an email address here would be a directory of addresses. + """ + response = await auth_client.get(f"/api/v1/users/{test_user_2['username']}") + + assert response.status_code == 200 + data = response.json() + assert data["username"] == test_user_2["username"] + assert "email" not in data + assert "is_superuser" not in data diff --git a/backend/tests/integration/api/v1/users/test_update.py b/backend/tests/integration/api/v1/users/test_update.py index d6ce6337..cc0d7aab 100644 --- a/backend/tests/integration/api/v1/users/test_update.py +++ b/backend/tests/integration/api/v1/users/test_update.py @@ -4,6 +4,8 @@ from httpx import AsyncClient from sqlalchemy.ext.asyncio import AsyncSession +from src.modules.user.models import User + from .test_create import generate_unique_user_data logging.basicConfig(level=logging.INFO) @@ -35,11 +37,10 @@ async def test_update_user_profile_success( assert "message" in data assert data["message"] == "User updated successfully" - get_response = await auth_client.get(f"/api/v1/users/{username}") - assert get_response.status_code == 200 - user_data = get_response.json() - assert user_data["name"] == update_data["name"] - assert user_data["email"] == update_data["email"] + stored = await db_session.get(User, test_user["id"]) + await db_session.refresh(stored) + assert stored.name == update_data["name"] + assert stored.email == update_data["email"] async def test_update_user_profile_invalid_email( diff --git a/docs/user-guide/api/index.md b/docs/user-guide/api/index.md index e8f56250..d3645693 100644 --- a/docs/user-guide/api/index.md +++ b/docs/user-guide/api/index.md @@ -182,9 +182,9 @@ What ships out of the box (40 total routes): | Prefix | Source | Notes | |--------|--------|-------| -| `POST/GET/PATCH/DELETE /api/v1/users/*` | `modules/user/routes.py` | Open create, session/superuser-gated reads/updates | -| `GET /api/v1/tiers/*` | `modules/tier/routes.py` | Public list + lookup by name | -| `GET/PATCH/DELETE /api/v1/rate-limits/*` | `modules/rate_limit/routes.py` | List/get public; PATCH/DELETE require superuser | +| `POST/GET/PATCH/DELETE /api/v1/users/*` | `modules/user/routes.py` | Open create; reads/updates need a session, and a lookup by username returns no email | +| `GET /api/v1/tiers/*` | `modules/tier/routes.py` | Authenticated list + lookup by name | +| `GET/PATCH/DELETE /api/v1/rate-limits/*` | `modules/rate_limit/routes.py` | Superuser only | | `POST /api/v1/auth/login`, `logout`, `logout-all`, `refresh-csrf`, `check-auth` | `infrastructure/auth/routes.py` | Session auth | | `GET /api/v1/auth/oauth/google`, `oauth/callback/google` | `infrastructure/auth/routes.py` | Google OAuth | | `POST/GET/PATCH/DELETE /api/v1/api-keys/*` | `modules/api_keys/routes.py` | Authenticated key management | diff --git a/docs/user-guide/authentication/user-management.md b/docs/user-guide/authentication/user-management.md index 12e80a69..ac10afb3 100644 --- a/docs/user-guide/authentication/user-management.md +++ b/docs/user-guide/authentication/user-management.md @@ -11,7 +11,7 @@ All under `/api/v1/users/` (defined in `modules/user/routes.py`): | `POST` | `/api/v1/users/` | Create a new user | Open | | `GET` | `/api/v1/users/` | Paginated list of users | Superuser | | `GET` | `/api/v1/users/me` | Current user's profile | Session | -| `GET` | `/api/v1/users/{username}` | Get a user by username (active only) | Open | +| `GET` | `/api/v1/users/{username}` | Public profile by username (no email) | Session | | `GET` | `/api/v1/users/active-and-inactive/{username}` | Same as above, includes soft-deleted | Superuser | | `PATCH` | `/api/v1/users/{username}` | Update profile (own or admin) | Session | | `DELETE` | `/api/v1/users/{username}` | Soft-delete a user (own or admin) | Session |