From af08d69d64b6b7cefa2b9020dd60dca5fe91a4e2 Mon Sep 17 00:00:00 2001 From: Roelof Korporaal Date: Mon, 24 Apr 2023 17:05:49 +0200 Subject: [PATCH] Feature/member edit ux (#698) Co-authored-by: Mark Janssen <20283+praseodym@users.noreply.github.com> Co-authored-by: HeleenSG Co-authored-by: Jeroen Dekkers Co-authored-by: Patrick --- rocky/.ci/docker-compose.yml | 1 + rocky/account/forms/account_setup.py | 24 +++++-- rocky/account/mixins.py | 2 +- rocky/account/models.py | 5 +- rocky/assets/src/bundles/app/css/main.scss | 2 + .../css/themes/soft/manon/form-fieldset.scss | 5 ++ .../app/css/themes/soft/manon/form-radio.scss | 5 ++ .../app/css/themes/soft/manon/form.scss | 3 + .../src/bundles/app/css/themes/soft/soft.scss | 2 + .../form-fieldset-required.scss | 16 +++++ .../vendors/manon-overrides/form-radio.scss | 5 ++ rocky/onboarding/views.py | 2 + .../organization_member_add.html | 2 +- .../organization_member_list.html | 28 +++++--- .../templates/partials/form/field_input.html | 71 +++++++++---------- .../partials/form/field_input_checkbox.html | 27 +++---- .../partials/form/field_input_radio.html | 10 ++- .../partials/form/field_input_wrapper.html | 2 - .../templates/partials/form/fieldset.html | 1 - .../organization_member_list_filters.html | 27 +++++-- rocky/rocky/views/indemnification_add.py | 15 ++++ rocky/rocky/views/organization_member_add.py | 19 ++++- rocky/rocky/views/organization_member_edit.py | 15 ++-- rocky/rocky/views/organization_member_list.py | 61 +++++++++++++--- rocky/tests/conftest.py | 4 +- rocky/tests/test_members.py | 4 +- rocky/tests/test_organization.py | 25 ++++--- .../migrations/0033_auto_20230407_1113.py | 28 ++++++++ rocky/tools/models.py | 6 +- 29 files changed, 296 insertions(+), 121 deletions(-) create mode 100644 rocky/assets/src/bundles/app/css/themes/soft/manon/form-fieldset.scss create mode 100644 rocky/assets/src/bundles/app/css/themes/soft/manon/form-radio.scss create mode 100644 rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-fieldset-required.scss create mode 100644 rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-radio.scss create mode 100644 rocky/tools/migrations/0033_auto_20230407_1113.py diff --git a/rocky/.ci/docker-compose.yml b/rocky/.ci/docker-compose.yml index aa250c1505e..555868a627a 100644 --- a/rocky/.ci/docker-compose.yml +++ b/rocky/.ci/docker-compose.yml @@ -13,6 +13,7 @@ services: volumes: - .:/app/rocky - ./.ci/run_rocky.sh:/app/run_rocky.sh + - ../octopoes/octopoes:/app/rocky/octopoes env_file: - .ci/.env.test diff --git a/rocky/account/forms/account_setup.py b/rocky/account/forms/account_setup.py index bfc20aa713b..574575def51 100644 --- a/rocky/account/forms/account_setup.py +++ b/rocky/account/forms/account_setup.py @@ -5,7 +5,7 @@ from django.contrib.auth.password_validation import validate_password from django.utils.translation import gettext_lazy as _ from tools.enums import SCAN_LEVEL -from tools.forms.base import BaseRockyForm +from tools.forms.base import BaseRockyForm, BaseRockyModelForm from tools.models import ( GROUP_ADMIN, GROUP_CLIENT, @@ -135,7 +135,7 @@ def set_user(self): ) -class OrganizationMemberAddForm(UserAddForm, forms.ModelForm): +class OrganizationMemberAddForm(UserAddForm, BaseRockyModelForm): """ Form to add a new member """ @@ -172,18 +172,28 @@ class Meta: fields = ("account_type", "name", "email", "password") -class OrganizationMemberEditForm(forms.ModelForm): +class OrganizationMemberEditForm(BaseRockyModelForm): trusted_clearance_level = forms.ChoiceField( required=False, - label=_("Trusted clearance level"), + label=_("Assigned clearance level"), choices=[(-1, "")] + SCAN_LEVEL.choices, help_text=_("Select a clearance level you trust this member with."), widget=forms.RadioSelect(attrs={"radio_paws": True}), ) + blocked = forms.BooleanField( + required=False, + label=_("Blocked"), + help_text=_("Set the members status to blocked, so they don't have access to the organization anymore."), + widget=forms.CheckboxInput(), + ) + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - + self.fields["blocked"].widget.attrs["field_form_label"] = "Status" + if self.instance.user.is_superuser: + self.fields["trusted_clearance_level"].disabled = True + self.fields["acknowledged_clearance_level"].label = _("Accepted clearance level") self.fields["acknowledged_clearance_level"].required = False self.fields["acknowledged_clearance_level"].widget.attrs[ "fixed_paws" @@ -202,10 +212,10 @@ def save(self, commit=True): class Meta: model = OrganizationMember - fields = ["status", "trusted_clearance_level", "acknowledged_clearance_level"] + fields = ["blocked", "trusted_clearance_level", "acknowledged_clearance_level"] -class OrganizationForm(forms.ModelForm): +class OrganizationForm(BaseRockyModelForm): """ Form to create a new organization. """ diff --git a/rocky/account/mixins.py b/rocky/account/mixins.py index a6598702e62..39e8a51753e 100644 --- a/rocky/account/mixins.py +++ b/rocky/account/mixins.py @@ -44,7 +44,7 @@ def setup(self, request, *args, **kwargs): except OrganizationMember.DoesNotExist: raise Http404() - if self.organization_member.status == OrganizationMember.STATUSES.BLOCKED: + if self.organization_member.blocked: raise PermissionDenied() self.octopoes_api_connector = OctopoesAPIConnector(settings.OCTOPOES_API, organization_code) diff --git a/rocky/account/models.py b/rocky/account/models.py index 9fe3317e22d..fc8de59a3b3 100644 --- a/rocky/account/models.py +++ b/rocky/account/models.py @@ -104,10 +104,7 @@ def organizations(self) -> List[Organization]: """ if self.is_superuser: return self.all_organizations - return [ - m.organization - for m in filter(lambda o: o.status is not OrganizationMember.STATUSES.BLOCKED, self.organization_members) - ] + return [m.organization for m in self.organization_members if not m.blocked] @cached_property def organizations_including_blocked(self) -> List[Organization]: diff --git a/rocky/assets/src/bundles/app/css/main.scss b/rocky/assets/src/bundles/app/css/main.scss index 30c33386efd..50cff0dcd7e 100644 --- a/rocky/assets/src/bundles/app/css/main.scss +++ b/rocky/assets/src/bundles/app/css/main.scss @@ -12,6 +12,8 @@ @import "vendors/graph-override.scss"; @import "vendors/two-factor.scss"; @import "vendors/manon-overrides/dl.scss"; +@import "vendors/manon-overrides/form-radio.scss"; +@import "vendors/manon-overrides/form-fieldset-required.scss"; @import "vendors/manon-overrides/link.scss"; @import "vendors/manon-overrides/tile.scss"; diff --git a/rocky/assets/src/bundles/app/css/themes/soft/manon/form-fieldset.scss b/rocky/assets/src/bundles/app/css/themes/soft/manon/form-fieldset.scss new file mode 100644 index 00000000000..315ddb594c2 --- /dev/null +++ b/rocky/assets/src/bundles/app/css/themes/soft/manon/form-fieldset.scss @@ -0,0 +1,5 @@ +/* Form fieldset - Variables */ + +:root { + --form-horizontal-view-fieldset-label-margin-top: 0; +} diff --git a/rocky/assets/src/bundles/app/css/themes/soft/manon/form-radio.scss b/rocky/assets/src/bundles/app/css/themes/soft/manon/form-radio.scss new file mode 100644 index 00000000000..8248193be56 --- /dev/null +++ b/rocky/assets/src/bundles/app/css/themes/soft/manon/form-radio.scss @@ -0,0 +1,5 @@ +/* Form fieldset - Variables */ + +:root { + --form-radio-margin: 0.25rem var(--form-radio-margin-right) 0.25rem 0; +} diff --git a/rocky/assets/src/bundles/app/css/themes/soft/manon/form.scss b/rocky/assets/src/bundles/app/css/themes/soft/manon/form.scss index f6cbc9e1f1e..a456b853538 100644 --- a/rocky/assets/src/bundles/app/css/themes/soft/manon/form.scss +++ b/rocky/assets/src/bundles/app/css/themes/soft/manon/form.scss @@ -20,4 +20,7 @@ /* Fieldset */ --form-fieldset-legend-font-weight: 600; + + /* Nota bene */ + --form-horizontal-view-fieldset-nota-bene-required-margin-bottom: 0.5rem; } diff --git a/rocky/assets/src/bundles/app/css/themes/soft/soft.scss b/rocky/assets/src/bundles/app/css/themes/soft/soft.scss index 144f316d90c..212e526a05d 100644 --- a/rocky/assets/src/bundles/app/css/themes/soft/soft.scss +++ b/rocky/assets/src/bundles/app/css/themes/soft/soft.scss @@ -25,6 +25,8 @@ @import "manon/button-icon"; /* Icon button */ @import "manon/description-list"; @import "manon/filter"; +@import "manon/form-fieldset"; +@import "manon/form-radio"; @import "manon/layout"; @import "manon/login-meta"; @import "manon/navigation"; diff --git a/rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-fieldset-required.scss b/rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-fieldset-required.scss new file mode 100644 index 00000000000..bb869cfd84d --- /dev/null +++ b/rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-fieldset-required.scss @@ -0,0 +1,16 @@ +/* Form fieldset required */ + +form.horizontal-view div.required { + + label { + margin-top: calc(var(--form-horizontal-view-fieldset-nota-bene-required-margin-bottom) + var(--form-input-min-height) /2); + } + + .help-button { + margin-top: calc(var(--form-horizontal-view-fieldset-nota-bene-required-margin-bottom) + var(--form-input-min-height) /2); + } + + select + div + .help-button { + margin-top: 2rem; + } +} diff --git a/rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-radio.scss b/rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-radio.scss new file mode 100644 index 00000000000..9d8b4a1ec2a --- /dev/null +++ b/rocky/assets/src/bundles/app/css/vendors/manon-overrides/form-radio.scss @@ -0,0 +1,5 @@ +/* Form radio */ + +form input[type="radio"] { + margin: var(--form-radio-margin); +} diff --git a/rocky/onboarding/views.py b/rocky/onboarding/views.py index dea4d2b8f54..29d1a4173e0 100644 --- a/rocky/onboarding/views.py +++ b/rocky/onboarding/views.py @@ -355,6 +355,7 @@ def post(self, request, *args, **kwargs): def set_member_onboarded(self): member = OrganizationMember.objects.get(user=self.request.user, organization=self.organization) member.onboarded = True + member.status = OrganizationMember.STATUSES.ACTIVE member.save() @@ -648,5 +649,6 @@ def get(self, request, *args, **kwargs): redteam_group.user_set.add(self.request.user) return redirect(reverse("step_introduction", kwargs={"organization_code": self.organization.code})) member.onboarded = True + member.status = OrganizationMember.STATUSES.ACTIVE member.save() return redirect(reverse("crisis_room")) diff --git a/rocky/rocky/templates/organizations/organization_member_add.html b/rocky/rocky/templates/organizations/organization_member_add.html index fdc49bc893f..2b21bedced2 100644 --- a/rocky/rocky/templates/organizations/organization_member_add.html +++ b/rocky/rocky/templates/organizations/organization_member_add.html @@ -4,7 +4,7 @@ {% load static %} {% block content %} - {% include "header.html" with view_type="onboarding" %} + {% include "header.html" %}
diff --git a/rocky/rocky/templates/organizations/organization_member_list.html b/rocky/rocky/templates/organizations/organization_member_list.html index 8e60e79c3f9..d66901bba47 100644 --- a/rocky/rocky/templates/organizations/organization_member_list.html +++ b/rocky/rocky/templates/organizations/organization_member_list.html @@ -11,8 +11,8 @@

{% translate "Organization" %}: {{ organization.name }}

{% blocktranslate with organization_name=organization.name %} - An overview of "{{ organization_name }}" its members. -{% endblocktranslate %} + An overview of "{{ organization_name }}" its members. + {% endblocktranslate %}

{% translate "Members" %}

{% if perms.tools.add_organizationmember %}
@@ -33,7 +33,7 @@

{% translate "Members" %}

{% translate "Status" %} {% translate "Added" %} {% translate "Assigned clearance level" %} - {% translate "Agreed clearance level" %} + {% translate "Accepted clearance level" %} {% translate "Edit" %} {% translate "Blocked" %} @@ -60,7 +60,8 @@

{% translate "Members" %}

{% if member.user.is_superuser %}  {% translate "Active" %} {% else %} -  {{ member.status|title }} + +  {% if member.blocked %}{% translate "Blocked" %}{% else %}{{ member.status|title }}{% endif %} {% endif %}
@@ -88,17 +89,22 @@

{% translate "Members" %}

{% endif %} - - - + {% if member.user.is_superuser %} + + {% else %} + + {% endif %} - {% if member.status == "blocked" %} + {% if member.blocked %} {% include "partials/single_action_checkbox_form.html" with input_checked=member.blocked input_disabled=member.user.is_superuser action="unblock" key="member_id" value=member.id %} - {% else %} - {% include "partials/single_action_checkbox_form.html" with input_checked=member.blocked input_disabled=member.user.is_superuser action="block" key="member_id" value=member.id %} - + {% comment %} Disable the "block" checkbox for members that are super users or for the member of the user that's logged in so they can't lock themself out of the organisation by accident{% endcomment %} + {% if member.user.is_superuser or member.user.email == request.user.email %} + {% include "partials/single_action_checkbox_form.html" with input_checked=member.blocked input_disabled="disabled" action="block" key="member_id" value=member.id %} + {% else %} + {% include "partials/single_action_checkbox_form.html" with input_checked=member.blocked action="block" key="member_id" value=member.id %} + {% endif %} {% endif %} diff --git a/rocky/rocky/templates/partials/form/field_input.html b/rocky/rocky/templates/partials/form/field_input.html index 22f0db5859f..087db958d76 100644 --- a/rocky/rocky/templates/partials/form/field_input.html +++ b/rocky/rocky/templates/partials/form/field_input.html @@ -1,46 +1,45 @@ {% load i18n %} -
- - {{ field.label_tag }} - - {% if form_view != "vertical" %} -
- {% if field.field.required %} - {% translate "This field is required" %} - {% endif %} - {% if not field.field.widget.attrs.fixed_paws %} - {{ field }} - {% elif field.field.widget.attrs.fixed_paws < 0 %} - {% translate "Unset" %} - {% else %} - {% include "partials/scan_level_indicator.html" with value=field.field.widget.attrs.fixed_paws custom_class=field.field.widget.attrs.class %} +
+ {{ field.label_tag }} - {% endif %} - {% include "partials/form/field_input_help_text.html" with help_text=field.help_text %} - {% include "partials/form/field_input_errors.html" %} - - {% if form_name == 'login' and field.name == "username" %} -
- {% translate "Forgot email" %} -
- {% endif %} - {% if form_name == 'login' and field.name == "password" %} -
- {% translate "Forgot password" %} -
- {% endif %} -
-
-{% else %} - {{ field.label_tag }} -
+ {% if form_view != "vertical" %} +
{% if field.field.required %} {% translate "This field is required" %} {% endif %} - {{ field }} + {% if not field.field.widget.attrs.fixed_paws %} + {{ field }} + {% elif field.field.widget.attrs.fixed_paws < 0 %} + {% translate "Not set" %} + {% else %} + {% include "partials/scan_level_indicator.html" with value=field.field.widget.attrs.fixed_paws custom_class=field.field.widget.attrs.class %} + {% endif %} + {% include "partials/form/field_input_help_text.html" with help_text=field.help_text %} {% include "partials/form/field_input_errors.html" %} + {% if form_name == "login" and field.name == "username" %} +
+ {% translate "Forgot email" %} +
+ {% endif %} + {% if form_name == "login" and field.name == "password" %} +
+ {% translate "Forgot password" %} +
+ {% endif %}
-{% endif %} + {% else %} +
+ {% if field.field.required %} + {% translate "This field is required" %} + {% endif %} + + {{ field }} + + {% include "partials/form/field_input_help_text.html" with help_text=field.help_text %} + {% include "partials/form/field_input_errors.html" %} +
+ {% endif %} +
diff --git a/rocky/rocky/templates/partials/form/field_input_checkbox.html b/rocky/rocky/templates/partials/form/field_input_checkbox.html index 1fd8c4bcd10..277d22244d1 100644 --- a/rocky/rocky/templates/partials/form/field_input_checkbox.html +++ b/rocky/rocky/templates/partials/form/field_input_checkbox.html @@ -2,18 +2,21 @@ {% if field.field.widget.allow_multiple_selected %} {% include "partials/form/field_input.html" %} - {% else %} -
- {% if field.field.required %} - {% translate "This field is required" %} - {% endif %} -
- {{ field }} - {{ field.label_tag }} -
- {% include "partials/form/field_input_help_text.html" with help_text=field.help_text %} - {% include "partials/form/field_input_errors.html" %} +
+ +
+ {% if field.field.required %} + {% translate "This field is required" %} + {% endif %} + + {{ field }} + {{ field.label_tag }} -
+ {% include "partials/form/field_input_help_text.html" with help_text=field.help_text %} + {% include "partials/form/field_input_errors.html" %} +
+
{% endif %} diff --git a/rocky/rocky/templates/partials/form/field_input_radio.html b/rocky/rocky/templates/partials/form/field_input_radio.html index 4c981bc8f3f..5112fa2d47a 100644 --- a/rocky/rocky/templates/partials/form/field_input_radio.html +++ b/rocky/rocky/templates/partials/form/field_input_radio.html @@ -1,7 +1,6 @@ -
- - {{ field.label_tag }} - +
+ {{ field.label_tag }} + {% for choice in field %}
{% if choice.data.attrs.radio_paws %} {% include "partials/scan_level_indicator.html" with value=choice.data.value custom_class=choice.data.attrs.class %} - {% else %} {{ choice.choice_label }} {% endif %} @@ -23,4 +21,4 @@
{% endfor %} -
+
diff --git a/rocky/rocky/templates/partials/form/field_input_wrapper.html b/rocky/rocky/templates/partials/form/field_input_wrapper.html index 1bdd1e6d257..05b3cfd84a1 100644 --- a/rocky/rocky/templates/partials/form/field_input_wrapper.html +++ b/rocky/rocky/templates/partials/form/field_input_wrapper.html @@ -1,8 +1,6 @@ {# include template for input type, with fallback field_input.html #} {% if field.field.widget.input_type in "checkbox,radio,hidden" %} {% include "partials/form/field_input_"|add:field.field.widget.input_type|add:".html" %} - {% else %} {% include "partials/form/field_input.html" %} - {% endif %} diff --git a/rocky/rocky/templates/partials/form/fieldset.html b/rocky/rocky/templates/partials/form/fieldset.html index 9bb735e8b2e..a68acd33c2f 100644 --- a/rocky/rocky/templates/partials/form/fieldset.html +++ b/rocky/rocky/templates/partials/form/fieldset.html @@ -9,7 +9,6 @@ {% for field in fields %} {% if not fieldset or field.name in fieldset.split %} {% include "partials/form/field_input_wrapper.html" %} - {% endif %} {% endfor %} diff --git a/rocky/rocky/templates/partials/organization_member_list_filters.html b/rocky/rocky/templates/partials/organization_member_list_filters.html index 0b161d26fc9..67f37473d83 100644 --- a/rocky/rocky/templates/partials/organization_member_list_filters.html +++ b/rocky/rocky/templates/partials/organization_member_list_filters.html @@ -12,18 +12,35 @@ {% translate "Shown status types" %}
- {% for checkbox in checkbox_filters %} + {% for option in status_filters %}
- + value="{{ option.value }}" + {% if option.checked %} checked{% endif %}> +
{% endfor %}
+
+ + {% translate "Blocked status" %} + +
+ {% for option in blocked_filters %} +
+ + +
+ {% endfor %} +
+
{% endblock filter_form %} diff --git a/rocky/rocky/views/indemnification_add.py b/rocky/rocky/views/indemnification_add.py index 647e8bf3d78..2a8ee1c461e 100644 --- a/rocky/rocky/views/indemnification_add.py +++ b/rocky/rocky/views/indemnification_add.py @@ -3,6 +3,7 @@ from django.contrib import messages from django.contrib.auth.mixins import PermissionRequiredMixin from django.urls import reverse_lazy +from django.urls.base import reverse from django.utils.translation import gettext_lazy as _ from django.views.generic import FormView from django_otp.decorators import otp_required @@ -36,4 +37,18 @@ def get_context_data(self, **kwargs): context["indemnification_present"] = Indemnification.objects.filter( user=self.request.user, organization=self.organization ) + context["breadcrumbs"] = [ + { + "url": reverse("organization_settings", kwargs={"organization_code": self.organization.code}), + "text": "Settings", + }, + { + "url": reverse( + "indemnification_add", + kwargs={"organization_code": self.organization.code}, + ), + "text": _("Add indemnification"), + }, + ] + return context diff --git a/rocky/rocky/views/organization_member_add.py b/rocky/rocky/views/organization_member_add.py index 37bf82108e1..3c50887a4c6 100644 --- a/rocky/rocky/views/organization_member_add.py +++ b/rocky/rocky/views/organization_member_add.py @@ -1,19 +1,20 @@ from account.forms import OrganizationMemberToGroupAddForm +from account.mixins import OrganizationView from django.contrib import messages from django.contrib.auth import get_user_model from django.contrib.auth.mixins import PermissionRequiredMixin from django.urls import reverse_lazy +from django.urls.base import reverse from django.utils.translation import gettext_lazy as _ from django.views.generic.edit import CreateView from django_otp.decorators import otp_required -from tools.view_helpers import OrganizationMemberBreadcrumbsMixin from two_factor.views.utils import class_view_decorator User = get_user_model() @class_view_decorator(otp_required) -class OrganizationMemberAddView(PermissionRequiredMixin, OrganizationMemberBreadcrumbsMixin, CreateView): +class OrganizationMemberAddView(PermissionRequiredMixin, OrganizationView, CreateView): """ View to create a new member for a specific organization. """ @@ -33,7 +34,21 @@ def get_success_url(self, **kwargs): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) + context["breadcrumbs"] = [ + { + "url": reverse("organization_member_list", kwargs={"organization_code": self.organization.code}), + "text": "Members", + }, + { + "url": reverse( + "organization_member_add", + kwargs={"organization_code": self.organization.code}, + ), + "text": _("Add member"), + }, + ] context["organization"] = self.organization + return context def form_valid(self, form): diff --git a/rocky/rocky/views/organization_member_edit.py b/rocky/rocky/views/organization_member_edit.py index 74383e88ad0..30677f2c2d3 100644 --- a/rocky/rocky/views/organization_member_edit.py +++ b/rocky/rocky/views/organization_member_edit.py @@ -6,7 +6,7 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import UpdateView from django_otp.decorators import otp_required -from tools.models import GROUP_ADMIN, GROUP_CLIENT, OrganizationMember +from tools.models import GROUP_CLIENT, OrganizationMember from two_factor.views.utils import class_view_decorator @@ -26,9 +26,12 @@ def test_func(self): def get_form(self): form = super().get_form() group = self.object.user.groups.all().values_list("name", flat=True) - if self.object.user.is_superuser or GROUP_ADMIN in group: - # There could be a case where you block yourself out of the system - form.fields["status"].disabled = True + + # Make sure the logged in user can't block himself out of the organisation. + if self.object.user == self.request.user: + form.fields["blocked"].disabled = True + + # Since clients aren't allowed to scan and set clearance levels, disable the truste clearance level field. if GROUP_CLIENT in group: form.fields["trusted_clearance_level"].disabled = True return form @@ -46,8 +49,8 @@ def get_context_data(self, **kwargs): context["breadcrumbs"] = [ { - "url": reverse("organization_settings", kwargs={"organization_code": self.organization.code}), - "text": self.organization.name, + "url": reverse("organization_member_list", kwargs={"organization_code": self.organization.code}), + "text": "Members", }, { "url": reverse( diff --git a/rocky/rocky/views/organization_member_list.py b/rocky/rocky/views/organization_member_list.py index 0ab1a7c8f6a..23ad4db3aac 100644 --- a/rocky/rocky/views/organization_member_list.py +++ b/rocky/rocky/views/organization_member_list.py @@ -3,8 +3,10 @@ from django.contrib import messages from django.contrib.auth.mixins import PermissionRequiredMixin from django.core.exceptions import PermissionDenied +from django.db import models from django.shortcuts import redirect from django.urls.base import reverse +from django.utils.translation import gettext_lazy as _ from django.views.generic import ListView from django_otp.decorators import otp_required from requests.exceptions import RequestException @@ -13,6 +15,11 @@ from two_factor.views.utils import class_view_decorator +class BLOCK_STATUSES(models.TextChoices): + BLOCKED = _("Blocked"), "blocked" + UNBLOCKED = _("Not blocked"), "unblocked" + + class PageActions(Enum): BLOCK = "block" UNBLOCK = "unblock" @@ -33,18 +40,28 @@ def get_queryset(self): queryset = self.model.objects.filter(organization=self.organization) if "client_status" in self.request.GET: status_filter = self.request.GET.getlist("client_status", []) - queryset = self.filter_queryset(queryset, status_filter) - return queryset + queryset = [member for member in queryset if member.status in status_filter] + + if "blocked_status" in self.request.GET: + blocked_filter = self.request.GET.getlist("blocked_status", []) + blocked_filter_bools = [] - def filter_queryset(self, queryset, blocked_status_filter): - return [member for member in queryset if member.status in blocked_status_filter] + # Conversion from string values to boolean values + for filter_option in blocked_filter: + if filter_option == "blocked": + blocked_filter_bools.append(True) + if filter_option == "unblocked": + blocked_filter_bools.append(False) + + queryset = [member for member in queryset if member.blocked in blocked_filter_bools] + return queryset def setup(self, request, *args, **kwargs): super().setup(request, *args, **kwargs) self.filters_active = self.get_filters_active() def post(self, request, *args, **kwargs): - if not self.request.user.has_perms("tools.change_organizationmember"): + if not self.request.user.has_perm("tools.change_organizationmember"): raise PermissionDenied() if "action" not in self.request.POST: return self.get(request, *args, **kwargs) @@ -56,9 +73,19 @@ def handle_page_action(self, action: str): organizationmember = self.model.objects.get(id=member_id) try: if action == PageActions.BLOCK.value: - organizationmember.status = OrganizationMember.STATUSES.BLOCKED + organizationmember.blocked = True + messages.add_message( + self.request, + messages.SUCCESS, + _("Blocked member %s successfully.") % (organizationmember.user.full_name), + ) elif action == PageActions.UNBLOCK.value: - organizationmember.status = OrganizationMember.STATUSES.ACTIVE + organizationmember.blocked = False + messages.add_message( + self.request, + messages.SUCCESS, + _("Unblocked member %s successfully.") % (organizationmember.user.full_name), + ) else: raise Exception(f"Unhandled allowed action: {action}") organizationmember.save() @@ -66,9 +93,11 @@ def handle_page_action(self, action: str): messages.add_message(self.request, messages.ERROR, f"{action} failed: '{exception}'") def get_filters_active(self): - return self.request.GET.getlist("client_status", []) + active_filters = self.request.GET.getlist("client_status", []) + active_filters += [item.lower() for item in self.request.GET.getlist("blocked_status", [])] + return active_filters - def get_checkbox_filters(self): + def get_status_filters(self): return [ { "label": choice[0], @@ -78,7 +107,19 @@ def get_checkbox_filters(self): for choice in OrganizationMember.STATUSES.choices ] + def get_blocked_filters(self): + return [ + { + "label": choice[0], + "value": choice[1], + "checked": not self.filters_active or choice[1] in self.filters_active, + } + for choice in BLOCK_STATUSES.choices + ] + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - context["checkbox_filters"] = self.get_checkbox_filters() + context["status_filters"] = self.get_status_filters() + context["blocked_filters"] = self.get_blocked_filters() + return context diff --git a/rocky/tests/conftest.py b/rocky/tests/conftest.py index 0e0798646a3..71aeeceeeaf 100644 --- a/rocky/tests/conftest.py +++ b/rocky/tests/conftest.py @@ -55,6 +55,7 @@ def create_member(user, organization): user=user, organization=organization, status=OrganizationMember.STATUSES.ACTIVE, + blocked=False, trusted_clearance_level=4, acknowledged_clearance_level=4, onboarded=False, @@ -228,7 +229,8 @@ def active_member(django_user_model, organization): def blocked_member(django_user_model, organization): user = create_user(django_user_model, "cl3@openkat.nl", "TestTest123!!", "Blocked user", "default_blocked_user") member = create_member(user, organization) - member.status = OrganizationMember.STATUSES.BLOCKED + member.status = OrganizationMember.STATUSES.ACTIVE + member.blocked = True member.save() return member diff --git a/rocky/tests/test_members.py b/rocky/tests/test_members.py index d836a8f9cdf..19db5a533b8 100644 --- a/rocky/tests/test_members.py +++ b/rocky/tests/test_members.py @@ -154,7 +154,7 @@ def test_admin_edits_redteamer_to_block(rf, admin_member, redteam_member): request = setup_request( rf.post( "organization_member_edit", - {"status": "blocked", "trusted_clearance_level": 4}, + {"blocked": True, "trusted_clearance_level": 4}, ), admin_member.user, ) @@ -163,4 +163,4 @@ def test_admin_edits_redteamer_to_block(rf, admin_member, redteam_member): ) redteam_member.refresh_from_db() - assert redteam_member.status == "blocked" + assert redteam_member.blocked is True diff --git a/rocky/tests/test_organization.py b/rocky/tests/test_organization.py index 6a593c4d875..34eef0a1ba9 100644 --- a/rocky/tests/test_organization.py +++ b/rocky/tests/test_organization.py @@ -183,7 +183,7 @@ def test_organization_member_list(rf, admin_member): assertContains(response, admin_member.user.date_joined.strftime("%m/%d/%Y")) assertContains(response, "Assigned clearance level") assertContains(response, admin_member.trusted_clearance_level) - assertContains(response, "Agreed clearance level") + assertContains(response, "Accepted clearance level") assertContains(response, admin_member.acknowledged_clearance_level) assertContains(response, "Edit") assertContains(response, admin_member.id) @@ -191,35 +191,42 @@ def test_organization_member_list(rf, admin_member): def test_organization_filtered_member_list(rf, superuser_member, new_member, blocked_member): - request = setup_request(rf.get("organization_member_list", {"client_status": "blocked"}), superuser_member.user) + # Test with only filter option blocked status "blocked" + request = setup_request(rf.get("organization_member_list", {"blocked_status": "blocked"}), superuser_member.user) response = OrganizationMemberListView.as_view()(request, organization_code=superuser_member.organization.code) assertNotContains(response, new_member.user.full_name) assertContains(response, blocked_member.user.full_name) assertContains(response, 'class="blocked"') - assertNotContains(response, "New") - assertNotContains(response, "Active") + assertNotContains(response, 'class="new"') + assertNotContains(response, 'class="active"') + # Test with only filter option status "new" checked request2 = setup_request(rf.get("organization_member_list", {"client_status": "new"}), superuser_member.user) response2 = OrganizationMemberListView.as_view()(request2, organization_code=superuser_member.organization.code) assertContains(response2, new_member.user.full_name) assertNotContains(response2, blocked_member.user.full_name) - assertContains(response2, "New") + assertContains(response2, 'class="new"') assertNotContains(response2, 'class="blocked"') - assertNotContains(response2, "Active") + assertNotContains(response2, 'class="active"') + # Test with every filter option checked (new, active, blocked and unblocked) request3 = setup_request( - rf.get("organization_member_list", {"client_status": ["new", "active", "blocked"]}), superuser_member.user + rf.get( + "organization_member_list", + {"client_status": ["new", "active"], "blocked_status": ["blocked", "unblocked"]}, + ), + superuser_member.user, ) response3 = OrganizationMemberListView.as_view()(request3, organization_code=superuser_member.organization.code) assertContains(response3, superuser_member.user.full_name) assertContains(response3, new_member.user.full_name) assertContains(response3, blocked_member.user.full_name) - assertContains(response3, "New") + assertContains(response3, 'class="new"') assertContains(response3, 'class="blocked"') - assertContains(response3, "Active") + assertContains(response3, 'class="active"') def test_organization_does_not_exist(client, client_member): diff --git a/rocky/tools/migrations/0033_auto_20230407_1113.py b/rocky/tools/migrations/0033_auto_20230407_1113.py new file mode 100644 index 00000000000..87229fc3cbf --- /dev/null +++ b/rocky/tools/migrations/0033_auto_20230407_1113.py @@ -0,0 +1,28 @@ +# Generated by Django 3.2.18 on 2023-04-07 11:13 + +from django.db import migrations, models + + +def migrate_organizationmember_status_blocked_to_blocked_attribute(apps, schema_editor): + OrganizationMember = apps.get_model("tools", "OrganizationMember") + OrganizationMember.objects.filter(status="blocked").update(status="active", blocked=True) + + +class Migration(migrations.Migration): + dependencies = [ + ("tools", "0032_alter_organizationmember_user"), + ] + + operations = [ + migrations.AddField( + model_name="organizationmember", + name="blocked", + field=models.BooleanField(default=False), + ), + migrations.RunPython(migrate_organizationmember_status_blocked_to_blocked_attribute), + migrations.AlterField( + model_name="organizationmember", + name="status", + field=models.CharField(choices=[("active", "active"), ("new", "new")], default="new", max_length=64), + ), + ] diff --git a/rocky/tools/models.py b/rocky/tools/models.py index c892b99a80c..93a2c9c62f4 100644 --- a/rocky/tools/models.py +++ b/rocky/tools/models.py @@ -151,13 +151,13 @@ class OrganizationMember(models.Model): class STATUSES(models.TextChoices): ACTIVE = "active", _("active") NEW = "new", _("new") - BLOCKED = "blocked", _("blocked") scan_levels = [scan_level.value for scan_level in SCAN_LEVEL] user = models.ForeignKey("account.KATUser", on_delete=models.PROTECT, related_name="members") organization = models.ForeignKey(Organization, on_delete=models.CASCADE, related_name="members") status = models.CharField(choices=STATUSES.choices, max_length=64, default=STATUSES.NEW) + blocked = models.BooleanField(default=False) onboarded = models.BooleanField(default=False) trusted_clearance_level = models.IntegerField( default=-1, validators=[MinValueValidator(-1), MaxValueValidator(max(scan_levels))] @@ -166,10 +166,6 @@ class STATUSES(models.TextChoices): default=-1, validators=[MinValueValidator(-1), MaxValueValidator(max(scan_levels))] ) - @property - def blocked(self): - return self.status == OrganizationMember.STATUSES.BLOCKED - class Meta: unique_together = ["user", "organization"]