Add multi-role permission system with a Role Management admin page

Users can now hold multiple roles, each with its own editable set of create/edit/delete-style permissions across assets, shots, tasks, task assignment, review approve/retake, submissions, uploads, and notes (including internal vs. client note visibility). The 4 existing roles (coordinator/director/artist/developer) are migrated into the new system as system roles, seeded to reproduce today's actual behavior exactly; admins can create custom roles (e.g. "Reviewer", "Outsourcing") via the new Role Management page and assign multiple roles to a user via a new "Manage Roles" action on the Team page. Backend: - New Role/Permission models and role_permissions/user_roles tables, plus a one-off, idempotent seed/backfill migration script. - New require_permission()/user_has_permission() dependency, wired into the actual mutation endpoints across shots/assets/tasks/reviews, always preserving existing ownership- and self-service-based access (e.g. artists editing their own task status, own notes, own uploads, own submissions) as an unconditional fallback alongside the new permission checks - nothing that worked before now requires a role. - New endpoints: PUT/DELETE on task submissions (wires up soft-deletion columns that existed on the model but were never exposed), plus full role CRUD and per-user role assignment. - Along the way: fixed newly-created users not being linked to their matching system role (silently leaving them with zero permissions), and unified an inconsistency between the single vs. bulk task status endpoints that allowed different roles to bulk-update status. Frontend: - Role Management page with a grouped, human-readable permission editor (icons, plain-language action labels, per-resource select-all, live selected count) replacing an earlier dense matrix prototype. - hasPermission() added to the existing usePermission() composable without touching its current isAdmin/isCoordinatorOrAdmin consumers. - Note composer gets an Internal/Client toggle; submissions gain inline edit/delete actions gated the same ownership-or-permission way as notes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-18 20:41:17 +08:00
parent 976ec40b52
commit db2c414c1a
33 changed files with 1697 additions and 137 deletions
@@ -9,7 +9,7 @@ from models.task import Task, TaskType, TaskStatus
 from models.user import User, UserRole
 from schemas.asset import AssetCreate, AssetUpdate, AssetResponse, AssetListResponse, TaskStatusInfo
 from schemas.task import TaskCreate
-from utils.auth import get_current_user_from_token
+from utils.auth import get_current_user_from_token, require_permission
 from services.asset_soft_deletion import AssetSoftDeletionService

 router = APIRouter()
@@ -24,22 +24,6 @@ def get_current_user_with_db(
    return _get_user_from_db(db, token_data["user_id"])


-def require_coordinator_or_admin(
-    token_data: dict = Depends(get_current_user_from_token),
-    db: Session = Depends(get_db)
-):
-    """Require coordinator or admin role."""
-    from utils.auth import _get_user_from_db
-    current_user = _get_user_from_db(db, token_data["user_id"])
-    
-    if current_user.role != UserRole.COORDINATOR and not current_user.is_admin:
-        raise HTTPException(
-            status_code=status.HTTP_403_FORBIDDEN,
-            detail="Insufficient permissions"
-        )
-    return current_user
-
-
 def get_status_sort_order(status: str, project_custom_statuses: list = None) -> int:
    """Get sort order for task status, including custom statuses."""
    # Default system status order
@@ -374,7 +358,7 @@ async def create_asset(
    asset: AssetCreate,
    project_id: int,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('asset', 'create'))
 ):
    """Create a new asset in a project with optional default tasks"""
    # Check project access
@@ -536,7 +520,7 @@ async def create_asset_task(
    asset_id: int,
    task_type: str,  # Changed from TaskType enum to str
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('task', 'create'))
 ):
    """Create a new task for an asset"""
    # Exclude soft deleted assets
@@ -595,7 +579,7 @@ async def update_asset(
    asset_id: int,
    asset_update: AssetUpdate,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('asset', 'edit'))
 ):
    """Update an asset"""
    # Exclude soft deleted assets
@@ -651,7 +635,7 @@ async def update_asset(
 async def get_asset_deletion_info(
    asset_id: int,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('asset', 'delete'))
 ):
    """Get information about what will be deleted when deleting an asset"""
    # Exclude soft deleted assets
