From aef0ff3fd18d9e16e6ba552f0a63995f13b6f410 Mon Sep 17 00:00:00 2001 From: PuQing Date: Sun, 28 Jan 2024 19:55:47 +0800 Subject: [PATCH] Update logging configuration and user authentication --- backend/app/app/core/security.py | 13 +++-- backend/app/app/crud/base.py | 61 --------------------- backend/app/app/crud/crud_item.py | 50 ----------------- backend/app/app/crud/crud_user.py | 62 ---------------------- backend/app/app/db/init_db.py | 3 +- backend/app/app/log.py | 3 ++ backend/app/app/models.py | 44 +++++++++++++-- backend/app/app/web/api/endpoints/items.py | 23 ++++---- backend/app/app/web/api/endpoints/login.py | 20 ++++--- backend/app/app/web/api/endpoints/users.py | 57 ++++++++++---------- 10 files changed, 106 insertions(+), 230 deletions(-) delete mode 100644 backend/app/app/crud/base.py delete mode 100644 backend/app/app/crud/crud_item.py delete mode 100644 backend/app/app/crud/crud_user.py diff --git a/backend/app/app/core/security.py b/backend/app/app/core/security.py index 6c6ee8b..3240f56 100644 --- a/backend/app/app/core/security.py +++ b/backend/app/app/core/security.py @@ -1,5 +1,5 @@ from datetime import datetime, timedelta -from typing import Any, Union +from typing import Any, Optional, Union from jose import jwt from passlib.context import CryptContext @@ -13,16 +13,21 @@ def create_access_token( - subject: Union[str, Any], expires_delta: timedelta = None + subject: Union[str, Any], + expires_delta: Optional[timedelta] = None, ) -> str: if expires_delta: expire = datetime.utcnow() + expires_delta else: expire = datetime.utcnow() + timedelta( - minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES + minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES, ) to_encode = {"exp": expire, "sub": str(subject)} - encoded_jwt = jwt.encode(to_encode, settings.SECRET_KEY, algorithm=ALGORITHM) + encoded_jwt = jwt.encode( + to_encode, + settings.SECRET_KEY, + algorithm=ALGORITHM, + ) return encoded_jwt diff --git a/backend/app/app/crud/base.py b/backend/app/app/crud/base.py deleted file mode 100644 index ce9b01d..0000000 --- a/backend/app/app/crud/base.py +++ /dev/null @@ -1,61 +0,0 @@ -from typing import Any, Generic, Optional, TypeVar, Union - -from fastapi.encoders import jsonable_encoder -from pydantic import BaseModel -from sqlmodel import Session - -ModelType = TypeVar("ModelType", bound=Any) -CreateSchemaType = TypeVar("CreateSchemaType", bound=BaseModel) -UpdateSchemaType = TypeVar("UpdateSchemaType", bound=BaseModel) - - -class CRUDBase(Generic[ModelType, CreateSchemaType, UpdateSchemaType]): - def __init__(self, model: type[ModelType]): - """ - CRUD object with default methods to Create, Read, Update, Delete (CRUD). - - **Parameters** - - * `model`: A SQLAlchemy model class - * `schema`: A Pydantic model (schema) class - """ - self.model = model - - def get(self, db: Session, id: Any) -> Optional[ModelType]: - return db.get(self.model, id) - - def create(self, db: Session, *, obj_in: CreateSchemaType) -> ModelType: - obj_in_data = jsonable_encoder(obj_in) - db_obj = self.model(**obj_in_data) # type: ignore - db.add(db_obj) - db.commit() - db.refresh(db_obj) - return db_obj - - def update( - self, - db: Session, - *, - db_obj: ModelType, - obj_in: Union[UpdateSchemaType, dict[str, Any]] - ) -> ModelType: - obj_data = jsonable_encoder(db_obj) - if isinstance(obj_in, dict): - update_data = obj_in - else: - update_data = obj_in.model_dump(exclude_unset=True) - for field in obj_data: - if field in update_data: - setattr(db_obj, field, update_data[field]) - db.add(db_obj) - db.commit() - db.refresh(db_obj) - return db_obj - - def remove(self, db: Session, *, id: int) -> ModelType | None: - obj = db.query(self.model).get(id) - if obj: - db.delete(obj) - db.commit() - return obj - return None diff --git a/backend/app/app/crud/crud_item.py b/backend/app/app/crud/crud_item.py deleted file mode 100644 index 04a5c61..0000000 --- a/backend/app/app/crud/crud_item.py +++ /dev/null @@ -1,50 +0,0 @@ -from typing import Optional - -from sqlmodel import Session, col - -from app.crud.base import CRUDBase -from app.models import Item -from app.schemas.item import ItemCreate, ItemUpdate - - -class CRUDItem(CRUDBase[Item, ItemCreate, ItemUpdate]): - def create(self, db: Session, *, obj_in: ItemCreate) -> Item: - db_obj = Item( - title=obj_in.title, - description=obj_in.description, - keywords=obj_in.keywords, - raw_url=str(obj_in.raw_url), - ) - db.add(db_obj) - db.commit() - db.refresh(db_obj) - return db_obj - - def update(self, db: Session, *, db_obj: Item, obj_in: ItemUpdate) -> Item: - if obj_in.title: - db_obj.title = obj_in.title - if obj_in.description: - db_obj.description = obj_in.description - if obj_in.keywords: - db_obj.keywords = obj_in.keywords - if obj_in.raw_url: - db_obj.raw_url = obj_in.raw_url - return super().update(db, db_obj=db_obj, obj_in=obj_in) - - def get_by_id(self, db: Session, *, id: int) -> Optional[Item]: - return db.get(Item, id) - - def get_by_fuzzy_title(self, db: Session, *, title: str) -> list[Item]: - return db.query(Item).filter(col("title").ilike(f"%{title}%")).all() - - def create_bulk( - self, db: Session, *, objs_in: list[ItemCreate] - ) -> list[Item]: - objs = [Item(**obj_in.model_dump()) for obj_in in objs_in] - db.add_all(objs) - db.commit() - db.refresh(objs) - return objs - - -item = CRUDItem(Item) diff --git a/backend/app/app/crud/crud_user.py b/backend/app/app/crud/crud_user.py deleted file mode 100644 index 22092a9..0000000 --- a/backend/app/app/crud/crud_user.py +++ /dev/null @@ -1,62 +0,0 @@ -from typing import Any, Optional, Union - -from sqlmodel import Session, select - -from app.core.security import get_password_hash, verify_password -from app.crud.base import CRUDBase -from app.models import User -from app.schemas.user import UserCreate, UserUpdate - - -class CRUDUser(CRUDBase[User, UserCreate, UserUpdate]): - def get_by_email(self, db: Session, *, email: str) -> Optional[User]: - statement = select(User).where(User.email == email) - return db.exec(statement).first() - - def create(self, db: Session, *, obj_in: UserCreate) -> User: - db_obj = User( - email=obj_in.email, - hashed_password=get_password_hash(obj_in.password), - full_name=obj_in.full_name, - is_superuser=obj_in.is_superuser, - ) - db.add(db_obj) - db.commit() - db.refresh(db_obj) - return db_obj - - def update( - self, - db: Session, - *, - db_obj: User, - obj_in: Union[UserUpdate, dict[str, Any]] - ) -> User: - if isinstance(obj_in, dict): - update_data = obj_in - else: - update_data = obj_in.model_dump(exclude_unset=True) - if update_data["password"]: - hashed_password = get_password_hash(update_data["password"]) - del update_data["password"] - update_data["hashed_password"] = hashed_password - return super().update(db, db_obj=db_obj, obj_in=update_data) - - def authenticate( - self, db: Session, *, email: str, password: str - ) -> Optional[User]: - user = self.get_by_email(db, email=email) - if not user: - return None - if not verify_password(password, user.hashed_password): - return None - return user - - def is_active(self, user: User) -> bool: - return user.is_active - - def is_superuser(self, user: User) -> bool: - return user.is_superuser - - -user = CRUDUser(User) diff --git a/backend/app/app/db/init_db.py b/backend/app/app/db/init_db.py index 41de50f..20b9c0d 100644 --- a/backend/app/app/db/init_db.py +++ b/backend/app/app/db/init_db.py @@ -1,7 +1,6 @@ from sqlmodel import Session, select from app.core.config import settings -from app.crud.crud_user import user as crud from app.models import User, UserCreate # noqa: F401 # make sure all SQLModel models are imported (app.models) before initializing DB @@ -23,4 +22,4 @@ def init_db(session: Session) -> None: password=settings.FIRST_SUPERUSER_PASSWORD, is_superuser=True, ) - user = crud.create(db=session, obj_in=user_in) + user = User.create(session, user_in) diff --git a/backend/app/app/log.py b/backend/app/app/log.py index 63db5cd..c6c32c3 100644 --- a/backend/app/app/log.py +++ b/backend/app/app/log.py @@ -58,6 +58,9 @@ def configure_logging() -> None: # pragma: no cover logging.getLogger("gunicorn").handlers = [intercept_handler] logging.getLogger("gunicorn.access").handlers = [intercept_handler] + + logging.getLogger("passlib").setLevel(logging.ERROR) + # set logs output, level and format logger.remove() logger.add( diff --git a/backend/app/app/models.py b/backend/app/app/models.py index d1715d0..c404d75 100644 --- a/backend/app/app/models.py +++ b/backend/app/app/models.py @@ -7,6 +7,8 @@ from sqlalchemy.orm.exc import FlushError from sqlmodel import JSON, AutoString, Column, Field, SQLModel, select +from app.core.security import get_password_hash, verify_password + class ActiveRecordMixin: __config__ = None @@ -87,11 +89,10 @@ def create( update: Optional[dict] = None, ) -> Optional[SQLModel]: obj = cls.convert_without_saving(source, update) - if obj is None: - return None if obj.save(session): return obj - return None + else: + return None @classmethod def create_or_update( @@ -195,6 +196,43 @@ class User(ActiveRecordMixin, UserBase, table=True): id: Union[int, None] = Field(default=None, primary_key=True) hashed_password: str + @classmethod + def authenticate( + cls, session, *, email: str, password: str + ) -> Optional["User"]: + user = User.one_by_field(session, "email", email) + if not user: + return None + if not verify_password(password, user.hashed_password): + return None + return user + + @classmethod + def create( + cls, + session, + source: Union[UserCreate, UserCreateOpen], + ) -> Optional[SQLModel]: + obj = User( + **source.model_dump(exclude={"password"}, exclude_unset=True), + hashed_password=get_password_hash(source.password), + ) + if obj.save(session): + return obj + else: + return None + + def update(self, session, source: Union[dict, SQLModel]): + if isinstance(source, SQLModel): + source = source.model_dump(exclude_unset=True) + + if "password" in source: + source["hashed_password"] = get_password_hash(source["password"]) + del source["password"] + for key, value in source.items(): + setattr(self, key, value) + self.save(session) + # Properties to return via API, id is always required class UserOut(UserBase): diff --git a/backend/app/app/web/api/endpoints/items.py b/backend/app/app/web/api/endpoints/items.py index 3d558c9..3b5629d 100644 --- a/backend/app/app/web/api/endpoints/items.py +++ b/backend/app/app/web/api/endpoints/items.py @@ -3,7 +3,6 @@ from fastapi import APIRouter, HTTPException from sqlmodel import select -from app.crud.crud_item import item as crud from app.models import Item, ItemCreate, ItemOut, ItemUpdate from app.web.api.deps import CurrentUser, SessionDep @@ -28,7 +27,7 @@ def read_item(session: SessionDep, id: int) -> Any: """ Get item by ID. """ - item = session.get(Item, id) + item = Item.one_by_id(session, id=id) if not item: raise HTTPException(status_code=404, detail="Item not found") return item @@ -39,8 +38,7 @@ def read_item_fuzzy(session: SessionDep, title: str) -> Any: """ Get item by fuzzy title. """ - statement = crud.get_by_fuzzy_title(session, title=title) - return statement + pass @router.post("/", response_model=ItemOut) @@ -48,7 +46,9 @@ def create_item(*, session: SessionDep, item_in: ItemCreate) -> Any: """ Create new item. """ - item = crud.create(session, obj_in=item_in) + item = Item.create(session, source=item_in) + if not item: + raise HTTPException(status_code=400, detail="Item not created") return item @@ -57,8 +57,7 @@ def create_items(*, session: SessionDep, items_in: list[ItemCreate]) -> Any: """ Create new items. """ - items = crud.create_bulk(session, objs_in=items_in) - return items + pass @router.put("/{id}", response_model=ItemOut) @@ -74,10 +73,10 @@ def update_item( """ if not current_user.is_superuser: raise HTTPException(status_code=400, detail="Not enough permissions") - item = crud.get(session, id=id) + item: Item = Item.one_by_id(session, id=id) if not item: raise HTTPException(status_code=404, detail="Item not found") - item = crud.update(session, db_obj=item, obj_in=item_in) + item.update(session, item_in) return item @@ -86,14 +85,14 @@ def delete_item( session: SessionDep, current_user: CurrentUser, id: int, -) -> ItemOut: +) -> Any: """ Delete an item. """ - item = crud.get(session, id=id) + item: Item = Item.one_by_id(session, id=id) if not item: raise HTTPException(status_code=404, detail="Item not found") if not current_user.is_superuser: raise HTTPException(status_code=400, detail="Not enough permissions") - item = crud.remove(session, id=id) + item.delete(session) return item diff --git a/backend/app/app/web/api/endpoints/login.py b/backend/app/app/web/api/endpoints/login.py index 5cac2e6..5a26d4c 100644 --- a/backend/app/app/web/api/endpoints/login.py +++ b/backend/app/app/web/api/endpoints/login.py @@ -6,8 +6,7 @@ from app.core import security from app.core.config import settings -from app.crud.crud_user import user as crud -from app.models import Message, NewPassword, Token, UserOut +from app.models import Message, NewPassword, Token, User, UserOut from app.utils import ( generate_password_reset_token, send_reset_password_email, @@ -26,16 +25,21 @@ def login_access_token( """ OAuth2 compatible token login, get an access token for future requests """ - user = crud.authenticate( - db=session, + user = User.authenticate( + session, email=form_data.username, password=form_data.password, ) if not user: - raise HTTPException(status_code=400, detail="Incorrect email or password") + raise HTTPException( + status_code=400, + detail="Incorrect email or password", + ) elif not user.is_active: raise HTTPException(status_code=400, detail="Inactive user") - access_token_expires = timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES) + access_token_expires = timedelta( + minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES, + ) return Token( access_token=security.create_access_token( user.id, @@ -57,7 +61,7 @@ async def recover_password(email: str, session: SessionDep) -> Message: """ Password Recovery """ - user = crud.get_by_email(db=session, email=email) + user = User.first_by_field(session, "email", email) if not user: raise HTTPException( @@ -81,7 +85,7 @@ def reset_password(session: SessionDep, body: NewPassword) -> Message: email = verify_password_reset_token(token=body.token) if not email: raise HTTPException(status_code=400, detail="Invalid token") - user = crud.get_by_email(db=session, email=email) + user = User.first_by_field(session, "email", email) if not user: raise HTTPException( status_code=404, diff --git a/backend/app/app/web/api/endpoints/users.py b/backend/app/app/web/api/endpoints/users.py index f25b09f..fa1cb84 100644 --- a/backend/app/app/web/api/endpoints/users.py +++ b/backend/app/app/web/api/endpoints/users.py @@ -1,11 +1,9 @@ from typing import Any from fastapi import APIRouter, Depends, HTTPException -from fastapi.encoders import jsonable_encoder from sqlmodel import select from app.core.config import settings -from app.crud.crud_user import user as crud from app.models import ( User, UserCreate, @@ -47,48 +45,44 @@ def read_users( dependencies=[Depends(get_current_active_superuser)], response_model=UserOut, ) -def create_user(*, session: SessionDep, user_in: UserCreate) -> UserOut: +def create_user(*, session: SessionDep, user_in: UserCreate) -> Any: """ Create new user. """ - user = crud.get_by_email(db=session, email=user_in.email) + user = User.first_by_field(session, "email", user_in.email) if user: raise HTTPException( status_code=400, detail="The user with this username already exists in the system.", ) - - user = crud.create(db=session, user_create=user_in) + user = User.create(session, user_in) if settings.EMAILS_ENABLED and user_in.email: send_new_account_email( email_to=user_in.email, username=user_in.email, password=user_in.password, ) + if user is None: + raise HTTPException( + status_code=400, + detail="Failed to create user.", + ) return user @router.put("/me", response_model=UserOut) def update_user_me( *, session: SessionDep, body: UserUpdateMe, current_user: CurrentUser -) -> UserOut: +) -> Any: """ Update own user. """ - current_user_data = jsonable_encoder(current_user) - user_in = UserUpdate(**current_user_data) - if body.password is not None: - user_in.password = body.password - if body is not None: - user_in.full_name = body.full_name - if body is not None: - user_in.email = body.email - user = crud.update(db=session, session_obj=current_user, obj_in=user_in) - return user + current_user.update(session, body) + return current_user @router.get("/me", response_model=UserOut) -def read_user_me(session: SessionDep, current_user: CurrentUser) -> UserOut: +def read_user_me(session: SessionDep, current_user: CurrentUser) -> Any: """ Get current user. """ @@ -96,7 +90,7 @@ def read_user_me(session: SessionDep, current_user: CurrentUser) -> UserOut: @router.post("/open", response_model=UserOut) -def create_user_open(session: SessionDep, user_in: UserCreateOpen) -> UserOut: +def create_user_open(session: SessionDep, user_in: UserCreateOpen) -> Any: """ Create new user without the need to be logged in. """ @@ -105,14 +99,18 @@ def create_user_open(session: SessionDep, user_in: UserCreateOpen) -> UserOut: status_code=403, detail="Open user registration is forbidden on this server", ) - user = crud.get_by_email(db=session, email=user_in.email) + user = User.first_by_field(session, "email", user_in.email) if user: raise HTTPException( status_code=400, detail="The user with this username already exists in the system", ) - user_create = UserCreate.model_validate(user_in) - user = crud.create(db=session, obj_in=user_create) + user = User.create(session, user_in) + if user is None: + raise HTTPException( + status_code=400, + detail="Failed to create user.", + ) return user @@ -121,19 +119,22 @@ def read_user_by_id( user_id: int, session: SessionDep, current_user: CurrentUser, -) -> UserOut: +) -> Any: """ Get a specific user by id. """ - user = session.get(User, user_id) - if user == current_user: - return user if not current_user.is_superuser: raise HTTPException( # TODO: Review status code status_code=400, detail="The user doesn't have enough privileges", ) + user = session.get(User, user_id) + if not user: + raise HTTPException( + status_code=404, + detail="The user with this username does not exist in the system", + ) return user @@ -147,7 +148,7 @@ def update_user( session: SessionDep, user_id: int, user_in: UserUpdate, -) -> UserOut: +) -> Any: """ Update a user. """ @@ -158,5 +159,5 @@ def update_user( status_code=404, detail="The user with this username does not exist in the system", ) - user = crud.update(db=session, db_obj=user, obj_in=user_in) + user.update(session, user_in) return user