Skip to content

Commit

Permalink
fix user update endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
ciur committed Jan 16, 2025
1 parent d2123ed commit e0d748e
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 28 deletions.
7 changes: 6 additions & 1 deletion papermerge/core/dbapi.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -41,5 +42,9 @@
"get_document_types_without_pagination",
"delete_document_type",
"update_document_type",
"create_custom_field"
"create_custom_field",
"update_user",
"get_user",
"change_password",
"get_users_count"
]
11 changes: 11 additions & 0 deletions papermerge/core/features/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,3 +573,14 @@ 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
13 changes: 0 additions & 13 deletions papermerge/core/features/groups/tests/conftest.py

This file was deleted.

25 changes: 25 additions & 0 deletions papermerge/core/features/users/db/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,28 @@ 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
44 changes: 36 additions & 8 deletions papermerge/core/features/users/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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""",
Expand Down Expand Up @@ -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)
Expand All @@ -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
13 changes: 9 additions & 4 deletions papermerge/core/features/users/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@


class Group(BaseModel):
id: int
id: UUID
name: str


Expand All @@ -22,7 +22,7 @@ class RemoteUser(BaseModel):
username: str
email: str = ""
name: str = ""
groups: list[str] = []
groups: list[str] | None = None


class User(BaseModel):
Expand Down Expand Up @@ -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)
Expand All @@ -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
80 changes: 79 additions & 1 deletion papermerge/core/features/users/tests/test_users_router.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
from papermerge.core import schema
import uuid
from passlib.hash import pbkdf2_sha256
from sqlalchemy import select

from papermerge.core import schema, dbapi, orm
from papermerge.core.tests.types import AuthTestClient



def test_list_users(make_user, auth_api_client: AuthTestClient):
for i in range(3):
make_user(username=f"user {i}")
Expand All @@ -11,6 +16,79 @@ def test_list_users(make_user, auth_api_client: AuthTestClient):
assert response.status_code == 200, response.json()


def test_create_user(make_group, auth_api_client: AuthTestClient):
group = make_group(name="demo")
data = {
"username": "friedrich",
"email": "[email protected]",
"group_ids": [str(group.id)],
"scopes": [],
"is_active": True,
"is_superuser": False,
"password": "blah"
}
response = auth_api_client.post("/users/", json=data)

assert response.status_code == 201, response.json()


def test_get_user_details(
make_user,
make_group,
auth_api_client: AuthTestClient,
db_session
):
"""In this scenario user belongs to one group"""
user = make_user(username="Karl")
group = make_group(name="demo")

attrs = schema.UpdateUser(group_ids=[group.id])
dbapi.update_user(db_session, user_id=user.id, attrs=attrs)

response = auth_api_client.get(f"/users/{user.id}")

assert response.status_code == 200, response.json()


def test_delete_user(
make_user,
make_group,
auth_api_client: AuthTestClient,
db_session
):
user = make_user(username="Karl")
group = make_group(name="demo")

attrs = schema.UpdateUser(group_ids=[group.id])
dbapi.update_user(db_session, user_id=user.id, attrs=attrs)

response = auth_api_client.delete(f"/users/{user.id}")

assert response.status_code == 204, response.text


def test_change_user_password(
make_user,
auth_api_client: AuthTestClient,
db_session
):
user = make_user(username="Karl")

data = {
"userId": str(user.id),
"password": "secret"
}

response = auth_api_client.post(f"/users/{user.id}/change-password", json=data)

assert response.status_code == 200, response.text
stmt = select(orm.User).where(orm.User.id == user.id)

db_user = db_session.execute(stmt).scalar()

assert pbkdf2_sha256.verify("secret", db_user.password)


def test_users_paginated_result__9_items_first_page(
make_user, auth_api_client: AuthTestClient
):
Expand Down
3 changes: 2 additions & 1 deletion papermerge/core/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
ExtractStrategy,
MoveStrategy
)
from .features.users.schema import User, CreateUser, UserDetails, UpdateUser
from .features.users.schema import User, CreateUser, UserDetails, UpdateUser, ChangeUserPassword
from .features.custom_fields.schema import CustomField, UpdateCustomField, CustomFieldType, CustomFieldValue
from .features.tags.schema import Tag, UpdateTag, CreateTag
from .features.document_types.schema import DocumentType, UpdateDocumentType, CreateDocumentType
Expand All @@ -39,6 +39,7 @@
'CreateUser',
'UpdateUser',
'UserDetails',
'ChangeUserPassword',
'Tag',
'UpdateTag',
'CreateTag',
Expand Down

0 comments on commit e0d748e

Please sign in to comment.