Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 83 additions & 4 deletions backend/app/api/v1/users.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from fastapi import APIRouter, Depends, HTTPException, status
from sqlalchemy.ext.asyncio import AsyncSession

from app.core.dependencies import get_db
from app.core.dependencies import get_current_user, get_db, verify_api_key
from app.crud import user as crud_user
from app.schemas.user import UserCreate, UserOut
from app.models.user import User
from app.schemas.user import UserCreate, UserOut, UserUpdate

router = APIRouter()

Expand All @@ -17,6 +18,84 @@
async def signup(user_in: UserCreate, db: AsyncSession = Depends(get_db)):
existing = await crud_user.get_user_by_username(db, user_in.username)
if existing:
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Username already taken")
user = await crud_user.create_user(db, user_in.username, user_in.password)
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST, detail="Username already taken"
)
if user_in.email:
email_existing = await crud_user.get_user_by_email(db, user_in.email)
if email_existing:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST, detail="Email already taken"
)
user = await crud_user.create_user(
db,
user_in.username,
user_in.password,
email=user_in.email,
display_name=user_in.display_name,
)
return user


@router.get(
"/users/me",
response_model=UserOut,
dependencies=[Depends(verify_api_key)],
status_code=status.HTTP_200_OK,
)
async def get_me(current_user: User = Depends(get_current_user)):
return current_user


@router.put(
"/users/me",
response_model=UserOut,
dependencies=[Depends(verify_api_key)],
status_code=status.HTTP_200_OK,
)
async def update_me(
update: UserUpdate,
current_user: User = Depends(get_current_user),
db: AsyncSession = Depends(get_db),
):
data = update.model_dump(exclude_unset=True)
if "username" in data:
existing = await crud_user.get_user_by_username(db, data["username"])
if existing and existing.id != current_user.id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST, detail="Username already taken"
)
if "email" in data:
existing = await crud_user.get_user_by_email(db, data["email"])
if existing and existing.id != current_user.id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST, detail="Email already taken"
Comment on lines +61 to +72

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Skip email uniqueness check when clearing address

Both update_me and patch_me run a uniqueness check whenever the payload contains an email key, even when the value is null. Because get_user_by_email is then called with None, the query matches the first user that already has a NULL email (which is common because signups default to no email). The existing.id != current_user.id branch fires and returns 400 "Email already taken", making it impossible for a user to remove their email address while any other user also lacks one. The check should only run when a non-NULL email is supplied.

Useful? React with 👍 / 👎.

)
return await crud_user.update_user(db, current_user, data)


@router.patch(
"/users/me",
response_model=UserOut,
dependencies=[Depends(verify_api_key)],
status_code=status.HTTP_200_OK,
)
async def patch_me(
update: UserUpdate,
current_user: User = Depends(get_current_user),
db: AsyncSession = Depends(get_db),
):
data = update.model_dump(exclude_unset=True)
if "username" in data:
existing = await crud_user.get_user_by_username(db, data["username"])
if existing and existing.id != current_user.id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST, detail="Username already taken"
)
if "email" in data:
existing = await crud_user.get_user_by_email(db, data["email"])
if existing and existing.id != current_user.id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST, detail="Email already taken"
)
return await crud_user.update_user(db, current_user, data)
30 changes: 28 additions & 2 deletions backend/app/crud/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,36 @@ async def get_user_by_username(db: AsyncSession, username: str) -> Optional[User
return result.scalars().first()


async def create_user(db: AsyncSession, username: str, password: str):
async def get_user_by_email(db: AsyncSession, email: str) -> Optional[User]:
result = await db.execute(select(User).filter(User.email == email))
return result.scalars().first()


async def create_user(
db: AsyncSession,
username: str,
password: str,
*,
email: str | None = None,
display_name: str | None = None,
):
hashed_pw = hash_password(password)
new_user = User(username=username, hashed_password=hashed_pw)
new_user = User(
username=username,
hashed_password=hashed_pw,
email=email,
display_name=display_name,
)
db.add(new_user)
await db.commit()
await db.refresh(new_user)
return new_user


async def update_user(db: AsyncSession, user: User, updates: dict) -> User:
for field, value in updates.items():
setattr(user, field, value)
db.add(user)
await db.commit()
await db.refresh(user)
return user
2 changes: 2 additions & 0 deletions backend/app/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ class User(Base):
__tablename__ = "users"
id = Column(Integer, primary_key=True, index=True)
username = Column(String, unique=True, index=True, nullable=False)
email = Column(String, unique=True, index=True, nullable=True)
display_name = Column(String, nullable=True)
hashed_password = Column(String, nullable=False)

tasks = relationship("Task", back_populates="user", cascade="all, delete-orphan")
14 changes: 13 additions & 1 deletion backend/app/schemas/user.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
from pydantic import BaseModel, ConfigDict
from typing import Optional

from pydantic import BaseModel, ConfigDict, EmailStr


class UserCreate(BaseModel):
username: str
password: str
email: Optional[EmailStr] = None
display_name: Optional[str] = None

model_config = ConfigDict(
json_schema_extra={
Expand All @@ -20,5 +24,13 @@ class UserCreate(BaseModel):
class UserOut(BaseModel):
id: int
username: str
email: Optional[EmailStr] = None
display_name: Optional[str] = None

model_config = {"from_attributes": True}


class UserUpdate(BaseModel):
username: Optional[str] = None
email: Optional[EmailStr] = None
display_name: Optional[str] = None
38 changes: 38 additions & 0 deletions backend/tests/test_users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import os

import pytest
import pytest_asyncio


@pytest_asyncio.fixture
async def auth_headers(async_client):
API_KEY = os.getenv("TSKZ_HTTP_API_KEY", "123456")
username = "user1"
password = "secret"
await async_client.post(
"/signup", json={"username": username, "password": password}
)
token_res = await async_client.post(
"/token", data={"username": username, "password": password}
)
token = token_res.json()["access_token"]
return {"Authorization": f"Bearer {token}", "X-API-Key": API_KEY}


@pytest.mark.asyncio
async def test_get_and_update_user(async_client, auth_headers):
res = await async_client.get("/users/me", headers=auth_headers)
assert res.status_code == 200
assert res.json()["username"] == "user1"

update = {"email": "[email protected]", "display_name": "User One"}
res = await async_client.put("/users/me", json=update, headers=auth_headers)
assert res.status_code == 200
data = res.json()
assert data["email"] == "[email protected]"
assert data["display_name"] == "User One"

patch_data = {"display_name": "User 1"}
res = await async_client.patch("/users/me", json=patch_data, headers=auth_headers)
assert res.status_code == 200
assert res.json()["display_name"] == "User 1"