diff --git a/backend/app/services/auth_service.py b/backend/app/services/auth_service.py index 4941671..7609358 100644 --- a/backend/app/services/auth_service.py +++ b/backend/app/services/auth_service.py @@ -15,13 +15,18 @@ from app.core.auth import ( ) from app.models.user import User from app.schemas.users import Token, UserCreate +from app.crud.user import user as crud_user +from app.core.auth import decode_token, get_token_data logger = logging.getLogger(__name__) class AuthenticationError(Exception): - """Exception raised for authentication errors""" - pass + """Raised when authentication fails""" + + def __init__(self, message: str = "Authentication failed"): + self.message = message + super().__init__(self.message) class AuthService: @@ -40,19 +45,22 @@ class AuthService: Returns: User if authenticated, None otherwise """ - user = db.query(User).filter(User.email == email).first() - + user = crud_user.get_by_email(db, email=email) if not user: - return None + logger.warning(f"Login attempt failed: user not found for email {email}") + raise AuthenticationError("Invalid email or password") if not verify_password(password, user.password_hash): - return None + logger.warning(f"Login attempt failed: invalid password for user {email}") + raise AuthenticationError("Invalid email or password") - if not user.is_active: - raise AuthenticationError("User account is inactive") + if not crud_user.is_active(user): + logger.warning(f"Login attempt failed: inactive user {email}") + raise AuthenticationError("Inactive user") return user + @staticmethod def create_user(db: Session, user_data: UserCreate) -> User: """ @@ -66,29 +74,20 @@ class AuthService: Created user """ # Check if user already exists - existing_user = db.query(User).filter(User.email == user_data.email).first() + existing_user = crud_user.get_by_email(db, email=user_data.email) if existing_user: - raise AuthenticationError("User with this email already exists") + logger.warning(f"Registration failed: email already registered {user_data.email}") + raise AuthenticationError("Email already registered") - # Create new user - hashed_password = get_password_hash(user_data.password) + try: + # Create new user using CRUD user + user = crud_user.create(db, obj_in=user_data) + logger.info(f"New user created: {user.email}") + return user + except Exception as e: + logger.error(f"User creation failed: {str(e)}") + raise AuthenticationError("Could not create user") - # Create user object from model - user = User( - email=user_data.email, - password_hash=hashed_password, - first_name=user_data.first_name, - last_name=user_data.last_name, - phone_number=user_data.phone_number, - is_active=True, - is_superuser=False - ) - - db.add(user) - db.commit() - db.refresh(user) - - return user @staticmethod def create_tokens(user: User) -> Token: @@ -139,7 +138,6 @@ class AuthService: TokenExpiredError: If refresh token has expired TokenInvalidError: If refresh token is invalid """ - from app.core.auth import decode_token, get_token_data try: # Verify token is a refresh token @@ -148,12 +146,17 @@ class AuthService: # Get user ID from token token_data = get_token_data(refresh_token) user_id = token_data.user_id + if not user_id: + raise AuthenticationError("Invalid token") + # Get user from database - user = db.query(User).filter(User.id == user_id).first() + user: User | None = crud_user.get(db, id=UUID(str(user_id))) + if not user or not user.is_active: raise TokenInvalidError("Invalid user or inactive account") + user: User # Generate new tokens return AuthService.create_tokens(user) @@ -178,7 +181,7 @@ class AuthService: Raises: AuthenticationError: If current password is incorrect """ - user = db.query(User).filter(User.id == user_id).first() + user = crud_user.get(db, id=user_id) if not user: raise AuthenticationError("User not found") diff --git a/backend/tests/services/test_auth_service.py b/backend/tests/services/test_auth_service.py index b043a3c..b339475 100644 --- a/backend/tests/services/test_auth_service.py +++ b/backend/tests/services/test_auth_service.py @@ -32,13 +32,14 @@ class TestAuthServiceAuthentication: def test_authenticate_nonexistent_user(self, db_session): """Test authenticating with an email that doesn't exist""" - user = AuthService.authenticate_user( - db=db_session, - email="nonexistent@example.com", - password="password" - ) + with pytest.raises(AuthenticationError): + user = AuthService.authenticate_user( + db=db_session, + email="nonexistent@example.com", + password="password" + ) - assert user is None + assert user is None def test_authenticate_with_wrong_password(self, db_session, mock_user): """Test authenticating with the wrong password""" @@ -48,13 +49,14 @@ class TestAuthServiceAuthentication: db_session.commit() # Authenticate with wrong password - user = AuthService.authenticate_user( - db=db_session, - email=mock_user.email, - password="WrongPassword123" - ) + with pytest.raises(AuthenticationError): + user = AuthService.authenticate_user( + db=db_session, + email=mock_user.email, + password="WrongPassword123" + ) - assert user is None + assert user is None def test_authenticate_inactive_user(self, db_session, mock_user): """Test authenticating an inactive user"""