diff --git a/backend/app/auth/service.py b/backend/app/auth/service.py index fcd4699..9afe1af 100644 --- a/backend/app/auth/service.py +++ b/backend/app/auth/service.py @@ -4,8 +4,8 @@ from typing import Any, Dict, Optional import firebase_admin -from app.auth.schemas import UserResponse from app.auth.security import ( + create_access_token, create_refresh_token, generate_reset_token, get_password_hash, @@ -17,7 +17,8 @@ from fastapi import HTTPException, status from firebase_admin import auth as firebase_auth from firebase_admin import credentials -from pymongo.errors import DuplicateKeyError +from jose import JWTError +from pymongo.errors import DuplicateKeyError, PyMongoError # Initialize Firebase Admin SDK if not firebase_admin._apps: @@ -137,6 +138,12 @@ async def create_user_with_email( status_code=status.HTTP_400_BAD_REQUEST, detail="User with this email already exists", ) + except Exception as e: + logger.exception("Unexpected error while creating user with email") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Internal server error", + ) async def authenticate_user_with_email( self, email: str, password: str @@ -150,17 +157,31 @@ async def authenticate_user_with_email( A dictionary containing the authenticated user and a new refresh token. """ db = self.get_db() + try: + user = await db.users.find_one({"email": email}) + except PyMongoError as e: + logger.error(f"Database error during user lookup: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Internal server error", + ) - user = await db.users.find_one({"email": email}) if not user or not verify_password(password, user.get("hashed_password", "")): + logger.info("Authentication failed due to invalid credentials.") raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Incorrect email or password", ) # Create new refresh token - refresh_token = await self._create_refresh_token_record(str(user["_id"])) - + try: + refresh_token = await self._create_refresh_token_record(str(user["_id"])) + except Exception as e: + logger.error(f"Failed to generate refresh token: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to generate refresh token", + ) return {"user": user, "refresh_token": refresh_token} async def authenticate_with_google(self, id_token: str) -> Dict[str, Any]: @@ -177,7 +198,14 @@ async def authenticate_with_google(self, id_token: str) -> Dict[str, Any]: """ try: # Verify the Firebase ID token - decoded_token = firebase_auth.verify_id_token(id_token) + try: + decoded_token = firebase_auth.verify_id_token(id_token) + except firebase_auth.InvalidIdTokenError: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid Google ID token", + ) + firebase_uid = decoded_token["uid"] email = decoded_token.get("email") name = decoded_token.get( @@ -193,10 +221,16 @@ async def authenticate_with_google(self, id_token: str) -> Dict[str, Any]: db = self.get_db() # Check if user exists - user = await db.users.find_one( - {"$or": [{"email": email}, {"firebase_uid": firebase_uid}]} - ) - + try: + user = await db.users.find_one( + {"$or": [{"email": email}, {"firebase_uid": firebase_uid}]} + ) + except PyMongoError as e: + logger.error("Database error while checking user: %s", str(e)) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Internal server error", + ) if user: # Update user info if needed update_data = {} @@ -206,10 +240,14 @@ async def authenticate_with_google(self, id_token: str) -> Dict[str, Any]: update_data["avatar"] = picture if update_data: - await db.users.update_one( - {"_id": user["_id"]}, {"$set": update_data} - ) - user.update(update_data) + try: + await db.users.update_one( + {"_id": user["_id"]}, {"$set": update_data} + ) + user.update(update_data) + except PyMongoError as e: + logger.warning( + "Failed to update user profile: %s", str(e)) else: # Create new user user_doc = { @@ -222,22 +260,38 @@ async def authenticate_with_google(self, id_token: str) -> Dict[str, Any]: "firebase_uid": firebase_uid, "hashed_password": None, } - - result = await db.users.insert_one(user_doc) - user_doc["_id"] = result.inserted_id - user = user_doc + try: + result = await db.users.insert_one(user_doc) + user_doc["_id"] = result.inserted_id + user = user_doc + except PyMongoError as e: + logger.error( + "Failed to create new Google user: %s", str(e)) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to create user", + ) # Create refresh token - refresh_token = await self._create_refresh_token_record(str(user["_id"])) + try: + refresh_token = await self._create_refresh_token_record( + str(user["_id"]) + ) + except Exception as e: + logger.error( + "Failed to issue refresh token for Google login: %s", str( + e) + ) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to generate refresh token", + ) return {"user": user, "refresh_token": refresh_token} - - except firebase_auth.InvalidIdTokenError: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid Google ID token", - ) + except HTTPException: + raise except Exception as e: + logger.exception("Unexpected error during Google authentication") raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=f"Google authentication failed: {str(e)}", @@ -258,13 +312,21 @@ async def refresh_access_token(self, refresh_token: str) -> str: db = self.get_db() # Find and validate refresh token - token_record = await db.refresh_tokens.find_one( - { - "token": refresh_token, - "revoked": False, - "expires_at": {"$gt": datetime.now(timezone.utc)}, - } - ) + try: + token_record = await db.refresh_tokens.find_one( + { + "token": refresh_token, + "revoked": False, + "expires_at": {"$gt": datetime.now(timezone.utc)}, + } + ) + except PyMongoError as e: + logger.error( + "Database error while validating refresh token: %s", str(e)) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Internal server error", + ) if not token_record: raise HTTPException( @@ -273,19 +335,39 @@ async def refresh_access_token(self, refresh_token: str) -> str: ) # Get user - user = await db.users.find_one({"_id": token_record["user_id"]}) + try: + user = await db.users.find_one({"_id": token_record["user_id"]}) + except PyMongoError as e: + logger.error("Error while fetching user: %s", str(e)) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Internal server error", + ) if not user: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found" ) # Create new refresh token (token rotation) - new_refresh_token = await self._create_refresh_token_record(str(user["_id"])) + try: + new_refresh_token = await self._create_refresh_token_record( + str(user["_id"]) + ) + except Exception as e: + logger.error("Failed to create new refresh token: %s", str(e)) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to create refresh token", + ) # Revoke old token - await db.refresh_tokens.update_one( - {"_id": token_record["_id"]}, {"$set": {"revoked": True}} - ) + try: + await db.refresh_tokens.update_one( + {"_id": token_record["_id"]}, {"$set": {"revoked": True}} + ) + except PyMongoError as e: + logger.error("Failed to revoke old refresh token: %s", str(e)) + # No raise here since new token is safely issued return new_refresh_token @@ -304,8 +386,14 @@ async def verify_access_token(self, token: str) -> Dict[str, Any]: """ from app.auth.security import verify_token - payload = verify_token(token) - user_id = payload.get("sub") + try: + payload = verify_token(token) + user_id = payload.get("sub") + except JWTError as e: + logger.warning("JWT verification failed: %s", str(e)) + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token" + ) if not user_id: raise HTTPException( @@ -313,7 +401,15 @@ async def verify_access_token(self, token: str) -> Dict[str, Any]: ) db = self.get_db() - user = await db.users.find_one({"_id": user_id}) + + try: + user = await db.users.find_one({"_id": user_id}) + except Exception as e: + logger.error("Error while verifying token: %s", str(e)) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Internal server error", + ) if not user: raise HTTPException( @@ -330,7 +426,16 @@ async def request_password_reset(self, email: str) -> bool: """ db = self.get_db() - user = await db.users.find_one({"email": email}) + try: + user = await db.users.find_one({"email": email}) + except PyMongoError as e: + logger.error( + f"Database error while fetching user by email {email}: {str(e)}" + ) + raise HTTPException( + status_code=500, detail="Internal server error during user lookup." + ) + if not user: # Don't reveal if email exists or not return True @@ -340,16 +445,24 @@ async def request_password_reset(self, email: str) -> bool: reset_expires = datetime.now( timezone.utc) + timedelta(hours=1) # 1 hour expiry - # Store reset token - await db.password_resets.insert_one( - { - "user_id": user["_id"], - "token": reset_token, - "expires_at": reset_expires, - "used": False, - "created_at": datetime.utcnow(), - } - ) + try: + # Store reset token + await db.password_resets.insert_one( + { + "user_id": user["_id"], + "token": reset_token, + "expires_at": reset_expires, + "used": False, + "created_at": datetime.utcnow(), + } + ) + except PyMongoError as e: + logger.error( + f"Database error while storing reset token for user {email}: {str(e)}" + ) + raise HTTPException( + status_code=500, detail="Internal server error during token storage." + ) # For development/free tier: just log the reset token # In production, you would send this via email @@ -378,39 +491,54 @@ async def confirm_password_reset(self, reset_token: str, new_password: str) -> b """ db = self.get_db() - # Find and validate reset token - reset_record = await db.password_resets.find_one( - { - "token": reset_token, - "used": False, - "expires_at": {"$gt": datetime.now(timezone.utc)}, - } - ) - - if not reset_record: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Invalid or expired reset token", + try: + # Find and validate reset token + reset_record = await db.password_resets.find_one( + { + "token": reset_token, + "used": False, + "expires_at": {"$gt": datetime.now(timezone.utc)}, + } ) - # Update user password - new_hash = get_password_hash(new_password) - await db.users.update_one( - {"_id": reset_record["user_id"]}, { - "$set": {"hashed_password": new_hash}} - ) + if not reset_record: + logger.warning("Invalid or expired reset token") + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid or expired reset token", + ) - # Mark token as used - await db.password_resets.update_one( - {"_id": reset_record["_id"]}, {"$set": {"used": True}} - ) + # Update user password + new_hash = get_password_hash(new_password) + await db.users.update_one( + {"_id": reset_record["user_id"]}, + {"$set": {"hashed_password": new_hash}}, + ) - # Revoke all refresh tokens for this user (force re-login) - await db.refresh_tokens.update_many( - {"user_id": reset_record["user_id"]}, {"$set": {"revoked": True}} - ) + # Mark token as used + await db.password_resets.update_one( + {"_id": reset_record["_id"]}, {"$set": {"used": True}} + ) - return True + # Revoke all refresh tokens for this user (force re-login) + await db.refresh_tokens.update_many( + {"user_id": reset_record["user_id"]}, { + "$set": {"revoked": True}} + ) + logger.info( + f"Password reset successful for user_id: {reset_record['user_id']}" + ) + return True + + except HTTPException: + raise # Raising HTTPException to avoid logging again + except Exception as e: + logger.exception( + f"Unexpected error during password reset: {str(e)}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Internal server error during password reset", + ) async def _create_refresh_token_record(self, user_id: str) -> str: """ @@ -431,15 +559,27 @@ async def _create_refresh_token_record(self, user_id: str) -> str: days=settings.refresh_token_expire_days ) - await db.refresh_tokens.insert_one( - { - "token": refresh_token, - "user_id": ObjectId(user_id) if isinstance(user_id, str) else user_id, - "expires_at": expires_at, - "revoked": False, - "created_at": datetime.now(timezone.utc), - } - ) + try: + await db.refresh_tokens.insert_one( + { + "token": refresh_token, + "user_id": ( + ObjectId(user_id) if isinstance( + user_id, str) else user_id + ), + "expires_at": expires_at, + "revoked": False, + "created_at": datetime.now(timezone.utc), + } + ) + except Exception as e: + logger.error( + f"Failed to create refresh token for user_id: {user_id}. Error: {str(e)}" + ) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to create refresh token: {str(e)}", + ) return refresh_token diff --git a/backend/app/expenses/service.py b/backend/app/expenses/service.py index 609fb0f..0d676e7 100644 --- a/backend/app/expenses/service.py +++ b/backend/app/expenses/service.py @@ -1,7 +1,6 @@ -import asyncio -from collections import defaultdict, deque +from collections import defaultdict from datetime import datetime, timedelta -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional from app.config import logger from app.database import mongodb @@ -15,7 +14,8 @@ SettlementStatus, SplitType, ) -from bson import ObjectId +from bson import ObjectId, errors +from fastapi import HTTPException class ExpenseService: @@ -46,15 +46,22 @@ async def create_expense( # Validate and convert group_id to ObjectId try: group_obj_id = ObjectId(group_id) - except Exception: - raise ValueError("Group not found or user not a member") + except errors.InvalidId: # Incorrect ObjectId format + logger.warning(f"Invalid group ID format: {group_id}") + raise HTTPException(status_code=400, detail="Invalid group ID") + except Exception as e: + logger.error(f"Unexpected error parsing groupId: {e}") + raise HTTPException( + status_code=500, detail="Failed to process group ID") # Verify user is member of the group group = await self.groups_collection.find_one( {"_id": group_obj_id, "members.userId": user_id} ) - if not group: - raise ValueError("Group not found or user not a member") + if not group: # User not a member of the group + raise HTTPException( + status_code=403, detail="You are not a member of this group" + ) # Create expense document expense_doc = { @@ -231,21 +238,32 @@ async def get_expense_by_id( try: group_obj_id = ObjectId(group_id) expense_obj_id = ObjectId(expense_id) - except Exception: - raise ValueError("Group not found or user not a member") + except errors.InvalidId: # Incorrect ObjectId format for group_id or expense_id + logger.warning( + f"Invalid ObjectId(s): group_id={group_id}, expense_id={expense_id}" + ) + raise HTTPException( + status_code=400, detail="Invalid group ID or expense ID" + ) + except Exception as e: + logger.error(f"Unexpected error parsing IDs: {e}") + raise HTTPException( + status_code=500, detail="Unable to process IDs") # Verify user access group = await self.groups_collection.find_one( {"_id": group_obj_id, "members.userId": user_id} ) - if not group: - raise ValueError("Group not found or user not a member") + if not group: # Unauthorized access + raise HTTPException( + status_code=403, detail="You are not a member of this group" + ) expense_doc = await self.expenses_collection.find_one( {"_id": expense_obj_id, "groupId": group_id} ) - if not expense_doc: - raise ValueError("Expense not found") + if not expense_doc: # Expense not found + raise HTTPException(status_code=404, detail="Expense not found") expense = await self._expense_doc_to_response(expense_doc) @@ -274,30 +292,39 @@ async def update_expense( # Validate ObjectId format try: expense_obj_id = ObjectId(expense_id) - except Exception as e: - raise ValueError(f"Invalid expense ID format: {expense_id}") + except errors.InvalidId: + logger.warning(f"Invalid expense ID format: {expense_id}") + raise HTTPException( + status_code=400, detail="Invalid expense ID format") # Verify user access and that they created the expense expense_doc = await self.expenses_collection.find_one( {"_id": expense_obj_id, "groupId": group_id, "createdBy": user_id} ) - if not expense_doc: - raise ValueError("Expense not found or not authorized to edit") + if not expense_doc: # Expense not found or user not authorized + raise HTTPException( + status_code=403, + detail="Not authorized to update this expense or it does not exist", + ) # Validate splits against current or new amount if both are being updated if updates.splits is not None and updates.amount is not None: total_split = sum(split.amount for split in updates.splits) if abs(total_split - updates.amount) > 0.01: - raise ValueError( - "Split amounts must sum to total expense amount") + raise HTTPException( + status_code=400, + detail="Split amounts must sum to total expense amount", + ) # If only splits are being updated, validate against current amount elif updates.splits is not None: current_amount = expense_doc["amount"] total_split = sum(split.amount for split in updates.splits) if abs(total_split - current_amount) > 0.01: - raise ValueError( - "Split amounts must sum to current expense amount") + raise HTTPException( + status_code=400, + detail="Split amounts must sum to total expense amount", + ) # Store original data for history original_data = { @@ -332,7 +359,8 @@ async def update_expense( user.get( "name", "Unknown User") if user else "Unknown User" ) - except: + except Exception as e: + logger.warning(f"Failed to fetch user for history: {e}") user_name = "Unknown User" history_entry = { @@ -349,8 +377,10 @@ async def update_expense( {"$set": update_doc, "$push": {"history": history_entry}}, ) - if result.matched_count == 0: - raise ValueError("Expense not found during update") + if result.matched_count == 0: # Expense not found during update + raise HTTPException( + status_code=404, detail="Expense not found during update" + ) else: # No actual changes, just update the timestamp result = await self.expenses_collection.update_one( @@ -358,7 +388,9 @@ async def update_expense( ) if result.matched_count == 0: - raise ValueError("Expense not found during update") + raise HTTPException( + status_code=404, detail="Expense not found during update" + ) # If splits changed, recalculate settlements if updates.splits is not None or updates.amount is not None: @@ -378,10 +410,9 @@ async def update_expense( await self._create_settlements_for_expense( updated_expense, user_id ) - except Exception as e: + except Exception: logger.error( - f"Warning: Failed to recalculate settlements: {e}", - exc_info=True, + f"Warning: Failed to recalculate settlements", exc_info=True ) # Continue anyway, as the expense update succeeded @@ -390,14 +421,26 @@ async def update_expense( {"_id": expense_obj_id} ) if not updated_expense: - raise ValueError("Failed to retrieve updated expense") + raise HTTPException( + status_code=500, detail="Failed to retrieve updated expense" + ) return await self._expense_doc_to_response(updated_expense) - except ValueError: + # Allowing FastAPI exception to bubble up for proper handling + except HTTPException: raise - except Exception as e: - logger.error(f"Error in update_expense: {str(e)}", exc_info=True) + except ValueError as ve: + raise HTTPException(status_code=400, detail=str(ve)) + except ( + Exception + ) as e: # logger.exception() will provide the entire traceback, so its safe to remove traceback + logger.exception( + f"Unhandled error in update_expense for expense {expense_id}: {e}" + ) + import traceback + + traceback.print_exc() raise Exception(f"Database error during expense update: {str(e)}") async def delete_expense( @@ -411,7 +454,13 @@ async def delete_expense( "createdBy": user_id} ) if not expense_doc: - raise ValueError("Expense not found or not authorized to delete") + logger.warning( + f"Unauthorized delete attempt or missing expense: {expense_id} by user {user_id}" + ) + raise HTTPException( + status_code=403, + detail="Not authorized to delete this expense or it does not exist", + ) # Delete settlements for this expense await self.settlements_collection.delete_many({"expenseId": expense_id}) @@ -576,7 +625,12 @@ async def create_manual_settlement( {"_id": ObjectId(group_id), "members.userId": user_id} ) if not group: - raise ValueError("Group not found or user not a member") + logger.warning( + f"Unauthorized access attempt to group {group_id} by user {user_id}" + ) + raise HTTPException( + status_code=403, detail="Group not found or user not a member" + ) # Get user names users = await self.users_collection.find( @@ -666,7 +720,12 @@ async def get_group_settlements( {"_id": ObjectId(group_id), "members.userId": user_id} ) if not group: - raise ValueError("Group not found or user not a member") + logger.warning( + f"Unauthorized access attempt to group {group_id} by user {user_id}" + ) + raise HTTPException( + status_code=403, detail="Group not found or user not a member" + ) # Build query query = {"groupId": group_id} @@ -705,17 +764,24 @@ async def get_settlement_by_id( # Verify user access group = await self.groups_collection.find_one( - {"_id": ObjectId(group_id), "members.userId": user_id} + { + "_id": ObjectId( + group_id + ), # Assuming valid object ID format (same as above functions) + "members.userId": user_id, + } ) if not group: - raise ValueError("Group not found or user not a member") + raise HTTPException( + status_code=403, detail="Group not found or user not a member" + ) settlement_doc = await self.settlements_collection.find_one( {"_id": ObjectId(settlement_id), "groupId": group_id} ) if not settlement_doc: - raise ValueError("Settlement not found") + raise HTTPException(status_code=404, detail="Settlement not found") return Settlement(**{**settlement_doc, "_id": str(settlement_doc["_id"])}) @@ -740,7 +806,7 @@ async def update_settlement_status( ) if result.matched_count == 0: - raise ValueError("Settlement not found") + raise HTTPException(status_code=404, detail="Settlement not found") # Get updated settlement settlement_doc = await self.settlements_collection.find_one( @@ -759,7 +825,9 @@ async def delete_settlement( {"_id": ObjectId(group_id), "members.userId": user_id} ) if not group: - raise ValueError("Group not found or user not a member") + raise HTTPException( + status_code=403, detail="Group not found or user not a member" + ) result = await self.settlements_collection.delete_one( {"_id": ObjectId(settlement_id), "groupId": group_id} @@ -777,7 +845,9 @@ async def get_user_balance_in_group( {"_id": ObjectId(group_id), "members.userId": current_user_id} ) if not group: - raise ValueError("Group not found or user not a member") + raise HTTPException( + status_code=403, detail="Group not found or user not a member" + ) # Get user info user = await self.users_collection.find_one({"_id": ObjectId(target_user_id)}) @@ -1109,7 +1179,9 @@ async def get_group_analytics( {"_id": ObjectId(group_id), "members.userId": user_id} ) if not group: - raise ValueError("Group not found or user not a member") + raise HTTPException( + status_code=403, detail="Group not found or user not a member" + ) # Build date range if period == "month" and year and month: diff --git a/backend/app/groups/service.py b/backend/app/groups/service.py index 6d40a7b..0dcdcc9 100644 --- a/backend/app/groups/service.py +++ b/backend/app/groups/service.py @@ -3,9 +3,10 @@ from datetime import datetime, timezone from typing import Any, Dict, List, Optional +from app.config import logger from app.database import get_database -from bson import ObjectId -from fastapi import HTTPException, status +from bson import ObjectId, errors +from fastapi import HTTPException class GroupService: @@ -69,7 +70,24 @@ async def _enrich_members_with_user_details( ), } enriched_members.append(enriched_member) + except errors.InvalidId: # exception for invalid ObjectId + logger.warning( + f"Invalid ObjectId for userId: {member_user_id}") + enriched_members.append( + { + "userId": member_user_id, + "role": member.get("role", "member"), + "joinedAt": member.get("joinedAt"), + "user": { + "name": f"User {member_user_id[-4:]}", + "email": f"{member_user_id}@example.com", + "avatar": None, + }, + } + ) except Exception as e: + logger.error( + f"Error enriching userId {member_user_id}: {e}") # If user lookup fails, add member with basic info enriched_members.append( { @@ -83,6 +101,7 @@ async def _enrich_members_with_user_details( }, } ) + else: # Add member without user details if userId is missing enriched_members.append(member) @@ -95,7 +114,8 @@ def transform_group_document(self, group: dict) -> dict: return None try: group_id = str(group["_id"]) - except Exception: + except Exception as e: + logger.warning(f"Failed to get _id from group document: {e}") return None return { @@ -157,7 +177,12 @@ async def get_group_by_id(self, group_id: str, user_id: str) -> Optional[dict]: db = self.get_db() try: obj_id = ObjectId(group_id) - except Exception: + except errors.InvalidId: + logger.warning(f"Invalid group_id: {group_id}") + return None + except Exception as e: + logger.error( + f"Unexpected error converting group_id to ObjectId: {e}") return None group = await db.groups.find_one({"_id": obj_id, "members.userId": user_id}) @@ -184,7 +209,12 @@ async def update_group( db = self.get_db() try: obj_id = ObjectId(group_id) - except Exception: + except errors.InvalidId: + logger.warning(f"Invalid group_id: {group_id}") + return None + except Exception as e: + logger.error( + f"Unexpected error converting group_id to ObjectId: {e}") return None # Check if user is admin @@ -209,7 +239,12 @@ async def delete_group(self, group_id: str, user_id: str) -> bool: db = self.get_db() try: obj_id = ObjectId(group_id) - except Exception: + except errors.InvalidId: + logger.warning(f"Invalid group_id: {group_id}") + return False + except Exception as e: + logger.error( + f"Unexpected error converting group_id to ObjectId: {e}") return False # Check if user is admin diff --git a/backend/app/user/service.py b/backend/app/user/service.py index 589a682..e91d8b6 100644 --- a/backend/app/user/service.py +++ b/backend/app/user/service.py @@ -1,9 +1,9 @@ from datetime import datetime, timezone from typing import Any, Dict, Optional +from app.config import logger from app.database import get_database -from bson import ObjectId -from fastapi import Depends, HTTPException, status +from bson import ObjectId, errors class UserService: @@ -31,11 +31,15 @@ def iso(dt): ) return dt_utc.isoformat().replace("+00:00", "Z") except AttributeError: + logger.warning( + "DateTime conversion failed, returning raw string" + ) # Logging failed datetime transformation return str(dt) try: user_id = str(user["_id"]) - except Exception: + except (KeyError, TypeError) as e: + logger.error(f"Invalid user document format: {e}") return None # Handle invalid ObjectId gracefully return { "id": user_id, @@ -51,7 +55,9 @@ async def get_user_by_id(self, user_id: str) -> Optional[dict]: db = self.get_db() try: obj_id = ObjectId(user_id) - except Exception: + except errors.InvalidId as e: + # Invalid ObjectId format + logger.warning(f"Invalid User ID format: {e}") return None # Handle invalid ObjectId gracefully user = await db.users.find_one({"_id": obj_id}) return self.transform_user_document(user) @@ -60,7 +66,10 @@ async def update_user_profile(self, user_id: str, updates: dict) -> Optional[dic db = self.get_db() try: obj_id = ObjectId(user_id) - except Exception: + except errors.InvalidId as e: + logger.warning( + f"Invalid User ID format: {e}" + ) # Invalid ObjectId format for profile update return None # Handle invalid ObjectId gracefully # Only allow certain fields allowed = {"name", "imageUrl", "currency"} @@ -75,7 +84,10 @@ async def delete_user(self, user_id: str) -> bool: db = self.get_db() try: obj_id = ObjectId(user_id) - except Exception: + except errors.InvalidId as e: + logger.warning( + f"Invalid User ID format: {e}" + ) # Invalid ObjectId format for deletion return False # Handle invalid ObjectId gracefully result = await db.users.delete_one({"_id": obj_id}) return result.deleted_count > 0 diff --git a/backend/tests/auth/test_auth_service.py b/backend/tests/auth/test_auth_service.py new file mode 100644 index 0000000..422d6fb --- /dev/null +++ b/backend/tests/auth/test_auth_service.py @@ -0,0 +1,806 @@ +""" +Test suite for the `AuthService` class in the `app.auth.service` module. + +This file contains extensive async tests using `pytest` and `unittest.mock` to validate +the behavior of authentication-related features in a FastAPI-based application, including: + +1. **User Registration** + - Successful creation of a new user with email and password. + - Handling of duplicate email entries (via `find_one` and `DuplicateKeyError`). + - Failure to create refresh token during user registration. + +2. **Email/Password Login** + - Successful authentication with correct credentials. + - Handling incorrect password or nonexistent user. + - Missing hashed password in the database. + - DB errors during user retrieval. + - Failure in refresh token generation after successful login. + +3. **Google Sign-In (OAuth)** + - Successful authentication using a valid Google ID token. + - Handling invalid or missing tokens. + - Missing email in decoded token. + - MongoDB-related errors during find/insert operations. + +4. **Refresh Token Workflow** + - Successful issuance of new access token using a valid refresh token. + - Handling of invalid, expired, or revoked tokens. + - Missing user associated with the refresh token. + - DB errors during token or user fetch. + - Failure during new token generation. + +5. **Access Token Verification** + - Verifying access tokens for authenticated requests. + - Handling invalid tokens (JWT errors or missing `sub` claim). + - DB errors or missing user during verification. + +6. **Password Reset Flow** + - Requesting a password reset for an existing user and generating reset token. + - Ignoring requests for nonexistent users. + - Handling DB errors during lookup or insertion of reset tokens. + - Confirming password reset with valid token and updating user password. + - Rejecting invalid, expired, or already-used reset tokens. + +7. **Refresh Token Record Creation** + - Inserting a new refresh token record for a user. + - Handling DB insertion errors and invalid user IDs. + +All tests simulate various edge cases and exceptions to ensure robust handling of errors +with appropriate HTTP response codes and messages. +""" + +import logging +from datetime import datetime, timedelta, timezone +from unittest.mock import AsyncMock, MagicMock, patch + +import firebase_admin +import pytest +from app.auth.security import create_refresh_token, get_password_hash, verify_password +from app.auth.service import AuthService +from bson import ObjectId +from bson.errors import InvalidId +from fastapi import HTTPException, status +from firebase_admin import auth as firebase_auth +from firebase_admin import credentials +from jose import JWTError +from pymongo.errors import DuplicateKeyError, PyMongoError + + +def validate_object_id(id_str: str, field_name: str = "ID") -> ObjectId: + try: + return ObjectId(id_str) + except InvalidId: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, detail=f"Invalid {field_name}" + ) + + +@pytest.mark.asyncio +async def test_create_user_with_email_success(monkeypatch): + service = AuthService() + + # Mock DB behavior + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None # No existing user + mock_insert_result = AsyncMock(inserted_id=ObjectId()) + mock_db.users.insert_one.return_value = mock_insert_result + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + monkeypatch.setattr( + service, + "_create_refresh_token_record", + AsyncMock(return_value="mock_refresh_token"), + ) + monkeypatch.setattr( + "app.auth.service.get_password_hash", lambda pwd: f"hashed_{pwd}" + ) + + result = await service.create_user_with_email( + "new@example.com", "securepass", "Test User" + ) + + assert result["user"]["email"] == "new@example.com" + assert result["user"]["hashed_password"] == "hashed_securepass" + assert result["refresh_token"] == "mock_refresh_token" + + +@pytest.mark.asyncio +async def test_create_user_with_email_already_exists(monkeypatch): + service = AuthService() + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = {"email": "existing@example.com"} + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + with pytest.raises(HTTPException) as exc: + await service.create_user_with_email("existing@example.com", "pass", "User") + + assert exc.value.status_code == 400 + assert exc.value.detail == "User with this email already exists" + + +@pytest.mark.asyncio +async def test_create_user_with_email_refresh_token_error(monkeypatch): + service = AuthService() + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None + mock_insert_result = AsyncMock(inserted_id=ObjectId()) + mock_db.users.insert_one.return_value = mock_insert_result + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + monkeypatch.setattr( + "app.auth.service.get_password_hash", lambda pwd: f"hashed_{pwd}" + ) + + async def fail_refresh_token(*args, **kwargs): + raise Exception("Token generation failed") + + monkeypatch.setattr( + service, "_create_refresh_token_record", fail_refresh_token) + + with pytest.raises(HTTPException) as exc: + await service.create_user_with_email("fail@example.com", "pass", "User") + + assert exc.value.status_code == 500 + assert exc.value.detail == "Internal server error" + + +@pytest.mark.asyncio +async def test_create_user_with_email_duplicate_key(monkeypatch): + service = AuthService() + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None + mock_db.users.insert_one.side_effect = DuplicateKeyError("dup key") + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + monkeypatch.setattr( + "app.auth.service.get_password_hash", lambda pwd: f"hashed_{pwd}" + ) + monkeypatch.setattr(service, "_create_refresh_token_record", AsyncMock()) + + with pytest.raises(HTTPException) as exc: + await service.create_user_with_email("race@example.com", "pass", "User") + + assert exc.value.status_code == 400 + assert exc.value.detail == "User with this email already exists" + + +@pytest.mark.asyncio +async def test_authenticate_user_success(monkeypatch): + service = AuthService() + mock_user = { + "_id": ObjectId(), + "email": "test@example.com", + "hashed_password": "mocked_hash", + } + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = mock_user + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + monkeypatch.setattr( + "app.auth.service.verify_password", lambda pwd, hash: pwd == "correct-password" + ) + monkeypatch.setattr( + service, "_create_refresh_token_record", AsyncMock( + return_value="refresh-token") + ) + + result = await service.authenticate_user_with_email( + "test@example.com", "correct-password" + ) + + assert result.get("user") == mock_user + assert result.get("refresh_token") == "refresh-token" + + +@pytest.mark.asyncio +async def test_authenticate_user_db_error(monkeypatch): + service = AuthService() + mock_db = AsyncMock() + mock_db.users.find_one.side_effect = PyMongoError("DB failure") + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + with pytest.raises(HTTPException) as e: + await service.authenticate_user_with_email("email", "pass") + + assert e.value.status_code == 500 + assert "Internal server error" in e.value.detail + + +@pytest.mark.asyncio +async def test_authenticate_user_not_found(monkeypatch): + service = AuthService() + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + with pytest.raises(HTTPException) as e: + await service.authenticate_user_with_email("email", "pass") + + assert e.value.status_code == 401 + assert "Incorrect email or password" in e.value.detail + + +@pytest.mark.asyncio +async def test_authenticate_user_password_incorrect(monkeypatch): + service = AuthService() + mock_user = { + "_id": ObjectId(), + "email": "test@example.com", + "hashed_password": "hashed", + } + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = mock_user + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + monkeypatch.setattr("app.auth.service.verify_password", + lambda pwd, hash: False) + + with pytest.raises(HTTPException) as e: + await service.authenticate_user_with_email("email", "wrongpass") + + assert e.value.status_code == 401 + assert "Incorrect email or password" in e.value.detail + + +@pytest.mark.asyncio +async def test_authenticate_user_missing_hashed_password(monkeypatch): + service = AuthService() + # no hashed_password + mock_user = {"_id": ObjectId(), "email": "test@example.com"} + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = mock_user + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + monkeypatch.setattr("app.auth.service.verify_password", + lambda pwd, hash: False) + + with pytest.raises(HTTPException) as e: + await service.authenticate_user_with_email("email", "pass") + + assert e.value.status_code == 401 + + +@pytest.mark.asyncio +async def test_authenticate_user_refresh_token_error(monkeypatch): + service = AuthService() + mock_user = { + "_id": ObjectId(), + "email": "test@example.com", + "hashed_password": "mocked_hash", + } + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = mock_user + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + monkeypatch.setattr("app.auth.service.verify_password", + lambda pwd, hash: True) + monkeypatch.setattr( + service, + "_create_refresh_token_record", + AsyncMock(side_effect=Exception("fail")), + ) + + with pytest.raises(HTTPException) as e: + await service.authenticate_user_with_email("email", "pass") + + assert e.value.status_code == 500 + assert "Failed to generate refresh token" in e.value.detail + + +@pytest.mark.asyncio +async def test_authenticate_with_google_success(mocker): + mock_token = "valid-id-token" + mock_user_id = ObjectId() + decoded_token = { + "uid": "firebase-uid-123", + "email": "test@example.com", + "name": "Test User", + "picture": "http://example.com/avatar.jpg", + } + + # Mock firebase_auth.verify_id_token + mocker.patch( + "app.auth.service.firebase_auth.verify_id_token", return_value=decoded_token + ) + + # Mock db + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None # Simulate new user + mock_db.users.insert_one.return_value.inserted_id = mock_user_id + + mocker.patch.object(AuthService, "get_db", return_value=mock_db) + mocker.patch.object( + AuthService, "_create_refresh_token_record", return_value="new_refresh_token" + ) + + service = AuthService() + result = await service.authenticate_with_google(mock_token) + + assert result["user"]["email"] == "test@example.com" + assert result["refresh_token"] == "new_refresh_token" + + +@pytest.mark.asyncio +async def test_authenticate_with_google_invalid_token(mocker): + mocker.patch( + "app.auth.service.firebase_auth.verify_id_token", + side_effect=firebase_auth.InvalidIdTokenError("bad token"), + ) + + service = AuthService() + with pytest.raises(HTTPException) as exc_info: + await service.authenticate_with_google("invalid-token") + + assert exc_info.value.status_code == 401 + assert "Invalid Google ID token" in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_authenticate_with_google_missing_email(mocker): + + decoded_token = {"uid": "uid123"} # no email + + mocker.patch( + "app.auth.service.firebase_auth.verify_id_token", return_value=decoded_token + ) + + service = AuthService() + with pytest.raises(HTTPException) as exc_info: + await service.authenticate_with_google("any") + + assert exc_info.value.status_code == 400 + assert "Email not provided" in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_authenticate_with_google_db_find_error(mocker): + decoded_token = {"uid": "uid123", "email": "test@example.com"} + + mocker.patch( + "app.auth.service.firebase_auth.verify_id_token", return_value=decoded_token + ) + + mock_db = AsyncMock() + mock_db.users.find_one.side_effect = PyMongoError("db error") + mocker.patch.object(AuthService, "get_db", return_value=mock_db) + + service = AuthService() + with pytest.raises(HTTPException) as exc_info: + await service.authenticate_with_google("any") + + assert exc_info.value.status_code == 500 + + +@pytest.mark.asyncio +async def test_authenticate_with_google_insert_error(mocker): + decoded_token = { + "uid": "uid123", + "email": "test@example.com", + } + + mocker.patch( + "app.auth.service.firebase_auth.verify_id_token", return_value=decoded_token + ) + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None + mock_db.users.insert_one.side_effect = PyMongoError("insert failed") + mocker.patch.object(AuthService, "get_db", return_value=mock_db) + + service = AuthService() + with pytest.raises(HTTPException) as exc_info: + await service.authenticate_with_google("token") + + assert exc_info.value.status_code == 500 + assert "Failed to create user" in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_refresh_access_token_success(): + service = AuthService() + mock_db = MagicMock() + service.get_db = MagicMock(return_value=mock_db) + + now = datetime.now(timezone.utc) + mock_token_record = { + "token": "valid_refresh_token", + "revoked": False, + "expires_at": now + timedelta(hours=1), + "user_id": "user123", + "_id": "token_id", + } + mock_user = {"_id": "user123", "email": "test@example.com"} + + mock_db.refresh_tokens.find_one = AsyncMock(return_value=mock_token_record) + mock_db.users.find_one = AsyncMock(return_value=mock_user) + mock_db.refresh_tokens.update_one = AsyncMock() + + service._create_refresh_token_record = AsyncMock( + return_value="new_refresh_token") + + token = await service.refresh_access_token("valid_refresh_token") + assert token == "new_refresh_token" + mock_db.refresh_tokens.update_one.assert_called_once() + + +@pytest.mark.asyncio +async def test_refresh_access_token_invalid_or_expired(): + service = AuthService() + service.get_db = MagicMock(return_value=MagicMock()) + service.get_db().refresh_tokens.find_one = AsyncMock(return_value=None) + + with pytest.raises(HTTPException) as e: + await service.refresh_access_token("invalid_or_expired_token") + + assert e.value.status_code == status.HTTP_401_UNAUTHORIZED + assert "Invalid or expired" in e.value.detail + + +@pytest.mark.asyncio +async def test_refresh_access_token_user_not_found(): + service = AuthService() + mock_db = MagicMock() + service.get_db = MagicMock(return_value=mock_db) + + mock_token_record = { + "token": "valid_token", + "revoked": False, + "expires_at": datetime.now(timezone.utc) + timedelta(hours=1), + "user_id": "user123", + } + + mock_db.refresh_tokens.find_one = AsyncMock(return_value=mock_token_record) + mock_db.users.find_one = AsyncMock(return_value=None) + + with pytest.raises(HTTPException) as e: + await service.refresh_access_token("valid_token") + + assert e.value.status_code == status.HTTP_401_UNAUTHORIZED + assert "User not found" in e.value.detail + + +@pytest.mark.asyncio +async def test_refresh_access_token_db_failure_on_token(): + service = AuthService() + mock_db = MagicMock() + service.get_db = MagicMock(return_value=mock_db) + mock_db.refresh_tokens.find_one = AsyncMock( + side_effect=PyMongoError("DB error")) + + with pytest.raises(HTTPException) as e: + await service.refresh_access_token("any_token") + + assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + + +@pytest.mark.asyncio +async def test_refresh_access_token_db_failure_on_user_fetch(): + service = AuthService() + mock_db = MagicMock() + service.get_db = MagicMock(return_value=mock_db) + + mock_token_record = { + "token": "token", + "revoked": False, + "expires_at": datetime.now(timezone.utc) + timedelta(hours=1), + "user_id": "user123", + } + + mock_db.refresh_tokens.find_one = AsyncMock(return_value=mock_token_record) + mock_db.users.find_one = AsyncMock(side_effect=PyMongoError("DB error")) + + with pytest.raises(HTTPException) as e: + await service.refresh_access_token("token") + + assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + + +@pytest.mark.asyncio +async def test_refresh_access_token_generation_failure(): + service = AuthService() + mock_db = MagicMock() + service.get_db = MagicMock(return_value=mock_db) + + mock_token_record = { + "token": "token", + "revoked": False, + "expires_at": datetime.now(timezone.utc) + timedelta(hours=1), + "user_id": "user123", + "_id": "token_id", + } + + mock_user = {"_id": "user123"} + mock_db.refresh_tokens.find_one = AsyncMock(return_value=mock_token_record) + mock_db.users.find_one = AsyncMock(return_value=mock_user) + + service._create_refresh_token_record = AsyncMock( + side_effect=Exception("Token gen fail") + ) + + with pytest.raises(HTTPException) as e: + await service.refresh_access_token("token") + + assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert "Failed to create refresh token" in e.value.detail + + +@pytest.mark.asyncio +async def test_verify_access_token_valid(monkeypatch): + service = AuthService() + + # Mock verify_token to return a payload + monkeypatch.setattr( + "app.auth.security.verify_token", lambda token: {"sub": "user123"} + ) + + # Mock DB response + mock_user = {"_id": "user123", "email": "test@example.com"} + mock_db = AsyncMock() + mock_db.users.find_one.return_value = mock_user + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + user = await service.verify_access_token("validtoken") + assert user == mock_user + + +@pytest.mark.asyncio +async def test_verify_access_token_invalid_token(monkeypatch): + service = AuthService() + + def raise_jwt_error(token): + raise JWTError("Invalid signature") + + monkeypatch.setattr("app.auth.security.verify_token", raise_jwt_error) + + with pytest.raises(HTTPException) as exc_info: + await service.verify_access_token("badtoken") + + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "Invalid token" + + +@pytest.mark.asyncio +async def test_verify_access_token_missing_sub(monkeypatch): + service = AuthService() + + monkeypatch.setattr("app.auth.security.verify_token", lambda token: {}) + + with pytest.raises(HTTPException) as exc_info: + await service.verify_access_token("token") + + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "Invalid token" + + +@pytest.mark.asyncio +async def test_verify_access_token_db_error(monkeypatch): + service = AuthService() + + monkeypatch.setattr( + "app.auth.security.verify_token", lambda token: {"sub": "user123"} + ) + + mock_db = AsyncMock() + mock_db.users.find_one.side_effect = Exception("DB failure") + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + with pytest.raises(HTTPException) as exc_info: + await service.verify_access_token("token") + + assert exc_info.value.status_code == 500 + assert exc_info.value.detail == "Internal server error" + + +@pytest.mark.asyncio +async def test_verify_access_token_user_not_found(monkeypatch): + service = AuthService() + + monkeypatch.setattr( + "app.auth.security.verify_token", lambda token: {"sub": "user123"} + ) + + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + with pytest.raises(HTTPException) as exc_info: + await service.verify_access_token("token") + + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "User not found" + + +@pytest.mark.asyncio +async def test_request_password_reset_user_exists(monkeypatch, caplog): + service = AuthService() + mock_db = AsyncMock() + mock_user = {"_id": "mock_user_id", "email": "test@example.com"} + + mock_db.users.find_one.return_value = mock_user + mock_db.password_resets.insert_one.return_value = None + + monkeypatch.setattr( + service, "get_db", lambda: mock_db + ) # temporarily override get_db in function scope + + # Mock the token generator + with patch("app.auth.service.generate_reset_token", return_value="mocktoken"): + with caplog.at_level( + logging.INFO + ): # caplog captures the log messages (previously print statements) + result = await service.request_password_reset("test@example.com") + + # Assert + assert result is True + assert "mocktoken" in caplog.text + assert "Reset link" in caplog.text + mock_db.users.find_one.assert_awaited_once_with( + {"email": "test@example.com"}) + mock_db.password_resets.insert_one.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_request_password_reset_user_does_not_exist(monkeypatch): + service = AuthService() + mock_db = AsyncMock() + mock_db.users.find_one.return_value = None # No user found + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + result = await service.request_password_reset("nonexistent@example.com") + + assert result is True + mock_db.users.find_one.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_request_password_reset_db_error_on_lookup(monkeypatch): + service = AuthService() + mock_db = AsyncMock() + mock_db.users.find_one.side_effect = PyMongoError("DB failure") + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + with pytest.raises(HTTPException) as exc_info: + await service.request_password_reset("test@example.com") + + assert exc_info.value.status_code == 500 + assert "user lookup" in exc_info.value.detail.lower() + + +@pytest.mark.asyncio +async def test_request_password_reset_db_error_on_insert(monkeypatch): + service = AuthService() + mock_db = AsyncMock() + mock_user = {"_id": "mock_user_id", "email": "test@example.com"} + mock_db.users.find_one.return_value = mock_user + mock_db.password_resets.insert_one.side_effect = PyMongoError( + "Insert failure") + + monkeypatch.setattr(service, "get_db", lambda: mock_db) + + with patch("app.auth.service.generate_reset_token", return_value="mocktoken"): + with pytest.raises(HTTPException) as exc_info: + await service.request_password_reset("test@example.com") + + assert exc_info.value.status_code == 500 + assert "token storage" in exc_info.value.detail.lower() + + +@pytest.mark.asyncio +async def test_confirm_password_reset_success(): + service = AuthService() + mock_db = MagicMock() + mock_user_id = ObjectId() + + future_time = datetime.now(timezone.utc) + timedelta(hours=1) + + with patch.object(service, "get_db", return_value=mock_db): + # Mock reset token lookup + mock_db.password_resets.find_one = AsyncMock( + return_value={ + "token": "validtoken", + "used": False, + "expires_at": future_time, + "_id": ObjectId(), + "user_id": mock_user_id, + } + ) + + # Mock user update + mock_db.users.update_one = AsyncMock( + return_value=MagicMock(modified_count=1)) + mock_db.password_resets.update_one = AsyncMock() + mock_db.refresh_tokens.update_many = AsyncMock() + + result = await service.confirm_password_reset("validtoken", "newpassword123") + assert result is True + + mock_db.users.update_one.assert_awaited_once() + mock_db.refresh_tokens.update_many.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_confirm_password_reset_invalid_or_expired_token(): + service = AuthService() + mock_db = MagicMock() + + with patch.object(service, "get_db", return_value=mock_db): + # Simulate token not found (invalid, used, or expired) + mock_db.password_resets.find_one = AsyncMock(return_value=None) + + with pytest.raises(HTTPException) as exc_info: + await service.confirm_password_reset("badtoken", "irrelevantpassword") + + assert exc_info.value.status_code == 400 + assert exc_info.value.detail == "Invalid or expired reset token" + + # No DB updates should have been made + mock_db.users.update_one.assert_not_called() + mock_db.password_resets.update_one.assert_not_called() + mock_db.refresh_tokens.update_many.assert_not_called() + + +@pytest.mark.asyncio +async def test_create_refresh_token_record_success(): + service = AuthService() + mock_db = MagicMock() + mock_token = "mocked_refresh_token" + mock_user_id = str(ObjectId()) + + # Patch get_db and create_refresh_token + with patch.object(service, "get_db", return_value=mock_db), patch( + "app.auth.service.create_refresh_token", return_value=mock_token + ): + + # Mock insert + mock_db.refresh_tokens.insert_one = AsyncMock() + + result = await service._create_refresh_token_record(mock_user_id) + + assert result == mock_token + mock_db.refresh_tokens.insert_one.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_create_refresh_token_record_db_failure(): + service = AuthService() + mock_db = MagicMock() + mock_user_id = str(ObjectId()) + + with patch.object(service, "get_db", return_value=mock_db), patch( + "app.auth.service.create_refresh_token", return_value="badtoken" + ): + + # DB failure + mock_db.refresh_tokens.insert_one = AsyncMock( + side_effect=Exception("DB write error") + ) + + with pytest.raises(HTTPException) as exc_info: + await service._create_refresh_token_record(mock_user_id) + + assert exc_info.value.status_code == 500 + assert "Failed to create refresh token" in exc_info.value.detail + + +@pytest.mark.asyncio +async def test_create_refresh_token_record_invalid_user_id(): + service = AuthService() + invalid_user_id = "not-a-valid-objectid" + + with pytest.raises(HTTPException) as exc_info: + validate_object_id(invalid_user_id, field_name="user ID") + + assert exc_info.value.status_code == 400 + assert exc_info.value.detail == "Invalid user ID" diff --git a/backend/tests/expenses/test_expense_routes.py b/backend/tests/expenses/test_expense_routes.py index f55ee2f..351d7c5 100644 --- a/backend/tests/expenses/test_expense_routes.py +++ b/backend/tests/expenses/test_expense_routes.py @@ -3,6 +3,7 @@ import pytest from app.expenses.schemas import ExpenseCreateRequest, ExpenseSplit from fastapi import status +from firebase_admin import auth as firebase_auth from httpx import ASGITransport, AsyncClient from main import app # Adjusted import diff --git a/backend/tests/expenses/test_expense_service.py b/backend/tests/expenses/test_expense_service.py index a51cfdb..aed9818 100644 --- a/backend/tests/expenses/test_expense_service.py +++ b/backend/tests/expenses/test_expense_service.py @@ -5,7 +5,8 @@ import pytest from app.expenses.schemas import ExpenseCreateRequest, ExpenseSplit, SplitType from app.expenses.service import ExpenseService -from bson import ObjectId +from bson import ObjectId, errors +from fastapi import HTTPException @pytest.fixture @@ -120,7 +121,7 @@ async def test_create_expense_invalid_group(expense_service): mock_mongodb.database = mock_db mock_db.groups.find_one = AsyncMock(return_value=None) - # Test with invalid ObjectId format + """# Test with invalid ObjectId format with pytest.raises(ValueError, match="Group not found or user not a member"): await expense_service.create_expense( "invalid_group", expense_request, "user_a" @@ -128,9 +129,23 @@ async def test_create_expense_invalid_group(expense_service): # Test with valid ObjectId format but non-existent group with pytest.raises(ValueError, match="Group not found or user not a member"): + await expense_service.create_expense("65f1a2b3c4d5e6f7a8b9c0d0", expense_request, "user_a")""" + # Updated after stricter exception handling (July 2025) + # Case 1: Invalid ObjectId format + with pytest.raises(HTTPException) as exc_info_1: await expense_service.create_expense( - "65f1a2b3c4d5e6f7a8b9c0d0", expense_request, "user_a" + "invalid_group", expense_request, "user_a" + ) + assert exc_info_1.value.status_code == 400 + assert "Invalid group ID" in str(exc_info_1.value.detail) + + # Case 2: Valid ObjectId format but group not found or user not a member + with pytest.raises(HTTPException) as exc_info_2: + await expense_service.create_expense( + str(ObjectId()), expense_request, "user_a" ) + assert exc_info_2.value.status_code == 403 + assert "not a member of this group" in str(exc_info_2.value.detail) @pytest.mark.asyncio @@ -335,7 +350,9 @@ async def test_update_expense_unauthorized(expense_service): """Test expense update by non-creator""" from app.expenses.schemas import ExpenseUpdateRequest - update_request = ExpenseUpdateRequest(description="Unauthorized Update") + update_request = ExpenseUpdateRequest( + description="Unauthorized Update", amount=150.0 + ) with patch("app.expenses.service.mongodb") as mock_mongodb: mock_db = MagicMock() @@ -344,15 +361,23 @@ async def test_update_expense_unauthorized(expense_service): # Mock finding no expense (user not creator) mock_db.expenses.find_one = AsyncMock(return_value=None) - with pytest.raises( - ValueError, match="Expense not found or not authorized to edit" - ): + """with pytest.raises(ValueError, match="Expense not found or not authorized to edit"): + await expense_service.update_expense( + "group_id", + "65f1a2b3c4d5e6f7a8b9c0d1", + update_request, + "unauthorized_user" + )""" + # Updated test + with pytest.raises(HTTPException) as exc_info: await expense_service.update_expense( "group_id", "65f1a2b3c4d5e6f7a8b9c0d1", update_request, "unauthorized_user", ) + assert exc_info.value.status_code == 403 + assert "Not authorized" in str(exc_info.value.detail) def test_expense_split_validation(): @@ -477,10 +502,15 @@ async def test_get_expense_by_id_not_found(expense_service): # Mock expense not found mock_db.expenses.find_one = AsyncMock(return_value=None) - with pytest.raises(ValueError, match="Expense not found"): + """ with pytest.raises(ValueError, match="Expense not found"): + await expense_service.get_expense_by_id("65f1a2b3c4d5e6f7a8b9c0d0", "65f1a2b3c4d5e6f7a8b9c0d1", "user_a")""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.get_expense_by_id( "65f1a2b3c4d5e6f7a8b9c0d0", "65f1a2b3c4d5e6f7a8b9c0d1", "user_a" ) + assert exc_info.value.status_code == 404 + assert "Expense not found" in exc_info.value.detail @pytest.mark.asyncio @@ -765,11 +795,21 @@ async def test_delete_expense_not_found(expense_service): ) # Should not be called if expense not found mock_db.expenses.delete_one = AsyncMock() # Should not be called - with pytest.raises( - ValueError, match="Expense not found or not authorized to delete" - ): + """with pytest.raises(ValueError, match="Expense not found or not authorized to delete"): await expense_service.delete_expense(group_id, expense_id, user_id) + mock_db.settlements.delete_many.assert_not_called() + mock_db.expenses.delete_one.assert_not_called()""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: + await expense_service.delete_expense(group_id, expense_id, user_id) + + assert exc_info.value.status_code == 403 + assert ( + exc_info.value.detail + == "Not authorized to delete this expense or it does not exist" + ) + mock_db.settlements.delete_many.assert_not_called() mock_db.expenses.delete_one.assert_not_called() @@ -899,11 +939,19 @@ async def test_create_manual_settlement_group_not_found(expense_service): mock_db.groups.find_one = AsyncMock( return_value=None) # Group not found - with pytest.raises(ValueError, match="Group not found or user not a member"): + """with pytest.raises(ValueError, match="Group not found or user not a member"): + await expense_service.create_manual_settlement(group_id, settlement_request, user_id) + + mock_db.settlements.insert_one.assert_not_called()""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.create_manual_settlement( group_id, settlement_request, user_id ) + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == "Group not found or user not a member" + mock_db.settlements.insert_one.assert_not_called() @@ -1045,9 +1093,15 @@ async def test_get_group_settlements_group_not_found(expense_service): mock_db.groups.find_one = AsyncMock( return_value=None) # Group not found - with pytest.raises(ValueError, match="Group not found or user not a member"): + """with pytest.raises(ValueError, match="Group not found or user not a member"): + await expense_service.get_group_settlements(group_id, user_id)""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.get_group_settlements(group_id, user_id) + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == "Group not found or user not a member" + mock_db.settlements.find.assert_not_called() mock_db.settlements.count_documents.assert_not_called() @@ -1114,11 +1168,17 @@ async def test_get_settlement_by_id_not_found(expense_service, mock_group_data): return_value=None ) # Settlement not found - with pytest.raises(ValueError, match="Settlement not found"): + """with pytest.raises(ValueError, match="Settlement not found"): + await expense_service.get_settlement_by_id(group_id, settlement_id_str, user_id)""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.get_settlement_by_id( group_id, settlement_id_str, user_id ) + assert exc_info.value.status_code == 404 + assert exc_info.value.detail == "Settlement not found" + @pytest.mark.asyncio async def test_get_settlement_by_id_group_access_denied(expense_service): @@ -1135,11 +1195,17 @@ async def test_get_settlement_by_id_group_access_denied(expense_service): return_value=None ) # User not in group / group doesn't exist - with pytest.raises(ValueError, match="Group not found or user not a member"): + """with pytest.raises(ValueError, match="Group not found or user not a member"): + await expense_service.get_settlement_by_id(group_id, settlement_id_str, user_id)""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.get_settlement_by_id( group_id, settlement_id_str, user_id ) + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == "Group not found or user not a member" + mock_db.settlements.find_one.assert_not_called() @@ -1232,11 +1298,19 @@ async def test_update_settlement_status_not_found(expense_service): mock_db.settlements.find_one = AsyncMock(return_value=None) - with pytest.raises(ValueError, match="Settlement not found"): + """with pytest.raises(ValueError, match="Settlement not found"): + await expense_service.update_settlement_status( + group_id, settlement_id_str, new_status + )""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.update_settlement_status( group_id, settlement_id_str, new_status ) + assert exc_info.value.status_code == 404 + assert exc_info.value.detail == "Settlement not found" + # Should not be called if update fails mock_db.settlements.find_one.assert_not_called() @@ -1314,11 +1388,17 @@ async def test_delete_settlement_group_access_denied(expense_service): mock_db.groups.find_one = AsyncMock( return_value=None) # User not in group - with pytest.raises(ValueError, match="Group not found or user not a member"): + """with pytest.raises(ValueError, match="Group not found or user not a member"): + await expense_service.delete_settlement(group_id, settlement_id_str, user_id)""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.delete_settlement( group_id, settlement_id_str, user_id ) + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == "Group not found or user not a member" + mock_db.settlements.delete_one.assert_not_called() @@ -1451,11 +1531,17 @@ async def test_get_user_balance_in_group_access_denied(expense_service): return_value=None ) # Current user not member - with pytest.raises(ValueError, match="Group not found or user not a member"): + """with pytest.raises(ValueError, match="Group not found or user not a member"): + await expense_service.get_user_balance_in_group(group_id, target_user_id_str, current_user_id)""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.get_user_balance_in_group( group_id, target_user_id_str, current_user_id ) + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == "Group not found or user not a member" + mock_db.users.find_one.assert_not_called() mock_db.settlements.aggregate.assert_not_called() mock_db.settlements.find.assert_not_called() @@ -2013,9 +2099,15 @@ async def test_get_group_analytics_group_not_found(expense_service): mock_db.groups.find_one = AsyncMock( return_value=None) # Group not found - with pytest.raises(ValueError, match="Group not found or user not a member"): + """with pytest.raises(ValueError, match="Group not found or user not a member"): + await expense_service.get_group_analytics(group_id, user_id)""" + # Updated after stricter exception handling (July 2025) + with pytest.raises(HTTPException) as exc_info: await expense_service.get_group_analytics(group_id, user_id) + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == "Group not found or user not a member" + mock_db.expenses.find.assert_not_called() mock_db.users.find_one.assert_not_called() diff --git a/backend/tests/groups/test_groups_service.py b/backend/tests/groups/test_groups_service.py index 0bc9daa..ca95b64 100644 --- a/backend/tests/groups/test_groups_service.py +++ b/backend/tests/groups/test_groups_service.py @@ -442,3 +442,39 @@ async def test_leave_group_allow_member_to_leave(self): ) assert result is True + + # Adding invalid object ID and partial input tests for modified exception handling + @pytest.mark.asyncio + async def test_get_group_by_id_invalid_objectid(self): + """Test get_group_by_id with invalid ObjectId format""" + with patch.object(self.service, "get_db"): + result = await self.service.get_group_by_id("invalid-id", "user123") + assert result is None + + @pytest.mark.asyncio + async def test_update_group_invalid_objectid(self): + """Test update_group with invalid ObjectId""" + with patch.object(self.service, "get_db"): + result = await self.service.update_group( + "invalid-id", {"name": "test"}, "user123" + ) + assert result is None + + @pytest.mark.asyncio + async def test_delete_group_invalid_objectid(self): + """Test delete_group with invalid ObjectId""" + with patch.object(self.service, "get_db"): + result = await self.service.delete_group("invalid-id", "user123") + assert result is False + + def test_transform_group_document_partial_input(self): + """Test transform with partial group fields""" + group_doc = { + "_id": ObjectId("642f1e4a9b3c2d1f6a1b2c3d"), + "name": "Partial Group", + } + + result = self.service.transform_group_document(group_doc) + assert result["name"] == "Partial Group" + assert result["currency"] == "USD" # default fallback + assert result["members"] == [] # default fallback diff --git a/backend/tests/user/test_user_service.py b/backend/tests/user/test_user_service.py index 0a64d73..3d82f19 100644 --- a/backend/tests/user/test_user_service.py +++ b/backend/tests/user/test_user_service.py @@ -180,6 +180,18 @@ async def test_get_user_by_id_not_found(mock_db_client, mock_get_database): assert user is None +# Added Test for invalid ObjectId format +@pytest.mark.asyncio +async def test_get_user_by_id_invalid_objectid(mock_db_client, mock_get_database): + invalid_id = "invalid-objectid" + + user = await user_service.get_user_by_id(invalid_id) + + # No DB calls should be made + mock_db_client.users.find_one.assert_not_called() + assert user is None + + # --- Tests for update_user_profile --- @@ -240,6 +252,19 @@ async def test_update_user_profile_user_not_found(mock_db_client, mock_get_datab assert updated_user is None +# Added Test for invalid ObjectId format for user update +@pytest.mark.asyncio +async def test_update_user_profile_invalid_object_id(mock_db_client, mock_get_database): + invalid_user_id = "invalid_object_id" # Not a 24-char hex string + update_data = {"name": "Test Name"} + + updated_user = await user_service.update_user_profile(invalid_user_id, update_data) + + # Should return None and never hit the DB + mock_db_client.users.find_one_and_update.assert_not_called() + assert updated_user is None + + # --- Tests for delete_user --- @@ -267,3 +292,15 @@ async def test_delete_user_not_found(mock_db_client, mock_get_database): mock_db_client.users.delete_one.assert_called_once_with( {"_id": TEST_OBJECT_ID}) assert result is False + + +# Added Test for invalid ObjectId format for user deletion +@pytest.mark.asyncio +async def test_delete_user_invalid_object_id(mock_db_client, mock_get_database): + invalid_user_id = "invalid_object_id" # Not a valid 24-char hex string + + result = await user_service.delete_user(invalid_user_id) + + # Expected result: False and never hit the DB + mock_db_client.users.delete_one.assert_not_called() + assert result is False