Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Cherry-Pick] Update clone perms: allow only template or same committee #2572

Merged
merged 2 commits into from
Aug 14, 2024
Merged

[Cherry-Pick] Update clone perms: allow only template or same committee #2572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37738c5
Update clone perms: allow only template or same committee (#2568)
reiterl Aug 13, 2024
686c1a6
Fix the merge conflicts
reiterl Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/actions/meeting.clone.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,7 @@ If set_as_template is given, template_for_organization_id has to be set to 1.

## Permission

The request user must have the CML `can_manage` in the target committee (where the meeting is created).
If the organization setting `require_duplicate_from` is set, a committee manager can only clone template meetings.
It is not allowed to clone a meeting from a different committee if said meeting isn't a template.

Otherwise the request user only needs the CML `can_manage` in the target committee (where the meeting is created).

36 changes: 13 additions & 23 deletions openslides_backend/action/actions/meeting/clone.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,12 @@
)
from openslides_backend.models.models import Meeting, MeetingUser
from openslides_backend.services.datastore.interface import GetManyRequest
from openslides_backend.shared.exceptions import ActionException
from openslides_backend.shared.exceptions import ActionException, PermissionDenied
from openslides_backend.shared.interfaces.event import Event, EventType
from openslides_backend.shared.patterns import fqid_from_collection_and_id
from openslides_backend.shared.schema import id_list_schema, required_id_schema

from ....permissions.management_levels import OrganizationManagementLevel
from ....permissions.permission_helper import has_organization_management_level
from ....shared.export_helper import export_meeting
from ....shared.util import ONE_ORGANIZATION_FQID
from ...util.default_schema import DefaultSchema
from ...util.register import register_action
from ...util.typing import ActionData
Expand Down Expand Up @@ -79,28 +76,21 @@ def preprocess_data(self, instance: dict[str, Any]) -> dict[str, Any]:
return instance

def check_permissions(self, instance: dict[str, Any]) -> None:
if "committee_id" in instance:
meeting = self.datastore.get(
fqid_from_collection_and_id("meeting", instance["meeting_id"]),
["committee_id", "template_for_organization_id"],
lock_result=False,
)
if meeting["committee_id"] != instance["committee_id"] and not meeting.get(
"template_for_organization_id"
):
raise PermissionDenied(
"Cannot clone meeting to a different committee if it is a non-template meeting."
)
MeetingPermissionMixin.check_permissions(self, instance)

def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
organization = self.datastore.get(
ONE_ORGANIZATION_FQID,
["require_duplicate_from", "template_meeting_ids"],
lock_result=False,
)
if (
organization.get("require_duplicate_from")
and not has_organization_management_level(
self.datastore,
self.user_id,
OrganizationManagementLevel.CAN_MANAGE_ORGANIZATION,
)
and not instance["meeting_id"]
in (organization.get("template_meeting_ids") or [])
):
raise ActionException(
"Committee manager cannot clone a non-template meeting if duplicate-from is required."
)

meeting_json = export_meeting(self.datastore, instance["meeting_id"])
instance["meeting"] = meeting_json
additional_user_ids = instance.pop("user_ids", None) or []
Expand Down
72 changes: 49 additions & 23 deletions tests/system/action/meeting/test_clone.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def setUp(self) -> None:
"active_meeting_ids": [1],
"organization_tag_ids": [1],
"user_ids": [1],
"template_meeting_ids": [1],
},
"organization_tag/1": {
"name": "TEST",
Expand All @@ -25,6 +26,7 @@ def setUp(self) -> None:
},
"committee/1": {"organization_id": 1},
"meeting/1": {
"template_for_organization_id": 1,
"committee_id": 1,
"language": "en",
"name": "Test",
Expand Down Expand Up @@ -112,7 +114,7 @@ def test_clone_without_users(self) -> None:
},
)
self.assert_model_exists(
"organization/1", {"template_meeting_ids": [2], "user_ids": [1]}
"organization/1", {"template_meeting_ids": [1, 2], "user_ids": [1]}
)

def test_clone_group_with_weight(self) -> None:
Expand Down Expand Up @@ -516,9 +518,6 @@ def test_clone_with_ex_users(self) -> None:
)

def test_clone_with_set_fields(self) -> None:
self.test_models["meeting/1"][
"template_for_organization_id"
] = ONE_ORGANIZATION_ID
self.set_models(self.test_models)

