fix user update endpoint (#576)

Author: ciur

.github/workflows/tests.yml

@@ -10,6 +10,7 @@ on:
       - 'ui/**'
       - 'pyproject.toml'
       - '.github/workflows/tests.yml'
+      - 'docker/standard/**'
 
 jobs:
   tests_with_sqlite:
@@ -21,11 +22,11 @@ jobs:
         with:
           python-version: '3.13'
       - name: Install required debian packages
-        run: sudo apt-get install -y poppler-utils  # required by pdf2image
+        run: sudo apt-get update && sudo apt-get install -y poppler-utils  # required by pdf2image
       - name: Install python dependencies
         run: |
           python -m pip install --upgrade pip
-          pip install poetry==1.8
+          pip install poetry==1.8.2
           poetry install --with test && poetry run pytest papermerge/
         env:
           PAPERMERGE__DATABASE__URL: 'sqlite:///test.sqlite'
@@ -54,11 +55,11 @@ jobs:
         with:
           python-version: '3.13'
       - name: Install required debian packages
-        run: sudo apt-get install -y poppler-utils  # required by pdf2image
+        run: sudo apt-get update && sudo apt-get install -y poppler-utils  # required by pdf2image
       - name: Install python dependencies
         run: |
           python -m pip install --upgrade pip
-          pip install poetry==1.8
+          pip install poetry==1.8.2
           poetry install --with test -E pg && poetry run pytest papermerge/
         env:
           PAPERMERGE__DATABASE__URL: 'postgresql://pmguser:pmgpass@localhost:5432/test_pmgdb'

papermerge/core/dbapi.py

@@ -1,4 +1,5 @@
 from .features.page_mngm.db.api import move_pages
+from .features.users.db.api import update_user, get_users_count, change_password, get_user
 from .features.document.db.api import (
     get_last_doc_ver,
     upload,
@@ -12,6 +13,7 @@
     get_doc_ver_pages
 )
 from .features.nodes.db.api import get_nodes
+from .features.groups.db.api import get_group, sync_perms
 from .features.document_types.db.api import (
     create_document_type,
     get_document_types,
@@ -41,5 +43,13 @@
     "get_document_types_without_pagination",
     "delete_document_type",
     "update_document_type",
-    "create_custom_field"
+    "create_custom_field",
+    # users
+    "update_user",
+    "get_user",
+    "change_password",
+    "get_users_count",
+    # groups
+    "get_group",
+    "sync_perms"
 ]

papermerge/core/features/conftest.py

@@ -573,3 +573,23 @@ def _make_tax(title: str, user: orm.User, parent=None):
         return doc
 
     return _make_tax
+
+
+@pytest.fixture()
+def make_group(db_session):
+    def _maker(name: str):
+        group = orm.Group(name=name)
+        db_session.add(group)
+        db_session.commit()
+        return group
+
+    return _maker
+
+
+@pytest.fixture()
+def random_string():
+    from random import choice
+    from string import ascii_uppercase
+
+    ret = "".join(choice(ascii_uppercase) for i in range(12))
+    return ret

papermerge/core/features/groups/db/api.py

@@ -1,5 +1,6 @@
 import logging
 import math
+import uuid
 
 from sqlalchemy import delete, select, func
 from sqlalchemy.orm import joinedload
@@ -12,7 +13,7 @@
 logger = logging.getLogger(__name__)
 
 
-def get_group(db_session: Session, group_id: int) -> schema.GroupDetails:
+def get_group(db_session: Session, group_id: uuid.UUID) -> schema.GroupDetails:
     stmt = (
         select(orm.Group)
         .options(joinedload(orm.Group.permissions))
@@ -74,7 +75,7 @@ def create_group(
 
 
 def update_group(
-    db_session: Session, group_id: int, attrs: schema.UpdateGroup
+    db_session: Session, group_id: uuid.UUID, attrs: schema.UpdateGroup
 ) -> schema.Group:
     stmt = select(orm.Permission).where(orm.Permission.codename.in_(attrs.scopes))
     perms = db_session.execute(stmt).scalars().all()

papermerge/core/features/groups/router.py

@@ -1,4 +1,5 @@
 import logging
+import uuid
 from typing import Annotated
 
 from fastapi import APIRouter, Depends, HTTPException, Security
@@ -59,7 +60,7 @@ def get_groups(
 @router.get("/{group_id}", response_model=schema.GroupDetails)
 @utils.docstring_parameter(scope=scopes.GROUP_VIEW)
 def get_group(
-    group_id: int,
+    group_id: uuid.UUID,
     user: Annotated[
         schema.User, Security(get_current_user, scopes=[scopes.GROUP_VIEW])
     ],
@@ -136,7 +137,7 @@ def delete_group(
 @router.patch("/{group_id}", status_code=200, response_model=schema.Group)
 @utils.docstring_parameter(scope=scopes.GROUP_UPDATE)
 def update_group(
-    group_id: int,
+    group_id: uuid.UUID,
     attrs: schema.UpdateGroup,
     cur_user: Annotated[
         schema.User, Security(get_current_user, scopes=[scopes.GROUP_UPDATE])

papermerge/core/features/groups/tests/conftest.py

@@ -1,6 +1,7 @@
 from sqlalchemy import func
 
-from papermerge.core import schema, db
+from papermerge.core.db.engine import Session
+from papermerge.core import schema, db, dbapi
 from papermerge.core.features.groups.db import orm
 from papermerge.core.tests.types import AuthTestClient
 
@@ -19,6 +20,32 @@ def test_create_group_route(auth_api_client: AuthTestClient, db_session: db.Sess
     assert count_after == 1
 
 
+def test_update_group_route(auth_api_client: AuthTestClient, make_group, db_session):
+    group = make_group(name="demo")
+
+    dbapi.sync_perms(db_session)
+    response = auth_api_client.patch(
+        f"/groups/{group.id}",
+        json={"name": "Admin", "scopes": ["user.view", "custom_field.view"]},
+    )
+
+    assert response.status_code == 200, response.json()
+
+    updated_group = dbapi.get_group(db_session, group_id=group.id)
+
+    assert set(updated_group.scopes) == {"user.view", "custom_field.view"}
+
+
+def test_get_group_details(
+    make_group, auth_api_client: AuthTestClient, db_session: db.Session
+):
+    group = make_group(name="demo")
+
+    response = auth_api_client.get(f"/groups/{group.id}")
+
+    assert response.status_code == 200, response.json()
+
+
 def test_pagination_group_route_basic(auth_api_client: AuthTestClient):
     params = {"page_number": 1, "page_size": 1}
     response = auth_api_client.get("/groups/", params=params)

papermerge/core/features/groups/tests/test_router_groups.py

@@ -255,3 +255,26 @@ def delete_user(
     user = db_session.execute(stmt).scalars().one()
     db_session.delete(user)
     db_session.commit()
+
+
+def get_users_count(db_session: db.Session) -> int:
+    stmt = select(func.count(orm.User.id))
+    return db_session.execute(stmt).scalar()
+
+
+def change_password(
+    db_session: db.Session, user_id: uuid.UUID, password: str
+) -> Tuple[schema.User | None, err_schema.Error | None]:
+    db_user = db_session.get(User, user_id)
+
+    db_user.password = pbkdf2_sha256.hash(password)
+
+    try:
+        db_session.commit()
+    except Exception as e:
+        error = err_schema.Error(messages=[str(e)])
+        return None, error
+
+    user = schema.User.model_validate(db_user)
+
+    return user, None

papermerge/core/features/users/db/api.py

@@ -101,7 +101,7 @@ def create_user(
 
 @router.get(
     "/{user_id}",
-    status_code=201,
+    status_code=200,
     responses={
         404: {
             "description": """No user with specified UUID found""",
@@ -157,18 +157,20 @@ def delete_user(
 
     Required scope: `{scope}`
     """
-    if User.objects.count() == 1:
-        raise HTTPException(
-            status_code=432, detail="Deletion not possible. Only one user left."
-        )
+    with db.Session() as db_session:
+        if usr_dbapi.get_users_count(db_session) == 1:
+            raise HTTPException(
+                status_code=432, detail="Deletion not possible. Only one user left."
+            )
 
     try:
         if os.environ.get("PAPERMERGE__REDIS__URL"):
             delete_user_data.apply_async(kwargs={"user_id": str(user_id)})
         else:
-            delete_user_data(user_id)
-    except User.DoesNotExist:
-        raise HTTPException(status_code=404, detail="Does not exists")
+            usr_dbapi.delete_user(db_session, user_id=user_id)
+    except Exception as e:
+        logger.error(e)
+        raise HTTPException(status_code=469, detail=str(e))
 
 
 @router.patch("/{user_id}", status_code=200, response_model=schema.UserDetails)
@@ -191,3 +193,29 @@ def update_user(
         raise HTTPException(status_code=404, detail=error.model_dump())
 
     return user
+
+
+@router.post("/{user_id}/change-password", status_code=200, response_model=schema.UserDetails)
+@utils.docstring_parameter(scope=scopes.USER_UPDATE)
+def change_user_password(
+    user_id: UUID,
+    attrs: schema.ChangeUserPassword,
+    cur_user: Annotated[
+        schema.User, Security(get_current_user, scopes=[scopes.USER_UPDATE])
+    ],
+) -> schema.UserDetails:
+    """Change user password
+
+    Required scope: `{scope}`
+    """
+    with db.Session() as db_session:
+        user, error = usr_dbapi.change_password(
+            db_session,
+            user_id=UUID(attrs.userId),
+            password=attrs.password
+        )
+
+    if error:
+        raise HTTPException(status_code=469, detail=error.model_dump())
+
+    return user

papermerge/core/features/users/router.py

@@ -12,7 +12,7 @@
 
 
 class Group(BaseModel):
-    id: int
+    id: UUID
     name: str
 
 
@@ -22,7 +22,7 @@ class RemoteUser(BaseModel):
     username: str
     email: str = ""
     name: str = ""
-    groups: list[str] = []
+    groups: list[str] | None = None
 
 
 class User(BaseModel):
@@ -93,7 +93,7 @@ class CreateUser(BaseModel):
     is_superuser: bool
     is_active: bool
     scopes: list[str]  # list of scope names e.g. "user.create", "user.delete"
-    group_ids: list[int]  # list of group IDs e.g. 65, 72
+    group_ids: list[UUID]
 
     # Config
     model_config = ConfigDict(from_attributes=True)
@@ -106,4 +106,9 @@ class UpdateUser(BaseModel):
     is_superuser: bool | None = None
     is_active: bool | None = None
     scopes: list[str] | None = None
-    group_ids: list[int] | None = None
+    group_ids: list[UUID] | None = None
+
+
+class ChangeUserPassword(BaseModel):
+    userId: str
+    password: str

papermerge/core/features/users/schema.py

@@ -37,3 +37,16 @@ def test_user_update(db_session, make_user):
     updated_user = db_session.execute(stmt).scalar()
 
     assert updated_user.is_superuser == True
+
+
+def test_change_password(db_session, make_user):
+    user: orm.User = make_user("momo", is_superuser=False)
+
+    initial_password = user.password
+
+    dbapi.change_password(db_session, user_id=user.id, password="updatedpass")
+
+    stmt = select(orm.User).where(orm.User.id == user.id)
+    updated_user = db_session.execute(stmt).scalar()
+
+    assert updated_user.password != initial_password