@@ -701,7 +685,7 @@ async def get_asset_deletion_info(
 async def delete_asset(
    asset_id: int,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('asset', 'delete'))
 ):
    """Soft delete an asset and all its associated data"""
    # Exclude soft deleted assets
@@ -54,7 +54,11 @@ async def register(user_data: UserRegister, db: Session = Depends(get_db)):
    db.add(new_user)
    db.commit()
    db.refresh(new_user)
-    
+
+    from utils.auth import link_system_role
+    link_system_role(new_user, db)
+    db.commit()
+
    return {
        "message": "User registered successfully. Awaiting admin approval.",
        "user_id": new_user.id
@@ -7,7 +7,7 @@ from database import get_db
 from models.task import Task, Submission, Review, TaskStatus
 from models.user import User, UserRole
 from schemas.task import ReviewCreate, ReviewResponse, SubmissionResponse
-from utils.auth import get_current_user_from_token, _get_user_from_db
+from utils.auth import get_current_user_from_token, _get_user_from_db, require_permission
 from utils.notifications import notification_service

 router = APIRouter()
@@ -126,9 +126,9 @@ async def approve_submission(
    submission_id: int,
    review: ReviewCreate,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_director_coordinator_or_admin)
+    current_user: User = Depends(require_permission('review', 'publish'))
 ):
-    """Approve a submission. Only directors, coordinators, and users with admin permission can approve."""
+    """Approve a submission. Requires review:publish permission (or admin)."""
    
    submission = db.query(Submission).options(
        joinedload(Submission.task)
@@ -189,9 +189,9 @@ async def request_retake(
    submission_id: int,
    review: ReviewCreate,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_director_coordinator_or_admin)
+    current_user: User = Depends(require_permission('review', 'retake'))
 ):
-    """Request a retake for a submission. Only directors, coordinators, and users with admin permission can request retakes."""
+    """Request a retake for a submission. Requires review:retake permission (or admin)."""
    
    submission = db.query(Submission).options(
        joinedload(Submission.task)
@@ -0,0 +1,164 @@
+from fastapi import APIRouter, Depends, HTTPException, status
+from sqlalchemy.orm import Session
+from typing import List
+
+from database import get_db
+from models.role import Role, Permission
+from models.user import User
+from schemas.role import (
+    RoleResponse, RoleCreate, RoleUpdate, PermissionResponse
+)
+from utils.auth import require_admin_permission
+
+router = APIRouter()
+
+
+def _role_to_response(role: Role) -> RoleResponse:
+    return RoleResponse(
+        id=role.id,
+        name=role.name,
+        description=role.description,
+        is_system=role.is_system,
+        permissions=[PermissionResponse.model_validate(p) for p in role.permissions],
+        user_count=len(role.users),
+        created_at=role.created_at,
+        updated_at=role.updated_at,
+    )
+
+
+@router.get("/", response_model=List[RoleResponse])
+async def list_roles(
+    db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin_permission())
+):
+    """List all roles with their permissions and assigned-user count (admin only)."""
+    roles = db.query(Role).order_by(Role.is_system.desc(), Role.name).all()
+    return [_role_to_response(role) for role in roles]
+
+
+@router.get("/permissions", response_model=List[PermissionResponse])
+async def list_permissions(
+    db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin_permission())
+):
+    """List the full permission catalog (admin only)."""
+    return db.query(Permission).order_by(Permission.resource, Permission.action).all()
+
+
+@router.post("/", response_model=RoleResponse, status_code=status.HTTP_201_CREATED)
+async def create_role(
+    role_data: RoleCreate,
+    db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin_permission())
+):
+    """Create a custom role (admin only)."""
+    existing = db.query(Role).filter(Role.name == role_data.name).first()
+    if existing:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail=f"Role '{role_data.name}' already exists"
+        )
+
+    permissions = []
+    if role_data.permission_ids:
+        permissions = db.query(Permission).filter(Permission.id.in_(role_data.permission_ids)).all()
+        if len(permissions) != len(set(role_data.permission_ids)):
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="One or more permission_ids are invalid"
+            )
+
+    role = Role(
+        name=role_data.name,
+        description=role_data.description,
+        is_system=False,
+        permissions=permissions,
+    )
+    db.add(role)
+    db.commit()
+    db.refresh(role)
+    return _role_to_response(role)
+
+
+@router.put("/{role_id}", response_model=RoleResponse)
+async def update_role(
+    role_id: int,
+    role_update: RoleUpdate,
+    db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin_permission())
+):
+    """Update a role's name/description/permissions (admin only).
+
+    System roles cannot be renamed or have their description changed, but
+    their permissions ARE editable, same as custom roles.
+    """
+    role = db.query(Role).filter(Role.id == role_id).first()
+    if not role:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Role not found")
+
+    if role.is_system:
+        if role_update.name is not None and role_update.name != role.name:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="System role name cannot be changed"
+            )
+        if role_update.description is not None and role_update.description != role.description:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="System role description cannot be changed"
+            )
+    else:
+        if role_update.name is not None:
+            existing = db.query(Role).filter(Role.name == role_update.name, Role.id != role_id).first()
+            if existing:
+                raise HTTPException(
+                    status_code=status.HTTP_400_BAD_REQUEST,
+                    detail=f"Role '{role_update.name}' already exists"
+                )
+            role.name = role_update.name
+        if role_update.description is not None:
+            role.description = role_update.description
+
+    if role_update.permission_ids is not None:
+        permissions = db.query(Permission).filter(Permission.id.in_(role_update.permission_ids)).all()
+        if len(permissions) != len(set(role_update.permission_ids)):
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="One or more permission_ids are invalid"
+            )
+        role.permissions = permissions
+
+    db.commit()
+    db.refresh(role)
+    return _role_to_response(role)
+
+
+@router.delete("/{role_id}")
+async def delete_role(
+    role_id: int,
+    db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin_permission())
+):
+    """Delete a custom role (admin only). System roles and roles with assigned users cannot be deleted."""
+    role = db.query(Role).filter(Role.id == role_id).first()
+    if not role:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Role not found")
+
+    if role.is_system:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="System roles cannot be deleted"
+        )
+
+    user_count = len(role.users)
+    if user_count > 0:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail=f"{user_count} user(s) still have this role — reassign them first"
+        )
+
+    role.permissions = []
+    db.delete(role)
+    db.commit()
+
+    return {"message": f"Role '{role.name}' deleted successfully"}
@@ -12,7 +12,7 @@ from schemas.shot import (
    ShotCreate, ShotUpdate, ShotResponse, ShotListResponse,
    BulkShotCreate, BulkShotResponse, TaskStatusInfo
 )
