diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 5bef1169aa7..74103ae6134 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -17,7 +17,7 @@ ) -def get_db() -> Generator: +def get_db() -> Generator[Session, None, None]: with Session(engine) as session: yield session diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py index 19865908fc7..661525e3fc4 100644 --- a/backend/app/api/routes/users.py +++ b/backend/app/api/routes/users.py @@ -1,7 +1,7 @@ from typing import Any from fastapi import APIRouter, Depends, HTTPException -from sqlmodel import delete, func, select +from sqlmodel import col, delete, func, select from app import crud from app.api.deps import ( @@ -189,16 +189,17 @@ def delete_user( user = session.get(User, user_id) if not user: raise HTTPException(status_code=404, detail="User not found") - - if (user == current_user and not current_user.is_superuser) or ( - user != current_user and current_user.is_superuser - ): - statement = delete(Item).where(Item.owner_id == user_id) - session.exec(statement) - session.delete(user) - session.commit() - return Message(message="User deleted successfully") + elif user != current_user and not current_user.is_superuser: + raise HTTPException( + status_code=403, detail="The user doesn't have enough privileges" + ) elif user == current_user and current_user.is_superuser: raise HTTPException( status_code=403, detail="Super users are not allowed to delete themselves" ) + + statement = delete(Item).where(col(Item.owner_id) == user_id) + session.exec(statement) # type: ignore + session.delete(user) + session.commit() + return Message(message="User deleted successfully") diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 8aeb7eef2a7..a9803c048c4 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -68,7 +68,7 @@ def assemble_db_connection(cls, v: str | None, info: ValidationInfo) -> Any: @field_validator("EMAILS_FROM_NAME") def get_project_name(cls, v: str | None, info: ValidationInfo) -> str: if not v: - return info.data["PROJECT_NAME"] + return str(info.data["PROJECT_NAME"]) return v EMAIL_RESET_TOKEN_EXPIRE_HOURS: int = 48 @@ -89,7 +89,7 @@ def get_emails_enabled(cls, v: bool, info: ValidationInfo) -> bool: FIRST_SUPERUSER: str FIRST_SUPERUSER_PASSWORD: str USERS_OPEN_REGISTRATION: bool = False - model_config = SettingsConfigDict(case_sensitive=True) + model_config = SettingsConfigDict(env_file=".env") -settings = Settings() +settings = Settings() # type: ignore diff --git a/backend/app/core/security.py b/backend/app/core/security.py index c9e5085f087..217bb8d43e4 100644 --- a/backend/app/core/security.py +++ b/backend/app/core/security.py @@ -12,13 +12,8 @@ ALGORITHM = "HS256" -def create_access_token(subject: str | Any, expires_delta: timedelta = None) -> str: - if expires_delta: - expire = datetime.utcnow() + expires_delta - else: - expire = datetime.utcnow() + timedelta( - minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES - ) +def create_access_token(subject: str | Any, expires_delta: timedelta) -> str: + expire = datetime.utcnow() + expires_delta to_encode = {"exp": expire, "sub": str(subject)} encoded_jwt = jwt.encode(to_encode, settings.SECRET_KEY, algorithm=ALGORITHM) return encoded_jwt diff --git a/backend/app/main.py b/backend/app/main.py index e68508b2251..849b545ef23 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -6,7 +6,7 @@ from app.core.config import settings -def custom_generate_unique_id(route: APIRoute): +def custom_generate_unique_id(route: APIRoute) -> str: return f"{route.tags[0]}-{route.name}" diff --git a/backend/app/models.py b/backend/app/models.py index 10091303dc7..6b506502d38 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -25,7 +25,7 @@ class UserCreateOpen(SQLModel): # Properties to receive via API on update, all are optional # TODO replace email str with EmailStr when sqlmodel supports it class UserUpdate(UserBase): - email: str | None = None + email: str | None = None # type: ignore password: str | None = None @@ -70,7 +70,7 @@ class ItemCreate(ItemBase): # Properties to receive on item update class ItemUpdate(ItemBase): - title: str | None = None + title: str | None = None # type: ignore # Database model, database table inferred from class name diff --git a/backend/app/tests/api/api_v1/test_items.py b/backend/app/tests/api/api_v1/test_items.py index 20eca1e575b..56cfa95c4f7 100644 --- a/backend/app/tests/api/api_v1/test_items.py +++ b/backend/app/tests/api/api_v1/test_items.py @@ -6,7 +6,7 @@ def test_create_item( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: data = {"title": "Foo", "description": "Fighters"} response = client.post( @@ -23,7 +23,7 @@ def test_create_item( def test_read_item( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.get( @@ -39,7 +39,7 @@ def test_read_item( def test_read_item_not_found( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: response = client.get( f"{settings.API_V1_STR}/items/999", @@ -51,7 +51,7 @@ def test_read_item_not_found( def test_read_item_not_enough_permissions( - client: TestClient, normal_user_token_headers: dict, db: Session + client: TestClient, normal_user_token_headers: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.get( @@ -64,7 +64,7 @@ def test_read_item_not_enough_permissions( def test_read_items( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: create_random_item(db) create_random_item(db) @@ -78,7 +78,7 @@ def test_read_items( def test_update_item( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: item = create_random_item(db) data = {"title": "Updated title", "description": "Updated description"} @@ -96,7 +96,7 @@ def test_update_item( def test_update_item_not_found( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: data = {"title": "Updated title", "description": "Updated description"} response = client.put( @@ -110,7 +110,7 @@ def test_update_item_not_found( def test_update_item_not_enough_permissions( - client: TestClient, normal_user_token_headers: dict, db: Session + client: TestClient, normal_user_token_headers: dict[str, str], db: Session ) -> None: item = create_random_item(db) data = {"title": "Updated title", "description": "Updated description"} @@ -125,7 +125,7 @@ def test_update_item_not_enough_permissions( def test_delete_item( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.delete( @@ -138,7 +138,7 @@ def test_delete_item( def test_delete_item_not_found( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: response = client.delete( f"{settings.API_V1_STR}/items/999", @@ -150,7 +150,7 @@ def test_delete_item_not_found( def test_delete_item_not_enough_permissions( - client: TestClient, normal_user_token_headers: dict, db: Session + client: TestClient, normal_user_token_headers: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.delete( diff --git a/backend/app/tests/api/api_v1/test_login.py b/backend/app/tests/api/api_v1/test_login.py index 6102c68d61c..7b8231dc31a 100644 --- a/backend/app/tests/api/api_v1/test_login.py +++ b/backend/app/tests/api/api_v1/test_login.py @@ -1,7 +1,8 @@ -from app.utils import generate_password_reset_token from fastapi.testclient import TestClient +from pytest_mock import MockerFixture from app.core.config import settings +from app.utils import generate_password_reset_token def test_get_access_token(client: TestClient) -> None: @@ -38,7 +39,7 @@ def test_use_access_token( def test_recovery_password( - client: TestClient, normal_user_token_headers: dict[str, str], mocker + client: TestClient, normal_user_token_headers: dict[str, str], mocker: MockerFixture ) -> None: mocker.patch("app.utils.send_email", return_value=None) mocker.patch("app.core.config.settings.EMAILS_ENABLED", True) diff --git a/backend/app/tests/api/api_v1/test_users.py b/backend/app/tests/api/api_v1/test_users.py index ae2138e578e..b09a2b37b7b 100644 --- a/backend/app/tests/api/api_v1/test_users.py +++ b/backend/app/tests/api/api_v1/test_users.py @@ -1,4 +1,5 @@ from fastapi.testclient import TestClient +from pytest_mock import MockerFixture from sqlmodel import Session from app import crud @@ -30,7 +31,10 @@ def test_get_users_normal_user_me( def test_create_user_new_email( - client: TestClient, superuser_token_headers: dict, db: Session, mocker + client: TestClient, + superuser_token_headers: dict[str, str], + db: Session, + mocker: MockerFixture, ) -> None: mocker.patch("app.utils.send_email") mocker.patch("app.core.config.settings.EMAILS_ENABLED", True) @@ -50,7 +54,7 @@ def test_create_user_new_email( def test_get_existing_user( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -107,7 +111,7 @@ def test_get_existing_user_permissions_error( def test_create_user_existing_username( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: username = random_email() # username = email @@ -140,7 +144,7 @@ def test_create_user_by_normal_user( def test_retrieve_users( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -179,7 +183,7 @@ def test_update_user_me( def test_update_password_me( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: new_password = random_lower_string() data = { @@ -209,7 +213,7 @@ def test_update_password_me( def test_update_password_me_incorrect_password( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: new_password = random_lower_string() data = {"current_password": new_password, "new_password": new_password} @@ -224,7 +228,7 @@ def test_update_password_me_incorrect_password( def test_update_password_me_same_password_error( - client: TestClient, superuser_token_headers: dict, db: Session + client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: data = { "current_password": settings.FIRST_SUPERUSER_PASSWORD, @@ -242,7 +246,7 @@ def test_update_password_me_same_password_error( ) -def test_create_user_open(client: TestClient, mocker) -> None: +def test_create_user_open(client: TestClient, mocker: MockerFixture) -> None: mocker.patch("app.core.config.settings.USERS_OPEN_REGISTRATION", True) username = random_email() password = random_lower_string() @@ -258,7 +262,9 @@ def test_create_user_open(client: TestClient, mocker) -> None: assert created_user["full_name"] == full_name -def test_create_user_open_forbidden_error(client: TestClient, mocker) -> None: +def test_create_user_open_forbidden_error( + client: TestClient, mocker: MockerFixture +) -> None: mocker.patch("app.core.config.settings.USERS_OPEN_REGISTRATION", False) username = random_email() password = random_lower_string() @@ -272,7 +278,9 @@ def test_create_user_open_forbidden_error(client: TestClient, mocker) -> None: assert r.json()["detail"] == "Open user registration is forbidden on this server" -def test_create_user_open_already_exists_error(client: TestClient, mocker) -> None: +def test_create_user_open_already_exists_error( + client: TestClient, mocker: MockerFixture +) -> None: mocker.patch("app.core.config.settings.USERS_OPEN_REGISTRATION", True) password = random_lower_string() full_name = random_lower_string() @@ -382,6 +390,7 @@ def test_delete_user_current_super_user_error( client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: super_user = crud.get_user_by_email(session=db, email=settings.FIRST_SUPERUSER) + assert super_user user_id = super_user.id r = client.delete( diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index 502a45049a3..90ab39a357f 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -13,7 +13,7 @@ @pytest.fixture(scope="session", autouse=True) -def db() -> Generator: +def db() -> Generator[Session, None, None]: with Session(engine) as session: init_db(session) yield session @@ -25,7 +25,7 @@ def db() -> Generator: @pytest.fixture(scope="module") -def client() -> Generator: +def client() -> Generator[TestClient, None, None]: with TestClient(app) as c: yield c diff --git a/backend/app/tests/scripts/test_backend_pre_start.py b/backend/app/tests/scripts/test_backend_pre_start.py index 29173f996e7..4d9ec388008 100644 --- a/backend/app/tests/scripts/test_backend_pre_start.py +++ b/backend/app/tests/scripts/test_backend_pre_start.py @@ -1,21 +1,22 @@ from unittest.mock import MagicMock +from pytest_mock import MockerFixture from sqlmodel import select from app.backend_pre_start import init, logger -def test_init_successful_connection(mocker): +def test_init_successful_connection(mocker: MockerFixture) -> None: engine_mock = MagicMock() session_mock = MagicMock() exec_mock = MagicMock(return_value=True) - session_mock.configure_mock(**{'exec.return_value': exec_mock}) - mocker.patch('sqlmodel.Session', return_value=session_mock) + session_mock.configure_mock(**{"exec.return_value": exec_mock}) + mocker.patch("sqlmodel.Session", return_value=session_mock) - mocker.patch.object(logger, 'info') - mocker.patch.object(logger, 'error') - mocker.patch.object(logger, 'warn') + mocker.patch.object(logger, "info") + mocker.patch.object(logger, "error") + mocker.patch.object(logger, "warn") try: init(engine_mock) @@ -23,6 +24,10 @@ def test_init_successful_connection(mocker): except Exception: connection_successful = False - assert connection_successful, "The database connection should be successful and not raise an exception." + assert ( + connection_successful + ), "The database connection should be successful and not raise an exception." - assert session_mock.exec.called_once_with(select(1)), "The session should execute a select statement once." + assert session_mock.exec.called_once_with( + select(1) + ), "The session should execute a select statement once." diff --git a/backend/app/tests/scripts/test_celery_pre_start.py b/backend/app/tests/scripts/test_celery_pre_start.py index 67c3f5cbe08..a557e0d2f0f 100644 --- a/backend/app/tests/scripts/test_celery_pre_start.py +++ b/backend/app/tests/scripts/test_celery_pre_start.py @@ -1,21 +1,22 @@ from unittest.mock import MagicMock +from pytest_mock import MockerFixture from sqlmodel import select from app.celeryworker_pre_start import init, logger -def test_init_successful_connection(mocker): +def test_init_successful_connection(mocker: MockerFixture) -> None: engine_mock = MagicMock() session_mock = MagicMock() exec_mock = MagicMock(return_value=True) - session_mock.configure_mock(**{'exec.return_value': exec_mock}) - mocker.patch('sqlmodel.Session', return_value=session_mock) + session_mock.configure_mock(**{"exec.return_value": exec_mock}) + mocker.patch("sqlmodel.Session", return_value=session_mock) - mocker.patch.object(logger, 'info') - mocker.patch.object(logger, 'error') - mocker.patch.object(logger, 'warn') + mocker.patch.object(logger, "info") + mocker.patch.object(logger, "error") + mocker.patch.object(logger, "warn") try: init(engine_mock) @@ -23,6 +24,10 @@ def test_init_successful_connection(mocker): except Exception: connection_successful = False - assert connection_successful, "The database connection should be successful and not raise an exception." + assert ( + connection_successful + ), "The database connection should be successful and not raise an exception." - assert session_mock.exec.called_once_with(select(1)), "The session should execute a select statement once." + assert session_mock.exec.called_once_with( + select(1) + ), "The session should execute a select statement once." diff --git a/backend/app/tests/scripts/test_test_pre_start.py b/backend/app/tests/scripts/test_test_pre_start.py index 485338d4e23..de5b61ab3a7 100644 --- a/backend/app/tests/scripts/test_test_pre_start.py +++ b/backend/app/tests/scripts/test_test_pre_start.py @@ -1,21 +1,22 @@ from unittest.mock import MagicMock +from pytest_mock import MockerFixture from sqlmodel import select from app.tests_pre_start import init, logger -def test_init_successful_connection(mocker): +def test_init_successful_connection(mocker: MockerFixture) -> None: engine_mock = MagicMock() session_mock = MagicMock() exec_mock = MagicMock(return_value=True) - session_mock.configure_mock(**{'exec.return_value': exec_mock}) - mocker.patch('sqlmodel.Session', return_value=session_mock) + session_mock.configure_mock(**{"exec.return_value": exec_mock}) + mocker.patch("sqlmodel.Session", return_value=session_mock) - mocker.patch.object(logger, 'info') - mocker.patch.object(logger, 'error') - mocker.patch.object(logger, 'warn') + mocker.patch.object(logger, "info") + mocker.patch.object(logger, "error") + mocker.patch.object(logger, "warn") try: init(engine_mock) @@ -23,6 +24,10 @@ def test_init_successful_connection(mocker): except Exception: connection_successful = False - assert connection_successful, "The database connection should be successful and not raise an exception." + assert ( + connection_successful + ), "The database connection should be successful and not raise an exception." - assert session_mock.exec.called_once_with(select(1)), "The session should execute a select statement once." + assert session_mock.exec.called_once_with( + select(1) + ), "The session should execute a select statement once." diff --git a/backend/app/tests/utils/user.py b/backend/app/tests/utils/user.py index 4622c2d3f52..891f32a2bff 100644 --- a/backend/app/tests/utils/user.py +++ b/backend/app/tests/utils/user.py @@ -42,6 +42,8 @@ def authentication_token_from_email( user = crud.create_user(session=db, user_create=user_in_create) else: user_in_update = UserUpdate(password=password) + if not user.id: + raise Exception("User id not set") user = crud.update_user(session=db, user_id=user.id, user_in=user_in_update) return user_authentication_headers(client=client, email=email, password=password) diff --git a/backend/app/utils.py b/backend/app/utils.py index ff228cbf14e..8e459cffbba 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -4,7 +4,7 @@ from pathlib import Path from typing import Any -import emails +import emails # type: ignore from jinja2 import Template from jose import JWTError, jwt @@ -109,6 +109,6 @@ def generate_password_reset_token(email: str) -> str: def verify_password_reset_token(token: str) -> str | None: try: decoded_token = jwt.decode(token, settings.SECRET_KEY, algorithms=["HS256"]) - return decoded_token["sub"] + return str(decoded_token["sub"]) except JWTError: return None diff --git a/backend/app/worker.py b/backend/app/worker.py index 5e1f7d64e5e..5afbdbb48f4 100644 --- a/backend/app/worker.py +++ b/backend/app/worker.py @@ -3,7 +3,7 @@ from app.core.celery_app import celery_app from app.core.config import settings -sentry_sdk.init(dsn=settings.SENTRY_DSN) +sentry_sdk.init(dsn=str(settings.SENTRY_DSN)) @celery_app.task(acks_late=True) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 54415685b3e..04f468551c3 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -35,6 +35,9 @@ mypy = "^1.8.0" ruff = "^0.2.2" pre-commit = "^3.6.2" pytest-mock = "^3.12.0" +types-python-jose = "^3.3.4.20240106" +types-passlib = "^1.7.7.20240106" +celery-types = "^0.22.0" [tool.isort] multi_line_output = 3 @@ -47,6 +50,7 @@ build-backend = "poetry.masonry.api" [tool.mypy] strict = true +exclude = ["venv", "alembic"] [tool.ruff] target-version = "py310" diff --git a/backend/scripts/lint.sh b/backend/scripts/lint.sh index 9dc9ed427cf..148d37c5c0f 100644 --- a/backend/scripts/lint.sh +++ b/backend/scripts/lint.sh @@ -3,6 +3,5 @@ set -x mypy app -black app --check -isort --recursive --check-only app -flake8 +ruff app +ruff format app --check diff --git a/backend/tests-start.sh b/backend/tests-start.sh index 099c2b39731..53396b6ad8e 100644 --- a/backend/tests-start.sh +++ b/backend/tests-start.sh @@ -3,4 +3,5 @@ set -e python /app/app/tests_pre_start.py +bash ./scripts/lint.sh bash ./scripts/test.sh "$@"