Fix: use UUID

app/app.py

@@ -198,11 +198,11 @@ def initialize_schools(
     hyperion_error_logger.info("Startup: Adding new groups to the database")
     with Session(sync_engine) as db:
         for school in SchoolType:
-            exists = initialization.get_school_by_id_sync(school_id=school, db=db)
+            exists = initialization.get_school_by_id_sync(school_id=school.value, db=db)
             # We don't want to recreate the groups if they already exist
             if not exists:
                 db_school = models_core.CoreSchool(
-                    id=school,
+                    id=school.value,
                     name=school.name,
                     email_regex=".*",
                 )

app/core/models_core.py

@@ -1,6 +1,7 @@
 """Common model files for all core in order to avoid circular import due to bidirectional relationship"""
 
 from datetime import date, datetime
+from uuid import UUID
 
 from sqlalchemy import ForeignKey, String
 from sqlalchemy.orm import Mapped, mapped_column, relationship
@@ -29,7 +30,7 @@ class CoreUser(Base):
         index=True,
     )  # Use UUID later
     email: Mapped[str] = mapped_column(unique=True, index=True)
-    school_id: Mapped[str] = mapped_column(ForeignKey("core_school.id"))
+    school_id: Mapped[UUID] = mapped_column(ForeignKey("core_school.id"))
     password_hash: Mapped[str]
     # Depending on the account type, the user may have different rights and access to different features
     # External users may exist for:

app/core/schemas_core.py

@@ -79,7 +79,7 @@ class CoreUserSimple(CoreUserBase):
 
     id: str
     account_type: AccountType
-    school_id: str
+    school_id: UUID
 
     model_config = ConfigDict(from_attributes=True)
 
@@ -123,7 +123,7 @@ class CoreUserFusionRequest(BaseModel):
 
 class CoreUserUpdateAdmin(BaseModel):
     email: str | None = None
-    school_id: str | None = None
+    school_id: UUID | None = None
     account_type: AccountType | None = None
     name: str | None = None
     firstname: str | None = None

app/core/schools/endpoints_schools.py

@@ -4,6 +4,7 @@
 School management is part of the core of Hyperion. These endpoints allow managing schools.
 """
 
+import logging
 import re
 import uuid
 
@@ -23,6 +24,8 @@
 
 router = APIRouter(tags=["Schools"])
 
+hyperion_error_logger = logging.getLogger("hyperion.error")
+
 
 @router.get(
     "/schools/",
@@ -94,7 +97,7 @@ async def create_school(
         await cruds_schools.create_school(school=db_school, db=db)
         users = await cruds_users.get_users(
             db=db,
-            schools_ids=[SchoolType.no_school],
+            schools_ids=[SchoolType.no_school.value],
         )
         for db_user in users:
             if re.match(db_school.email_regex, db_user.email):
@@ -158,7 +161,10 @@ async def update_school(
         and school_update.email_regex != school.email_regex
     ):
         await cruds_users.remove_users_from_school(db, school_id=school_id)
-        users = await cruds_users.get_users(db, schools_ids=[SchoolType.no_school])
+        users = await cruds_users.get_users(
+            db,
+            schools_ids=[SchoolType.no_school.value],
+        )
         for db_user in users:
             if re.match(school_update.email_regex, db_user.email):
                 await cruds_users.update_user(
@@ -194,7 +200,7 @@ async def delete_school(
     **This endpoint is only usable by administrators**
     """
 
-    if school_id in (item.value for item in SchoolType):
+    if school_id in (SchoolType.list()):
         raise HTTPException(
             status_code=400,
             detail="SchoolTypes schools can not be deleted",

app/core/schools/schools_type.py

@@ -1,7 +1,8 @@
 from enum import Enum
+from uuid import UUID
 
 
-class SchoolType(str, Enum):
+class SchoolType(Enum):
     """
     In Hyperion, each user must have a school. Belonging to a school gives access to a set of specific endpoints.
 
@@ -10,10 +11,14 @@ class SchoolType(str, Enum):
     """
 
     # Account types
-    no_school = "dce19aa2-8863-4c93-861e-fb7be8f610ed"
-    centrale_lyon = "d9772da7-1142-4002-8b86-b694b431dfed"
+    no_school = UUID("dce19aa2-8863-4c93-861e-fb7be8f610ed")
+    centrale_lyon = UUID("d9772da7-1142-4002-8b86-b694b431dfed")
 
     # Auth related groups
 
     def __str__(self):
         return f"{self.name}<{self.value}>"
+
+    @staticmethod
+    def list():
+        return [school.value for school in SchoolType]

app/core/users/cruds_users.py

@@ -27,14 +27,14 @@ async def get_users(
     excluded_account_types: list[AccountType] | None = None,
     included_groups: list[str] | None = None,
     excluded_groups: list[str] | None = None,
-    schools_ids: list[str] | None = None,
+    schools_ids: list[UUID] | None = None,
 ) -> Sequence[models_core.CoreUser]:
     """
     Return all users from database.
 
     Parameters `excluded_account_types` and `excluded_groups` can be used to filter results.
     """
-    included_account_types = included_account_types or list(AccountType)
+    included_account_types = included_account_types or None
     excluded_account_types = excluded_account_types or []
     included_groups = included_groups or []
     excluded_groups = excluded_groups or []
@@ -48,12 +48,16 @@ async def get_users(
         )
         for group_id in included_groups
     ]