-from utils.auth import get_current_user_from_token
+from utils.auth import get_current_user_from_token, require_permission
 from services.shot_soft_deletion import ShotSoftDeletionService

 router = APIRouter()
@@ -27,22 +27,6 @@ def get_current_user_with_db(
    return _get_user_from_db(db, token_data["user_id"])


-def require_coordinator_or_admin(
-    token_data: dict = Depends(get_current_user_from_token),
-    db: Session = Depends(get_db)
-):
-    """Require coordinator or admin role."""
-    from utils.auth import _get_user_from_db
-    current_user = _get_user_from_db(db, token_data["user_id"])
-    
-    if current_user.role != UserRole.COORDINATOR and not current_user.is_admin:
-        raise HTTPException(
-            status_code=status.HTTP_403_FORBIDDEN,
-            detail="Insufficient permissions"
-        )
-    return current_user
-
-
 def check_episode_access(episode_id: int, current_user: User, db: Session):
    """Check if user has access to the episode and its project."""
    # Debug logging
@@ -381,7 +365,7 @@ async def create_shot(
    episode_id: int,
    create_default_tasks: bool = True,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('shot', 'create'))
 ):
    """Create a new shot in an episode"""
    # Check episode access
@@ -452,7 +436,7 @@ async def create_shots_bulk(
    bulk_shot: BulkShotCreate,
    episode_id: int,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('shot', 'create'))
 ):
    """Create multiple shots with naming pattern and default tasks"""
    # Check episode access
@@ -674,7 +658,7 @@ async def create_shot_task(
    shot_id: int,
    task_type: str,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('task', 'create'))
 ):
    """Create a new task for a shot"""
    # Exclude soft deleted shots
