From 26d7eb924b196bede57c045fa3d1b8b9924cc1e3 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Sun, 3 Nov 2024 14:18:27 -0500 Subject: [PATCH 01/16] add dependencies --- backend/pyproject.toml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 20833d9..9ccef99 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -33,6 +33,18 @@ precommit-install = "pre-commit install" revision = "alembic revision --autogenerate" upgrade = "alembic upgrade head" +[tool.pytest.ini_options] +asyncio_mode = "strict" +asyncio_default_fixture_loop_scope = "function" +pythonpath = ["."] + +[tool.pdm.dev-dependencies] +test = [ + "pytest>=7.0.0", + "pytest-asyncio>=0.24.0", + "pytest-mock>=3.10.0", +] + [tool.ruff] target-version = "py312" # Read more here https://docs.astral.sh/ruff/rules/ From c3e2868eee356dfe245b688ef849994e6fbc6ad0 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Sun, 3 Nov 2024 14:18:53 -0500 Subject: [PATCH 02/16] add pdyantic user models --- backend/app/schemas/user.py | 62 +++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 backend/app/schemas/user.py diff --git a/backend/app/schemas/user.py b/backend/app/schemas/user.py new file mode 100644 index 0000000..bd96e2d --- /dev/null +++ b/backend/app/schemas/user.py @@ -0,0 +1,62 @@ +from enum import Enum +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, EmailStr, Field, field_validator + + +class UserRole(str, Enum): + PARTICIPANT = "participant" + VOLUNTEER = "volunteer" + ADMIN = "admin" + + @classmethod + def to_role_id(cls, role: "UserRole") -> int: + role_map = {cls.PARTICIPANT: 1, cls.VOLUNTEER: 2, cls.ADMIN: 3} + return role_map[role] + + +class UserBase(BaseModel): + first_name: str = Field(..., min_length=1, max_length=50) + last_name: str = Field(..., min_length=1, max_length=50) + email: EmailStr + role: UserRole + + +class UserCreate(UserBase): + password: str = Field(..., min_length=8) + + @field_validator("password") + def password_complexity(cls, v): + if not any(char.isdigit() for char in v): + raise ValueError("Password must contain at least one digit") + if not any(char.isupper() for char in v): + raise ValueError("Password must contain at least one uppercase letter") + if not any(char.islower() for char in v): + raise ValueError("Password must contain at least one lowercase letter") + return v + + +class UserInDB(BaseModel): + id: UUID + first_name: str + last_name: str + email: EmailStr + role_id: int + auth_id: str + + model_config = ConfigDict(from_attributes=True) + + +class User(UserBase): + id: UUID + + model_config = ConfigDict(from_attributes=True) + + +class UserUpdate(BaseModel): + first_name: str | None = Field(None, min_length=1, max_length=50) + last_name: str | None = Field(None, min_length=1, max_length=50) + email: EmailStr | None = None + role: UserRole | None = None + + model_config = ConfigDict(from_attributes=True) From 06456a2975e6f4e49fa3078eb3a9c3fa0f613f3f Mon Sep 17 00:00:00 2001 From: mmiqball Date: Sun, 3 Nov 2024 14:19:07 -0500 Subject: [PATCH 03/16] add Firebase authentication initialization --- backend/app/server.py | 4 ++++ backend/app/utilities/firebase_init.py | 31 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 backend/app/utilities/firebase_init.py diff --git a/backend/app/server.py b/backend/app/server.py index a3830f0..10f6298 100644 --- a/backend/app/server.py +++ b/backend/app/server.py @@ -8,6 +8,8 @@ from app.routes import email from . import models +from .routes import user +from .utilities.firebase_init import initialize_firebase load_dotenv() @@ -18,6 +20,7 @@ async def lifespan(_: FastAPI): log.info("Starting up...") models.run_migrations() + initialize_firebase() yield log.info("Shutting down...") @@ -25,6 +28,7 @@ async def lifespan(_: FastAPI): # Source: https://stackoverflow.com/questions/77170361/ # running-alembic-migrations-on-fastapi-startup app = FastAPI(lifespan=lifespan) +app.include_router(user.router) app.include_router(email.router) diff --git a/backend/app/utilities/firebase_init.py b/backend/app/utilities/firebase_init.py new file mode 100644 index 0000000..f782b1d --- /dev/null +++ b/backend/app/utilities/firebase_init.py @@ -0,0 +1,31 @@ +import os + +import firebase_admin +from firebase_admin import credentials + + +def initialize_firebase(): + private_key = os.getenv("FIREBASE_SVC_ACCOUNT_PRIVATE_KEY") + if private_key: + private_key = private_key.replace("\\n", "\n") + + cred = credentials.Certificate( + { + "type": "service_account", + "project_id": os.getenv("FIREBASE_PROJECT_ID"), + "private_key_id": os.getenv("FIREBASE_SVC_ACCOUNT_PRIVATE_KEY_ID"), + "private_key": private_key, + "client_email": os.getenv("FIREBASE_SVC_ACCOUNT_CLIENT_EMAIL"), + "client_id": os.getenv("FIREBASE_SVC_ACCOUNT_CLIENT_ID"), + "auth_uri": os.getenv("FIREBASE_SVC_ACCOUNT_AUTH_URI"), + "token_uri": os.getenv("FIREBASE_SVC_ACCOUNT_TOKEN_URI"), + "auth_provider_x509_cert_url": os.getenv( + "FIREBASE_SVC_ACCOUNT_AUTH_PROVIDER_X509_CERT_URL" + ), + "client_x509_cert_url": os.getenv( + "FIREBASE_SVC_ACCOUNT_CLIENT_X509_CERT_URL" + ), + } + ) + + firebase_admin.initialize_app(cred) From f6975c19ef9c7190e29932ece5b9832c49834289 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Sun, 3 Nov 2024 14:19:24 -0500 Subject: [PATCH 04/16] implement user creation endpoint --- backend/app/interfaces/user_service.py | 2 +- backend/app/routes/user.py | 27 ++++++ .../services/implementations/user_service.py | 88 +++++++++++++++++++ backend/app/utilities/db_utils.py | 16 ++++ 4 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 backend/app/routes/user.py create mode 100644 backend/app/services/implementations/user_service.py create mode 100644 backend/app/utilities/db_utils.py diff --git a/backend/app/interfaces/user_service.py b/backend/app/interfaces/user_service.py index 46cdaec..2a22b5f 100644 --- a/backend/app/interfaces/user_service.py +++ b/backend/app/interfaces/user_service.py @@ -83,7 +83,7 @@ def get_users(self): pass @abstractmethod - def create_user(self, user, auth_id=None, signup_method="PASSWORD"): + def create_user(self, user, auth_id=None): """ Create a user, email verification configurable diff --git a/backend/app/routes/user.py b/backend/app/routes/user.py new file mode 100644 index 0000000..b12a4a4 --- /dev/null +++ b/backend/app/routes/user.py @@ -0,0 +1,27 @@ +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy.orm import Session + +from app.schemas.user import UserCreate, UserInDB +from app.services.implementations.user_service import UserService +from app.utilities.db_utils import get_db + +router = APIRouter( + prefix="/users", + tags=["users"], +) + +# to do: +# send email verification via auth_service +# allow signup methods other than email (like sign up w Google)?? + + +@router.post("/", response_model=UserInDB) +async def create_user(user: UserCreate, db: Session = Depends(get_db)): + user_service = UserService(db) + try: + created_user = await user_service.create_user(user) + return created_user + except HTTPException as http_ex: + raise http_ex + except Exception as e: + raise HTTPException(status_code=500, detail=str(e)) diff --git a/backend/app/services/implementations/user_service.py b/backend/app/services/implementations/user_service.py new file mode 100644 index 0000000..8d23e39 --- /dev/null +++ b/backend/app/services/implementations/user_service.py @@ -0,0 +1,88 @@ +import logging + +import firebase_admin.auth +from fastapi import HTTPException +from sqlalchemy.orm import Session + +from app.models import User +from app.schemas.user import UserCreate, UserInDB, UserRole +from app.services.interfaces.user_service import IUserService + + +class UserService(IUserService): + def __init__(self, db: Session): + self.db = db + self.logger = logging.getLogger(__name__) + + async def create_user(self, user: UserCreate) -> UserInDB: + firebase_user = None + try: + # Create user in Firebase + firebase_user = firebase_admin.auth.create_user( + email=user.email, password=user.password + ) + + role_id = UserRole.to_role_id(user.role) + + # create user in database + db_user = User( + first_name=user.first_name, + last_name=user.last_name, + email=user.email, + role_id=role_id, + auth_id=firebase_user.uid, + ) + + self.db.add(db_user) + self.db.commit() + self.db.refresh(db_user) + + return UserInDB.model_validate(db_user) + + except firebase_admin.exceptions.FirebaseError as firebase_error: + self.logger.error(f"Firebase error: {str(firebase_error)}") + if isinstance(firebase_error, firebase_admin.auth.EmailAlreadyExistsError): + raise HTTPException(status_code=409, detail="Email already exists") + else: + raise HTTPException(status_code=400, detail=str(firebase_error)) + except Exception as e: + if firebase_user: + try: + firebase_admin.auth.delete_user(firebase_user.uid) + except firebase_admin.auth.AuthError as firebase_error: + self.logger.error( + "Failed to delete Firebase user after database insertion failed. " + f"Firebase UID: {firebase_user.uid}. " + f"Error: {str(firebase_error)}" + ) + + self.db.rollback() + self.logger.error(f"Error creating user: {str(e)}") + raise HTTPException(status_code=500, detail=str(e)) + + def delete_user_by_email(self, email: str): + pass + + def delete_user_by_id(self, user_id: str): + pass + + def get_auth_id_by_user_id(self, user_id: str) -> str: + pass + + def get_user_by_email(self, email: str): + pass + + def get_user_by_id(self, user_id: str): + pass + + def get_user_id_by_auth_id(self, auth_id: str) -> str: + pass + + def get_user_role_by_auth_id(self, auth_id: str) -> str: + pass + + def get_users(self): + pass + + def update_user_by_id(self, user_id: str, user): + pass diff --git a/backend/app/utilities/db_utils.py b/backend/app/utilities/db_utils.py new file mode 100644 index 0000000..3a4a7fd --- /dev/null +++ b/backend/app/utilities/db_utils.py @@ -0,0 +1,16 @@ +import os + +from sqlalchemy import create_engine +from sqlalchemy.orm import Session, sessionmaker + +DATABASE_URL = os.getenv("POSTGRES_DATABASE_URL") +engine = create_engine(DATABASE_URL) +SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) + + +def get_db() -> Session: + db = SessionLocal() + try: + yield db + finally: + db.close() From ce34aa5fedbf131b901f9e8cb1176cccdd51d183 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Sun, 3 Nov 2024 14:19:59 -0500 Subject: [PATCH 05/16] add user creation unit test lint lint --- .../services/implementations/user_service.py | 2 +- backend/test.db | Bin 0 -> 20480 bytes backend/tests/unit/test_models.py | 25 ---- backend/tests/unit/test_user.py | 127 ++++++++++++++++++ 4 files changed, 128 insertions(+), 26 deletions(-) create mode 100644 backend/test.db delete mode 100644 backend/tests/unit/test_models.py create mode 100644 backend/tests/unit/test_user.py diff --git a/backend/app/services/implementations/user_service.py b/backend/app/services/implementations/user_service.py index 8d23e39..e47354c 100644 --- a/backend/app/services/implementations/user_service.py +++ b/backend/app/services/implementations/user_service.py @@ -51,7 +51,7 @@ async def create_user(self, user: UserCreate) -> UserInDB: firebase_admin.auth.delete_user(firebase_user.uid) except firebase_admin.auth.AuthError as firebase_error: self.logger.error( - "Failed to delete Firebase user after database insertion failed. " + "Failed to delete Firebase user after database insertion failed" f"Firebase UID: {firebase_user.uid}. " f"Error: {str(firebase_error)}" ) diff --git a/backend/test.db b/backend/test.db new file mode 100644 index 0000000000000000000000000000000000000000..39b503c61089193d268e3499a05761e2fa02d9b6 GIT binary patch literal 20480 zcmeI(&uZH+90zbY?$$Z5wJ=Jqf?jM`+B$AXE`?^9+Jz-<>)L~!j8ZGi;QXoWK-q0$ zx4p}rXb-VRm?hjm!bmQ=Ec-ghzgYU&l0P~rD7QELI2Uv@O~*W^4%sJ~M$RcEg!n~y zR+gP2o)ksKr$c-CvL^cE=<~x%nXpgD-uJz_{H)1G69gat0SG_<0uX=z1Rwwb2>gA4 zug8_$TCJviJInbn5%Wx>nL6oRk9B-T{m$DyqpFIURU?jQ5cJ+r_sXYk(C@#YRbv#V zSsqUKSkR9hulv5^ne7v6DJ9{XGQ^n2$>M&yvGRa)nuxIIyyyDtl6gxH=kxrNY%4oo zYPQ|jA zOVd$~M@`3{MApv5DoNE^tj&MPA>e=j1Rwwb2tWV=5P$##AOHafKwujLUTHcxobfb| z@8cPt#B=e;$FoEn-A~8zIxIGAh#}8JIFF;MWRZLG ze{O>h-T?v-fB*y_009U<00Izz00bcL{|OxG17p(<1*M8!MEOSq87pNS0t6rc0SG_< a0uX=z1Rwwb2tWV=|DeEYy>6^-2>1;aaLi!< literal 0 HcmV?d00001 diff --git a/backend/tests/unit/test_models.py b/backend/tests/unit/test_models.py deleted file mode 100644 index ba3bb91..0000000 --- a/backend/tests/unit/test_models.py +++ /dev/null @@ -1,25 +0,0 @@ -from app.models import db -from app.models.user import User - -""" -Sample python test. -For more information on pytest, visit: -https://docs.pytest.org/en/6.2.x/reference.html -""" - - -def test_create_user(): - user = { - "first_name": "Jane", - "last_name": "Doe", - "auth_id": "abc", - "role": "Admin", - } - - user = User(**user) - db.session.add(user) - db.session.commit() - assert user.first_name == "Jane" - assert user.last_name == "Doe" - assert user.auth_id == "abc" - assert user.role == "Admin" diff --git a/backend/tests/unit/test_user.py b/backend/tests/unit/test_user.py new file mode 100644 index 0000000..fb11c6a --- /dev/null +++ b/backend/tests/unit/test_user.py @@ -0,0 +1,127 @@ +import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from app.models import Role +from app.models.Base import Base +from app.models.User import User +from app.schemas.user import UserCreate, UserInDB, UserRole +from app.services.implementations.user_service import UserService + +# Test DB Configuration +SQLALCHEMY_DATABASE_URL = "sqlite:///./test.db" +engine = create_engine( + SQLALCHEMY_DATABASE_URL, connect_args={"check_same_thread": False} +) +TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) + + +class MockFirebaseUser: + """Mock Firebase user response""" + + def __init__(self): + self.uid = "test_firebase_uid" + self.email = "test@example.com" + + +class MockFirebaseError(Exception): + """Mock Firebase error""" + + pass + + +class MockAuthError(MockFirebaseError): + """Mock Firebase auth error""" + + def __init__(self, code, message): + self.code = code + self.message = message + super().__init__(f"{code}: {message}") + + +@pytest.fixture +def mock_firebase_auth(monkeypatch): + """Mock Firebase authentication service""" + + class MockAuth: + def create_user(self, email, password): + return MockFirebaseUser() + + def get_user_by_email(self, email): + return MockFirebaseUser() + + def delete_user(self, uid): + pass + + AuthError = MockAuthError + + mock_auth = MockAuth() + monkeypatch.setattr("firebase_admin.auth", mock_auth) + return mock_auth + + +@pytest.fixture(scope="function") +def db_session(): + """Provide a clean database session for each test""" + Base.metadata.create_all(bind=engine) + session = TestingSessionLocal() + + try: + # Clean up any existing data first + session.query(User).delete() + session.query(Role).delete() + session.commit() + + # Create test role + test_role = Role(id=1, name=UserRole.PARTICIPANT) + session.add(test_role) + session.commit() + + yield session + finally: + session.rollback() + session.close() + # Clean up + session = TestingSessionLocal() + session.query(User).delete() + session.query(Role).delete() + session.commit() + session.close() + Base.metadata.drop_all(bind=engine) + + +@pytest.mark.asyncio +async def test_create_user_service(mock_firebase_auth, db_session): + """Test user creation flow including Firebase auth and database storage""" + try: + # Arrange + user_service = UserService(db_session) + user_data = UserCreate( + first_name="Test", + last_name="User", + email="test@example.com", + password="TestPass@123", + role=UserRole.PARTICIPANT, + ) + + # Act + created_user = await user_service.create_user(user_data) + + # Assert response + assert isinstance(created_user, UserInDB) + assert created_user.first_name == "Test" + assert created_user.last_name == "User" + assert created_user.email == "test@example.com" + assert created_user.role_id == 1 + assert created_user.auth_id == "test_firebase_uid" + + # Assert database state + db_user = db_session.query(User).filter_by(email="test@example.com").first() + assert db_user is not None + assert db_user.auth_id == "test_firebase_uid" + assert db_user.role_id == 1 + + db_session.commit() # Commit successful test + except Exception: + db_session.rollback() # Rollback on error + raise From 751de859940c03f39e9ef95993e919ef69566f74 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Sun, 3 Nov 2024 17:05:33 -0500 Subject: [PATCH 06/16] separate user service initialization in user routes --- backend/app/routes/auth.py | 0 backend/app/routes/user.py | 9 +++++++-- backend/test.db | Bin 20480 -> 20480 bytes 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 backend/app/routes/auth.py diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/app/routes/user.py b/backend/app/routes/user.py index b12a4a4..2e5b854 100644 --- a/backend/app/routes/user.py +++ b/backend/app/routes/user.py @@ -15,9 +15,14 @@ # allow signup methods other than email (like sign up w Google)?? +def get_user_service(db: Session = Depends(get_db)): + return UserService(db) + + @router.post("/", response_model=UserInDB) -async def create_user(user: UserCreate, db: Session = Depends(get_db)): - user_service = UserService(db) +async def create_user( + user: UserCreate, user_service: UserService = Depends(get_user_service) +): try: created_user = await user_service.create_user(user) return created_user diff --git a/backend/test.db b/backend/test.db index 39b503c61089193d268e3499a05761e2fa02d9b6..7fdfb49ae010c1b0ced3fad95d6cebfa6cac5f1e 100644 GIT binary patch delta 109 zcmZozz}T>Wae}m93IhWJD-^Q;X^V+E#*8T&6TWae}m92m=EHD-^Q;X@iM6#*85w6TW# bO_P#R4UJPwQw&WFH(%H16d Date: Thu, 7 Nov 2024 21:19:20 -0500 Subject: [PATCH 07/16] simplify firebase initialization --- .gitignore | 2 +- backend/app/utilities/firebase_init.py | 24 +----------------------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/.gitignore b/.gitignore index c022680..416602c 100644 --- a/.gitignore +++ b/.gitignore @@ -4,7 +4,7 @@ **/venv **/__pycache__ **/*.log -**/firebaseServiceAccount.json +**/serviceAccountKey.json **/.DS_Store **/*.cache **/*.egg-info diff --git a/backend/app/utilities/firebase_init.py b/backend/app/utilities/firebase_init.py index f782b1d..3be40d1 100644 --- a/backend/app/utilities/firebase_init.py +++ b/backend/app/utilities/firebase_init.py @@ -5,27 +5,5 @@ def initialize_firebase(): - private_key = os.getenv("FIREBASE_SVC_ACCOUNT_PRIVATE_KEY") - if private_key: - private_key = private_key.replace("\\n", "\n") - - cred = credentials.Certificate( - { - "type": "service_account", - "project_id": os.getenv("FIREBASE_PROJECT_ID"), - "private_key_id": os.getenv("FIREBASE_SVC_ACCOUNT_PRIVATE_KEY_ID"), - "private_key": private_key, - "client_email": os.getenv("FIREBASE_SVC_ACCOUNT_CLIENT_EMAIL"), - "client_id": os.getenv("FIREBASE_SVC_ACCOUNT_CLIENT_ID"), - "auth_uri": os.getenv("FIREBASE_SVC_ACCOUNT_AUTH_URI"), - "token_uri": os.getenv("FIREBASE_SVC_ACCOUNT_TOKEN_URI"), - "auth_provider_x509_cert_url": os.getenv( - "FIREBASE_SVC_ACCOUNT_AUTH_PROVIDER_X509_CERT_URL" - ), - "client_x509_cert_url": os.getenv( - "FIREBASE_SVC_ACCOUNT_CLIENT_X509_CERT_URL" - ), - } - ) - + cred = credentials.Certificate("llsc/backend/app/utilities/serviceAccountKey.json") firebase_admin.initialize_app(cred) From 663260d150b5efcf1134e0df605ba09602b377a1 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Thu, 7 Nov 2024 21:20:06 -0500 Subject: [PATCH 08/16] load env before code executes --- backend/app/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/server.py b/backend/app/server.py index 10f6298..293d1c2 100644 --- a/backend/app/server.py +++ b/backend/app/server.py @@ -6,12 +6,12 @@ from fastapi import FastAPI from app.routes import email +load_dotenv() from . import models from .routes import user from .utilities.firebase_init import initialize_firebase -load_dotenv() log = logging.getLogger("uvicorn") From de58f13b2228596f442227d85522ad39fc1b7514 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Thu, 7 Nov 2024 21:38:29 -0500 Subject: [PATCH 09/16] construct firebase service account key path from pwd --- backend/app/utilities/firebase_init.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/app/utilities/firebase_init.py b/backend/app/utilities/firebase_init.py index 3be40d1..79ac2cf 100644 --- a/backend/app/utilities/firebase_init.py +++ b/backend/app/utilities/firebase_init.py @@ -5,5 +5,7 @@ def initialize_firebase(): - cred = credentials.Certificate("llsc/backend/app/utilities/serviceAccountKey.json") + current_dir = os.path.dirname(os.path.abspath(__file__)) + service_account_path = os.path.join(current_dir, "serviceAccountKey.json") + cred = credentials.Certificate(service_account_path) firebase_admin.initialize_app(cred) From 36edc431dc4d770007449d9f30b57faace6feeaf Mon Sep 17 00:00:00 2001 From: mmiqball Date: Thu, 14 Nov 2024 17:32:35 -0500 Subject: [PATCH 10/16] add google SSO signup functionality --- backend/app/interfaces/user_service.py | 2 +- backend/app/routes/user.py | 11 ++- backend/app/schemas/user.py | 76 ++++++++++++------- .../services/implementations/user_service.py | 51 +++++++++---- 4 files changed, 91 insertions(+), 49 deletions(-) diff --git a/backend/app/interfaces/user_service.py b/backend/app/interfaces/user_service.py index 2a22b5f..c1585e1 100644 --- a/backend/app/interfaces/user_service.py +++ b/backend/app/interfaces/user_service.py @@ -83,7 +83,7 @@ def get_users(self): pass @abstractmethod - def create_user(self, user, auth_id=None): + def create_user(self, user, signup_method="PASSWORD"): """ Create a user, email verification configurable diff --git a/backend/app/routes/user.py b/backend/app/routes/user.py index 2e5b854..0314341 100644 --- a/backend/app/routes/user.py +++ b/backend/app/routes/user.py @@ -1,7 +1,7 @@ from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.orm import Session -from app.schemas.user import UserCreate, UserInDB +from app.schemas.user import UserCreateRequest, UserCreateResponse from app.services.implementations.user_service import UserService from app.utilities.db_utils import get_db @@ -10,7 +10,7 @@ tags=["users"], ) -# to do: +# TODO: # send email verification via auth_service # allow signup methods other than email (like sign up w Google)?? @@ -19,13 +19,12 @@ def get_user_service(db: Session = Depends(get_db)): return UserService(db) -@router.post("/", response_model=UserInDB) +@router.post("/", response_model=UserCreateResponse) async def create_user( - user: UserCreate, user_service: UserService = Depends(get_user_service) + user: UserCreateRequest, user_service: UserService = Depends(get_user_service) ): try: - created_user = await user_service.create_user(user) - return created_user + return await user_service.create_user(user) except HTTPException as http_ex: raise http_ex except Exception as e: diff --git a/backend/app/schemas/user.py b/backend/app/schemas/user.py index bd96e2d..fe19a52 100644 --- a/backend/app/schemas/user.py +++ b/backend/app/schemas/user.py @@ -1,10 +1,26 @@ +""" +Pydantic schemas for user-related data validation and serialization. +Handles user CRUD and response models for the API. +""" from enum import Enum from uuid import UUID +from typing import Optional from pydantic import BaseModel, ConfigDict, EmailStr, Field, field_validator +# TODO: +# confirm complexity rules for fields (such as password) + +class SignUpMethod(str, Enum): + """Authentication methods supported for user signup""" + PASSWORD = "PASSWORD" + GOOGLE = "GOOGLE" + class UserRole(str, Enum): + """ + Enum for possible user roles. + """ PARTICIPANT = "participant" VOLUNTEER = "volunteer" ADMIN = "admin" @@ -16,27 +32,45 @@ def to_role_id(cls, role: "UserRole") -> int: class UserBase(BaseModel): + """ + Base schema for user model with common attributes shared across schemas. + """ first_name: str = Field(..., min_length=1, max_length=50) last_name: str = Field(..., min_length=1, max_length=50) email: EmailStr role: UserRole -class UserCreate(UserBase): - password: str = Field(..., min_length=8) +class UserCreateRequest(UserBase): + """ + Request schema for user creation with conditional password validation + """ + password: Optional[str] = Field(None, min_length=8) + auth_id: Optional[str] = Field(None) + signup_method: SignUpMethod = Field(default=SignUpMethod.PASSWORD) @field_validator("password") - def password_complexity(cls, v): - if not any(char.isdigit() for char in v): - raise ValueError("Password must contain at least one digit") - if not any(char.isupper() for char in v): - raise ValueError("Password must contain at least one uppercase letter") - if not any(char.islower() for char in v): - raise ValueError("Password must contain at least one lowercase letter") - return v - - -class UserInDB(BaseModel): + def validate_password(cls, password: Optional[str], info): + signup_method = info.data.get('signup_method') + + if signup_method == SignUpMethod.PASSWORD and not password: + raise ValueError("Password is required for password signup") + + if password: + if not any(char.isdigit() for char in password): + raise ValueError("Password must contain at least one digit") + if not any(char.isupper() for char in password): + raise ValueError("Password must contain at least one uppercase letter") + if not any(char.islower() for char in password): + raise ValueError("Password must contain at least one lowercase letter") + + return password + + +class UserCreateResponse(BaseModel): + """ + Response schema for user creation, maps directly from ORM User object. + """ id: UUID first_name: str last_name: str @@ -44,19 +78,5 @@ class UserInDB(BaseModel): role_id: int auth_id: str - model_config = ConfigDict(from_attributes=True) - - -class User(UserBase): - id: UUID - - model_config = ConfigDict(from_attributes=True) - - -class UserUpdate(BaseModel): - first_name: str | None = Field(None, min_length=1, max_length=50) - last_name: str | None = Field(None, min_length=1, max_length=50) - email: EmailStr | None = None - role: UserRole | None = None - + # from_attributes enables automatic mapping from SQLAlchemy model to Pydantic model model_config = ConfigDict(from_attributes=True) diff --git a/backend/app/services/implementations/user_service.py b/backend/app/services/implementations/user_service.py index e47354c..994688b 100644 --- a/backend/app/services/implementations/user_service.py +++ b/backend/app/services/implementations/user_service.py @@ -1,11 +1,12 @@ import logging +from typing import Optional import firebase_admin.auth from fastapi import HTTPException from sqlalchemy.orm import Session from app.models import User -from app.schemas.user import UserCreate, UserInDB, UserRole +from app.schemas.user import SignUpMethod, UserCreateRequest, UserCreateResponse, UserRole from app.services.interfaces.user_service import IUserService @@ -14,17 +15,24 @@ def __init__(self, db: Session): self.db = db self.logger = logging.getLogger(__name__) - async def create_user(self, user: UserCreate) -> UserInDB: + async def create_user( + self, + user: UserCreateRequest + ) -> UserCreateResponse: firebase_user = None try: - # Create user in Firebase - firebase_user = firebase_admin.auth.create_user( - email=user.email, password=user.password - ) + if user.signup_method == SignUpMethod.PASSWORD: + firebase_user = firebase_admin.auth.create_user( + email=user.email, + password=user.password + ) + elif user.signup_method == SignUpMethod.GOOGLE: + # For signup with Google, Firebase users are automatically created + firebase_user = firebase_admin.auth.get_user(user.auth_id) role_id = UserRole.to_role_id(user.role) - # create user in database + # Create user in database db_user = User( first_name=user.first_name, last_name=user.last_name, @@ -34,31 +42,46 @@ async def create_user(self, user: UserCreate) -> UserInDB: ) self.db.add(db_user) + # Finish database transaction and run previously defined + # database operations (ie. db.add) self.db.commit() - self.db.refresh(db_user) - return UserInDB.model_validate(db_user) + return UserCreateResponse.model_validate(db_user) except firebase_admin.exceptions.FirebaseError as firebase_error: self.logger.error(f"Firebase error: {str(firebase_error)}") + if isinstance(firebase_error, firebase_admin.auth.EmailAlreadyExistsError): - raise HTTPException(status_code=409, detail="Email already exists") - else: - raise HTTPException(status_code=400, detail=str(firebase_error)) + raise HTTPException( + status_code=409, + detail="Email already exists" + ) + + raise HTTPException( + status_code=400, + detail=str(firebase_error) + ) + except Exception as e: + # Clean up Firebase user if a database exception occurs if firebase_user: try: firebase_admin.auth.delete_user(firebase_user.uid) except firebase_admin.auth.AuthError as firebase_error: self.logger.error( - "Failed to delete Firebase user after database insertion failed" + "Failed to delete Firebase user after database insertion failed. " f"Firebase UID: {firebase_user.uid}. " f"Error: {str(firebase_error)}" ) + # Rollback database changes self.db.rollback() self.logger.error(f"Error creating user: {str(e)}") - raise HTTPException(status_code=500, detail=str(e)) + + raise HTTPException( + status_code=500, + detail=str(e) + ) def delete_user_by_email(self, email: str): pass From 6a2851bad497b5ecb9fba910cb0b6c878e3d26c7 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Thu, 14 Nov 2024 17:35:03 -0500 Subject: [PATCH 11/16] minor pr comments fixes --- backend/app/utilities/db_utils.py | 6 ++- backend/app/utilities/firebase_init.py | 4 +- backend/test.db | Bin 20480 -> 20480 bytes backend/tests/unit/test_user.py | 52 ++++++++++++++++++++++--- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/backend/app/utilities/db_utils.py b/backend/app/utilities/db_utils.py index 3a4a7fd..78e8c3b 100644 --- a/backend/app/utilities/db_utils.py +++ b/backend/app/utilities/db_utils.py @@ -1,5 +1,8 @@ +from dotenv import load_dotenv import os +load_dotenv() + from sqlalchemy import create_engine from sqlalchemy.orm import Session, sessionmaker @@ -7,7 +10,8 @@ engine = create_engine(DATABASE_URL) SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) - +# explanation for using yield to get local db session: +# https://stackoverflow.com/questions/64763770/why-we-use-yield-to-get-sessionlocal-in-fastapi-with-sqlalchemy def get_db() -> Session: db = SessionLocal() try: diff --git a/backend/app/utilities/firebase_init.py b/backend/app/utilities/firebase_init.py index 79ac2cf..7ebc809 100644 --- a/backend/app/utilities/firebase_init.py +++ b/backend/app/utilities/firebase_init.py @@ -5,7 +5,7 @@ def initialize_firebase(): - current_dir = os.path.dirname(os.path.abspath(__file__)) - service_account_path = os.path.join(current_dir, "serviceAccountKey.json") + cwd = os.getcwd() + service_account_path = os.path.join(cwd, "serviceAccountKey.json") cred = credentials.Certificate(service_account_path) firebase_admin.initialize_app(cred) diff --git a/backend/test.db b/backend/test.db index 7fdfb49ae010c1b0ced3fad95d6cebfa6cac5f1e..05a9550e23adf89b25147dd5a3a3aeee796f57f4 100644 GIT binary patch delta 165 zcmZozz}T>Wae}nqW(EcZRw!lx(g_oFj2Sm?Ojs$;xN5VYzyf~D5Jq-iNl`;i{lp|A z6GLM&Q-dS}bCX2##H2(+i_}zeV?*NWae}m93IhWJD-^Q;X^V+E#*8T&6IRMIuG=gqu!P? Date: Thu, 14 Nov 2024 19:32:41 -0500 Subject: [PATCH 12/16] remove unusued test, add todo comment --- .../services/implementations/user_service.py | 3 + backend/test.db | Bin 20480 -> 20480 bytes backend/tests/unit/test_csv.py | 164 ------------------ backend/tests/unit/test_user.py | 9 +- 4 files changed, 5 insertions(+), 171 deletions(-) delete mode 100644 backend/tests/unit/test_csv.py diff --git a/backend/app/services/implementations/user_service.py b/backend/app/services/implementations/user_service.py index 994688b..f02a70a 100644 --- a/backend/app/services/implementations/user_service.py +++ b/backend/app/services/implementations/user_service.py @@ -26,6 +26,9 @@ async def create_user( email=user.email, password=user.password ) + ## TO DO: SSO functionality depends a lot on frontend implementation, + ## so we may need to update this when we have a better idea of what + ## that looks like elif user.signup_method == SignUpMethod.GOOGLE: # For signup with Google, Firebase users are automatically created firebase_user = firebase_admin.auth.get_user(user.auth_id) diff --git a/backend/test.db b/backend/test.db index 05a9550e23adf89b25147dd5a3a3aeee796f57f4..b75c4f9195860294e647483b1d89ecad57a9fd2f 100644 GIT binary patch delta 109 zcmZozz}T>Wae}nqB?bltRw!lx(ghQBj2SO&ObFIju(Yr+H#9R!N=;5QHc3l1v@}mm bGc!*%Gfy-!NHI@M-h5e~SAd|ZMGgW0#RnX( delta 109 zcmZozz}T>Wae}nqW(EcZRw!lx(g_oFj2Sm?ObFIjNK7&^F*G(aHApfrH%T;4OiDDg bNKG|2HZ)E#GO Date: Thu, 14 Nov 2024 19:49:19 -0500 Subject: [PATCH 13/16] lint --- backend/app/schemas/user.py | 17 ++++++--- backend/app/server.py | 9 ++--- .../services/implementations/user_service.py | 35 +++++++------------ backend/app/utilities/db_utils.py | 7 ++-- backend/tests/unit/test_user.py | 14 +++++--- 5 files changed, 44 insertions(+), 38 deletions(-) diff --git a/backend/app/schemas/user.py b/backend/app/schemas/user.py index fe19a52..b2c0719 100644 --- a/backend/app/schemas/user.py +++ b/backend/app/schemas/user.py @@ -2,17 +2,20 @@ Pydantic schemas for user-related data validation and serialization. Handles user CRUD and response models for the API. """ + from enum import Enum -from uuid import UUID from typing import Optional +from uuid import UUID from pydantic import BaseModel, ConfigDict, EmailStr, Field, field_validator # TODO: # confirm complexity rules for fields (such as password) + class SignUpMethod(str, Enum): """Authentication methods supported for user signup""" + PASSWORD = "PASSWORD" GOOGLE = "GOOGLE" @@ -21,6 +24,7 @@ class UserRole(str, Enum): """ Enum for possible user roles. """ + PARTICIPANT = "participant" VOLUNTEER = "volunteer" ADMIN = "admin" @@ -35,6 +39,7 @@ class UserBase(BaseModel): """ Base schema for user model with common attributes shared across schemas. """ + first_name: str = Field(..., min_length=1, max_length=50) last_name: str = Field(..., min_length=1, max_length=50) email: EmailStr @@ -45,17 +50,18 @@ class UserCreateRequest(UserBase): """ Request schema for user creation with conditional password validation """ + password: Optional[str] = Field(None, min_length=8) auth_id: Optional[str] = Field(None) signup_method: SignUpMethod = Field(default=SignUpMethod.PASSWORD) @field_validator("password") def validate_password(cls, password: Optional[str], info): - signup_method = info.data.get('signup_method') - + signup_method = info.data.get("signup_method") + if signup_method == SignUpMethod.PASSWORD and not password: raise ValueError("Password is required for password signup") - + if password: if not any(char.isdigit() for char in password): raise ValueError("Password must contain at least one digit") @@ -63,7 +69,7 @@ def validate_password(cls, password: Optional[str], info): raise ValueError("Password must contain at least one uppercase letter") if not any(char.islower() for char in password): raise ValueError("Password must contain at least one lowercase letter") - + return password @@ -71,6 +77,7 @@ class UserCreateResponse(BaseModel): """ Response schema for user creation, maps directly from ORM User object. """ + id: UUID first_name: str last_name: str diff --git a/backend/app/server.py b/backend/app/server.py index 293d1c2..9405741 100644 --- a/backend/app/server.py +++ b/backend/app/server.py @@ -6,12 +6,13 @@ from fastapi import FastAPI from app.routes import email -load_dotenv() -from . import models -from .routes import user -from .utilities.firebase_init import initialize_firebase +load_dotenv() +# we need to load env variables before initialization code runs +from . import models # noqa: E402 +from .routes import user # noqa: E402 +from .utilities.firebase_init import initialize_firebase # noqa: E402 log = logging.getLogger("uvicorn") diff --git a/backend/app/services/implementations/user_service.py b/backend/app/services/implementations/user_service.py index f02a70a..e95ca83 100644 --- a/backend/app/services/implementations/user_service.py +++ b/backend/app/services/implementations/user_service.py @@ -1,12 +1,16 @@ import logging -from typing import Optional import firebase_admin.auth from fastapi import HTTPException from sqlalchemy.orm import Session from app.models import User -from app.schemas.user import SignUpMethod, UserCreateRequest, UserCreateResponse, UserRole +from app.schemas.user import ( + SignUpMethod, + UserCreateRequest, + UserCreateResponse, + UserRole, +) from app.services.interfaces.user_service import IUserService @@ -15,16 +19,12 @@ def __init__(self, db: Session): self.db = db self.logger = logging.getLogger(__name__) - async def create_user( - self, - user: UserCreateRequest - ) -> UserCreateResponse: + async def create_user(self, user: UserCreateRequest) -> UserCreateResponse: firebase_user = None try: if user.signup_method == SignUpMethod.PASSWORD: firebase_user = firebase_admin.auth.create_user( - email=user.email, - password=user.password + email=user.email, password=user.password ) ## TO DO: SSO functionality depends a lot on frontend implementation, ## so we may need to update this when we have a better idea of what @@ -55,15 +55,9 @@ async def create_user( self.logger.error(f"Firebase error: {str(firebase_error)}") if isinstance(firebase_error, firebase_admin.auth.EmailAlreadyExistsError): - raise HTTPException( - status_code=409, - detail="Email already exists" - ) + raise HTTPException(status_code=409, detail="Email already exists") - raise HTTPException( - status_code=400, - detail=str(firebase_error) - ) + raise HTTPException(status_code=400, detail=str(firebase_error)) except Exception as e: # Clean up Firebase user if a database exception occurs @@ -72,7 +66,7 @@ async def create_user( firebase_admin.auth.delete_user(firebase_user.uid) except firebase_admin.auth.AuthError as firebase_error: self.logger.error( - "Failed to delete Firebase user after database insertion failed. " + "Failed to delete Firebase user after database insertion failed" f"Firebase UID: {firebase_user.uid}. " f"Error: {str(firebase_error)}" ) @@ -80,11 +74,8 @@ async def create_user( # Rollback database changes self.db.rollback() self.logger.error(f"Error creating user: {str(e)}") - - raise HTTPException( - status_code=500, - detail=str(e) - ) + + raise HTTPException(status_code=500, detail=str(e)) def delete_user_by_email(self, email: str): pass diff --git a/backend/app/utilities/db_utils.py b/backend/app/utilities/db_utils.py index 78e8c3b..986bdbc 100644 --- a/backend/app/utilities/db_utils.py +++ b/backend/app/utilities/db_utils.py @@ -1,15 +1,16 @@ -from dotenv import load_dotenv import os -load_dotenv() - +from dotenv import load_dotenv from sqlalchemy import create_engine from sqlalchemy.orm import Session, sessionmaker +load_dotenv() + DATABASE_URL = os.getenv("POSTGRES_DATABASE_URL") engine = create_engine(DATABASE_URL) SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) + # explanation for using yield to get local db session: # https://stackoverflow.com/questions/64763770/why-we-use-yield-to-get-sessionlocal-in-fastapi-with-sqlalchemy def get_db() -> Session: diff --git a/backend/tests/unit/test_user.py b/backend/tests/unit/test_user.py index 57a5850..aa082be 100644 --- a/backend/tests/unit/test_user.py +++ b/backend/tests/unit/test_user.py @@ -5,7 +5,12 @@ from app.models import Role from app.models.Base import Base from app.models.User import User -from app.schemas.user import SignUpMethod, UserCreateRequest, UserCreateResponse, UserRole +from app.schemas.user import ( + SignUpMethod, + UserCreateRequest, + UserCreateResponse, + UserRole, +) from app.services.implementations.user_service import UserService # Test DB Configuration @@ -102,7 +107,7 @@ async def test_create_user_service(mock_firebase_auth, db_session): email="test@example.com", password="TestPass@123", role=UserRole.PARTICIPANT, - signup_method=SignUpMethod.PASSWORD + signup_method=SignUpMethod.PASSWORD, ) # Act @@ -127,6 +132,7 @@ async def test_create_user_service(mock_firebase_auth, db_session): db_session.rollback() # Rollback on error raise + @pytest.mark.asyncio async def test_create_user_with_google(mock_firebase_auth, db_session): """Test user creation flow with Google authentication""" @@ -138,7 +144,7 @@ async def test_create_user_with_google(mock_firebase_auth, db_session): last_name="User", email="google@example.com", role=UserRole.PARTICIPANT, - signup_method=SignUpMethod.GOOGLE + signup_method=SignUpMethod.GOOGLE, ) # Act @@ -161,4 +167,4 @@ async def test_create_user_with_google(mock_firebase_auth, db_session): db_session.commit() # Commit successful test except Exception: db_session.rollback() # Rollback on error - raise \ No newline at end of file + raise From 70c1e174620684d42cf2485f68260d0007d591d2 Mon Sep 17 00:00:00 2001 From: mmiqball Date: Mon, 18 Nov 2024 19:44:00 -0500 Subject: [PATCH 14/16] fix incorrect import path --- backend/app/services/implementations/user_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/services/implementations/user_service.py b/backend/app/services/implementations/user_service.py index e95ca83..d3081ce 100644 --- a/backend/app/services/implementations/user_service.py +++ b/backend/app/services/implementations/user_service.py @@ -4,6 +4,7 @@ from fastapi import HTTPException from sqlalchemy.orm import Session +from app.interfaces.user_service import IUserService from app.models import User from app.schemas.user import ( SignUpMethod, @@ -11,7 +12,6 @@ UserCreateResponse, UserRole, ) -from app.services.interfaces.user_service import IUserService class UserService(IUserService): From 19011f25f197892d9aa919fb5757be46e8e2b918 Mon Sep 17 00:00:00 2001 From: Matthew Wang <53355975+mslwang@users.noreply.github.com> Date: Tue, 19 Nov 2024 03:01:03 +0100 Subject: [PATCH 15/16] Update backend/app/schemas/user.py --- backend/app/schemas/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/schemas/user.py b/backend/app/schemas/user.py index b2c0719..5c47fd4 100644 --- a/backend/app/schemas/user.py +++ b/backend/app/schemas/user.py @@ -52,7 +52,7 @@ class UserCreateRequest(UserBase): """ password: Optional[str] = Field(None, min_length=8) - auth_id: Optional[str] = Field(None) + auth_id: Optional[str] = Field(None) # for signup with google sso signup_method: SignUpMethod = Field(default=SignUpMethod.PASSWORD) @field_validator("password") From 81b7fbc788c5cbf971d86d9a8ce6d0bdca8e0a5d Mon Sep 17 00:00:00 2001 From: Matthew Wang Date: Mon, 18 Nov 2024 21:04:20 -0500 Subject: [PATCH 16/16] lint --- backend/app/schemas/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/schemas/user.py b/backend/app/schemas/user.py index 5c47fd4..5d4f360 100644 --- a/backend/app/schemas/user.py +++ b/backend/app/schemas/user.py @@ -52,7 +52,7 @@ class UserCreateRequest(UserBase): """ password: Optional[str] = Field(None, min_length=8) - auth_id: Optional[str] = Field(None) # for signup with google sso + auth_id: Optional[str] = Field(None) # for signup with google sso signup_method: SignUpMethod = Field(default=SignUpMethod.PASSWORD) @field_validator("password")