-    included_account_type_condition = or_(
-        False,
-        *[
-            models_core.CoreUser.account_type == account_type
-            for account_type in included_account_types
-        ],
+    included_account_type_condition = (
+        or_(
+            False,
+            *[
+                models_core.CoreUser.account_type == account_type
+                for account_type in included_account_types
+            ],
+        )
+        if included_account_types
+        else and_(True)
     )
     # We want, for each group that should not be included
     # check that the following condition is false :
@@ -303,7 +307,7 @@ async def remove_users_from_school(
             models_core.CoreUser.school_id == school_id,
         )
         .values(
-            school_id=SchoolType.no_school,
+            school_id=SchoolType.no_school.value,
             account_type=AccountType.external,
         ),
     )

app/core/users/endpoints_users.py

@@ -364,23 +364,7 @@ async def activate_user(
             detail=f"The account with the email {unconfirmed_user.email} is already confirmed",
         )
 
-    # Check the account type
-
-    # For staff and student
-    # ^[\w\-.]*@((etu(-enise)?|enise)\.)?ec-lyon\.fr$
-    # For staff
-    # ^[\w\-.]*@(enise\.)?ec-lyon\.fr$
-    # For student
-    # ^[\w\-.]*@etu(-enise)?\.ec-lyon\.fr$
-
-    # For former students
-    # ^[\w\-.]*@centraliens-lyon\.net$
-
-    # All accepted emails
-    # ^[\w\-.]*@(((etu(-enise)?|enise)\.)?ec-lyon\.fr|centraliens-lyon\.net)$
-
-    # By default we mark the user as external
-    # but if it has an ECL email address, we will mark it as member
+    # Get the account type and school_id from the email
     account_type, school_id = await get_account_type_and_school_id_from_email(
         email=unconfirmed_user.email,
         db=db,

app/core/users/tools_users.py

@@ -1,4 +1,5 @@
 import re
+from uuid import UUID
 
 from sqlalchemy.ext.asyncio import AsyncSession
 
@@ -14,7 +15,7 @@
 async def get_account_type_and_school_id_from_email(
     email: str,
     db: AsyncSession,
-) -> tuple[AccountType, str]:
+) -> tuple[AccountType, UUID]:
     """Return the account type from the email"""
     if re.match(ECL_STAFF_REGEX, email):
         return AccountType.staff, SchoolType.centrale_lyon.value
@@ -23,11 +24,8 @@ async def get_account_type_and_school_id_from_email(
     if re.match(ECL_FORMER_STUDENT_REGEX, email):
         return AccountType.former_student, SchoolType.centrale_lyon.value
     schools = await cruds_schools.get_schools(db)
-    schools = [
-        school
-        for school in schools
-        if school.id not in (SchoolType.centrale_lyon, SchoolType.no_school)
-    ]
+
+    schools = [school for school in schools if school.id not in SchoolType.list()]
     school = next(
         (school for school in schools if re.match(school.email_regex, email)),
         None,

tests/commons.py

@@ -112,7 +112,7 @@ def change_redis_client_status(activated: bool) -> None:
 async def create_user_with_groups(
     groups: list[GroupType],
     account_type: AccountType = AccountType.student,
-    school_id: SchoolType | str = SchoolType.centrale_lyon,
+    school_id: SchoolType | uuid.UUID = SchoolType.centrale_lyon,
     user_id: str | None = None,
     email: str | None = None,
     password: str | None = None,
@@ -129,6 +129,7 @@ async def create_user_with_groups(
 
     user_id = user_id or str(uuid.uuid4())
     password_hash = security.get_password_hash(password or get_random_string())
+    school_id = school_id.value if isinstance(school_id, SchoolType) else school_id
 
     user = models_core.CoreUser(
         id=user_id,

tests/test_schools.py

@@ -1,3 +1,5 @@
+from uuid import UUID
+
 import pytest_asyncio
 from fastapi.testclient import TestClient
 from pytest_mock import MockerFixture
@@ -17,7 +19,7 @@
 
 UNIQUE_TOKEN = "my_unique_token"
 
-id_test_ens = "4d133de7-24c4-4dbc-be73-4705a2ddd315"
+id_test_ens = UUID("4d133de7-24c4-4dbc-be73-4705a2ddd315")
 
 
 @pytest_asyncio.fixture(scope="module", autouse=True)
@@ -177,7 +179,7 @@ def test_create_user_corresponding_to_school(
     assert response.status_code == 201
 
     users = client.get(
-        "/users/",
+        "/users",
         headers={"Authorization": f"Bearer {token}"},
     )
     user = next(
@@ -225,4 +227,4 @@ def test_delete_school(client: TestClient) -> None:
         headers={"Authorization": f"Bearer {token}"},
     )
     assert response.status_code == 200
-    assert response.json()["school_id"] == SchoolType.no_school
+    assert response.json()["school_id"] == str(SchoolType.no_school.value)