221 lines
7.2 KiB
Python
221 lines
7.2 KiB
Python
# Copyright (c) 2026 Patrick Motsch
|
|
# All rights reserved.
|
|
"""
|
|
Integration tests for ``AppObjects.updateUser`` partial-update semantics.
|
|
|
|
Regression for the silent flag-flip bug (``isSysAdmin`` <-> ``isPlatformAdmin``)
|
|
on inline toggles in ``Admin > System > Mandanten/Benutzer``:
|
|
|
|
Symptom
|
|
-------
|
|
Toggling one privileged flag in the user table flipped the OTHER privileged
|
|
flag back to its Pydantic default (``False``).
|
|
|
|
Root cause
|
|
----------
|
|
The PUT ``/api/users/{id}`` route bound ``userData: User = Body(...)``. Pydantic
|
|
filled every field that the client did not explicitly send with model defaults
|
|
(``isSysAdmin=False``, ``isPlatformAdmin=False``). Combined with
|
|
``allowAdminFlagChange=True`` (PlatformAdmin updating another user), those
|
|
defaults were merged into the persisted record and silently overwrote the
|
|
"other" flag.
|
|
|
|
Fix
|
|
---
|
|
The route now accepts a plain ``Dict[str, Any]`` and ``AppObjects.updateUser``
|
|
treats the payload as a true partial patch — only the keys present in the
|
|
request body are applied to the stored record. Pydantic ``User`` callers are
|
|
still supported via ``model_dump(exclude_unset=True)`` so legacy paths keep
|
|
working without re-introducing the default-fill regression.
|
|
|
|
These tests assert the partial-update contract end-to-end at the interface
|
|
level, which is the layer where the bug lived.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import Any, Dict, List, Optional
|
|
from unittest.mock import Mock, patch
|
|
|
|
import pytest
|
|
|
|
from modules.datamodels.datamodelUam import User, UserInDB
|
|
from modules.interfaces.interfaceDbApp import AppObjects
|
|
|
|
|
|
class _FakeDb:
|
|
"""Minimal connector covering the access patterns used by ``updateUser``."""
|
|
|
|
def __init__(self, rows: List[Dict[str, Any]]):
|
|
self.rows: List[Dict[str, Any]] = [dict(r) for r in rows]
|
|
self.modifyCalls: List[Dict[str, Any]] = []
|
|
|
|
def getRecordset(self, model, recordFilter: Optional[Dict[str, Any]] = None):
|
|
if model is not UserInDB and model is not User:
|
|
return []
|
|
if not recordFilter:
|
|
return [dict(r) for r in self.rows]
|
|
out = []
|
|
for r in self.rows:
|
|
if all(r.get(k) == v for k, v in recordFilter.items()):
|
|
out.append(dict(r))
|
|
return out
|
|
|
|
def recordModify(self, model, recordId: str, payload):
|
|
if hasattr(payload, "model_dump"):
|
|
data = payload.model_dump()
|
|
elif isinstance(payload, dict):
|
|
data = dict(payload)
|
|
else:
|
|
data = {}
|
|
self.modifyCalls.append({"id": str(recordId), "data": dict(data)})
|
|
for r in self.rows:
|
|
if str(r.get("id")) == str(recordId):
|
|
r.update(data)
|
|
return r
|
|
return None
|
|
|
|
|
|
def _buildInterface(db: _FakeDb) -> AppObjects:
|
|
iface = AppObjects.__new__(AppObjects)
|
|
iface.db = db
|
|
iface.currentUser = Mock(id="caller", isPlatformAdmin=True, isSysAdmin=False)
|
|
iface.userId = "caller"
|
|
iface.mandateId = None
|
|
iface.featureInstanceId = None
|
|
iface.rbac = Mock()
|
|
return iface
|
|
|
|
|
|
def _row(uid: str = "u1", **extra) -> Dict[str, Any]:
|
|
base: Dict[str, Any] = {
|
|
"id": uid,
|
|
"username": "alice",
|
|
"email": "alice@example.com",
|
|
"fullName": "Alice Example",
|
|
"language": "de",
|
|
"enabled": True,
|
|
"isSysAdmin": False,
|
|
"isPlatformAdmin": True,
|
|
"authenticationAuthority": "local",
|
|
"roleLabels": [],
|
|
}
|
|
base.update(extra)
|
|
return base
|
|
|
|
|
|
def _stubGetUser(iface: AppObjects):
|
|
"""Wire ``getUser`` to read from the FakeDb so post-update reads reflect changes."""
|
|
db = iface.db
|
|
|
|
def _readUser(userId: str):
|
|
for r in db.rows:
|
|
if str(r.get("id")) == str(userId):
|
|
return User(**r)
|
|
return None
|
|
|
|
iface.getUser = Mock(side_effect=_readUser)
|
|
|
|
|
|
class TestPartialUpdateProtectsSiblingFlag:
|
|
"""The headline regression: toggling one flag must not touch the other."""
|
|
|
|
def test_togglingIsSysAdminKeepsIsPlatformAdmin(self):
|
|
row = _row(isSysAdmin=False, isPlatformAdmin=True)
|
|
db = _FakeDb([row])
|
|
iface = _buildInterface(db)
|
|
_stubGetUser(iface)
|
|
|
|
updated = iface.updateUser(
|
|
"u1",
|
|
{"isSysAdmin": True}, # only the toggled cell
|
|
allowAdminFlagChange=True,
|
|
)
|
|
|
|
assert updated.isSysAdmin is True
|
|
assert updated.isPlatformAdmin is True, (
|
|
"Partial update must not silently drop isPlatformAdmin to its default"
|
|
)
|
|
|
|
def test_togglingIsPlatformAdminKeepsIsSysAdmin(self):
|
|
row = _row(isSysAdmin=True, isPlatformAdmin=False)
|
|
db = _FakeDb([row])
|
|
iface = _buildInterface(db)
|
|
_stubGetUser(iface)
|
|
|
|
updated = iface.updateUser(
|
|
"u1",
|
|
{"isPlatformAdmin": True},
|
|
allowAdminFlagChange=True,
|
|
)
|
|
|
|
assert updated.isPlatformAdmin is True
|
|
assert updated.isSysAdmin is True, (
|
|
"Partial update must not silently drop isSysAdmin to its default"
|
|
)
|
|
|
|
def test_togglingUnrelatedFieldKeepsBothFlags(self):
|
|
row = _row(isSysAdmin=True, isPlatformAdmin=True, language="de")
|
|
db = _FakeDb([row])
|
|
iface = _buildInterface(db)
|
|
_stubGetUser(iface)
|
|
|
|
updated = iface.updateUser(
|
|
"u1",
|
|
{"language": "en"},
|
|
allowAdminFlagChange=False,
|
|
)
|
|
|
|
assert updated.language == "en"
|
|
assert updated.isSysAdmin is True
|
|
assert updated.isPlatformAdmin is True
|
|
|
|
|
|
class TestPrivilegedFlagGuard:
|
|
"""Without ``allowAdminFlagChange`` the protected flags must be dropped."""
|
|
|
|
def test_protectedFlagsDroppedWhenChangeNotAllowed(self):
|
|
row = _row(isSysAdmin=False, isPlatformAdmin=True)
|
|
db = _FakeDb([row])
|
|
iface = _buildInterface(db)
|
|
_stubGetUser(iface)
|
|
|
|
updated = iface.updateUser(
|
|
"u1",
|
|
{"isSysAdmin": True, "isPlatformAdmin": False, "language": "fr"},
|
|
allowAdminFlagChange=False,
|
|
)
|
|
|
|
assert updated.language == "fr", "non-protected fields still apply"
|
|
assert updated.isSysAdmin is False, "protected flag must be ignored"
|
|
assert updated.isPlatformAdmin is True, "protected flag must be ignored"
|
|
|
|
def test_legacyPydanticUserDoesNotDefaultFlipFlags(self):
|
|
"""Defense in depth: if a caller still passes a ``User`` instance,
|
|
``model_dump(exclude_unset=True)`` must keep unset fields out of the
|
|
merge so they do not pull live values down to Pydantic defaults.
|
|
"""
|
|
row = _row(isSysAdmin=True, isPlatformAdmin=True, fullName="Alice Example")
|
|
db = _FakeDb([row])
|
|
iface = _buildInterface(db)
|
|
_stubGetUser(iface)
|
|
|
|
partialModel = User(
|
|
id="u1",
|
|
username="alice",
|
|
fullName="Alice Updated",
|
|
)
|
|
|
|
updated = iface.updateUser(
|
|
"u1",
|
|
partialModel,
|
|
allowAdminFlagChange=True,
|
|
)
|
|
|
|
assert updated.fullName == "Alice Updated"
|
|
assert updated.isSysAdmin is True, (
|
|
"Pydantic User without explicit isSysAdmin must not flip stored True to default False"
|
|
)
|
|
assert updated.isPlatformAdmin is True, (
|
|
"Pydantic User without explicit isPlatformAdmin must not flip stored True to default False"
|
|
)
|