@@ -737,7 +721,7 @@ async def update_shot(
    shot_id: int,
    shot_update: ShotUpdate,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('shot', 'edit'))
 ):
    """Update a shot"""
    from sqlalchemy.orm import selectinload
@@ -818,7 +802,7 @@ async def update_shot(
 async def get_shot_deletion_info(
    shot_id: int,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('shot', 'delete'))
 ):
    """Get information about what will be deleted when deleting a shot"""
    # Exclude soft deleted shots
@@ -868,7 +852,7 @@ async def get_shot_deletion_info(
 async def delete_shot(
    shot_id: int,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_coordinator_or_admin)
+    current_user: User = Depends(require_permission('shot', 'delete'))
 ):
    """Soft delete a shot and all its associated data"""
    # Exclude soft deleted shots
@@ -8,7 +8,7 @@ import json
 from datetime import datetime

 from database import get_db
-from models.task import Task, ProductionNote, TaskAttachment, Submission, Review
+from models.task import Task, ProductionNote, TaskAttachment, Submission, Review, NoteType
 from models.user import User, UserRole, DepartmentRole
 from models.project import Project, ProjectMember
 from models.asset import Asset
@@ -19,10 +19,10 @@ from schemas.task import (
    TaskCreate, TaskUpdate, TaskResponse, TaskListResponse, TaskStatusUpdate, TaskAssignment,
    ProductionNoteCreate, ProductionNoteUpdate, ProductionNoteResponse,
    TaskAttachmentCreate, TaskAttachmentResponse,
-    SubmissionCreate, SubmissionResponse,
+    SubmissionCreate, SubmissionUpdate, SubmissionResponse,
    BulkStatusUpdate, BulkAssignment, BulkActionResult
 )
-from utils.auth import get_current_user_from_token, _get_user_from_db, require_role
+from utils.auth import get_current_user_from_token, _get_user_from_db, require_role, require_permission, user_has_permission
 from utils.notifications import notification_service
 from utils.file_handler import file_handler

@@ -93,20 +93,6 @@ def validate_task_status(db: Session, project_id: int, status_value: str) -> boo
    return False


-def require_admin_or_coordinator(
-    token_data: dict = Depends(get_current_user_from_token),
-    db: Session = Depends(get_db)
-):
-    """Dependency to require admin permission or coordinator role."""
-    current_user = _get_user_from_db(db, token_data["user_id"])
-    if not current_user.is_admin and current_user.role != UserRole.COORDINATOR:
-        raise HTTPException(
-            status_code=status.HTTP_403_FORBIDDEN,
-            detail="Admin permission or Coordinator role required"
-        )
-    return current_user
-
-
 def get_current_user(
    token_data: dict = Depends(get_current_user_from_token),
    db: Session = Depends(get_db)
@@ -372,7 +358,7 @@ async def get_my_tasks(
 async def create_task(
    task: TaskCreate,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_admin_or_coordinator)
+    current_user: User = Depends(require_permission('task', 'create'))
 ):
    """Create a new task. Only coordinators and users with admin permission can create tasks."""
    
@@ -520,7 +506,7 @@ async def bulk_update_task_status(
                    })
                    failed_count += 1
                    continue
