allow global/collection editors to grant upload access to titles

backend/src/cms_backend/api/routes/titles.py

@@ -668,9 +668,14 @@ class TitleUploaderGrantSchema(BaseModel):
     account_id: UUID
 
 
+class AvailableTitleUploadersGetSchema(BaseModel):
+    skip: SkipField = 0
+    limit: LimitFieldMax200 = 20
+
+
 @router.get(
     "/{title_identifier}/uploaders",
-    dependencies=[Depends(require_permission(namespace="account", name="update"))],
+    dependencies=[Depends(require_permission(namespace="title", name="update"))],
 )
 def get_title_uploaders(
     title_identifier: Annotated[NotEmptyString, Path()],
@@ -701,9 +706,45 @@ def get_title_uploaders(
     )
 
 
+@router.get(
+    "/{title_identifier}/uploaders/available",
+    dependencies=[Depends(require_permission(namespace="title", name="update"))],
+)
+def get_available_title_uploaders(
+    title_identifier: Annotated[NotEmptyString, Path()],
+    params: Annotated[AvailableTitleUploadersGetSchema, Query()],
+    session: Annotated[OrmSession, Depends(gen_dbsession)],
+    accessible_collection_ids: Annotated[
+        Sequence[UUID] | None, Depends(get_accessible_collection_ids)
+    ],
+    accessible_title_ids: Annotated[
+        Sequence[UUID] | None, Depends(get_accessible_title_ids)
+    ],
+) -> ListResponse[AccountSchema]:
+    """List title-uploader accounts which may be granted upload access to a title"""
+    title = db_title.get_title(
+        session,
+        title_identifier=title_identifier,
+        accessible_collection_ids=accessible_collection_ids,
+        accessible_title_ids=accessible_title_ids,
+    )
+    results = db_title_permission.get_available_title_uploaders(
+        session, title.id, skip=params.skip, limit=params.limit
+    )
+    return ListResponse[AccountSchema](
+        meta=calculate_pagination_metadata(
+            nb_records=results.nb_records,
+            skip=params.skip,
+            limit=params.limit,
+            page_size=len(results.records),
+        ),
+        items=results.records,
+    )
+
+
 @router.post(
     "/{title_identifier}/uploaders",
-    dependencies=[Depends(require_permission(namespace="account", name="update"))],
+    dependencies=[Depends(require_permission(namespace="title", name="update"))],
 )
 def grant_title_uploader(
     title_identifier: Annotated[NotEmptyString, Path()],
@@ -738,7 +779,7 @@ def grant_title_uploader(
 
 @router.delete(
     "/{title_identifier}/uploaders/{account_id}",
-    dependencies=[Depends(require_permission(namespace="account", name="update"))],
+    dependencies=[Depends(require_permission(namespace="title", name="update"))],
 )
 def revoke_title_uploader(
     title_identifier: Annotated[NotEmptyString, Path()],

backend/src/cms_backend/db/title_permission.py

@@ -1,11 +1,14 @@
 from collections.abc import Sequence
 from uuid import UUID
 
-from sqlalchemy import delete, select
+from sqlalchemy import delete, exists, select
 from sqlalchemy.orm import Session as OrmSession
 
+from cms_backend.db import account as db_account
+from cms_backend.db import count_from_stmt
 from cms_backend.db.models import Account, TitlePermission
 from cms_backend.roles import RoleEnum
+from cms_backend.schemas.orms import AccountSchema, ListResult
 
 
 def get_accessible_title_ids(
@@ -49,6 +52,31 @@ def get_title_uploaders(session: OrmSession, title_id: UUID) -> Sequence[Account
     ).all()
 
 
+def get_available_title_uploaders(
+    session: OrmSession, title_id: UUID, *, skip: int, limit: int
+) -> ListResult[AccountSchema]:
+    """Get title-uploader accounts not yet granted upload access to a title."""
+    stmt = (
+        select(Account)
+        .where(
+            Account.deleted.is_(False),
+            Account.role == RoleEnum.TITLE_UPLOADER,
+            ~exists().where(
+                TitlePermission.title_id == title_id,
+                TitlePermission.account_id == Account.id,
+            ),
+        )
+        .order_by(Account.display_name, Account.id)
+    )
+    return ListResult[AccountSchema](
+        nb_records=count_from_stmt(session, stmt),
+        records=[
+            db_account.create_account_schema(account)
+            for account in session.scalars(stmt.offset(skip).limit(limit)).all()
+        ],
+    )
+
+
 def get_title_permission_or_none(
     session: OrmSession, title_id: UUID, account_id: UUID
 ) -> TitlePermission | None:

backend/tests/api/routes/test_titles.py

@@ -14,6 +14,7 @@
 from cms_backend.api.context import Context as ApiContext
 from cms_backend.api.token import generate_access_token
 from cms_backend.context import Context
+from cms_backend.db.collection_permission import create_collection_permission
 from cms_backend.db.models import (
     Account,
     Book,
@@ -1507,8 +1508,20 @@ def test_grant_title_uploader_rejects_other_roles(
     ["permission", "expected_status_code"],
     [
         pytest.param(RoleEnum.ADMIN, HTTPStatus.NO_CONTENT, id="admin"),
+        pytest.param(RoleEnum.GLOBAL_EDITOR, HTTPStatus.NO_CONTENT, id="global-editor"),
+        pytest.param(
+            RoleEnum.TITLE_UPLOADER, HTTPStatus.UNAUTHORIZED, id="title-uploader"
+        ),
+        pytest.param(
+            RoleEnum.GLOBAL_VIEWER, HTTPStatus.UNAUTHORIZED, id="global-viewer"
+        ),
         pytest.param(
-            RoleEnum.GLOBAL_EDITOR, HTTPStatus.UNAUTHORIZED, id="global-editor"
+            RoleEnum.COLLECTION_VIEWER,
+            HTTPStatus.UNAUTHORIZED,
+            id="collection-viewer",
+        ),
+        pytest.param(
+            RoleEnum.PUBLIC_VIEWER, HTTPStatus.UNAUTHORIZED, id="public-viewer"
         ),
     ],
 )
@@ -1519,7 +1532,7 @@ def test_grant_title_uploader_required_permissions(
     permission: RoleEnum,
     expected_status_code: HTTPStatus,
 ):
-    """Granting upload access requires the account:update permission."""
+    """Granting upload access requires the title:update permission."""
     wikipedia_en_all = create_title(name="wikipedia_en_all")
     account1 = create_account(permission=RoleEnum.TITLE_UPLOADER)
     account2 = create_account(permission=permission)
@@ -1533,3 +1546,113 @@ def test_grant_title_uploader_required_permissions(
         headers={"Authorization": f"Bearer {access_token}"},
     )
     assert response.status_code == expected_status_code
+
+
+def test_collection_editor_can_grant_uploader_on_editable_title(
+    client: TestClient,
+    dbsession: OrmSession,
+    create_title: Callable[..., Title],
+    create_collection: Callable[..., Collection],
+    create_account: Callable[..., Account],
+):
+    """A collection editor can grant upload access for a title in their collection."""
+    wikipedia_en_all = create_title(name="wikipedia_en_all")
+    collection = create_collection(
+        title_ids_with_paths=[(wikipedia_en_all.id, "other")]
+    )
+    editor = create_account(permission=RoleEnum.COLLECTION_EDITOR)
+    create_collection_permission(dbsession, collection.id, editor.id)
+    uploader = create_account(permission=RoleEnum.TITLE_UPLOADER)
+    access_token = generate_access_token(issue_time=getnow(), account_id=str(editor.id))
+    headers = {"Authorization": f"Bearer {access_token}"}
+
+    response = client.post(
+        f"/v1/titles/{wikipedia_en_all.name}/uploaders",
+        json={"account_id": str(uploader.id)},
+        headers=headers,
+    )
+    assert response.status_code == HTTPStatus.NO_CONTENT
+
+    response = client.get(
+        f"/v1/titles/{wikipedia_en_all.name}/uploaders", headers=headers
+    )
+    assert response.status_code == HTTPStatus.OK
+    assert [item["id"] for item in response.json()["items"]] == [str(uploader.id)]
+
+    response = client.delete(
+        f"/v1/titles/{wikipedia_en_all.name}/uploaders/{uploader.id}",
+        headers=headers,
+    )
+    assert response.status_code == HTTPStatus.NO_CONTENT
+
+
+def test_collection_editor_cannot_grant_uploader_on_other_title(
+    client: TestClient,
+    dbsession: OrmSession,
+    create_title: Callable[..., Title],
+    create_collection: Callable[..., Collection],
+    create_account: Callable[..., Account],
+):
+    """A collection editor cannot grant upload access for other collections' titles."""
+    wikipedia_en_all = create_title(name="wikipedia_en_all")
+    create_collection(title_ids_with_paths=[(wikipedia_en_all.id, "other")])
+    other_collection = create_collection()
+    editor = create_account(permission=RoleEnum.COLLECTION_EDITOR)
+    create_collection_permission(dbsession, other_collection.id, editor.id)
+    uploader = create_account(permission=RoleEnum.TITLE_UPLOADER)
+    access_token = generate_access_token(issue_time=getnow(), account_id=str(editor.id))
+
+    response = client.post(
+        f"/v1/titles/{wikipedia_en_all.name}/uploaders",
+        json={"account_id": str(uploader.id)},
+        headers={"Authorization": f"Bearer {access_token}"},
+    )
+    assert response.status_code == HTTPStatus.NOT_FOUND
+
+
+def test_get_available_title_uploaders(
+    client: TestClient,
+    dbsession: OrmSession,
+    create_title: Callable[..., Title],
+    create_collection: Callable[..., Collection],
+    create_account: Callable[..., Account],
+):
+    """Editors can list title-uploader accounts without account:read permission."""
+    wikipedia_en_all = create_title(name="wikipedia_en_all")
+    collection = create_collection(
+        title_ids_with_paths=[(wikipedia_en_all.id, "other")]
+    )
+    editor = create_account(permission=RoleEnum.COLLECTION_EDITOR)
+    create_collection_permission(dbsession, collection.id, editor.id)
+    granted_uploader = create_account(permission=RoleEnum.TITLE_UPLOADER)
+    dbsession.add(
+        TitlePermission(title_id=wikipedia_en_all.id, account_id=granted_uploader.id)
+    )
+    dbsession.flush()
+
+    for _ in range(2):
+        create_account(permission=RoleEnum.TITLE_UPLOADER)
+
+    access_token = generate_access_token(issue_time=getnow(), account_id=str(editor.id))
+    headers = {"Authorization": f"Bearer {access_token}"}
+
+    response = client.get(
+        f"/v1/titles/{wikipedia_en_all.name}/uploaders/available?skip=0&limit=1",
+        headers=headers,
+    )
+    assert response.status_code == HTTPStatus.OK
+    first_page = response.json()
+    assert first_page["meta"]["count"] == 2
+    assert first_page["meta"]["skip"] == 0
+    assert first_page["meta"]["limit"] == 1
+    assert len(first_page["items"]) == 1
+
+    response = client.get(
+        f"/v1/titles/{wikipedia_en_all.name}/uploaders/available?skip=1&limit=1",
+        headers=headers,
+    )
+    assert response.status_code == HTTPStatus.OK
+    second_page = response.json()
+    assert second_page["meta"]["count"] == 2
+    assert second_page["meta"]["skip"] == 1
+    assert len(second_page["items"]) == 1

backend/tests/conftest.py

@@ -500,11 +500,12 @@ def _create_account(
         permission: RoleEnum = RoleEnum.ADMIN,
         password: str | None = None,
     ):
-        username = username or faker.first_name()
+        display_name = username or faker.first_name()
+        username = username or f"{display_name}_{uuid4().hex[:8]}"
         account = Account(
             role=permission,
             username=username,
-            display_name=username,
+            display_name=display_name,
             idp_sub=uuid4(),
             password_hash=(
                 None if password is None else generate_password_hash(password)

frontend/src/components/TitleUploadersDialog.vue

@@ -79,7 +79,6 @@ import { computed, ref, watch } from 'vue'
 import ConfirmDialog from '@/components/ConfirmDialog.vue'
 import { useNotificationStore } from '@/stores/notification'
 import { useTitleStore } from '@/stores/title'
-import { useUserStore } from '@/stores/user'
 import type { User } from '@/types/user'
 
 interface Props {
@@ -94,7 +93,6 @@ const emit = defineEmits<{
 }>()
 
 const titleStore = useTitleStore()
-const userStore = useUserStore()
 const notificationStore = useNotificationStore()
 
 const errors = ref<string[]>([])
@@ -121,7 +119,7 @@ const loadData = async () => {
   loading.value = true
   errors.value = []
   const [accounts, currentUploaders] = await Promise.all([
-    userStore.fetchUsers(0, 200, undefined, false, false, 'title-uploader'),
+    titleStore.fetchAvailableTitleUploaders(props.titleId),
     titleStore.fetchTitleUploaders(props.titleId),
   ])
   titleUploaderAccounts.value = accounts ?? []

frontend/src/stores/title.ts

@@ -281,6 +281,26 @@ export const useTitleStore = defineStore('title', () => {
     }
   }
 
+  const fetchAvailableTitleUploaders = async (
+    titleId: string,
+    skip: number = 0,
+    limit: number = 200,
+  ): Promise<User[]> => {
+    const service = await authStore.getApiService('titles')
+    try {
+      const response = await service.get<null, ListResponse<User>>(
+        `/${titleId}/uploaders/available`,
+        { params: { skip, limit } },
+      )
+      errors.value = []
+      return response.items
+    } catch (_error) {
+      console.error('Failed to fetch available title uploaders', _error)
+      errors.value = translateErrors(_error as ErrorResponse)
+      return []
+    }
+  }
+
   const fetchTitleUploaders = async (titleId: string): Promise<User[]> => {
     const service = await authStore.getApiService('titles')
     try {
@@ -345,6 +365,7 @@ export const useTitleStore = defineStore('title', () => {
     completeZimUploadByFile,
     completeZimUploadByUrl,
     fetchTitleUploaders,
+    fetchAvailableTitleUploaders,
     grantTitleUploader,
     revokeTitleUploader,
   }

frontend/src/views/TitleView.vue

@@ -724,7 +724,7 @@ const canArchiveTitle = computed(() => authStore.hasPermission('title', 'archive
 
 const canUploadZim = computed(() => authStore.hasPermission('book', 'create'))
 
-const canManageUploaders = computed(() => authStore.hasPermission('account', 'update'))
+const canManageUploaders = computed(() => authStore.hasPermission('title', 'update'))
 const showUploadersDialog = ref(false)
 
 const titleUploadsViewRef = ref<InstanceType<typeof TitleUploadsView>>()