From 9712be847cfc87bd5e8a31888bfe89aa3a360dc8 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 8 Sep 2020 17:38:41 +0200 Subject: [PATCH] policies/api: fix target returning pbm_uuid instead of proper primary key of the object --- passbook/flows/forms.py | 1 + .../migrations/0012_auto_20200830_1056.py | 18 --------- .../migrations/0012_auto_20200908_1542.py | 26 ++++++++++++ .../migrations/0013_auto_20200905_2142.py | 16 -------- passbook/flows/models.py | 2 +- passbook/flows/tests/test_transfer.py | 3 ++ passbook/flows/transfer/exporter.py | 1 + passbook/lib/api.py | 36 +++++++++++++++++ passbook/policies/api.py | 40 ++++++++++++++++++- .../migrations/0003_auto_20200908_1542.py | 25 ++++++++++++ swagger.yaml | 2 + 11 files changed, 134 insertions(+), 36 deletions(-) delete mode 100644 passbook/flows/migrations/0012_auto_20200830_1056.py create mode 100644 passbook/flows/migrations/0012_auto_20200908_1542.py delete mode 100644 passbook/flows/migrations/0013_auto_20200905_2142.py create mode 100644 passbook/lib/api.py create mode 100644 passbook/policies/migrations/0003_auto_20200908_1542.py diff --git a/passbook/flows/forms.py b/passbook/flows/forms.py index b15c1fb8d..7a4abe0ef 100644 --- a/passbook/flows/forms.py +++ b/passbook/flows/forms.py @@ -33,6 +33,7 @@ class FlowForm(forms.ModelForm): } widgets = { "name": forms.TextInput(), + "title": forms.TextInput(), } diff --git a/passbook/flows/migrations/0012_auto_20200830_1056.py b/passbook/flows/migrations/0012_auto_20200830_1056.py deleted file mode 100644 index 5fcbe28c2..000000000 --- a/passbook/flows/migrations/0012_auto_20200830_1056.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 3.1 on 2020-08-30 10:56 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("passbook_flows", "0011_flow_title"), - ] - - operations = [ - migrations.AlterField( - model_name="flow", - name="title", - field=models.TextField(blank=True, default=""), - ), - ] diff --git a/passbook/flows/migrations/0012_auto_20200908_1542.py b/passbook/flows/migrations/0012_auto_20200908_1542.py new file mode 100644 index 000000000..2bc7fb8fb --- /dev/null +++ b/passbook/flows/migrations/0012_auto_20200908_1542.py @@ -0,0 +1,26 @@ +# Generated by Django 3.1.1 on 2020-09-08 15:42 + +import django.db.models.deletion +from django.db import migrations, models + +import passbook.lib.models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0011_flow_title"), + ] + + operations = [ + migrations.AlterField( + model_name="flowstagebinding", + name="stage", + field=passbook.lib.models.InheritanceForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="passbook_flows.stage" + ), + ), + migrations.AlterField( + model_name="stage", name="name", field=models.TextField(unique=True), + ), + ] diff --git a/passbook/flows/migrations/0013_auto_20200905_2142.py b/passbook/flows/migrations/0013_auto_20200905_2142.py deleted file mode 100644 index 1f97c0581..000000000 --- a/passbook/flows/migrations/0013_auto_20200905_2142.py +++ /dev/null @@ -1,16 +0,0 @@ -# Generated by Django 3.1.1 on 2020-09-05 21:42 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("passbook_flows", "0012_auto_20200830_1056"), - ] - - operations = [ - migrations.AlterField( - model_name="stage", name="name", field=models.TextField(unique=True), - ), - ] diff --git a/passbook/flows/models.py b/passbook/flows/models.py index 87c60b193..2f7580773 100644 --- a/passbook/flows/models.py +++ b/passbook/flows/models.py @@ -93,7 +93,7 @@ class Flow(SerializerModel, PolicyBindingModel): name = models.TextField() slug = models.SlugField(unique=True) - title = models.TextField(default="", blank=True) + title = models.TextField() designation = models.CharField(max_length=100, choices=FlowDesignation.choices) diff --git a/passbook/flows/tests/test_transfer.py b/passbook/flows/tests/test_transfer.py index 576a6ed95..d9825e0ba 100644 --- a/passbook/flows/tests/test_transfer.py +++ b/passbook/flows/tests/test_transfer.py @@ -35,6 +35,7 @@ class TestFlowTransfer(TransactionTestCase): slug="test", designation=FlowDesignation.AUTHENTICATION, name="Welcome to passbook!", + title="test", ) FlowStageBinding.objects.update_or_create( target=flow, stage=login_stage, order=0, @@ -64,6 +65,7 @@ class TestFlowTransfer(TransactionTestCase): slug="default-source-authentication-test", designation=FlowDesignation.AUTHENTICATION, name="Welcome to passbook!", + title="test", ) PolicyBinding.objects.create(policy=flow_policy, target=flow, order=0) @@ -122,6 +124,7 @@ class TestFlowTransfer(TransactionTestCase): name="default-enrollment-flow", slug="default-enrollment-flow-test", designation=FlowDesignation.ENROLLMENT, + title="test", ) FlowStageBinding.objects.create(target=flow, stage=first_stage, order=0) diff --git a/passbook/flows/transfer/exporter.py b/passbook/flows/transfer/exporter.py index 9617e4286..44ca05b75 100644 --- a/passbook/flows/transfer/exporter.py +++ b/passbook/flows/transfer/exporter.py @@ -47,6 +47,7 @@ class FlowExporter: pbm_uuids += FlowStageBinding.objects.filter(target=self.flow).values_list( "pbm_uuid", flat=True ) + # Add policy objects first, so they are created first policies = Policy.objects.filter(bindings__in=pbm_uuids).select_related() for policy in policies: yield FlowBundleEntry.from_model(policy) diff --git a/passbook/lib/api.py b/passbook/lib/api.py new file mode 100644 index 000000000..3fed1bd11 --- /dev/null +++ b/passbook/lib/api.py @@ -0,0 +1,36 @@ +"""passbook API Helpers""" +from django.core.exceptions import ObjectDoesNotExist +from django.db.models.query import QuerySet +from model_utils.managers import InheritanceQuerySet +from rest_framework.serializers import ModelSerializer, PrimaryKeyRelatedField + + +class InheritancePrimaryKeyRelatedField(PrimaryKeyRelatedField): + """rest_framework PrimaryKeyRelatedField which resolves + model_manager's InheritanceQuerySet""" + + def get_queryset(self) -> QuerySet: + queryset = super().get_queryset() + if isinstance(queryset, InheritanceQuerySet): + return queryset.select_subclasses() + return queryset + + def to_internal_value(self, data): + if self.pk_field is not None: + data = self.pk_field.to_internal_value(data) + try: + queryset = self.get_queryset() + if isinstance(queryset, InheritanceQuerySet): + return queryset.get_subclass(pk=data) + return queryset.get(pk=data) + except ObjectDoesNotExist: + self.fail("does_not_exist", pk_value=data) + except (TypeError, ValueError): + self.fail("incorrect_type", data_type=type(data).__name__) + + +class InheritanceModelSerializer(ModelSerializer): + """rest_framework ModelSerializer which automatically uses InheritancePrimaryKeyRelatedField + for every primary key""" + + serializer_related_field = InheritancePrimaryKeyRelatedField diff --git a/passbook/policies/api.py b/passbook/policies/api.py index f2b272c57..b82c3190c 100644 --- a/passbook/policies/api.py +++ b/passbook/policies/api.py @@ -1,14 +1,52 @@ """policy API Views""" from rest_framework.serializers import ModelSerializer, SerializerMethodField +from rest_framework.utils.model_meta import get_field_info from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet +from passbook.lib.api import InheritancePrimaryKeyRelatedField from passbook.policies.forms import GENERAL_FIELDS -from passbook.policies.models import Policy, PolicyBinding +from passbook.policies.models import Policy, PolicyBinding, PolicyBindingModel class PolicyBindingSerializer(ModelSerializer): """PolicyBinding Serializer""" + # Because we're not interested in the PolicyBindingModel's PK but rather the subclasses PK, + # we have to manually declare this field + target = InheritancePrimaryKeyRelatedField( + queryset=PolicyBindingModel.objects.all().select_subclasses(), + source="target.pk", + required=True, + ) + + def update(self, instance, validated_data): + info = get_field_info(instance) + + # Simply set each attribute on the instance, and then save it. + # Note that unlike `.create()` we don't need to treat many-to-many + # relationships as being a special case. During updates we already + # have an instance pk for the relationships to be associated with. + m2m_fields = [] + for attr, value in validated_data.items(): + if attr in info.relations and info.relations[attr].to_many: + m2m_fields.append((attr, value)) + else: + if attr == "target": + instance.target_pk = value["pk"].pbm_uuid + else: + setattr(instance, attr, value) + + instance.save() + + # Note that many-to-many fields are set after updating instance. + # Setting m2m fields triggers signals which could potentially change + # updated instance and we do not want it to collide with .update() + for attr, value in m2m_fields: + field = getattr(instance, attr) + field.set(value) + + return instance + class Meta: model = PolicyBinding diff --git a/passbook/policies/migrations/0003_auto_20200908_1542.py b/passbook/policies/migrations/0003_auto_20200908_1542.py new file mode 100644 index 000000000..ce5cd2ad6 --- /dev/null +++ b/passbook/policies/migrations/0003_auto_20200908_1542.py @@ -0,0 +1,25 @@ +# Generated by Django 3.1.1 on 2020-09-08 15:42 + +import django.db.models.deletion +from django.db import migrations + +import passbook.lib.models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_policies", "0002_auto_20200528_1647"), + ] + + operations = [ + migrations.AlterField( + model_name="policybinding", + name="target", + field=passbook.lib.models.InheritanceForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="+", + to="passbook_policies.policybindingmodel", + ), + ), + ] diff --git a/swagger.yaml b/swagger.yaml index 3c7ff6776..1610adf7d 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -6041,6 +6041,7 @@ definitions: required: - name - slug + - title - designation type: object properties: @@ -6063,6 +6064,7 @@ definitions: title: title: Title type: string + minLength: 1 designation: title: Designation type: string