response = self.request(
Expand All @@ -536,7 +535,7 @@ def test_clone_with_set_fields(self) -> None:
},
)
self.assert_status_code(response, 200)
self.assert_model_exists("organization/1", {"template_meeting_ids": None})
self.assert_model_exists("organization/1", {"template_meeting_ids": [1]})
self.assert_model_exists(
"meeting/2",
{
Expand Down Expand Up @@ -689,7 +688,7 @@ def test_clone_user_ids_and_admin_ids(self) -> None:
},
)
self.assert_status_code(response, 200)
self.assert_model_exists("organization/1", {"template_meeting_ids": None})
self.assert_model_exists("organization/1", {"template_meeting_ids": [1]})
meeting2 = self.assert_model_exists(
"meeting/2", {"template_for_organization_id": None}
)
Expand Down Expand Up @@ -1285,25 +1284,44 @@ def test_meeting_name_too_long(self) -> None:
self.assert_status_code(response, 200)
self.assert_model_exists("meeting/2", {"name": "A" * 90 + "... - Copy"})

def test_permissions_both_okay(self) -> None:
def test_permissions_explicit_source_committee_permission(self) -> None:
self.set_models(self.test_models)
self.set_models(
{
"committee/2": {"organization_id": 1},
"user/1": {
"committee_management_ids": [1, 2],
"committee_management_ids": [1],
"committee_ids": [1, 2],
"organization_management_level": None,
},
"committee/2": {"user_ids": [1]},
}
)
response = self.request("meeting.clone", {"meeting_id": 1, "committee_id": 2})
response = self.request("meeting.clone", {"meeting_id": 1, "committee_id": 1})
self.assert_status_code(response, 200)
self.assert_model_exists(
"meeting/1", {"is_active_in_organization_id": 1, "committee_id": 1}
)
self.assert_model_exists(
"meeting/2", {"is_active_in_organization_id": 1, "committee_id": 2}
"meeting/2", {"is_active_in_organization_id": 1, "committee_id": 1}
)

def test_permissions_foreign_committee_cml_error(self) -> None:
self.set_models(self.test_models)
self.set_models(
{
"committee/2": {"organization_id": 1},
"user/1": {
"committee_management_ids": [1],
"committee_ids": [1],
"organization_management_level": None,
},
}
)
response = self.request("meeting.clone", {"meeting_id": 1, "committee_id": 2})
self.assert_status_code(response, 403)
self.assertIn(
"You are not allowed to perform action meeting.clone. Missing permission: CommitteeManagementLevel can_manage in committee 2",
response.json["message"],
)

def test_permissions_oml_can_manage(self) -> None:
Expand Down Expand Up @@ -1843,7 +1861,7 @@ def test_clone_datastore_calls(self) -> None:
with CountDatastoreCalls() as counter:
response = self.request("meeting.clone", {"meeting_id": 1})
self.assert_status_code(response, 200)
assert counter.calls == 25
assert counter.calls == 24

@performance
def test_clone_performance(self) -> None:
Expand Down Expand Up @@ -1903,19 +1921,26 @@ def test_clone_amendment_paragraphs(self) -> None:

def test_permissions_oml_locked_meeting(self) -> None:
self.create_meeting()
self.set_models({"meeting/1": {"locked_from_inside": True}})
self.set_models(
{
"meeting/1": {
"locked_from_inside": True,
"template_for_organization_id": 1,
},
ONE_ORGANIZATION_FQID: {"template_meeting_ids": [1]},
}
)
response = self.request("meeting.clone", {"meeting_id": 1, "committee_id": 2})
self.assert_status_code(response, 400)
assert "Cannot clone locked meeting." in response.json["message"]

def test_clone_require_duplicate_from_allowed(self) -> None:
def test_clone_template_allowed(self) -> None:
self.set_models(self.test_models)
self.set_models(
{
"meeting/1": {"template_for_organization_id": 1, "name": "m1"},
"organization/1": {
"template_meeting_ids": [1],
"require_duplicate_from": True,
},
"user/1": {
"organization_management_level": None,
Expand All @@ -1928,22 +1953,23 @@ def test_clone_require_duplicate_from_allowed(self) -> None:
response = self.request("meeting.clone", {"meeting_id": 1})
self.assert_status_code(response, 200)

def test_clone_require_duplicate_from_not_allowed(self) -> None:
def test_clone_non_template_and_committee_change_not_allowed(self) -> None:
self.test_models[ONE_ORGANIZATION_FQID]["template_meeting_ids"] = None
self.test_models["meeting/1"]["template_for_organization_id"] = None
self.set_models(self.test_models)
self.set_models(
{
"organization/1": {"require_duplicate_from": True},
"user/1": {
"organization_management_level": None,
"committee_ids": [1],
"committee_management_ids": [1],
"committee_ids": [1, 2],
"committee_management_ids": [1, 2],
},
"committee/1": {"user_ids": [1], "manager_ids": [1]},
"committee/2": {"user_ids": [1], "manager_ids": [1]},
}
)
response = self.request("meeting.clone", {"meeting_id": 1})
self.assert_status_code(response, 400)
response = self.request("meeting.clone", {"meeting_id": 1, "committee_id": 2})
self.assert_status_code(response, 403)
assert (
response.json["message"]
== "Committee manager cannot clone a non-template meeting if duplicate-from is required."
== "Cannot clone meeting to a different committee if it is a non-template meeting."
)