From ee31c1e633f6b88b7dcd1cd61cc6c747fc4ee4a9 Mon Sep 17 00:00:00 2001 From: Joshua Sangmeister Date: Mon, 12 Apr 2021 13:57:24 +0200 Subject: [PATCH] Lock poll to prevent race conditions Add migrations --- .../0025_assignmentpoll_fix_vote_values.py | 22 ++++++++++ .../0044_motionpoll_fix_vote_values.py | 18 +++++++++ .../poll/migrations/poll_migration_helper.py | 40 +++++++++++++++++++ server/openslides/poll/views.py | 26 +++++++----- 4 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 server/openslides/assignments/migrations/0025_assignmentpoll_fix_vote_values.py create mode 100644 server/openslides/motions/migrations/0044_motionpoll_fix_vote_values.py diff --git a/server/openslides/assignments/migrations/0025_assignmentpoll_fix_vote_values.py b/server/openslides/assignments/migrations/0025_assignmentpoll_fix_vote_values.py new file mode 100644 index 000000000..de897ba07 --- /dev/null +++ b/server/openslides/assignments/migrations/0025_assignmentpoll_fix_vote_values.py @@ -0,0 +1,22 @@ +# Generated by jsangmeister on 2021-04-15 08:01 + +from django.db import migrations + +from ...poll.migrations.poll_migration_helper import fix_wrongly_calculated_vote_fields + + +class Migration(migrations.Migration): + + dependencies = [ + ("assignments", "0024_assignmentpoll_entitled_users_remove_duplicates"), + ] + + operations = [ + migrations.RunPython( + fix_wrongly_calculated_vote_fields( + "assignments", + "AssignmentPoll", + lambda poll: not poll.is_pseudoanonymized, + ) + ), + ] diff --git a/server/openslides/motions/migrations/0044_motionpoll_fix_vote_values.py b/server/openslides/motions/migrations/0044_motionpoll_fix_vote_values.py new file mode 100644 index 000000000..8795992fc --- /dev/null +++ b/server/openslides/motions/migrations/0044_motionpoll_fix_vote_values.py @@ -0,0 +1,18 @@ +# Generated by jsangmeister on 2021-04-12 13:27 + +from django.db import migrations + +from ...poll.migrations.poll_migration_helper import fix_wrongly_calculated_vote_fields + + +class Migration(migrations.Migration): + + dependencies = [ + ("motions", "0043_motionpoll_entitled_users_remove_duplicates"), + ] + + operations = [ + migrations.RunPython( + fix_wrongly_calculated_vote_fields("motions", "MotionPoll") + ), + ] diff --git a/server/openslides/poll/migrations/poll_migration_helper.py b/server/openslides/poll/migrations/poll_migration_helper.py index 27b9414b3..a39f76e89 100644 --- a/server/openslides/poll/migrations/poll_migration_helper.py +++ b/server/openslides/poll/migrations/poll_migration_helper.py @@ -56,3 +56,43 @@ def remove_entitled_users_duplicates(poll_model_collection, poll_model_name): poll.save(skip_autoupdate=True) return _remove_entitled_users_duplicates + + +def fix_wrongly_calculated_vote_fields( + poll_model_collection, poll_model_name, filter=None +): + """ + Takes all polls of the given model and corrects votes* fields if: + - vote weight is disabled (checked in config and by asserting that votesvalid==votescast) + - poll type and state must be correct + - calculated value must be bigger than db value (should be the case anyway, but if + it's not, we don't want to break even more things by changing it) + """ + + def _fix_wrongly_calculated_vote_fields(apps, schema_editor): + ConfigStore = apps.get_model("core", "ConfigStore") + try: + config = ConfigStore.objects.get(key="users_activate_vote_weight") + value = config.value + except (ConfigStore.DoesNotExist, KeyError): + value = False + if not value: + PollModel = apps.get_model(poll_model_collection, poll_model_name) + for poll in PollModel.objects.all(): + if ( + poll.type != BasePoll.TYPE_ANALOG + and (not filter or filter(poll)) + and poll.state + in (BasePoll.STATE_FINISHED, BasePoll.STATE_PUBLISHED) + and poll.votesvalid == poll.votescast + ): + all_vote_tokens = set( + vote.user_token + for option in poll.options.all() + for vote in option.votes.all() + ) + if len(all_vote_tokens) > poll.votesvalid: + poll.votesvalid = poll.votescast = len(all_vote_tokens) + poll.save(skip_autoupdate=True) + + return _fix_wrongly_calculated_vote_fields diff --git a/server/openslides/poll/views.py b/server/openslides/poll/views.py index 06b4eaa5b..ca82542b7 100644 --- a/server/openslides/poll/views.py +++ b/server/openslides/poll/views.py @@ -38,6 +38,14 @@ class BasePollViewSet(ModelViewSet): else: return self.has_manage_permissions() + def get_locked_object(self): + """ + Enhance get_object to make sure to lock the underlying object to prevent + race conditions. + """ + poll = self.get_object() + return self.queryset.select_for_update().get(pk=poll.pk) + @transaction.atomic def create(self, request, *args, **kwargs): serializer = self.get_serializer(data=request.data) @@ -66,7 +74,7 @@ class BasePollViewSet(ModelViewSet): """ Customized view endpoint to update a poll. """ - poll = self.get_object() + poll = self.get_locked_object() partial = kwargs.get("partial", False) serializer = self.get_serializer(poll, data=request.data, partial=partial) @@ -122,7 +130,7 @@ class BasePollViewSet(ModelViewSet): @action(detail=True, methods=["POST"]) @transaction.atomic def start(self, request, pk): - poll = self.get_object() + poll = self.get_locked_object() if poll.state != BasePoll.STATE_CREATED: raise ValidationError({"detail": "Wrong poll state"}) poll.state = BasePoll.STATE_STARTED @@ -135,8 +143,8 @@ class BasePollViewSet(ModelViewSet): @action(detail=True, methods=["POST"]) @transaction.atomic def stop(self, request, pk): - poll = self.get_object() - # Analog polls could not be stopped; they are stopped when + poll = self.get_locked_object() + # Analog polls cannot be stopped; they are stopped when # the results are entered. if poll.type == BasePoll.TYPE_ANALOG: raise ValidationError( @@ -155,7 +163,7 @@ class BasePollViewSet(ModelViewSet): @action(detail=True, methods=["POST"]) @transaction.atomic def publish(self, request, pk): - poll = self.get_object() + poll = self.get_locked_object() if poll.state != BasePoll.STATE_FINISHED: raise ValidationError({"detail": "Wrong poll state"}) @@ -175,7 +183,7 @@ class BasePollViewSet(ModelViewSet): @action(detail=True, methods=["POST"]) @transaction.atomic def pseudoanonymize(self, request, pk): - poll = self.get_object() + poll = self.get_locked_object() if poll.state not in (BasePoll.STATE_FINISHED, BasePoll.STATE_PUBLISHED): raise ValidationError( @@ -191,7 +199,7 @@ class BasePollViewSet(ModelViewSet): @action(detail=True, methods=["POST"]) @transaction.atomic def reset(self, request, pk): - poll = self.get_object() + poll = self.get_locked_object() poll.reset() self.extend_history_information(["Voting reset"]) return Response() @@ -202,7 +210,7 @@ class BasePollViewSet(ModelViewSet): """ For motion polls: Just "Y", "N" or "A" (if pollmethod is "YNA") """ - poll = self.get_object() + poll = self.get_locked_object() # Disable history for these requests disable_history() @@ -257,7 +265,7 @@ class BasePollViewSet(ModelViewSet): @action(detail=True, methods=["POST"]) @transaction.atomic def refresh(self, request, pk): - poll = self.get_object() + poll = self.get_locked_object() inform_changed_data(poll) inform_changed_data(poll.get_options()) inform_changed_data(poll.get_votes())