diff --git a/src/api/urls.py b/src/api/urls.py index 0e20413..ad50dc9 100644 --- a/src/api/urls.py +++ b/src/api/urls.py @@ -8,6 +8,7 @@ import image_asset.urls import nominees.urls import officers.urls +import users.urls from api.auth import require_trusted_origin router = APIRouter( @@ -22,3 +23,4 @@ router.include_router(event.urls.router) router.include_router(honorary.urls.router) router.include_router(image_asset.urls.router) +router.include_router(users.urls.router) diff --git a/src/auth/crud.py b/src/auth/crud.py index 2d37725..141dd50 100644 --- a/src/auth/crud.py +++ b/src/auth/crud.py @@ -36,7 +36,12 @@ async def create_user_session(db_session: AsyncSession, session_id: str, computi ) # ...or just update their "last_logged_in" time user_query = user_query.on_conflict_do_update( - index_elements=[SiteUserDB.computing_id], set_={"last_logged_in": now} + index_elements=[SiteUserDB.computing_id], + set_={ + "last_logged_in": now, + # If the user's first_logged_in is null, set it to now, otherwise leave it as is. + "first_logged_in": sqlalchemy.func.coalesce(SiteUserDB.first_logged_in, now), + }, ) await db_session.execute(user_query) diff --git a/src/dependencies.py b/src/dependencies.py index e611852..897ab29 100644 --- a/src/dependencies.py +++ b/src/dependencies.py @@ -5,8 +5,8 @@ import auth import auth.crud import database -from auth.constants import COOKIE_SESSION_KEY -from utils.permissions import is_user_election_admin, is_user_website_admin +from auth.constants import COOKIE_SESSION_KEY, UserRole +from utils.permissions import is_user_election_admin, is_user_role, is_user_website_admin # Dependency to ensure years in paths are valid # Honestly don't know if this DB will be running past year 3000 @@ -83,7 +83,7 @@ async def perm_election(db_session: database.DBSession, computing_id: LoggedInUs async def perm_admin(db_session: database.DBSession, computing_id: LoggedInUser): - if not await is_user_website_admin(computing_id, db_session): + if not await is_user_role(db_session, computing_id, UserRole.ADMIN): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="must be an admin") return computing_id diff --git a/src/load_test_db.py b/src/load_test_db.py index d251323..38ec4c7 100644 --- a/src/load_test_db.py +++ b/src/load_test_db.py @@ -31,6 +31,7 @@ ) from officers.tables import OfficerInfoDB, OfficerTermDB from translink.tables import TransLinkStaticScheduleDB +from users.crud import UserRole, create_site_user, create_user_roles async def reset_db(engine): @@ -231,7 +232,8 @@ async def load_test_officers_data(db_session: AsyncSession): await db_session.commit() -SYSADMIN_COMPUTING_ID = "pkn4" +SYSADMIN_COMPUTING_ID = "sysadmin" +SYSADMIN_FULL_NAME = "System Administrator" async def load_sysadmin(db_session: AsyncSession): @@ -241,7 +243,7 @@ async def load_sysadmin(db_session: AsyncSession): await create_new_officer_info( db_session, OfficerInfoDB( - legal_name="Puneet North", + legal_name=SYSADMIN_FULL_NAME, discord_id=None, discord_name=None, discord_nickname=None, @@ -300,10 +302,20 @@ async def load_sysadmin(db_session: AsyncSession): photo_url=None, ), ) + user_roles = [ + SiteUserRoleDB( + computing_id=SYSADMIN_COMPUTING_ID, + role=UserRole.ADMIN, + added_by=SYSADMIN_COMPUTING_ID, + created_at=datetime.now(UTC), + ) + ] + create_user_roles(db_session, user_roles) await db_session.commit() -WEBMASTER_COMPUTING_ID = "jbriones" +WEBMASTER_COMPUTING_ID = "webmaster" +WEBMASTER_FULL_NAME = "Web Master" async def load_webmaster(db_session: AsyncSession): @@ -313,7 +325,7 @@ async def load_webmaster(db_session: AsyncSession): await create_new_officer_info( db_session, OfficerInfoDB( - legal_name="Jon Andre Briones", + legal_name=WEBMASTER_FULL_NAME, discord_id=None, discord_name=None, discord_nickname=None, @@ -330,7 +342,7 @@ async def load_webmaster(db_session: AsyncSession): position=OfficerPositionEnum.FIRST_YEAR_REPRESENTATIVE, start_date=date.today() - timedelta(days=(365 * 3)), end_date=date.today() - timedelta(days=(365 * 2)), - nickname="Jon Andre Briones", + nickname=WEBMASTER_FULL_NAME, favourite_course_0="CMPT 379", favourite_course_1="CMPT 371", favourite_pl_0="TypeScript", @@ -355,6 +367,15 @@ async def load_webmaster(db_session: AsyncSession): photo_url=None, ), ) + user_roles = [ + SiteUserRoleDB( + computing_id=WEBMASTER_COMPUTING_ID, + role=UserRole.ADMIN, + added_by=SYSADMIN_COMPUTING_ID, + created_at=datetime.now(UTC), + ) + ] + create_user_roles(db_session, user_roles) await db_session.commit() @@ -413,8 +434,8 @@ async def load_test_elections_data(db_session: AsyncSession): await create_nominee_info( db_session, NomineeInfoDB( - computing_id="pkn4", - full_name="Puneet North", + computing_id="sysadmin", + full_name=SYSADMIN_FULL_NAME, linked_in="linkedin.com/john-doe3", instagram="john_doe 3", email="john_do3e@doe.com", diff --git a/src/users/crud.py b/src/users/crud.py new file mode 100644 index 0000000..65c7f79 --- /dev/null +++ b/src/users/crud.py @@ -0,0 +1,50 @@ +from collections.abc import Sequence + +from sqlalchemy import delete, select +from sqlalchemy.orm import selectinload + +import database +from auth.constants import UserRole +from auth.tables import SiteUserDB, SiteUserRoleDB + + +def create_site_user(db_session: database.DBSession, user: SiteUserDB) -> None: + db_session.add(user) + + +async def get_all_users(db_session: database.DBSession) -> Sequence[SiteUserDB]: + query = select(SiteUserDB).options(selectinload(SiteUserDB.roles)).order_by(SiteUserDB.last_logged_in.desc()) + + return (await db_session.scalars(query)).all() + + +def create_user_roles(db_session: database.DBSession, roles: list[SiteUserRoleDB]) -> None: + db_session.add_all(roles) + + +async def get_user_for_role_update(db_session: database.DBSession, computing_id: str) -> SiteUserDB | None: + query = ( + select(SiteUserDB) + .where(SiteUserDB.computing_id == computing_id) + .options(selectinload(SiteUserDB.roles)) + .with_for_update() + ) + user = await db_session.scalar(query) + + return user + + +async def delete_user_roles(db_session: database.DBSession, computing_id: str, roles_to_remove: set[UserRole]) -> None: + query = ( + delete(SiteUserRoleDB) + .where(SiteUserRoleDB.computing_id == computing_id, SiteUserRoleDB.role.in_(roles_to_remove)) + .execution_options(synchronize_session="fetch") + ) + + await db_session.execute(query) + + +async def delete_user(db_session: database.DBSession, computing_id: str) -> None: + query = delete(SiteUserDB).where(SiteUserDB.computing_id == computing_id) + + await db_session.execute(query) diff --git a/src/users/models.py b/src/users/models.py new file mode 100644 index 0000000..00d600c --- /dev/null +++ b/src/users/models.py @@ -0,0 +1,35 @@ +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from auth.constants import UserRole +from constants import COMPUTING_ID_LEN + + +class SiteUserBase(BaseModel): + computing_id: str = Field( + ..., + min_length=1, + max_length=COMPUTING_ID_LEN, + description="Computing ID of the user", + ) + + roles: list[UserRole] = Field( + [], + description="List of roles assigned to the user", + ) + + @field_validator("roles") + @classmethod + def roles_must_be_unique(cls, roles: list[UserRole]) -> list[UserRole]: + if len(roles) != len(set(roles)): + raise ValueError("Roles must be unique") + return roles + + +class SiteUserCreate(SiteUserBase): + pass + + +class SiteUserUpdate(BaseModel): + model_config = ConfigDict(extra="forbid") + + roles: list[UserRole] = Field(..., description="Complete set of roles assigned to the user") diff --git a/src/users/urls.py b/src/users/urls.py new file mode 100644 index 0000000..cb62c57 --- /dev/null +++ b/src/users/urls.py @@ -0,0 +1,193 @@ +from datetime import datetime + +from fastapi import APIRouter, Depends, HTTPException, Response, status +from pydantic import ValidationError +from sqlalchemy.exc import IntegrityError + +import database +import users.crud +from auth.models import SiteUser +from auth.tables import SiteUserDB, SiteUserRoleDB +from constants import TZ_INFO +from dependencies import SiteAdmin, perm_admin +from users.models import SiteUserCreate, SiteUserUpdate +from utils.shared_models import DetailModel + +router = APIRouter( + prefix="/user", + tags=["user"], +) + + +@router.get( + "", + description="Get all site users.", + response_model=list[SiteUser], + responses={403: {"description": "need to be an admin", "model": DetailModel}}, + operation_id="get_all_users", + dependencies=[Depends(perm_admin)], +) +async def get_all_users( + db_session: database.DBSession, +): + users_list = await users.crud.get_all_users(db_session) + + try: + response = [SiteUser.model_validate(user) for user in users_list] + except ValidationError as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="validation error from database" + ) from e + + return response + + +@router.post( + "", + description="Create a site user.", + response_model=SiteUser, + status_code=status.HTTP_201_CREATED, + responses={ + 401: {"description": "need to be a logged in admin", "model": DetailModel}, + 403: {"description": "need to be an admin", "model": DetailModel}, + 409: {"description": "user already existed", "model": DetailModel}, + }, + operation_id="create_site_user", +) +async def create_site_user(db_session: database.DBSession, admin_id: SiteAdmin, body: SiteUserCreate): + user_db = SiteUserDB(computing_id=body.computing_id) + + users.crud.create_site_user(db_session, user_db) + + if body.roles: + now = datetime.now(tz=TZ_INFO) + user_roles = [ + SiteUserRoleDB( + computing_id=body.computing_id, + role=role, + added_by=admin_id, + created_at=now, + ) + for role in body.roles + ] + users.crud.create_user_roles(db_session, user_roles) + + try: + await db_session.flush() + await db_session.refresh( + user_db, + attribute_names=[ + "computing_id", + "first_logged_in", + "last_logged_in", + "roles", + ], + ) + new_user = SiteUser.model_validate(user_db) + await db_session.commit() + except IntegrityError: + await db_session.rollback() + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Site user already exists", + ) from None + + return new_user + + +@router.put( + "/{computing_id}", + description="Update a site user's roles.", + response_model=SiteUser, + status_code=status.HTTP_200_OK, + responses={ + 401: {"description": "need to be a logged in admin", "model": DetailModel}, + 403: {"description": "need to be an admin", "model": DetailModel}, + 404: {"description": "site user not found", "model": DetailModel}, + 409: {"description": "role update conflicted with another change", "model": DetailModel}, + }, + operation_id="update_site_user_roles", +) +async def update_site_user_roles( + db_session: database.DBSession, + admin_id: SiteAdmin, + computing_id: str, + body: SiteUserUpdate, +): + user = await users.crud.get_user_for_role_update(db_session, computing_id) + + if user is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Site user not found") + + existing_roles = {assignment.role for assignment in user.roles} + + new_roles = set(body.roles) + + roles_to_add = new_roles - existing_roles + roles_to_remove = existing_roles - new_roles + + try: + if roles_to_remove: + await users.crud.delete_user_roles(db_session, computing_id, roles_to_remove) + + if roles_to_add: + now = datetime.now(tz=TZ_INFO) + users.crud.create_user_roles( + db_session, + [ + SiteUserRoleDB( + computing_id=computing_id, + role=role, + added_by=admin_id, + created_at=now, + ) + for role in roles_to_add + ], + ) + + await db_session.flush() + await db_session.refresh( + user, + attribute_names=[ + "computing_id", + "first_logged_in", + "last_logged_in", + "roles", + ], + ) + updated_user = SiteUser.model_validate(user) + await db_session.commit() + except IntegrityError: + await db_session.rollback() + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Site user roles changed concurrently", + ) from None + + return updated_user + + +@router.delete( + "/{computing_id}", + description="Delete a site user's roles. Does not cascade deletes.", + status_code=status.HTTP_204_NO_CONTENT, + responses={ + 401: {"description": "Need to be a logged in admin", "model": DetailModel}, + 403: {"description": "Need to be an admin", "model": DetailModel}, + 404: {"description": "Site user not found", "model": DetailModel}, + 409: {"description": "Site user still referenced by another", "model": DetailModel}, + }, + operation_id="delete_site_user_roles", +) +async def delete_site_user_roles(db_session: database.DBSession, computing_id: str): + try: + await users.crud.delete_user(db_session, computing_id) + await db_session.commit() + except IntegrityError: + await db_session.rollback() + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Site user {computing_id} still referenced by others.", + ) from None + + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/tests/integration/test_elections.py b/tests/integration/test_elections.py index b5c6c0c..e23bbf3 100644 --- a/tests/integration/test_elections.py +++ b/tests/integration/test_elections.py @@ -120,7 +120,7 @@ async def test__get_single_candidates(client: AsyncClient): response = await client.get(f"/api/candidate/{TEST_ELECTION_2}") assert response.status_code == 401 - response = await client.get("/api/nominee/pkn4") + response = await client.get("/api/nominee/sysadmin") assert response.status_code == 401 @@ -375,7 +375,7 @@ async def test__admin_update_candidate(admin_client: AsyncClient): # ensure that candidates can be viewed # update the candidate response = await admin_client.patch( - f"/api/candidate/{TEST_ELECTION_2}/vice-president/pkn4", + f"/api/candidate/{TEST_ELECTION_2}/vice-president/sysadmin", json={"speech": "Vote for me as treasurer"}, ) assert response.status_code == 200 @@ -383,7 +383,7 @@ async def test__admin_update_candidate(admin_client: AsyncClient): # ensure that candidates can be viewed # try updating a non-registered election response = await admin_client.patch( - "/api/candidate/testElection4/pkn4", + "/api/candidate/testElection4/sysadmin", json={"position": "president", "speech": "Vote for me as president, I am good at valorant."}, ) assert response.status_code == 404 @@ -412,7 +412,7 @@ async def test__admin_update_nominee(admin_client: AsyncClient): response = await admin_client.patch( f"/api/nominee/{load_test_db.SYSADMIN_COMPUTING_ID}", json={ - "full_name": "Puneet N", + "full_name": load_test_db.SYSADMIN_FULL_NAME, "linked_in": "linkedin.com/not-my-linkedin", }, ) diff --git a/tests/unit/test_users.py b/tests/unit/test_users.py new file mode 100644 index 0000000..cb9c91f --- /dev/null +++ b/tests/unit/test_users.py @@ -0,0 +1,77 @@ +from unittest.mock import AsyncMock, Mock + +import pytest +from fastapi import HTTPException, status +from sqlalchemy.ext.asyncio import AsyncSession + +import users.crud +from auth.constants import UserRole +from auth.tables import SiteUserDB, SiteUserRoleDB +from users.models import SiteUserUpdate +from users.urls import update_site_user_roles + +pytestmark = pytest.mark.unit + + +def make_user(computing_id: str, roles: set[UserRole]) -> SiteUserDB: + return SiteUserDB( + computing_id=computing_id, + roles=[SiteUserRoleDB(computing_id=computing_id, role=role) for role in roles], + ) + + +async def test__update_site_user_roles_replaces_role_set(monkeypatch: pytest.MonkeyPatch): + computing_id = "target" + admin_id = "admin" + user = make_user(computing_id, {UserRole.USER, UserRole.EXEC}) + desired_roles = {UserRole.EXEC, UserRole.ADMIN} + + db_session = AsyncMock(spec=AsyncSession) + + async def refresh_user(instance: SiteUserDB, *, attribute_names: list[str]): + assert instance is user + assert "roles" in attribute_names + instance.roles = [ + SiteUserRoleDB(computing_id=computing_id, role=role, added_by=admin_id) for role in desired_roles + ] + + db_session.refresh.side_effect = refresh_user + + get_user = AsyncMock(return_value=user) + delete_roles = AsyncMock() + create_roles = Mock() + monkeypatch.setattr(users.crud, "get_user_for_role_update", get_user) + monkeypatch.setattr(users.crud, "delete_user_roles", delete_roles) + monkeypatch.setattr(users.crud, "create_user_roles", create_roles) + + result = await update_site_user_roles( + db_session=db_session, + admin_id=admin_id, + computing_id=computing_id, + body=SiteUserUpdate(roles=desired_roles), + ) + + delete_roles.assert_awaited_once_with(db_session, computing_id, {UserRole.USER}) + created_assignments = create_roles.call_args.args[1] + assert {assignment.role for assignment in created_assignments} == {UserRole.ADMIN} + assert all(assignment.added_by == admin_id for assignment in created_assignments) + assert set(result.roles) == desired_roles + db_session.flush.assert_awaited_once() + db_session.commit.assert_awaited_once() + db_session.rollback.assert_not_awaited() + + +async def test__update_site_user_roles_returns_not_found(monkeypatch: pytest.MonkeyPatch): + db_session = AsyncMock(spec=AsyncSession) + monkeypatch.setattr(users.crud, "get_user_for_role_update", AsyncMock(return_value=None)) + + with pytest.raises(HTTPException) as exc_info: + await update_site_user_roles( + db_session=db_session, + admin_id="admin", + computing_id="missing", + body=SiteUserUpdate(roles=set()), + ) + + assert exc_info.value.status_code == status.HTTP_404_NOT_FOUND + db_session.commit.assert_not_awaited()