Update services/auth.py

services/auth.py

@@ -1,132 +1,121 @@
-from passlib.context import CryptContext
-from sqlmodel import Session, select
+# /home/user/app/services/auth.py
 from typing import Optional
+import bcrypt # For password hashing
+from sqlmodel import select
 
-from models.user import User, UserCreate # Direct import
-from models.db import get_session_context
-from services.logger import app_logger
+from models import User, UserCreate, get_session_context # Your SQLModel User and session
+from services.logger import app_logger # Your application logger
 
-pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
+# --- Password Hashing Utilities ---
+def hash_password(password: str) -> str:
+    """Hashes a plain-text password using bcrypt."""
+    try:
+        password_bytes = password.encode('utf-8')
+        salt = bcrypt.gensalt()
+        hashed_bytes = bcrypt.hashpw(password_bytes, salt)
+        return hashed_bytes.decode('utf-8')
+    except Exception as e:
+        app_logger.error(f"Error during password hashing: {e}", exc_info=True)
+        # Re-raise or return a specific error indicator if preferred,
+        # but for security, failing to hash should prevent account creation.
+        raise ValueError("Password hashing failed") from e
 
 def verify_password(plain_password: str, hashed_password: str) -> bool:
-    return pwd_context.verify(plain_password, hashed_password)
-
-def get_password_hash(password: str) -> str:
-    return pwd_context.hash(password)
-
-def get_user_by_username(db: Session, username: str) -> Optional[User]:
-    """Fetches a user by username."""
-    statement = select(User).where(User.username == username)
-    user = db.exec(statement).first()
-    return user
+    """Verifies a plain-text password against a stored bcrypt hash."""
+    try:
+        plain_password_bytes = plain_password.encode('utf-8')
+        hashed_password_bytes = hashed_password.encode('utf-8')
+        return bcrypt.checkpw(plain_password_bytes, hashed_password_bytes)
+    except Exception as e:
+        app_logger.error(f"Error during password verification: {e}", exc_info=True)
+        # If bcrypt itself is broken, verification will fail.
+        return False
 
-def create_user_in_db(user_data: UserCreate) -> Optional[User]:
+# --- User Creation ---
+def create_user_in_db(user_create_data: UserCreate) -> Optional[User]:
     """
-    Creates a new user in the database.
-    Returns the User object with basic attributes loaded, suitable for immediate
-    use even after the session creating it has closed.
+    Creates a new user in the database with a hashed password.
+    Returns the User object if successful, None otherwise.
     """
-    hashed_password = get_password_hash(user_data.password)
-    
-    # Create the Python object instance.
-    # Initialize relationships like chat_sessions to an empty list if appropriate,
-    # as they won't be populated from the DB for a new user yet.
-    new_user_instance = User(
-        username=user_data.username,
-        hashed_password=hashed_password,
-        email=user_data.email,
-        full_name=user_data.full_name,
-        chat_sessions=[] # Good practice to initialize for new instances
-    )
-    
-    created_user_id: Optional[int] = None
-
+    app_logger.info(f"Attempting to create user: {user_create_data.username}")
     try:
-        with get_session_context() as db:
-            # Check if user already exists
-            existing_user = get_user_by_username(db, user_data.username)
-            if existing_user:
-                app_logger.warning(f"User {user_data.username} already exists.")
-                return None # User already exists
-            
-            db.add(new_user_instance)
-            db.commit() # Commit to save the user and allow DB to generate ID
-            db.refresh(new_user_instance) # Refresh to get all attributes, especially the DB-generated ID
-            
-            # Store the ID so we can re-fetch a "clean" instance if needed,
-            # or to ensure we are working with the persisted state.
-            created_user_id = new_user_instance.id
-            
-            # To ensure the returned object is safe for use after this session closes,
-            # especially for attributes that might be displayed immediately,
-            # we can load them explicitly or re-fetch.
-            # For simple attributes, refresh should be enough.
-            # We will return a freshly fetched instance to be absolutely sure.
+        with get_session_context() as session:
+            # Check if username already exists
+            statement_username = select(User).where(User.username == user_create_data.username)
+            existing_user_by_username = session.exec(statement_username).first()
+            if existing_user_by_username:
+                app_logger.warning(f"Signup failed: Username '{user_create_data.username}' already exists.")
+                return None
 
-    except Exception as e:
-        app_logger.error(f"Error during database operation for user {user_data.username}: {e}")
-        return None # DB operation failed
+            # Check if email already exists (if provided and should be unique)
+            if user_create_data.email:
+                statement_email = select(User).where(User.email == user_create_data.email)
+                existing_user_by_email = session.exec(statement_email).first()
+                if existing_user_by_email:
+                    app_logger.warning(f"Signup failed: Email '{user_create_data.email}' already exists.")
+                    return None
+            
+            try:
+                hashed_pw = hash_password(user_create_data.password)
+            except ValueError: # Catch hashing specific error
+                app_logger.error(f"Could not hash password for user {user_create_data.username} during signup.")
+                return None
 
