rohskiddo/UMBRA
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Fix QA critical/warning findings on profile feature

Browse Source 
C-01: Replace setattr loop with explicit field assignment in update_profile
C-02: Fix useEffect dependency to profileQuery.dataUpdatedAt for re-sync
W-01: Add audit log entry for profile updates
W-02: Use less misleading generic error for email uniqueness on registration
W-03: Early return on empty PUT body to avoid unnecessary commit

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Kyle 2026-03-02 19:09:15 +08:00
parent 45f3788fb0
commit 02efe04fc4
2 changed files with 19 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
backend/app/routers/auth.py
View File

@ -448,7 +448,7 @@ async def register(
select(User).where(User.email == data.email) select(User).where(User.email == data.email)
) )
if existing_email.scalar_one_or_none(): if existing_email.scalar_one_or_none():
raise HTTPException(status_code=400, detail="Registration could not be completed. Please try a different username.") raise HTTPException(status_code=400, detail="Registration could not be completed. Please check your details and try again.")
password_hash = hash_password(data.password) password_hash = hash_password(data.password)
# SEC-01: Explicit field assignment — never **data.model_dump() # SEC-01: Explicit field assignment — never **data.model_dump()
@ -645,12 +645,16 @@ async def get_profile(
@router.put("/profile", response_model=ProfileResponse) @router.put("/profile", response_model=ProfileResponse)
async def update_profile( async def update_profile(
data: ProfileUpdate, data: ProfileUpdate,
request: Request,
db: AsyncSession = Depends(get_db), db: AsyncSession = Depends(get_db),
current_user: User = Depends(get_current_user), current_user: User = Depends(get_current_user),
): ):
"""Update the current user's profile fields (first_name, last_name, email).""" """Update the current user's profile fields (first_name, last_name, email)."""
update_data = data.model_dump(exclude_unset=True) update_data = data.model_dump(exclude_unset=True)
if not update_data:
return ProfileResponse.model_validate(current_user)
# Email uniqueness check if email is changing # Email uniqueness check if email is changing
if "email" in update_data and update_data["email"] != current_user.email: if "email" in update_data and update_data["email"] != current_user.email:
new_email = update_data["email"] new_email = update_data["email"]
@ -661,9 +665,19 @@ async def update_profile(
if existing.scalar_one_or_none(): if existing.scalar_one_or_none():
raise HTTPException(status_code=400, detail="Email is already in use") raise HTTPException(status_code=400, detail="Email is already in use")
# SEC-01: Explicit field assignment # SEC-01: Explicit field assignment — only allowed profile fields
for key, value in update_data.items(): if "first_name" in update_data:
setattr(current_user, key, value) current_user.first_name = update_data["first_name"]
if "last_name" in update_data:
current_user.last_name = update_data["last_name"]
if "email" in update_data:
current_user.email = update_data["email"]
await log_audit_event(
db, action="auth.profile_updated", actor_id=current_user.id,
detail={"fields": list(update_data.keys())},
ip=get_client_ip(request),
)
await db.commit() await db.commit()
await db.refresh(current_user) await db.refresh(current_user)

2
frontend/src/components/settings/SettingsPage.tsx
View File

@ -73,7 +73,7 @@ export default function SettingsPage() {
setLastName(profileQuery.data.last_name ?? ''); setLastName(profileQuery.data.last_name ?? '');
setProfileEmail(profileQuery.data.email ?? ''); setProfileEmail(profileQuery.data.email ?? '');
} }
}, [profileQuery.data?.username]); }, [profileQuery.dataUpdatedAt]);
// Sync state when settings load // Sync state when settings load
useEffect(() => { useEffect(() => {