Skip to content

Commit

Permalink
Feat: prevent school change through email migration
Browse files Browse the repository at this point in the history
  • Loading branch information
Rotheem committed Jan 7, 2025
1 parent 42d1007 commit 010097a
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 20 deletions.
31 changes: 12 additions & 19 deletions app/core/users/endpoints_users.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import logging
import re
import string
import uuid
from datetime import UTC, datetime, timedelta
Expand Down Expand Up @@ -53,10 +52,6 @@

templates = Jinja2Templates(directory="assets/templates")

ECL_STAFF_REGEX = r"^[\w\-.]*@(enise\.)?ec-lyon\.fr$"
ECL_STUDENT_REGEX = r"^[\w\-.]*@((etu(-enise)?)|(ecl\d{2}))\.ec-lyon\.fr$"
ECL_FORMER_STUDENT_REGEX = r"^[\w\-.]*@centraliens-lyon\.net$"


@router.get(
"/users",
Expand Down Expand Up @@ -580,16 +575,9 @@ async def migrate_mail(
settings: Settings = Depends(get_settings),
):
"""
Due to a change in the email format, all student users need to migrate their email address.
This endpoint will send a confirmation code to the user's new email address. He will need to use this code to confirm the change with `/users/confirm-mail-migration` endpoint.
"""

if not re.match(ECL_STUDENT_REGEX, mail_migration.new_email):
raise HTTPException(
status_code=400,
detail="The new email address must match the new ECL format for student users",
)

existing_user = await cruds_users.get_user_by_email(
db=db,
email=mail_migration.new_email,
Expand All @@ -610,6 +598,17 @@ async def migrate_mail(
)
return

# We need to make sur the user will keep the same school if he is not a no_school user
_, new_school_id = await get_account_type_and_school_id_from_email(
email=mail_migration.new_email,
db=db,
)
if user.school_id is not SchoolType.no_school and user.school_id != new_school_id:
raise HTTPException(
status_code=400,
detail="New email address is not compatible with the current school",
)

await create_and_send_email_migration(
user_id=user.id,
new_email=mail_migration.new_email,
Expand All @@ -629,8 +628,8 @@ async def migrate_mail_confirm(
db: AsyncSession = Depends(get_db),
):
"""
Due to a change in the email format, all student users need to migrate their email address.
This endpoint will updates the user new email address.
The user will need to use the confirmation code sent by the `/users/migrate-mail` endpoint.
"""

migration_object = await cruds_users.get_email_migration_code_by_token(
Expand Down Expand Up @@ -671,12 +670,6 @@ async def migrate_mail_confirm(
email=migration_object.new_email,
db=db,
)

if user.school_id is not SchoolType.no_school and user.school_id != new_school_id:
raise HTTPException(
status_code=400,
detail="User cannot change school",
)
try:
await cruds_users.update_user(
db=db,
Expand Down
14 changes: 13 additions & 1 deletion tests/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ def test_create_and_activate_user(
headers={"Authorization": f"Bearer {token_admin_user}"},
)

assert user_detail.json()["school_id"] == expected_school_id.value
assert user_detail.json()["school_id"] == str(expected_school_id.value)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -362,6 +362,18 @@ def test_update_user(client: TestClient) -> None:
assert response.status_code == 204


async def test_migrate_mail_with_school_change(client: TestClient) -> None:
token = create_api_access_token(student_user_with_old_email)

# Start the migration process
response = client.post(
"/users/migrate-mail",
json={"new_email": "[email protected]"},
headers={"Authorization": f"Bearer {token}"},
)
assert response.status_code == 400


async def test_migrate_mail(mocker: MockerFixture, client: TestClient) -> None:
# NOTE: we don't want to mock app.core.security.generate_token but
# app.core.users.endpoints_users.security.generate_token which is the imported version of the function
Expand Down

0 comments on commit 010097a

Please sign in to comment.