From 34b8e6105f3362b3dd3235d4ef1224360f5dda97 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Fri, 17 Sep 2021 16:21:51 +0200 Subject: [PATCH 1/2] prevent adding/removing guardians without global change_subject permission This prevents leaking the subject ID to users who would otherwise not have access to it. --- castellum/contacts/forms.py | 12 ++++-- .../templates/contacts/contact_form.html | 41 +++++++++++++------ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/castellum/contacts/forms.py b/castellum/contacts/forms.py index fd6a6e029..488ba542b 100644 --- a/castellum/contacts/forms.py +++ b/castellum/contacts/forms.py @@ -65,8 +65,6 @@ class ContactForm(forms.ModelForm): ('self', _('Full of age')), ('guardians', _('Has legal guardian')), ], widget=forms.RadioSelect) - guardians_remove = forms.ModelMultipleChoiceField(Subject.objects, required=False) - guardians_add = SubjectMultipleChoiceField(Subject.objects, required=False) class Meta: model = Contact @@ -93,7 +91,15 @@ class ContactForm(forms.ModelForm): elif any([self.instance.get_address(), self.instance.phone_number, self.instance.email]): self.fields['guardians_pane'].initial = 'self' - self.fields['guardians_remove'].choices = self.get_guardians_rm_choices(user) + self.guardians = self.get_guardians_rm_choices(user) + if user.has_perm('subjects.change_subject'): + self.fields['guardians_remove'] = forms.ModelMultipleChoiceField( + Subject.objects, required=False + ) + self.fields['guardians_remove'].choices = self.guardians + self.fields['guardians_add'] = SubjectMultipleChoiceField( + Subject.objects, required=False + ) def get_address_form(self, **kwargs): address_kwargs = kwargs.copy() diff --git a/castellum/contacts/templates/contacts/contact_form.html b/castellum/contacts/templates/contacts/contact_form.html index bf47abf06..d0856e847 100644 --- a/castellum/contacts/templates/contacts/contact_form.html +++ b/castellum/contacts/templates/contacts/contact_form.html @@ -1,5 +1,5 @@ {% extends view.base_template|default:"subjects/base.html" %} -{% load static i18n bootstrap4 %} +{% load static i18n auth bootstrap4 %} {% block title %} {% if object %} @@ -29,25 +29,42 @@
- - {% if form.guardians_blocked > 0 %}

{% blocktranslate with count=form.guardians_blocked %}{{ count }} guardians not listed because of insufficient privacy level.{% endblocktranslate %}

{% endif %} - {% for widget in form.guardians_remove %} - {% include 'contacts/__guardian_item.html' with name=form.guardians_remove.name pk=widget.data.value label=widget.choice_label slug=widget.choice_label.subject.slug removed=widget.data.selected %} - {% endfor %} + {% has_perm 'subjects.change_subject' user as can_change_subject %} + {% if can_change_subject %} + + + {% for widget in form.guardians_remove %} + {% include 'contacts/__guardian_item.html' with name=form.guardians_remove.name pk=widget.data.value label=widget.choice_label slug=widget.choice_label.subject.slug removed=widget.data.selected %} + {% endfor %} - {% for subject in form.cleaned_data.guardians_add %} - {% include 'contacts/__guardian_item.html' with name=form.guardians_add.name pk=subject.pk label=subject label=subject.contact.full_name removed=False %} - {% endfor %} + {% for subject in form.cleaned_data.guardians_add %} + {% include 'contacts/__guardian_item.html' with name=form.guardians_add.name pk=subject.pk label=subject label=subject.contact.full_name removed=False %} + {% endfor %} - + + {% else %} + {% has_perm 'subjects.view_subject' user as can_view_subject %} + {% for subject_id, contact in form.guardians %} +
+
+
{% translate 'Guardian' %}
+
{{ contact }}
+ +
+ {% if can_view_subject %} + {% translate 'Details' %} + {% endif %} +
+ {% endfor %} + {% endif %}
-- GitLab From 338e15a4b7a575374d75721061a380fe936fea72 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Tue, 21 Sep 2021 14:41:52 +0200 Subject: [PATCH 2/2] prevent adding guardians with high privacy level --- castellum/contacts/forms.py | 2 +- tests/contacts/forms/test_contact_form.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/castellum/contacts/forms.py b/castellum/contacts/forms.py index 488ba542b..0947b0c79 100644 --- a/castellum/contacts/forms.py +++ b/castellum/contacts/forms.py @@ -98,7 +98,7 @@ class ContactForm(forms.ModelForm): ) self.fields['guardians_remove'].choices = self.guardians self.fields['guardians_add'] = SubjectMultipleChoiceField( - Subject.objects, required=False + Subject.objects.filter(privacy_level__lte=user.get_privacy_level()), required=False ) def get_address_form(self, **kwargs): diff --git a/tests/contacts/forms/test_contact_form.py b/tests/contacts/forms/test_contact_form.py index 4b2545e74..f862023b7 100644 --- a/tests/contacts/forms/test_contact_form.py +++ b/tests/contacts/forms/test_contact_form.py @@ -234,6 +234,22 @@ def test_contact_form_with_guardians_add(user, full_data, contact): assert form.cleaned_data['email'] == '' +@pytest.mark.django_db +def test_contact_form_with_guardians_add_privacy_level(user, full_data, contact): + full_data.update({ + 'guardians_pane': 'guardians', + 'guardians_add': [contact.subject.pk], + }) + + contact.subject.privacy_level = 2 + contact.subject.save() + + form = ContactForm(user=user, data=full_data, instance=contact) + + assert form.is_valid() is False + assert 'Select a valid choice.' in str(form.errors) + + @pytest.mark.django_db def test_contact_form_with_guardians_removes_address(user, full_data): form = ContactForm(user=user, data=full_data) -- GitLab