-    # If user was created, fetch a fresh instance to return.
-    # This new instance will be detached but will have its basic attributes loaded.
-    if created_user_id is not None:
-        try:
-            with get_session_context() as db:
-                # .get() is efficient for fetching by primary key
-                final_user_to_return = db.get(User, created_user_id)
-                if final_user_to_return:
-                    # "Touch" attributes that will be used immediately after this function returns
-                    # to ensure they are loaded before this (new) session closes.
-                    _ = final_user_to_return.id
-                    _ = final_user_to_return.username
-                    _ = final_user_to_return.email
-                    _ = final_user_to_return.full_name
-                    # Do NOT try to access final_user_to_return.chat_sessions here unless you
-                    # intend to load them, which might be a heavy operation.
-                    # They will be lazy-loaded when accessed within an active session later.
-                    return final_user_to_return
-        except Exception as e:
-            app_logger.error(f"Error re-fetching created user {created_user_id}: {e}")
-            return None # Failed to re-fetch
 
-    return None # Fallback if user was not created or couldn't be re-fetched
+            # Create the new user object
+            db_user = User(
+                username=user_create_data.username,
+                email=user_create_data.email,
+                full_name=user_create_data.full_name, # Assuming UserCreate and User models have this
+                disabled=user_create_data.disabled if hasattr(user_create_data, 'disabled') else False,
+                hashed_password=hashed_pw
+            )
+            
+            session.add(db_user)
+            # The commit is handled by the get_session_context manager upon successful exit.
+            # We need to refresh to get DB-generated values like ID before the session closes.
+            session.refresh(db_user)
+            app_logger.info(f"User '{db_user.username}' (ID: {db_user.id}) created successfully in DB.")
+            return db_user
 
+    except Exception as e:
+        app_logger.error(f"Database error during user creation for '{user_create_data.username}': {e}", exc_info=True)
+        # session.rollback() is handled by get_session_context
+        return None
 
-def authenticate_user(username: str, password: str) -> Optional[User]:
+# --- User Authentication ---
+def authenticate_user(username_in: str, password_in: str) -> Optional[User]:
     """
-    Authenticates a user.
-    Returns the User object with basic attributes loaded.
+    Authenticates a user by username and password.
+    Returns the User object if authentication is successful, None otherwise.
     """
+    app_logger.info(f"Attempting to authenticate user: {username_in}")
     try:
-        with get_session_context() as db:
-            user = get_user_by_username(db, username)
+        with get_session_context() as session:
+            statement = select(User).where(User.username == username_in)
+            user = session.exec(statement).first()
+
             if not user:
-                return None # User not found
-            if not verify_password(password, user.hashed_password):
-                return None # Invalid password
+                app_logger.warning(f"Authentication failed: User '{username_in}' not found.")
+                return None
             
-            # User is authenticated. Before returning the user object,
-            # ensure any attributes that will be immediately accessed by the caller
-            # (e.g., when setting st.session_state.authenticated_user) are loaded.
-            # This helps avoid DetachedInstanceError if Streamlit or other parts
-            # of the app try to access these attributes after this session closes.
+            if user.disabled: # Assuming your User model has a 'disabled' field
+                app_logger.warning(f"Authentication failed: User '{username_in}' is disabled.")
+                return None
+
+            if not verify_password(password_in, user.hashed_password):
+                app_logger.warning(f"Authentication failed: Invalid password for user '{username_in}'.")
+                return None
             
-            user_id_to_return = user.id # Get ID to re-fetch
+            # If login is successful, you might want to update a last_login timestamp here
+            # user.last_login_at = datetime.utcnow()
+            # session.add(user)
+            # session.commit() # (handled by context manager)
+            # session.refresh(user)
 
-        # Re-fetch the user in a new session context to return a "clean" detached object
-        # This is a robust way to prevent issues with detached instances being used across session boundaries.
-        if user_id_to_return is not None:
-            with get_session_context() as db:
-                authenticated_user = db.get(User, user_id_to_return)
-                if authenticated_user:
-                    # "Touch" attributes
-                    _ = authenticated_user.id
-                    _ = authenticated_user.username
-                    _ = authenticated_user.email
-                    # Do not touch relationships unless intended for eager load here.
-                    return authenticated_user
-        return None # Should not happen if authenticated
+            app_logger.info(f"User '{user.username}' (ID: {user.id}) authenticated successfully.")
+            return user # Return the ORM object
 
     except Exception as e:
-        app_logger.error(f"Error during authentication for user {username}: {e}")
+        app_logger.error(f"Database or unexpected error during authentication for '{username_in}': {e}", exc_info=True)
         return None