diff --git a/alembic/versions/29dd08592b64_replace_user_role_enum_with_boolean_.py b/alembic/versions/29dd08592b64_replace_user_role_enum_with_boolean_.py new file mode 100644 index 0000000..44312bc --- /dev/null +++ b/alembic/versions/29dd08592b64_replace_user_role_enum_with_boolean_.py @@ -0,0 +1,96 @@ +"""replace user role enum with boolean flags + +Revision ID: 29dd08592b64 +Revises: f2a7b7ac51e6 +Create Date: 2026-07-15 21:05:10.541763 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from sqlalchemy.dialects import mysql + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "29dd08592b64" +down_revision: Union[str, None] = "f2a7b7ac51e6" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "users", + sa.Column("is_leader", sa.Boolean(), nullable=False, server_default="0"), + ) + op.add_column( + "users", + sa.Column("is_admin", sa.Boolean(), nullable=False, server_default="0"), + ) + op.add_column( + "users", + sa.Column("is_president", sa.Boolean(), nullable=False, server_default="0"), + ) + + op.execute( + "UPDATE users SET is_leader = 1 " + "WHERE role IN ('LEADER', 'ADMIN_AND_LEADER', 'LEADER_AND_PRESIDENT')" + ) + op.execute( + "UPDATE users SET is_admin = 1 " + "WHERE role IN ('ADMIN', 'PRESIDENT', 'ADMIN_AND_LEADER', 'LEADER_AND_PRESIDENT')" + ) + op.execute( + "UPDATE users SET is_president = 1 " + "WHERE role IN ('PRESIDENT', 'LEADER_AND_PRESIDENT')" + ) + + op.drop_index("idx_users_role", table_name="users") + op.drop_column("users", "role") + + op.create_index("idx_users_is_leader", "users", ["is_leader"]) + op.create_index("idx_users_is_admin", "users", ["is_admin"]) + op.create_index("idx_users_is_president", "users", ["is_president"]) + + +def downgrade() -> None: + op.add_column( + "users", + sa.Column( + "role", + mysql.ENUM( + "MEMBER", + "LEADER", + "ADMIN", + "PRESIDENT", + "ADMIN_AND_LEADER", + "LEADER_AND_PRESIDENT", + name="userrole", + ), + nullable=False, + server_default="MEMBER", + ), + ) + op.execute( + """ + UPDATE users SET role = CASE + WHEN is_leader = 1 AND is_president = 1 THEN 'LEADER_AND_PRESIDENT' + WHEN is_leader = 1 AND is_admin = 1 THEN 'ADMIN_AND_LEADER' + WHEN is_president = 1 THEN 'PRESIDENT' + WHEN is_admin = 1 THEN 'ADMIN' + WHEN is_leader = 1 THEN 'LEADER' + ELSE 'MEMBER' + END + """ + ) + + op.drop_index("idx_users_is_leader", table_name="users") + op.drop_index("idx_users_is_admin", table_name="users") + op.drop_index("idx_users_is_president", table_name="users") + op.drop_column("users", "is_leader") + op.drop_column("users", "is_admin") + op.drop_column("users", "is_president") + + op.create_index("idx_users_role", "users", ["role"]) diff --git a/alembic/versions/f2a7b7ac51e6_add_president_roles_to_user_role_enum.py b/alembic/versions/f2a7b7ac51e6_add_president_roles_to_user_role_enum.py new file mode 100644 index 0000000..9a0e365 --- /dev/null +++ b/alembic/versions/f2a7b7ac51e6_add_president_roles_to_user_role_enum.py @@ -0,0 +1,53 @@ +"""add president roles to user_role enum + +Revision ID: f2a7b7ac51e6 +Revises: a1b2c3d4e5f6 +Create Date: 2026-07-12 08:05:54.190502 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from sqlalchemy.dialects import mysql + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "f2a7b7ac51e6" +down_revision: Union[str, None] = "a1b2c3d4e5f6" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.alter_column( + "users", + "role", + existing_type=mysql.ENUM("MEMBER", "LEADER", "ADMIN", "ADMIN_AND_LEADER"), + type_=mysql.ENUM( + "MEMBER", + "LEADER", + "ADMIN", + "PRESIDENT", + "ADMIN_AND_LEADER", + "LEADER_AND_PRESIDENT", + ), + nullable=False, + ) + + +def downgrade() -> None: + op.alter_column( + "users", + "role", + existing_type=mysql.ENUM( + "MEMBER", + "LEADER", + "ADMIN", + "PRESIDENT", + "ADMIN_AND_LEADER", + "LEADER_AND_PRESIDENT", + ), + type_=mysql.ENUM("MEMBER", "LEADER", "ADMIN", "ADMIN_AND_LEADER"), + nullable=False, + ) diff --git a/app/deps/auth.py b/app/deps/auth.py index 6ee37b1..f48495d 100644 --- a/app/deps/auth.py +++ b/app/deps/auth.py @@ -96,7 +96,7 @@ async def require_admin(user: User = Depends(get_current_user)) -> User: Require user to be admin. Raises 403 if user is not admin. """ - if not user.is_admin: + if not user.has_admin_access: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail="Admin permission required" ) diff --git a/app/deps/project.py b/app/deps/project.py index 6d65706..b6df022 100644 --- a/app/deps/project.py +++ b/app/deps/project.py @@ -11,7 +11,7 @@ def require_leader_or_admin(project_id: int, user: User, db: Session) -> Project Returns the project if authorized, raises 403 otherwise. """ # Admin has full access - if user.is_admin: + if user.has_admin_access: project = ProjectService.get(db, project_id) if not project: raise HTTPException( diff --git a/app/models/__init__.py b/app/models/__init__.py index d580bf1..02e4819 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -7,7 +7,6 @@ MemberRole, ProjectStatus, Qualification, - UserRole, ) from app.models.project import Project from app.models.project_member import ProjectMember @@ -23,7 +22,6 @@ "ProjectStatus", "MemberRole", "ApprovalStatus", - "UserRole", "User", "AuditLog", "UserActivity", diff --git a/app/models/enums.py b/app/models/enums.py index 7fa2549..1482c3d 100644 --- a/app/models/enums.py +++ b/app/models/enums.py @@ -19,13 +19,6 @@ class MemberRole(str, Enum): MEMBER = "member" -class UserRole(str, Enum): - MEMBER = "member" - LEADER = "leader" - ADMIN = "admin" - ADMIN_AND_LEADER = "admin_and_leader" - - class AuditAction(str, Enum): QUALIFICATION_CHANGED = "qualification_changed" ROLE_CHANGED = "role_changed" diff --git a/app/models/user.py b/app/models/user.py index 10e4be4..1365639 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -18,7 +18,6 @@ NotificationChannel, ProjectStatus, Qualification, - UserRole, ) @@ -48,7 +47,18 @@ class User(Base, TimestampMixin, SoftDeleteMixin): qualification = Column( Enum(Qualification), nullable=False, default=Qualification.PENDING ) - role = Column(Enum(UserRole), nullable=False, default=UserRole.MEMBER) + is_leader = Column(Boolean, nullable=False, default=False) + is_admin = Column(Boolean, nullable=False, default=False) + is_president = Column(Boolean, nullable=False, default=False) + + @property + def has_admin_access(self) -> bool: + """Admin-gated actions should check this, not is_admin directly. + + The president is the top of the org hierarchy and always has admin + access, whether or not is_admin was also explicitly granted. + """ + return self.is_admin or self.is_president @property def current_projects(self): @@ -58,14 +68,6 @@ def current_projects(self): if m.left_at is None and m.project.status != ProjectStatus.ENDED ] - @property - def is_admin(self) -> bool: - return self.role in (UserRole.ADMIN, UserRole.ADMIN_AND_LEADER) - - @property - def is_leader(self) -> bool: - return self.role in (UserRole.LEADER, UserRole.ADMIN_AND_LEADER) - # Profile (optional) phone = Column(String(20), nullable=True) affiliation = Column(String(200), nullable=True) @@ -113,7 +115,9 @@ def is_leader(self) -> bool: __table_args__ = ( Index("idx_users_qualification", "qualification"), - Index("idx_users_role", "role"), + Index("idx_users_is_leader", "is_leader"), + Index("idx_users_is_admin", "is_admin"), + Index("idx_users_is_president", "is_president"), Index("idx_users_created_at", "created_at"), Index("idx_users_is_temporary", "is_temporary"), # Roster import matches existing members by student_id. diff --git a/app/routes/auth.py b/app/routes/auth.py index cc0ec00..fbe6db9 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -24,7 +24,7 @@ InvalidAuthTokenError, UserNotRegisteredError, ) -from app.models import Qualification, User, UserRole +from app.models import Qualification, User from app.schemas import ( AuthResult, AuthStatus, @@ -508,7 +508,6 @@ async def signup( bio=request.bio, github_username=request.github_username, qualification=Qualification.PENDING, - role=UserRole.MEMBER, ) # Generate JWT @@ -624,7 +623,14 @@ async def signin_dev( if user: # Update existing user's admin status and qualification - UserService.update(db, user, role=request.role, qualification=qualification) + UserService.update( + db, + user, + is_leader=request.is_leader, + is_admin=request.is_admin, + is_president=request.is_president, + qualification=qualification, + ) else: # Create new user with dev google_id # Handle race condition: if concurrent request created user, catch and retry @@ -638,14 +644,21 @@ async def signin_dev( email=request.email, name=request.name, qualification=qualification, - role=request.role, + is_leader=request.is_leader, + is_admin=request.is_admin, + is_president=request.is_president, ) except IntegrityError: db.rollback() user = UserService.get_by_email(db, request.email) if user: UserService.update( - db, user, role=request.role, qualification=qualification + db, + user, + is_leader=request.is_leader, + is_admin=request.is_admin, + is_president=request.is_president, + qualification=qualification, ) else: raise HTTPException( diff --git a/app/routes/users.py b/app/routes/users.py index 1e3fef1..f0d84e8 100644 --- a/app/routes/users.py +++ b/app/routes/users.py @@ -14,7 +14,7 @@ RosterFileTooLargeError, TemporaryMemberApprovalError, ) -from app.models import AuditAction, Qualification, User, UserRole +from app.models import AuditAction, Qualification, User from app.schemas import ( ActivityCreateRequest, ActivityDetail, @@ -153,6 +153,31 @@ async def get_my_projects( return Response(ok=True, data=projects) +@router.get( + "/me/activities", + response_model=Response[list[ActivityDetail]], + summary="Get my activities", + description="Returns activities of a current member.", + responses={ + 200: {"description": "Activities retrieved successfully"}, + 401: {"description": "Not authenticated"}, + 403: {"description": "Requires REGULAR qualification or higher"}, + }, +) +async def get_my_activities( + current_user: User = Depends(require_regular), db: Session = Depends(get_db) +): + """ + Get activities of a current member. + + **Requires**: REGULAR qualification or higher. + + Returns a list of activities (current projects and histories). + """ + activities = ActivityService.list_by_user(db, current_user.id) + return Response(ok=True, data=activities) + + # === Admin management === @router.get( "", @@ -374,12 +399,17 @@ async def update_user( ) # Log role changes - if "role" in update_data and update_data["role"] != user.role: + role_changes = { + field: {"from": getattr(user, field), "to": update_data[field]} + for field in ("is_leader", "is_admin", "is_president") + if field in update_data and update_data[field] != getattr(user, field) + } + if role_changes: AuditLogService.log( db=db, user_id=user.id, action=AuditAction.ROLE_CHANGED, - payload={"from": user.role.value, "to": update_data["role"].value}, + payload=role_changes, actor_id=admin.id, ) diff --git a/app/schemas/auth.py b/app/schemas/auth.py index 2e0ecec..2a3d6fe 100644 --- a/app/schemas/auth.py +++ b/app/schemas/auth.py @@ -2,7 +2,6 @@ from pydantic import BaseModel, EmailStr, Field -from app.models.enums import UserRole from app.schemas.user import UserDetail @@ -86,10 +85,9 @@ class DevSigninRequest(BaseModel): description="User name for dev signin", examples=["Admin User"], ) - role: UserRole = Field( - default=UserRole.MEMBER, - description="User role: member, leader, admin, or admin_and_leader", - ) + is_leader: bool = Field(default=False, description="Grant leader privileges") + is_admin: bool = Field(default=False, description="Grant admin privileges") + is_president: bool = Field(default=False, description="Grant president privileges") qualification: Literal["pending", "associate", "regular", "active"] = Field( default="active", description="User qualification level", diff --git a/app/schemas/user.py b/app/schemas/user.py index fab0382..40e67f6 100644 --- a/app/schemas/user.py +++ b/app/schemas/user.py @@ -1,11 +1,6 @@ from pydantic import BaseModel, Field -from app.models.enums import ( - GraduationStatus, - NotificationChannel, - Qualification, - UserRole, -) +from app.models.enums import GraduationStatus, NotificationChannel, Qualification from app.schemas.common import Website @@ -139,9 +134,10 @@ class UserUpdateRequest(ProfileUpdateRequest): "ACTIVE: fully active member with all privileges" ), ) - role: UserRole | None = Field( - default=None, - description="User role: member, leader, admin, or admin_and_leader", + is_leader: bool | None = Field(default=None, description="Grant leader privileges") + is_admin: bool | None = Field(default=None, description="Grant admin privileges") + is_president: bool | None = Field( + default=None, description="Grant president privileges" ) generation: str | None = Field( default=None, @@ -236,9 +232,9 @@ class UserDetail(BaseModel): description=("Graduation status of the user. One of [학부생, 졸업생, 휴학생, 대학원생]"), examples=["학부생", "졸업생", "휴학생", "대학원생"], ) - role: UserRole = Field( - description="User role determining access level: member, leader, admin, or admin_and_leader" - ) + is_leader: bool = Field(description="Whether the user has leader privileges") + is_admin: bool = Field(description="Whether the user has admin privileges") + is_president: bool = Field(description="Whether the user has president privileges") phone: str | None = Field(description="Contact phone number") affiliation: str | None = Field(description="Current organization or company") bio: str | None = Field(description="Short self-introduction") diff --git a/app/services/request.py b/app/services/request.py index 5fff41e..5b9250e 100644 --- a/app/services/request.py +++ b/app/services/request.py @@ -96,7 +96,7 @@ def _can_view_or_review( actor: User, include_requester: bool = True, ) -> bool: - if actor.is_admin: + if actor.has_admin_access: return True if include_requester and approval_request.requester_id == actor.id: return True @@ -127,7 +127,7 @@ def _build_create_body( request: ApprovalRequestCreateRequest, ) -> tuple[int | None, dict]: target_user_id = request.target_user_id or actor.id - if target_user_id != actor.id and not actor.is_admin: + if target_user_id != actor.id and not actor.has_admin_access: raise ForbiddenError("Cannot create a request for another user") _ensure_user_exists(db, target_user_id) @@ -313,11 +313,11 @@ def list( ) if scope == RequestScope.ALL: - if not actor.is_admin: + if not actor.has_admin_access: raise ForbiddenError("Admin access required") elif scope == RequestScope.SENT: query = query.filter(ApprovalRequest.requester_id == actor.id) - elif not actor.is_admin: + elif not actor.has_admin_access: query = query.filter(_received_condition(db, actor)) if status != RequestStatusFilter.ALL: @@ -394,7 +394,7 @@ def update( @staticmethod def delete(db: Session, *, actor: User, approval_request: ApprovalRequest) -> None: - if not actor.is_admin: + if not actor.has_admin_access: if approval_request.requester_id != actor.id: raise ForbiddenError("Only the requester can delete this request") if approval_request.status != ApprovalStatus.PENDING: diff --git a/app/services/user.py b/app/services/user.py index 54c9fed..310b8ac 100644 --- a/app/services/user.py +++ b/app/services/user.py @@ -127,7 +127,7 @@ def bulk_create_temporary( Temporary members are created with only name and student_id populated; email is NULL and all other columns fall back to their model defaults - (qualification=PENDING, role=MEMBER, etc.). + (qualification=PENDING, is_leader/is_admin/is_president=False, etc.). Returns (created_users, skipped) where skipped is a list of (name, student_id, reason). diff --git a/tests/conftest.py b/tests/conftest.py index 97a1d95..cc53bfb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -37,7 +37,7 @@ from app.config.database import Base, Engine, get_db from app.main import app -from app.models import Qualification, User, UserRole +from app.models import Qualification, User from app.services import UserService # JWT config (must match app/deps/auth.py default) @@ -179,7 +179,7 @@ def admin_user(db: Session) -> User: name="Admin User", generation="26", qualification=Qualification.ACTIVE, - role=UserRole.ADMIN, + is_admin=True, google_id="admin_google_id", ) return user @@ -219,3 +219,54 @@ def active_token(active_user: User) -> str: def admin_token(admin_user: User) -> str: """Create JWT token for admin user.""" return create_access_token(admin_user.id, admin_user.email, admin_user.google_id) + + +@pytest.fixture +def president_user(db: Session) -> User: + """Create a president user for testing.""" + user = UserService.create( + db, + email="president@example.com", + name="President User", + generation="26", + qualification=Qualification.ACTIVE, + is_admin=True, + is_president=True, + google_id="president_google_id", + ) + return user + + +@pytest.fixture +def leader_and_president_user(db: Session) -> User: + """Create a leader_and_president user for testing.""" + user = UserService.create( + db, + email="leader_president@example.com", + name="Leader President User", + generation="26", + qualification=Qualification.ACTIVE, + is_leader=True, + is_admin=True, + is_president=True, + google_id="leader_president_google_id", + ) + return user + + +@pytest.fixture +def president_token(president_user: User) -> str: + """Create JWT token for president user.""" + return create_access_token( + president_user.id, president_user.email, president_user.google_id + ) + + +@pytest.fixture +def leader_and_president_token(leader_and_president_user: User) -> str: + """Create JWT token for leader_and_president user.""" + return create_access_token( + leader_and_president_user.id, + leader_and_president_user.email, + leader_and_president_user.google_id, + ) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index d01c743..9c15e5a 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -679,3 +679,128 @@ def test_deleted_project_not_found( headers={"Authorization": f"Bearer {regular_token}"}, ) assert response.status_code == 404 + + +class TestPresidentAndLeaderRoles: + """Test role properties and access control for PRESIDENT and LEADER_AND_PRESIDENT roles.""" + + def test_president_role_properties(self, client: TestClient, president_user: User): + """President has is_admin=True, is_president=True, is_leader=False""" + assert president_user.is_admin is True + assert president_user.is_president is True + assert president_user.is_leader is False + + def test_leader_and_president_role_properties( + self, client: TestClient, leader_and_president_user: User + ): + """Leader_and_president has is_admin, is_president, and is_leader all True""" + assert leader_and_president_user.is_admin is True + assert leader_and_president_user.is_president is True + assert leader_and_president_user.is_leader is True + + def test_president_can_access_admin_endpoint( + self, client: TestClient, president_token: str + ): + """President can access admin-only endpoints""" + response = client.get( + "/users", + headers={"Authorization": f"Bearer {president_token}"}, + ) + assert response.status_code == 200 + + def test_leader_and_president_can_access_admin_endpoint( + self, client: TestClient, leader_and_president_token: str + ): + """Leader_and_president can access admin-only endpoints""" + response = client.get( + "/users", + headers={"Authorization": f"Bearer {leader_and_president_token}"}, + ) + assert response.status_code == 200 + + def test_regular_user_cannot_access_admin_endpoint( + self, client: TestClient, regular_token: str + ): + """Regular user is denied admin-only endpoints""" + response = client.get( + "/users", + headers={"Authorization": f"Bearer {regular_token}"}, + ) + assert response.status_code == 403 + + def test_president_can_create_project( + self, + client: TestClient, + db: Session, + president_user: User, + president_token: str, + ): + """President (is_admin=True) can create a project""" + response = client.post( + "/projects", + json={ + "name": "President Project", + "started_at": str(date.today()), + "members": [{"user_id": president_user.id, "role": "leader"}], + }, + headers={"Authorization": f"Bearer {president_token}"}, + ) + assert response.status_code == 200 + assert response.json()["data"]["name"] == "President Project" + + def test_president_can_update_project_as_admin( + self, + client: TestClient, + db: Session, + president_user: User, + president_token: str, + admin_user: User, + admin_token: str, + ): + """President can update any project (admin-level access in require_leader_or_admin)""" + # Admin creates the project (president is not a member) + create_resp = client.post( + "/projects", + json={ + "name": "Admin Only Project", + "started_at": str(date.today()), + "members": [{"user_id": admin_user.id, "role": "leader"}], + }, + headers={"Authorization": f"Bearer {admin_token}"}, + ) + assert create_resp.status_code == 200 + project_id = create_resp.json()["data"]["id"] + + # President (not a member) can still update it via admin access + update_resp = client.patch( + f"/projects/{project_id}", + json={"name": "Updated By President"}, + headers={"Authorization": f"Bearer {president_token}"}, + ) + assert update_resp.status_code == 200 + assert update_resp.json()["data"]["name"] == "Updated By President" + + def test_leader_role_alone_not_admin(self, db: Session, client: TestClient): + """Plain LEADER role does not grant admin access""" + from tests.conftest import create_access_token + + leader = UserService.create( + db, + email="plain_leader@example.com", + name="Plain Leader", + generation="26", + qualification=Qualification.ACTIVE, + is_leader=True, + ) + token = create_access_token(leader.id, leader.email, leader.google_id) + + assert leader.is_leader is True + assert leader.is_admin is False + assert leader.is_president is False + + # Should be denied admin-only endpoint + response = client.get( + "/users", + headers={"Authorization": f"Bearer {token}"}, + ) + assert response.status_code == 403 diff --git a/tests/test_member_detail.py b/tests/test_member_detail.py index 19c9b2c..931e250 100644 --- a/tests/test_member_detail.py +++ b/tests/test_member_detail.py @@ -352,7 +352,7 @@ def test_role_change_creates_audit_log( """Role change creates a role_changed audit log entry.""" client.patch( f"/users/{regular_user.id}", - json={"role": "admin"}, + json={"is_admin": True}, headers={"Authorization": f"Bearer {admin_token}"}, ) @@ -363,8 +363,8 @@ def test_role_change_creates_audit_log( logs = response.json()["data"] assert any(log["action"] == "role_changed" for log in logs) role_log = next(log for log in logs if log["action"] == "role_changed") - assert role_log["payload"]["from"] == "member" - assert role_log["payload"]["to"] == "admin" + assert role_log["payload"]["is_admin"]["from"] is False + assert role_log["payload"]["is_admin"]["to"] is True class TestProfileUpdate: