From 39da976ef3dabe6ad594332494471378204fae65 Mon Sep 17 00:00:00 2001 From: Esa Kataja Date: Sat, 10 May 2025 14:28:34 +0300 Subject: [PATCH] Add more comprehensive logging --- README.md | 4 +++ src/routes/admin.py | 76 +++++++++++++++++++++++++++++++++------------ src/routes/auth.py | 31 +++++++++++++----- src/routes/users.py | 21 ++++++++++--- 4 files changed, 100 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index ab93487..18033df 100644 --- a/README.md +++ b/README.md @@ -220,6 +220,10 @@ Automated tests are not implemented at this time. Manual testing via the API doc ## TODOs * [ ] Replace password hashing with passlib +* [ ] Switch to PostgreSQL from DuckDB +* [x] Implement more comprehensive logging +* [ ] Implement user disabling functionality +* [ ] Implement user password change functionality ## Contributing diff --git a/src/routes/admin.py b/src/routes/admin.py index 9aa2b90..bab536e 100644 --- a/src/routes/admin.py +++ b/src/routes/admin.py @@ -1,9 +1,10 @@ -from fastapi import APIRouter, Body, HTTPException +from fastapi import APIRouter, Body, HTTPException, Request from models.user import CreateUser from models.team import TeamBase, Team from lib.db import get_connection from lib.helpers import hash_password +from lib.logger import logger router = APIRouter(prefix="/admin", tags=["Admin"]) @@ -19,36 +20,73 @@ router = APIRouter(prefix="/admin", tags=["Admin"]) @router.post("/teams") -async def create_team(team: TeamBase): - with get_connection() as conn: - conn.execute("INSERT INTO Team (name) VALUES (?)", (team.name,)) - return {"message": "Team created successfully"} +async def create_team(team: TeamBase, request: Request): + logger.info( + f"Admin action: Creating new team '{team.name}' from IP: {request.client.host}" + ) + try: + with get_connection() as conn: + conn.execute("INSERT INTO Team (name) VALUES (?)", (team.name,)) + logger.info(f"Team '{team.name}' created successfully") + return {"message": "Team created successfully"} + except Exception as e: + logger.error(f"Failed to create team '{team.name}': {str(e)}") + raise HTTPException(status_code=500, detail="Failed to create team") @router.get("/teams", response_model=list[Team]) -async def list_teams(): +async def list_teams(request: Request): + logger.debug(f"Admin action: Listing all teams from IP: {request.client.host}") with get_connection() as conn: teams = conn.execute("SELECT * FROM Team").fetchdf() return teams.to_dict(orient="records") @router.post("/users") -async def create_user(user: CreateUser): - with get_connection() as conn: - conn.execute( - "INSERT INTO User (username, hashed_password, team_id, is_admin) VALUES (?, ?, ?, ?)", - ( - user.username, - hash_password(user.password), - user.team_id, - user.is_admin, - ), +async def create_user(user: CreateUser, request: Request): + logger.info( + f"Admin action: Creating new user '{user.username}' from IP: {request.client.host}" + ) + try: + with get_connection() as conn: + # Check if username already exists + existing = conn.execute( + "SELECT COUNT(*) as count FROM User WHERE username = ?", + (user.username,), + ).fetchdf() + if existing["count"][0] > 0: + logger.warning( + f"Failed to create user: Username '{user.username}' already exists" + ) + raise HTTPException(status_code=400, detail="Username already exists") + + conn.execute( + "INSERT INTO User (username, hashed_password, team_id, is_admin) VALUES (?, ?, ?, ?)", + ( + user.username, + hash_password(user.password), + user.team_id, + user.is_admin, + ), + ) + logger.info( + f"User '{user.username}' created successfully with team_id: {user.team_id}, admin status: {user.is_admin}" ) - return {"message": "User created successfully"} + return {"message": "User created successfully"} + except HTTPException: + # Re-raise HTTP exceptions + raise + except Exception as e: + logger.error(f"Failed to create user '{user.username}': {str(e)}") + raise HTTPException(status_code=500, detail="Failed to create user") -@router.post("/users/") -async def disable_user(user_id: int = Body(..., embed=True)): +@router.post("/users/disable") +async def disable_user(user_id: int = Body(..., embed=True), request: Request = None): + logger.info( + f"Admin action: Attempting to disable user with ID: {user_id} from IP: {request.client.host}" + ) + logger.warning(f"Disable user functionality not implemented for user_id: {user_id}") raise HTTPException(status_code=405, detail="Method not yet implemented") diff --git a/src/routes/auth.py b/src/routes/auth.py index 77c7d5a..de78202 100644 --- a/src/routes/auth.py +++ b/src/routes/auth.py @@ -1,24 +1,34 @@ -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, HTTPException, Request from lib.db import get_connection from models.user import User, UserLogin from lib.helpers import verify_password +from lib.logger import logger router = APIRouter(prefix="/auth", tags=["auth"]) @router.post("/token", response_model=User) -async def login(user_login: UserLogin): +async def login(user_login: UserLogin, request: Request): + logger.debug( + f"Login attempt for user: {user_login.username} from IP: {request.client.host}" + ) with get_connection() as conn: user_df = conn.execute( 'SELECT * FROM "User" WHERE username = ?', (user_login.username,) ).fetchdf() if user_df.empty: + logger.warning( + f"Failed login: Username {user_login.username} not found - IP: {request.client.host}" + ) raise HTTPException(status_code=401, detail="Invalid username or password") user_data = user_df.to_dict(orient="records")[0] if not verify_password(user_login.password, user_data["hashed_password"]): + logger.warning( + f"Failed login: Incorrect password for user {user_login.username} - IP: {request.client.host}" + ) raise HTTPException(status_code=401, detail="Invalid username or password") # Update the login timestamp in the database @@ -27,11 +37,16 @@ async def login(user_login: UserLogin): 'UPDATE "User" SET last_login = CURRENT_TIMESTAMP WHERE id = ?', (user_data["id"],), ) - + # Fetch the updated user data with the new last_login timestamp - updated_user = conn.execute( - 'SELECT * FROM "User" WHERE id = ?', - (user_data["id"],) - ).fetchdf().to_dict(orient="records")[0] - + updated_user = ( + conn.execute('SELECT * FROM "User" WHERE id = ?', (user_data["id"],)) + .fetchdf() + .to_dict(orient="records")[0] + ) + + logger.info( + f"Successful login: User {user_login.username} (ID: {user_data['id']}) logged in from {request.client.host}" + ) + return User(**updated_user) diff --git a/src/routes/users.py b/src/routes/users.py index 905f5db..7c624d5 100644 --- a/src/routes/users.py +++ b/src/routes/users.py @@ -1,14 +1,15 @@ -from fastapi import APIRouter -from fastapi import HTTPException +from fastapi import APIRouter, HTTPException, Request from models.user import User, UserChangePassword from lib.db import get_connection +from lib.logger import logger router = APIRouter(prefix="/users", tags=["users"]) @router.get("/", response_model=list[User]) -async def list_users(): +async def list_users(request: Request): + logger.info(f"User list requested from {request.client.host}") with get_connection() as conn: users = conn.execute('SELECT * FROM "User"').fetchdf() @@ -18,19 +19,29 @@ async def list_users(): @router.get("/{user_id}", response_model=User) -async def get_user(user_id: int): +async def get_user(user_id: int, request: Request): + logger.debug( + f"User details requested for user_id: {user_id} from {request.client.host}" + ) with get_connection() as conn: user_df = conn.execute( 'SELECT * FROM "User" WHERE id = ?', (user_id,) ).fetchdf() if user_df.empty: + logger.warning(f"Failed user lookup: user_id {user_id} not found") raise HTTPException(status_code=404, detail="User not found") user_data = user_df.to_dict(orient="records")[0] + logger.debug(f"User details retrieved: {user_data}") return User(**user_data) @router.patch("/{user_id}") -async def change_password(user: UserChangePassword): +async def change_password(user_id: int, user: UserChangePassword, request: Request): + logger.info( + f"Password change attempt for user_id: {user_id} from {request.client.host}" + ) + # Implementation will go here when completed + logger.warning(f"Password change not implemented for user_id: {user_id}") raise HTTPException(status_code=401, detail="Not implemented")