-            elif current_user.role not in [UserRole.COORDINATOR, UserRole.DIRECTOR] and not current_user.is_admin:
+            elif not user_has_permission(current_user, 'task', 'change_status', db):
                errors.append({
                    "task_id": task_id,
                    "error": "Insufficient permissions"
@@ -597,7 +583,7 @@ async def bulk_update_task_status(
 async def bulk_assign_tasks(
    bulk_assignment: BulkAssignment,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_admin_or_coordinator)
+    current_user: User = Depends(require_permission('assignment', 'edit'))
 ):
    """
    Assign multiple tasks to a user atomically.
@@ -780,7 +766,7 @@ async def update_task(
        # Artists can only update status
        if task_update.model_dump(exclude_unset=True).keys() - {"status"}:
            raise HTTPException(status_code=403, detail="Artists can only update task status")
-    elif current_user.role != UserRole.COORDINATOR and not current_user.is_admin:
+    elif not user_has_permission(current_user, 'task', 'edit', db):
        raise HTTPException(status_code=403, detail="Not authorized to update tasks")
    
    # Verify assigned user if being updated
@@ -789,7 +775,11 @@ async def update_task(
            assigned_user = db.query(User).filter(User.id == task_update.assigned_user_id).first()
            if not assigned_user:
                raise HTTPException(status_code=404, detail="Assigned user not found")
-            
+
+            assignment_action = 'edit' if task.assigned_user_id else 'create'
+            if not user_has_permission(current_user, 'assignment', assignment_action, db):
+                raise HTTPException(status_code=403, detail="Not authorized to assign this task")
+
            # Check if user is a project member
            project_member = db.query(ProjectMember).filter(
                and_(
@@ -800,6 +790,8 @@ async def update_task(
            if not project_member:
                raise HTTPException(status_code=400, detail="Assigned user is not a member of this project")
        else:
+            if not user_has_permission(current_user, 'assignment', 'delete', db):
+                raise HTTPException(status_code=403, detail="Not authorized to unassign this task")
            task_update.assigned_user_id = None
    
    # Validate status if being updated
@@ -893,9 +885,10 @@ async def update_task_status(
        raise HTTPException(status_code=404, detail="Task not found")
    
    # Permission check
-    if current_user.role == UserRole.ARTIST and task.assigned_user_id != current_user.id:
-        raise HTTPException(status_code=403, detail="Not authorized to update this task")
-    elif current_user.role not in [UserRole.ARTIST, UserRole.COORDINATOR] and not current_user.is_admin:
+    if current_user.role == UserRole.ARTIST:
+        if task.assigned_user_id != current_user.id:
+            raise HTTPException(status_code=403, detail="Not authorized to update this task")
+    elif not user_has_permission(current_user, 'task', 'change_status', db):
        raise HTTPException(status_code=403, detail="Not authorized to update task status")
    
    # Validate the status for the task's project
@@ -966,10 +959,10 @@ async def assign_task(
    task_id: int,
    assignment: TaskAssignment,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_admin_or_coordinator)
+    current_user: User = Depends(get_current_user)
 ):
    """Assign a task to a user with department role filtering."""
-    
+
    task = db.query(Task).outerjoin(Shot, Task.shot_id == Shot.id).outerjoin(Asset, Task.asset_id == Asset.id).filter(
        Task.id == task_id,
        Task.deleted_at.is_(None),
@@ -981,7 +974,13 @@ async def assign_task(
    ).first()
    if not task:
        raise HTTPException(status_code=404, detail="Task not found")
-    
+
+    # Assigning a previously-unassigned task requires assignment:create;
+    # reassigning an already-assigned task requires assignment:edit.
+    assignment_action = 'edit' if task.assigned_user_id else 'create'
+    if not user_has_permission(current_user, 'assignment', assignment_action, db):
+        raise HTTPException(status_code=403, detail="Not authorized to assign this task")
+
    # Verify assigned user exists and is a project member
    assigned_user = db.query(User).filter(User.id == assignment.assigned_user_id).first()
    if not assigned_user:
@@ -1064,7 +1063,7 @@ async def assign_task(
 async def delete_task(
    task_id: int,
    db: Session = Depends(get_db),
-    current_user: User = Depends(require_admin_or_coordinator)
+    current_user: User = Depends(require_permission('task', 'delete'))
 ):
    """Delete a task. Only coordinators and users with admin permission can delete tasks."""
    
@@ -1118,7 +1117,18 @@ async def get_task_notes(
        ProductionNote.task_id == task_id,
        ProductionNote.deleted_at.is_(None)
    ).order_by(ProductionNote.created_at).all()
-    
+
+    # Filter by note-type view permission (a note whose type the viewer can't
+    # see is dropped entirely - any reply to it is dropped too, since it
+    # would otherwise reference content the viewer isn't allowed to see)
+    can_view_internal = user_has_permission(current_user, 'note', 'view_internal', db)
+    can_view_client = user_has_permission(current_user, 'note', 'view_client', db)
+    notes = [
+        note for note in notes
+        if (note.note_type == NoteType.INTERNAL and can_view_internal)
+        or (note.note_type == NoteType.CLIENT and can_view_client)
+    ]
+
    # Build threaded structure
    notes_dict = {}
    root_notes = []
@@ -1127,6 +1137,7 @@ async def get_task_notes(
        note_data = {
            "id": note.id,
            "content": note.content,
+            "note_type": note.note_type,
            "parent_note_id": note.parent_note_id,
            "task_id": note.task_id,
            "user_id": note.user_id,
@@ -1171,10 +1182,13 @@ async def create_task_note(
    if not task:
        raise HTTPException(status_code=404, detail="Task not found")
    
-    # Artists can only add notes to their own tasks
-    if current_user.role == UserRole.ARTIST and task.assigned_user_id != current_user.id:
-        raise HTTPException(status_code=403, detail="Not authorized to add notes to this task")
-    
+    # Artists can only add notes to their own tasks; everyone else needs note:create
+    if current_user.role == UserRole.ARTIST:
+        if task.assigned_user_id != current_user.id:
+            raise HTTPException(status_code=403, detail="Not authorized to add notes to this task")
+    elif not user_has_permission(current_user, 'note', 'create', db):
+        raise HTTPException(status_code=403, detail="Not authorized to add notes")
+
    # Verify parent note exists if specified
    if note.parent_note_id:
        parent_note = db.query(ProductionNote).filter(
@@ -1192,6 +1206,7 @@ async def create_task_note(
        task_id=task_id,
        user_id=current_user.id,
        content=note.content,
+        note_type=note.note_type,
        parent_note_id=note.parent_note_id
    )
    db.add(db_note)
@@ -1206,6 +1221,7 @@ async def create_task_note(
    note_data = {
        "id": db_note.id,
        "content": db_note.content,
+        "note_type": db_note.note_type,
        "parent_note_id": db_note.parent_note_id,
        "task_id": db_note.task_id,
        "user_id": db_note.user_id,
@@ -1217,7 +1233,7 @@ async def create_task_note(
        "user_avatar_url": db_note.user.avatar_url,
        "child_notes": []
    }
-    
+
    return ProductionNoteResponse(**note_data)


@@ -1244,8 +1260,9 @@ async def update_task_note(
    if not note:
        raise HTTPException(status_code=404, detail="Note not found")
    
-    # Users can only update their own notes, unless they have admin permission
-    if note.user_id != current_user.id and not current_user.is_admin:
+    # Users can only update their own notes, unless they have admin permission or note:edit
+    if (note.user_id != current_user.id and not current_user.is_admin
+            and not user_has_permission(current_user, 'note', 'edit', db)):
        raise HTTPException(status_code=403, detail="Not authorized to update this note")
    
    note.content = note_update.content
@@ -1255,6 +1272,7 @@ async def update_task_note(
    note_data = {
        "id": note.id,
        "content": note.content,
+        "note_type": note.note_type,
        "parent_note_id": note.parent_note_id,
        "task_id": note.task_id,
        "user_id": note.user_id,
@@ -1290,8 +1308,9 @@ async def delete_task_note(
    if not note:
        raise HTTPException(status_code=404, detail="Note not found")
    
-    # Users can only delete their own notes, unless they have admin permission
-    if note.user_id != current_user.id and not current_user.is_admin:
+    # Users can only delete their own notes, unless they have admin permission or note:delete
+    if (note.user_id != current_user.id and not current_user.is_admin
+            and not user_has_permission(current_user, 'note', 'delete', db)):
        raise HTTPException(status_code=403, detail="Not authorized to delete this note")
    
    db.delete(note)
@@ -1368,9 +1387,12 @@ async def upload_task_attachment(
    if not task:
        raise HTTPException(status_code=404, detail="Task not found")
    
-    # Artists can only upload attachments to their own tasks
-    if current_user.role == UserRole.ARTIST and task.assigned_user_id != current_user.id:
-        raise HTTPException(status_code=403, detail="Not authorized to upload attachments to this task")
+    # Artists can only upload attachments to their own tasks; everyone else needs upload:create
+    if current_user.role == UserRole.ARTIST:
+        if task.assigned_user_id != current_user.id:
+            raise HTTPException(status_code=403, detail="Not authorized to upload attachments to this task")
+    elif not user_has_permission(current_user, 'upload', 'create', db):
+        raise HTTPException(status_code=403, detail="Not authorized to upload attachments")
    
    # Validate file using file handler
    file_handler.validate_file(file, file_handler.MAX_ATTACHMENT_SIZE, db)
@@ -1443,9 +1465,9 @@ async def delete_task_attachment(
    if not attachment:
        raise HTTPException(status_code=404, detail="Attachment not found")
    
-    # Users can only delete their own attachments, unless they're admin/coordinator
-    if (attachment.user_id != current_user.id and 
-        not current_user.is_admin and current_user.role != UserRole.COORDINATOR):
+    # Users can only delete their own attachments, unless they have admin permission or upload:delete
+    if (attachment.user_id != current_user.id and not current_user.is_admin
+            and not user_has_permission(current_user, 'upload', 'delete', db)):
        raise HTTPException(status_code=403, detail="Not authorized to delete this attachment")
    
    # Delete file from filesystem using file handler
@@ -1534,8 +1556,9 @@ async def submit_work(
    if not task:
        raise HTTPException(status_code=404, detail="Task not found")
    
-    # Only assigned artist can submit work
-    if task.assigned_user_id != current_user.id:
+    # Only the assigned artist can submit work, unless the caller holds submission:create
+    if (task.assigned_user_id != current_user.id
+            and not user_has_permission(current_user, 'submission', 'create', db)):
        raise HTTPException(status_code=403, detail="Only the assigned artist can submit work for this task")
    
    # Validate file using file handler
@@ -1598,5 +1621,84 @@ async def submit_work(
        "thumbnail_url": f"/files/submissions/{db_submission.id}?thumbnail=true" if file_handler.is_image_file(db_submission.file_path) else None,
        "stream_url": f"/files/submissions/{db_submission.id}/stream" if file_handler.is_video_file(db_submission.file_path) else None
    }
-    
-    return SubmissionResponse(**submission_data)
+
+    return SubmissionResponse(**submission_data)
+
+
+@router.put("/{task_id}/submissions/{submission_id}", response_model=SubmissionResponse)
+async def update_task_submission(
+    task_id: int,
+    submission_id: int,
+    submission_update: SubmissionUpdate,
+    db: Session = Depends(get_db),
+    current_user: User = Depends(get_current_user)
+):
+    """Update a submission's notes. Users can only update their own submissions."""
+
+    submission = db.query(Submission).options(
+        joinedload(Submission.user)
+    ).filter(
+        Submission.id == submission_id,
+        Submission.task_id == task_id,
+        Submission.deleted_at.is_(None)
+    ).first()
+    if not submission:
+        raise HTTPException(status_code=404, detail="Submission not found")
+
+    # Users can only update their own submissions, unless they have admin permission or submission:edit
+    if (submission.user_id != current_user.id and not current_user.is_admin
+            and not user_has_permission(current_user, 'submission', 'edit', db)):
+        raise HTTPException(status_code=403, detail="Not authorized to update this submission")
+
+    if submission_update.notes is not None:
+        submission.notes = submission_update.notes
+    db.commit()
+    db.refresh(submission)
+
+    submission_data = {
+        "id": submission.id,
+        "task_id": submission.task_id,
+        "user_id": submission.user_id,
+        "file_path": submission.file_path,
+        "file_name": submission.file_name,
+        "version_number": submission.version_number,
+        "notes": submission.notes,
+        "submitted_at": submission.submitted_at,
+        "user_first_name": submission.user.first_name,
+        "user_last_name": submission.user.last_name,
+        "latest_review": None,
+        "download_url": f"/files/submissions/{submission.id}",
+        "thumbnail_url": f"/files/submissions/{submission.id}?thumbnail=true" if file_handler.is_image_file(submission.file_path) else None,
+        "stream_url": f"/files/submissions/{submission.id}/stream" if file_handler.is_video_file(submission.file_path) else None
+    }
+
+    return SubmissionResponse(**submission_data)
+
+
+@router.delete("/{task_id}/submissions/{submission_id}")
+async def delete_task_submission(
+    task_id: int,
+    submission_id: int,
+    db: Session = Depends(get_db),
+    current_user: User = Depends(get_current_user)
+):
+    """Soft delete a submission. Users can only delete their own submissions."""
+
+    submission = db.query(Submission).filter(
+        Submission.id == submission_id,
+        Submission.task_id == task_id,
+        Submission.deleted_at.is_(None)
+    ).first()
+    if not submission:
+        raise HTTPException(status_code=404, detail="Submission not found")
+
+    # Users can only delete their own submissions, unless they have admin permission or submission:delete
+    if (submission.user_id != current_user.id and not current_user.is_admin
+            and not user_has_permission(current_user, 'submission', 'delete', db)):
+        raise HTTPException(status_code=403, detail="Not authorized to delete this submission")
+
+    submission.deleted_at = datetime.utcnow()
+    submission.deleted_by = current_user.id
+    db.commit()
+
+    return {"message": "Submission deleted successfully"}
@@ -1,5 +1,5 @@
 from fastapi import APIRouter, Depends, HTTPException, status, UploadFile, File
-from sqlalchemy.orm import Session
+from sqlalchemy.orm import Session, selectinload
 from typing import List, Optional
 from passlib.context import CryptContext
 from pathlib import Path
@@ -12,7 +12,9 @@ from database import get_db
 from models.user import User, UserRole
 from models.project import ProjectMember
 from models.task import Task
+from models.role import Role
 from schemas.user import UserResponse, UserApproval, UserRoleUpdate, UserUpdate, UserAdminUpdate, UserAdminCreate, UserAdminEdit, UserPasswordReset, UserPasswordChange
+from schemas.role import UserRolesUpdate
 from utils.auth import get_current_user_from_token, _get_user_from_db, require_admin_permission

 pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
@@ -98,7 +100,7 @@ async def update_user_role(
    
    user.role = role_data.role
    db.commit()
-    
+
    return {
        "message": f"User {user.email} role updated to {role_data.role}",
        "user_id": user.id,
@@ -106,6 +108,42 @@ async def update_user_role(
    }


+@router.put("/{user_id}/roles", response_model=dict)
+async def update_user_roles(
+    user_id: int,
+    roles_data: UserRolesUpdate,
+    db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin_permission_with_db)
+):
+    """Replace a user's full set of assigned custom roles (Admin permission required).
+
+    This is the new multi-role system and is independent of the legacy
+    single `role` field updated by PUT /{user_id}/role above.
+    """
+    user = db.query(User).filter(User.id == user_id).first()
+    if not user:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail="User not found"
+        )
+
+    roles = db.query(Role).filter(Role.id.in_(roles_data.role_ids)).all()
+    if len(roles) != len(set(roles_data.role_ids)):
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="One or more role_ids are invalid"
+        )
+
+    user.roles = roles
+    db.commit()
+
+    return {
+        "message": f"User {user.email} roles updated",
+        "user_id": user.id,
+        "role_ids": [role.id for role in user.roles]
+    }
+
+
@router.put("/{user_id}/admin", response_model=dict)
 async def update_user_admin_permission(
    user_id: int,
@@ -149,7 +187,7 @@ async def list_users(
    current_user: User = Depends(require_admin_or_coordinator)
 ):
    """List all users (Admin and Coordinator only)."""
-    users = db.query(User).offset(skip).limit(limit).all()
+    users = db.query(User).options(selectinload(User.roles)).offset(skip).limit(limit).all()
    return users


@@ -169,7 +207,10 @@ async def get_current_user_profile(
    db: Session = Depends(get_db)
 ):
    """Get current user's profile."""
+    from utils.auth import compute_effective_permissions
+
    current_user = _get_user_from_db(db, token_data["user_id"])
+    current_user.permissions = compute_effective_permissions(current_user, db)
    return current_user


@@ -214,7 +255,7 @@ async def get_user(
    current_user: User = Depends(require_admin_or_coordinator)
 ):
    """Get user by ID (Admin and Coordinator only)."""
-    user = db.query(User).filter(User.id == user_id).first()
+    user = db.query(User).options(selectinload(User.roles)).filter(User.id == user_id).first()
    if not user:
        raise HTTPException(
            status_code=status.HTTP_404_NOT_FOUND,
@@ -255,7 +296,12 @@ async def admin_create_user(
    db.add(new_user)
    db.commit()
    db.refresh(new_user)
-    
+
+    from utils.auth import link_system_role
+    link_system_role(new_user, db)
+    db.commit()
+    db.refresh(new_user)
+
    return new_user