diff --git a/app/api/routes/auth.py b/app/api/routes/auth.py index a71acc8..719bf3f 100644 --- a/app/api/routes/auth.py +++ b/app/api/routes/auth.py @@ -1,38 +1,22 @@ -from datetime import datetime - -from authlib.integrations.starlette_client import OAuth, OAuthError -from fastapi import APIRouter, Depends, HTTPException, Request -from fastapi.responses import JSONResponse +from fastapi import APIRouter, Depends, Request +from fastapi.security import OAuth2PasswordRequestForm from sqlalchemy.orm import Session -from starlette.config import Config from app.config.database import get_db -from app.models.user import AuthType, User -from app.settings import settings -from app.utils import auth +from app.config.github import github +from app.schemas.users import Token +from app.services.auth import login_github_user, login_user router = APIRouter() -# TODO: Move this to a separate config file -oauth = OAuth( - Config( - environ={ - "GITHUB_CLIENT_ID": settings.GITHUB_CLIENT_ID, - "GITHUB_CLIENT_SECRET": settings.GITHUB_CLIENT_SECRET, - } - ) -) -github = oauth.register( - name="github", - client_id=settings.GITHUB_CLIENT_ID, - client_secret=settings.GITHUB_CLIENT_SECRET, - access_token_url="https://github.com/login/oauth/access_token", - access_token_params=None, - authorize_url="https://github.com/login/oauth/authorize", - authorize_params=None, - api_base_url="https://api.github.com/", - client_kwargs={"scope": "user:email"}, -) + +# Standard username/password login route +@router.post("/login", response_model=Token) +def login( + form_data: OAuth2PasswordRequestForm = Depends(), db: Session = Depends(get_db) +): + """Simple login route using username and password.""" + return login_user(db, form_data.username, form_data.password) @router.get("/github/login") @@ -45,49 +29,4 @@ async def login_via_github(request: Request): @router.get("/github/callback") async def auth_github_callback(request: Request, db: Session = Depends(get_db)): """Handles the GitHub OAuth callback and logs in or registers the user.""" - try: - token = await github.authorize_access_token(request) - resp = await github.get("user", token=token) - profile = resp.json() - email = profile.get("email") - name = profile.get("name") or profile.get("login") - provider_id = str(profile.get("id")) - - # If email is not public, fetch from emails endpoint - if not email: - emails_resp = await github.get("user/emails", token=token) - emails = emails_resp.json() - email = next( - (e["email"] for e in emails if e.get("primary") and e.get("verified")), - None, - ) - if not email: - raise HTTPException( - status_code=400, detail="GitHub account has no accessible email." - ) - - user = db.query(User).filter(User.email == email).first() - if not user: - # TODO: Use existing create user logic - user = User( - email=email, - name=name, - provider_id=provider_id, - auth_type=AuthType.GITHUB, - is_active=True, - created_at=datetime.utcnow(), - ) - db.add(user) - db.commit() - db.refresh(user) - - access_token = auth.create_access_token( - {"sub": str(user.id), "email": user.email} - ) - return JSONResponse({"access_token": access_token, "token_type": "bearer"}) - except OAuthError as e: - # Log the detailed exception for debugging purposes - import logging - logging.error("OAuthError occurred during GitHub authentication", exc_info=e) - # Return a generic error message to the client - return JSONResponse({"error": "An error occurred during authentication. Please try again later."}, status_code=400) + return await login_github_user(request, db) diff --git a/app/api/routes/user.py b/app/api/routes/user.py index 848308d..94ef567 100644 --- a/app/api/routes/user.py +++ b/app/api/routes/user.py @@ -1,10 +1,9 @@ from fastapi import APIRouter, Depends -from fastapi.security import OAuth2PasswordRequestForm from sqlalchemy.orm import Session from app.config.database import get_db -from app.schemas.users import OTPVerifyRequest, Token, UserCreate, UserResponse -from app.services.user import (get_current_user, login_user, register_with_otp, +from app.schemas.users import OTPVerifyRequest, UserCreate, UserResponse +from app.services.user import (get_current_user, register_with_otp, verify_otp_and_create_user) router = APIRouter() @@ -12,22 +11,21 @@ @router.post("/register", status_code=201) def register_user(user_in: UserCreate, db: Session = Depends(get_db)): + """Register a new user with OTP verification. + It initiates the OTP process and returns a response to the user. + The user must then verify the OTP to complete registration.""" return register_with_otp(db, user_in) @router.post("/verify-otp", response_model=UserResponse) def verify_otp(request: OTPVerifyRequest, db: Session = Depends(get_db)): + """Verify OTP and create user. + This endpoint is called after the user has received an OTP via email. + It checks the OTP and creates the user if the OTP is valid.""" user = verify_otp_and_create_user(db, request.email, request.otp_code) return user -@router.post("/login", response_model=Token) -def login( - form_data: OAuth2PasswordRequestForm = Depends(), db: Session = Depends(get_db) -): - return login_user(db, form_data.username, form_data.password) - - @router.get("/me") def read_users_me(current_user=Depends(get_current_user)): if hasattr(current_user, "id"): diff --git a/app/config/github.py b/app/config/github.py new file mode 100644 index 0000000..e2c94ff --- /dev/null +++ b/app/config/github.py @@ -0,0 +1,25 @@ +from authlib.integrations.starlette_client import OAuth +from starlette.config import Config + +from app.settings import settings + +oauth = OAuth( + Config( + environ={ + "GITHUB_CLIENT_ID": settings.GITHUB_CLIENT_ID, + "GITHUB_CLIENT_SECRET": settings.GITHUB_CLIENT_SECRET, + } + ) +) + +github = oauth.register( + name="github", + client_id=settings.GITHUB_CLIENT_ID, + client_secret=settings.GITHUB_CLIENT_SECRET, + access_token_url="https://github.com/login/oauth/access_token", + access_token_params=None, + authorize_url="https://github.com/login/oauth/authorize", + authorize_params=None, + api_base_url="https://api.github.com/", + client_kwargs={"scope": "user:email"}, +) diff --git a/app/models/user.py b/app/models/user.py index a13f9b8..044f4d0 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -7,6 +7,7 @@ from app.config.database import Base +# TODO: removed as AuthType is now in app.schemas.users class AuthType(Enum): LOCAL = "local" GOOGLE = "google" diff --git a/app/services/auth.py b/app/services/auth.py new file mode 100644 index 0000000..f3930a7 --- /dev/null +++ b/app/services/auth.py @@ -0,0 +1,89 @@ +import logging + +from authlib.integrations.starlette_client import OAuthError +from fastapi import HTTPException +from fastapi.responses import JSONResponse + +from app.config.github import github +from app.models.user import User +from app.schemas.users import AuthType, UserCreate +from app.services.user import create_user +from app.utils.auth import create_access_token, verify_password + + +def authenticate_user(db, email: str, password: str): + """Authenticate user with email and password using hashed password.""" + user = db.query(User).filter(User.email == email).first() + if not user or not user.password: + return None + if not verify_password(password, user.password): + return None + return user + + +def login_user(db, email: str, password: str): + """Login user with email and password. Returns an access token if successful. Raises HTTPException if authentication fails.""" + user = authenticate_user(db, email, password) + if not user: + raise HTTPException(status_code=401, detail="Invalid credentials") + access_token = create_access_token({"sub": str(user.id), "email": user.email}) + return {"access_token": access_token, "token_type": "bearer"} + + +def create_github_user(profile, email: str, db): + """Create a new user from GitHub profile data.""" + name = profile.get("name") or profile.get("login") + provider_id = str(profile.get("id")) + + user_in = UserCreate( + email=email, + name=name, + is_active=True, + provider_id=provider_id, + auth_type=AuthType.github, + ) + user = create_user(db, user_in) + return user + + +async def login_github_user(request, db): + """Login user via GitHub OAuth. Returns an access token if successful. Raises HTTPException if authentication fails.""" + try: + token = await github.authorize_access_token(request) + resp = await github.get("user", token=token) + profile = resp.json() + email = profile.get("email") + + # If email is not public, fetch from emails endpoint + if not email: + emails_resp = await github.get("user/emails", token=token) + emails = emails_resp.json() + email = next( + ( + e.get("email") + for e in emails + if e.get("primary") and e.get("verified") + ), + None, + ) + if not email: + raise HTTPException( + status_code=400, detail="GitHub account has no accessible email." + ) + + user = db.query(User).filter(User.email == email).first() + + # If user does not exist, create a new one + if not user: + user = create_github_user(profile, email, db) + + access_token = create_access_token({"sub": str(user.id), "email": user.email}) + return JSONResponse({"access_token": access_token, "token_type": "bearer"}) + except OAuthError as e: + logging.error("OAuthError occurred during GitHub authentication", exc_info=e) + return JSONResponse( + { + "error": "An error occurred during authentication. Please try again later." + }, + status_code=400, + ) diff --git a/app/services/user.py b/app/services/user.py index 30b6348..d36d0bc 100644 --- a/app/services/user.py +++ b/app/services/user.py @@ -1,5 +1,3 @@ -import random -import re from datetime import datetime, timedelta from fastapi import Depends, HTTPException, status @@ -7,21 +5,21 @@ from jose import JWTError, jwt from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session -from sqlalchemy.orm import Session as _Session from app.config.database import get_db from app.models.otp import OTP from app.models.user import AuthType, User from app.schemas.users import UserCreate from app.settings import settings -from app.utils.auth import create_access_token, hash_password, verify_password +from app.utils.auth import (cleanup_expired_otps, generate_otp, hash_password, + validate_strong_password) from app.utils.email import send_otp_email security = HTTPBearer() def create_user(db: Session, user_in: UserCreate) -> User: - # Check if user already exists + """Create a new user in the database.""" existing = db.query(User).filter(User.email == user_in.email).first() if existing: raise HTTPException(status_code=400, detail="Email already registered") @@ -40,34 +38,17 @@ def create_user(db: Session, user_in: UserCreate) -> User: try: db.commit() db.refresh(user) - except IntegrityError: + except IntegrityError as exc: db.rollback() raise HTTPException( status_code=400, detail="Could not create user (integrity error)" - ) + ) from exc return user -def authenticate_user(db: Session, email: str, password: str): - user = db.query(User).filter(User.email == email).first() - if not user or not user.password: - return None - if not verify_password(password, user.password): - return None - return user - - -def login_user(db: Session, email: str, password: str): - user = authenticate_user(db, email, password) - if not user: - raise HTTPException(status_code=401, detail="Invalid credentials") - access_token = create_access_token({"sub": str(user.id), "email": user.email}) - return {"access_token": access_token, "token_type": "bearer"} - - def get_current_user( credentials: HTTPAuthorizationCredentials = Depends(security), - db: _Session = Depends(get_db), + db: Session = Depends(get_db), ): token = credentials.credentials credentials_exception = HTTPException( @@ -87,54 +68,24 @@ def get_current_user( if user is None: raise credentials_exception return user - except JWTError: - raise JWTError("JWT validation failed") - - -# TODO: move this function to a utils module or similar -def generate_otp(): - return str(random.randint(100000, 999999)) - - -# TODO: move this function to a utils module or similar -def validate_strong_password(password: str): - # At least 8 chars, one uppercase, one lowercase, one digit, one special char - if not password or len(password) < 8: - raise HTTPException( - status_code=400, detail="Password must be at least 8 characters long." - ) - if not re.search(r"[A-Z]", password): - raise HTTPException( - status_code=400, - detail="Password must contain at least one uppercase letter.", - ) - if not re.search(r"[a-z]", password): - raise HTTPException( - status_code=400, - detail="Password must contain at least one lowercase letter.", - ) - if not re.search(r"\d", password): - raise HTTPException( - status_code=400, detail="Password must contain at least one digit." - ) - if not re.search(r"[!@#$%^&*(),.?\":{}|<>]", password): - raise HTTPException( - status_code=400, - detail="Password must contain at least one special character.", - ) + except JWTError as exc: + raise JWTError("JWT validation failed") from exc def register_with_otp(db: Session, user_in: UserCreate): - # Check if user already exists + """Register a new user, initiate OTP process, and send OTP to email.""" existing = db.query(User).filter(User.email == user_in.email).first() if existing: raise HTTPException(status_code=400, detail="Email already registered") if not user_in.name: raise HTTPException(status_code=400, detail="Name is required.") + validate_strong_password(user_in.password) otp_code = generate_otp() + expires_at = datetime.utcnow() + timedelta(minutes=5) hashed_password = hash_password(user_in.password) if user_in.password else None + otp_entry = OTP( email=user_in.email, otp_code=otp_code, @@ -142,6 +93,7 @@ def register_with_otp(db: Session, user_in: UserCreate): password=hashed_password, is_active=1 if getattr(user_in, "is_active", True) else 0, provider_id=getattr(user_in, "provider_id", None), + # TODO: update this to use only local auth type auth_type=user_in.auth_type.value if hasattr(user_in, "auth_type") else "local", created_at=datetime.utcnow(), expires_at=expires_at, @@ -159,32 +111,30 @@ def verify_otp_and_create_user(db: Session, email: str, otp_code: str): ) if not otp_entry or otp_entry.is_expired(): raise HTTPException(status_code=400, detail="Invalid or expired OTP") - # Check if user already exists + existing = db.query(User).filter(User.email == email).first() if existing: db.delete(otp_entry) db.commit() raise HTTPException(status_code=400, detail="User already registered.") - # Create user from OTP details - # TODO: use existing user creation logic - user = User( + + # Use the existing create_user function with OTP data, in a transaction + user_in = UserCreate( email=otp_entry.email, name=otp_entry.name, - password=otp_entry.password, + password=otp_entry.password, # Already hashed is_active=bool(otp_entry.is_active), provider_id=otp_entry.provider_id, - auth_type=AuthType(otp_entry.auth_type), - created_at=datetime.utcnow(), + auth_type=otp_entry.auth_type, ) - db.add(user) - db.delete(otp_entry) - db.commit() - db.refresh(user) + try: + user = create_user(db, user_in) + db.delete(otp_entry) + db.commit() + except Exception as exc: + db.rollback() + raise HTTPException( + status_code=500, detail="User creation failed. Please try again." + ) from exc cleanup_expired_otps(db) return user - - -def cleanup_expired_otps(db: Session): - threshold = datetime.utcnow() - timedelta(minutes=5) - db.query(OTP).filter(OTP.expires_at < threshold).delete() - db.commit() diff --git a/app/utils/auth.py b/app/utils/auth.py index d1abd89..6332d07 100644 --- a/app/utils/auth.py +++ b/app/utils/auth.py @@ -1,9 +1,14 @@ +import random +import re from datetime import datetime, timedelta from typing import Optional +from fastapi import HTTPException from jose import jwt from passlib.context import CryptContext +from sqlalchemy.orm import Session +from app.models.otp import OTP from app.settings import settings # Password hashing context @@ -30,3 +35,44 @@ def create_access_token(data: dict, expires_delta: Optional[timedelta] = None): to_encode.update({"exp": expire}) encoded_jwt = jwt.encode(to_encode, SECRET_KEY, algorithm=ALGORITHM) return encoded_jwt + + +def validate_strong_password(password: str): + """Validate that the password meets strong security requirements. + Raises HTTPException if the password is not strong enough. + At least 8 chars, one uppercase, one lowercase, one digit, one special char + """ + if not password or len(password) < 8: + raise HTTPException( + status_code=400, detail="Password must be at least 8 characters long." + ) + if not re.search(r"[A-Z]", password): + raise HTTPException( + status_code=400, + detail="Password must contain at least one uppercase letter.", + ) + if not re.search(r"[a-z]", password): + raise HTTPException( + status_code=400, + detail="Password must contain at least one lowercase letter.", + ) + if not re.search(r"\d", password): + raise HTTPException( + status_code=400, detail="Password must contain at least one digit." + ) + if not re.search(r"[!@#$%^&*(),.?\":{}|<>]", password): + raise HTTPException( + status_code=400, + detail="Password must contain at least one special character.", + ) + + +def generate_otp(): + return str(random.randint(100000, 999999)) + + +def cleanup_expired_otps(db: Session): + """Remove expired OTP entries from the database.""" + threshold = datetime.utcnow() - timedelta(minutes=5) + db.query(OTP).filter(OTP.expires_at < threshold).delete() + db.commit()