Skip to content

Commit

Permalink
Check for poll permission correctly (#2622)
Browse files Browse the repository at this point in the history
* Check for poll permission correctly

* Update documentation
  • Loading branch information
luisa-beerboom authored Nov 13, 2024
1 parent cb07383 commit b5921c5
Show file tree
Hide file tree
Showing 14 changed files with 39 additions and 22 deletions.
2 changes: 1 addition & 1 deletion docs/actions/option.update.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ If the poll's state is *created* and at least one vote value is given (`Y`, `N`
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.anonymize.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ Only for non-analog polls in the state *finished* or *published*. Sets all `vote
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.create.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,4 @@ The `entitled_group_ids` may not contain the meetings `anonymous_group_id`.
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.delete.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ Deletes the given poll and all linked options with all votes, too.
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.publish.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ Sets the state to *published*. Only allowed for polls in the *finished* state.
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.reset.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ If `type != "pseudoanonymous"`, `is_pseudoanonymized` may be reset to `false` (i
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.start.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ If `meeting/poll_couple_countdown` is true and the poll is an electronic poll, t
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.stop.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ Some fields have to be calculated upon stopping a poll:
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
2 changes: 1 addition & 1 deletion docs/actions/poll.update.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@ The `entitled_group_ids` may not contain the meetings `anonymous_group_id`.
The request user needs:
- `motion.can_manage_polls` if the poll's content object is a motion
- `assignment.can_manage` if the poll's content object is an assignment
- `poll.can_manage` else
- `poll.can_manage` if the poll's content object is a topic
10 changes: 7 additions & 3 deletions openslides_backend/action/actions/poll/functions.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from ....permissions.permission_helper import has_perm
from ....permissions.permissions import Permission, Permissions
from ....services.datastore.interface import DatastoreService
from ....shared.exceptions import MissingPermission
from ....shared.patterns import KEYSEPARATOR
from ....shared.exceptions import ActionException, MissingPermission
from ....shared.patterns import KEYSEPARATOR, collection_from_fqid


def check_poll_or_option_perms(
Expand All @@ -15,7 +15,11 @@ def check_poll_or_option_perms(
perm: Permission = Permissions.Motion.CAN_MANAGE_POLLS
elif content_object_id.startswith("assignment" + KEYSEPARATOR):
perm = Permissions.Assignment.CAN_MANAGE
else:
elif content_object_id.startswith("topic" + KEYSEPARATOR):
perm = Permissions.Poll.CAN_MANAGE
else:
raise ActionException(
f"'{collection_from_fqid(content_object_id)}' is not a valid poll collection."
)
if not has_perm(datastore, user_id, perm, meeting_id):
raise MissingPermission(perm)
4 changes: 3 additions & 1 deletion openslides_backend/action/actions/poll/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from openslides_backend.shared.typing import HistoryInformation

from ....services.datastore.commands import GetManyRequest
from ....shared.exceptions import VoteServiceException
from ....shared.exceptions import ActionException, VoteServiceException
from ....shared.interfaces.write_request import WriteRequest
from ....shared.patterns import (
collection_from_fqid,
Expand Down Expand Up @@ -33,6 +33,8 @@ def check_permissions(self, instance: dict[str, Any]) -> None:
)
content_object_id = poll.get("content_object_id", "")
meeting_id = poll["meeting_id"]
if not content_object_id:
raise ActionException("No 'content_object_id' was given")
check_poll_or_option_perms(
content_object_id, self.datastore, self.user_id, meeting_id
)
Expand Down
6 changes: 2 additions & 4 deletions tests/system/action/poll/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ def test_create_wrong_publish_immediately(self) -> None:
response = self.request(
"poll.create",
{
"content_object_id": "assignment/1",
"title": "test_title_ahThai4pae1pi4xoogoo",
"pollmethod": "YN",
"type": "pseudoanonymous",
Expand Down Expand Up @@ -799,10 +800,7 @@ def test_create_without_content_object(self) -> None:
},
)
self.assert_status_code(response, 400)
assert (
response.json["message"]
== "Creation of poll/1: You try to set following required fields to an empty value: ['content_object_id']"
)
assert response.json["message"] == "No 'content_object_id' was given"

def test_create_no_permissions_assignment(self) -> None:
self.base_permission_test(
Expand Down
14 changes: 11 additions & 3 deletions tests/system/action/poll/test_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,18 @@ def test_delete_wrong_id(self) -> None:
def test_delete_correct_cascading(self) -> None:
self.set_models(
{
"topic/1": {"poll_ids": [111], "meeting_id": 1},
"poll/111": {
"option_ids": [42],
"meeting_id": 1,
"projection_ids": [1],
"content_object_id": "topic/1",
},
"option/42": {"poll_id": 111, "meeting_id": 1},
"meeting/1": {
"is_active_in_organization_id": 1,
"all_projection_ids": [1],
"topic_ids": [1],
},
"projection/1": {
"content_object_id": "poll/111",
Expand All @@ -64,9 +67,11 @@ def test_delete_correct_cascading(self) -> None:
def test_delete_cascading_poll_candidate_list(self) -> None:
self.set_models(
{
"topic/1": {"poll_ids": [111], "meeting_id": 1},
"poll/111": {
"option_ids": [42],
"meeting_id": 1,
"content_object_id": "topic/1",
},
"option/42": {
"poll_id": 111,
Expand All @@ -77,6 +82,7 @@ def test_delete_cascading_poll_candidate_list(self) -> None:
"is_active_in_organization_id": 1,
"poll_candidate_list_ids": [12],
"poll_candidate_ids": [13],
"topic_ids": [1],
},
"poll_candidate_list/12": {
"meeting_id": 1,
Expand All @@ -100,20 +106,22 @@ def test_delete_cascading_poll_candidate_list(self) -> None:

def test_delete_no_permissions(self) -> None:
self.base_permission_test(
{"poll/111": {"meeting_id": 1}}, "poll.delete", {"id": 111}
{"poll/111": {"meeting_id": 1, "content_object_id": "topic/1"}},
"poll.delete",
{"id": 111},
)

def test_delete_permissions(self) -> None:
self.base_permission_test(
{"poll/111": {"meeting_id": 1}},
{"poll/111": {"meeting_id": 1, "content_object_id": "topic/1"}},
"poll.delete",
{"id": 111},
Permissions.Poll.CAN_MANAGE,
)

def test_delete_permissions_locked_meeting(self) -> None:
self.base_locked_out_superadmin_permission_test(
{"poll/111": {"meeting_id": 1}},
{"poll/111": {"meeting_id": 1, "content_object_id": "topic/1"}},
"poll.delete",
{"id": 111},
)
Expand Down
9 changes: 7 additions & 2 deletions tests/system/action/poll/test_publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,13 @@ def test_publish_assignment(self) -> None:
def test_publish_wrong_state(self) -> None:
self.set_models(
{
"poll/1": {"state": "created", "meeting_id": 1},
"meeting/1": {"is_active_in_organization_id": 1},
"topic/1": {"poll_ids": [111], "meeting_id": 1},
"poll/1": {
"state": "created",
"meeting_id": 1,
"content_object_id": "topic/1",
},
"meeting/1": {"is_active_in_organization_id": 1, "topic_ids": [1]},
}
)
response = self.request("poll.publish", {"id": 1})
Expand Down

0 comments on commit b5921c5

Please sign in to comment.