Fixes #548: Admin users can no longer delete themselves from an organ…

…ization

administrator can no longer change their own role in the organization.

cadasta/organization/forms.py

@@ -187,20 +187,20 @@ def save(self):
 class EditOrganizationMemberForm(forms.Form):
     org_role = forms.ChoiceField(choices=ADMIN_CHOICES)
 
-    def __init__(self, data, organization, user, *args, **kwargs):
+    def __init__(self, data, org, user, current_user, *args, **kwargs):
         super(EditOrganizationMemberForm, self).__init__(data, *args, **kwargs)
         self.data = data
-        self.organization = organization
+        self.organization = org
         self.user = user
-
+        self.current_user = current_user
         self.org_role_instance = OrganizationRole.objects.get(
             user=user,
             organization=self.organization)
 
         self.initial['org_role'] = 'A' if self.org_role_instance.admin else 'M'
 
         project_roles = ProjectRole.objects.filter(
-            project__organization=organization).values('project__id', 'role')
+            project__organization=org).values('project__id', 'role')
         project_roles = {r['project__id']: r['role'] for r in project_roles}
 
         for p in self.organization.projects.values_list('id', 'name'):
@@ -213,6 +213,15 @@ def __init__(self, data, organization, user, *args, **kwargs):
                 initial=role
             )
 
+    def clean_org_role(self):
+        org_role = self.cleaned_data['org_role']
+        if self.org_role_instance.admin and self.current_user == self.user:
+            if self.data.get('org_role') != 'A':
+                raise forms.ValidationError(
+                    _("Organization administrators cannot change their own" +
+                        " role in the organization."))
+        return org_role
+
     def save(self):
         self.org_role_instance.admin = self.data.get('org_role') == 'A'
         self.org_role_instance.save()

cadasta/organization/serializers.py

@@ -151,6 +151,15 @@ def validate_username(self, value):
             if error:
                 raise serializers.ValidationError(error.format(value))
 
+    def validate_admin(self, role_value):
+        if 'request' in self.context:
+            if self.context['request'].user == self.instance:
+                if role_value != self.get_roles_object(self.instance).admin:
+                    raise serializers.ValidationError(
+                        _("Organization administrators cannot change their "
+                          "own permissions within the organization"))
+        return role_value
+
     def create(self, validated_data):
         obj = self.context[self.Meta.context_key]
 
@@ -171,7 +180,6 @@ def create(self, validated_data):
 
     def update(self, instance, validated_data):
         role = self.get_roles_object(instance)
-
         role_value = validated_data[self.Meta.role_key]
 
         if self.Meta.role_key in validated_data:

cadasta/organization/tests/test_forms.py

@@ -239,18 +239,40 @@ class EditOrganizationMemberFormTest(UserTestCase):
     def test_edit_org_role(self):
         org = OrganizationFactory.create()
         user = UserFactory.create()
+        current_user = UserFactory.create()
 
         data = {'org_role': 'A'}
 
         org_role = OrganizationRole.objects.create(organization=org, user=user)
-        form = forms.EditOrganizationMemberForm(data, org, user)
+        OrganizationRole.objects.create(
+            organization=org, user=current_user, admin=True)
+        form = forms.EditOrganizationMemberForm(data, org, user, current_user)
 
         form.save()
         org_role.refresh_from_db()
 
         assert form.is_valid() is True
         assert org_role.admin is True
 
+    def test_edit_admin_role(self):
+        # Should fail, since admins are not allowed to alter
+        # their own permissions.
+        org = OrganizationFactory.create()
+        user = UserFactory.create()
+
+        data = {'org_role': 'M'}
+
+        org_role = OrganizationRole.objects.create(
+            organization=org, user=user, admin=True)
+        form = forms.EditOrganizationMemberForm(data, org, user, user)
+        assert not form.is_valid()
+        assert form.errors == {
+            'org_role': ["Organization administrators cannot change their" +
+                         " own role in the organization."]
+        }
+        org_role.refresh_from_db()
+        assert org_role.admin is True
+
     def test_edit_project_roles(self):
         user = UserFactory.create()
         org = OrganizationFactory.create()
@@ -270,7 +292,7 @@ def test_edit_project_roles(self):
             prj_4.id: 'Pb'
         }
 
-        form = forms.EditOrganizationMemberForm(data, org, user)
+        form = forms.EditOrganizationMemberForm(data, org, user, user)
 
         form.save()
         org_role.refresh_from_db()

cadasta/organization/tests/test_views_api_organizations.py

@@ -73,7 +73,6 @@ def test_list_only_one_organization_is_authorized(self):
         """
         OrganizationFactory.create_from_kwargs([{}, {'slug': 'unauthorized'}])
         content = self._get(user=self.unauth_user, status=200, length=1)
-        print(content[0])
         assert content[0]['slug'] != 'unauthorized'
 
     def test_full_list_with_unauthorized_user(self):
@@ -427,12 +426,14 @@ def _get(self, org, username, user=None, status=None):
             assert response.status_code == status
         return content
 
-    def _patch(self, org, username, data, status=None, count=None):
+    def _patch(self, org, username, data, user=None, status=None, count=None):
+        if not user:
+            user = self.user
         url = '/v1/organizations/{org}/users/{username}'
         request = APIRequestFactory().patch(
             url.format(org=org.slug, username=username),
             data=data, format='json')
-        force_authenticate(request, user=self.user)
+        force_authenticate(request, user=user)
         response = self.view(request, slug=org.slug,
                              username=username).render()
         content = json.loads(response.content.decode('utf-8'))
@@ -478,6 +479,22 @@ def test_update_user(self):
         role = OrganizationRole.objects.get(organization=org, user=user)
         assert role.admin is True
 
+    def test_update_admin_user(self):
+        user = UserFactory.create()
+        org = OrganizationFactory.create(add_users=[user])
+        self._patch(org, user.username, data={'admin': 'true'},
+                    status=200, count=1)
+        role = OrganizationRole.objects.get(organization=org, user=user)
+        assert role.admin is True
+
+        content = self._patch(org, user.username, user=user,
+                              data={'admin': 'false'}, status=400, count=1)
+        assert content['admin'][0] == ("Organization administrators cannot"
+                                       " change their own permissions within"
+                                       " the organization")
+        role = OrganizationRole.objects.get(organization=org, user=user)
+        assert role.admin is True
+
     def test_remove_user(self):
         user = UserFactory.create()
         user_to_remove = UserFactory.create()
@@ -486,6 +503,14 @@ def test_remove_user(self):
         assert user_to_remove not in org.users.all()
         assert user_to_remove in User.objects.all()
 
+    def test_remove_admin_user(self):
+        user = UserFactory.create()
+        org = OrganizationFactory.create(add_users=[user])
+        self._patch(org, user.username, data={'admin': 'true'},
+                    status=200, count=1)
+        self._delete(org, user.username, user=user, status=403, count=1)
+        assert user in org.users.all()
+
     def test_remove_with_unauthorized_user(self):
         user = UserFactory.create()
         user_to_remove = UserFactory.create()

cadasta/organization/tests/test_views_default_organizations.py

@@ -697,6 +697,7 @@ def test_get_with_authorized_user(self):
         user = UserFactory.create()
         assign_user_policies(user, self.policy)
         setattr(self.request, 'user', user)
+        OrganizationRole.objects.create(organization=self.org, user=user)
 
         response = self.view(
             self.request,
@@ -708,7 +709,8 @@ def test_get_with_authorized_user(self):
         context['object'] = self.member
         context['organization'] = self.org
         context['form'] = forms.EditOrganizationMemberForm(
-            None, self.org, self.member)
+            None, self.org, self.member, user)
+        # context['current_user'] = user
 
         expected = render_to_string(
             'organization/organization_members_edit.html',
@@ -791,6 +793,7 @@ def test_post_with_unauthenticated_user(self):
     def test_post_with_invalid_form(self):
         user = UserFactory.create()
         assign_user_policies(user, self.policy)
+        OrganizationRole.objects.create(organization=self.org, user=user)
         setattr(self.request, 'user', user)
         setattr(self.request, 'method', 'POST')
         setattr(self.request, 'POST', {'org_role': 'X'})

cadasta/organization/views/api.py

@@ -1,3 +1,4 @@
+from django.utils.translation import ugettext as _
 from rest_framework.response import Response
 from rest_framework import generics, filters, status
 from tutelary.mixins import APIPermissionRequiredMixin
@@ -63,11 +64,19 @@ class OrganizationUsersDetail(APIPermissionRequiredMixin,
     permission_required = 'org.users.remove'
 
     def destroy(self, request, *args, **kwargs):
+        user = self.get_object()
+        if user == request.user:
+            return Response(
+                {
+                    _('WARNING'):
+                    _("Organization administrators cannot delete themselves.")
+                },
+                status=status.HTTP_403_FORBIDDEN)
         OrganizationRole.objects.get(
-            organization=self.org, user=self.get_object()
+            organization=self.org, user=user
         ).delete()
 
-        return Response(status=status.HTTP_204_NO_CONTENT)
+        return Response(status=status.HTTP_204_NO_CONTENT, )
 
 
 class UserAdminList(APIPermissionRequiredMixin, generics.ListAPIView):

cadasta/organization/views/default.py

@@ -186,16 +186,25 @@ def get_form(self):
         if self.request.method == 'POST':
             return self.form_class(self.request.POST,
                                    self.get_organization(),
-                                   self.get_object())
+                                   self.get_object(),
+                                   self.request.user)
         else:
             return self.form_class(None,
                                    self.get_organization(),
-                                   self.get_object())
+                                   self.get_object(),
+                                   self.request.user)
 
     def get_context_data(self, *args, **kwargs):
         context = super().get_context_data(*args, **kwargs)
         context['organization'] = self.get_organization()
         context['form'] = self.get_form()
+
+        org_admin = OrganizationRole.objects.get(
+           user=self.request.user,
+           organization=context['organization']).admin
+        context['org_admin'] = (org_admin and
+                                not self.is_superuser and
+                                context['org_member'] == self.request.user)
         return context
 
     def post(self, request, *args, **kwargs):

cadasta/templates/organization/organization_members_edit.html

@@ -33,11 +33,21 @@ <h4>{{ object.username }}</h4>
                 <div class="member-role form-group{% if form.org_role.errors %} has-error{% endif %}">
                   <label for="{{ form.org_role.id_for_label }}">Role</label>
                   <label class="pull-right control-label">{{ form.org_role.errors }}</label>
-                  {% render_field form.org_role class+="form-control" %}
+                  {% if org_admin %}
+                    {% render_field form.org_role class+="form-control" %}
+                  {% else %}
+                    {% render_field form.org_role class+="form-control" %}
+                  {% endif %}
                 </div>
                 <div class="btn-full">
-                  <button type="button" class="btn btn-danger" name="remove" data-toggle="modal" data-target="#remove_confirm">
+                  <button type="button" class="btn btn-danger" name="remove" data-toggle="modal" data-target="#remove_confirm"{% if org_admin %} disabled{% endif %}>
                     <span class="glyphicon glyphicon-remove" aria-hidden="true"></span> Remove member</button>
+                    {% if org_admin %}
+                      <div class="alert alert-full" role="alert">
+                        <div class="pull-left"><span class="glyphicon glyphicon-info-sign"></span></div>
+                        <div>An organization administrator cannot remove themself.</div>
+                      </div>
+                    {% endif %}
                 </div>
               </div>
             </div>
@@ -83,7 +93,7 @@ <h4>{{ object.username }}</h4>
 {% endblock %}
 
 {% block modals %}
-
+{% if not org_admin  %}
 <div class="modal fade" id="remove_confirm" tabindex="-1" role="dialog">
   <div class="modal-dialog">
     <div class="modal-content">
@@ -102,5 +112,6 @@ <h3 class="modal-title">Remove Member</h3>
     </div><!-- /.modal-content -->
   </div><!-- /.modal-dialog -->
 </div><!-- /.modal -->
+{% endif %}
 
 {% endblock %}

functional_tests/organizations/test_organization_member.py

@@ -21,6 +21,7 @@ def setUp(self):
                 organization=test_objs['organizations'][0],
                 user=UserFactory.create(
                         username='admin_user',
+                        full_name='Admin User',
                         password='password'),
                 admin=True)
 
@@ -64,6 +65,29 @@ def test_changing_a_members_organizational_role(self):
         roles = page.get_role_options()
         assert roles["admin"].text == roles["selected"].text
 
+    def test_changing_an_admins_organizational_role(self):
+        """An admin member can change a member's role in the organization."""
+
+        LoginPage(self).login('admin_user', 'password')
+        page = OrganizationMemberPage(self)
+        page.go_to()
+        OrganizationPage(self).go_to_organization_page()
+        OrganizationMemberListPage(self).go_to_member_list_page()
+        page.go_to_admin_member_page()
+
+        roles = page.get_role_options()
+        assert roles["admin"].text == roles["selected"].text
+
+        roles["member"].click()
+        page.click_submit_button()
+        self.get_screenshot()
+        # get error message
+        # assert you stayed on the same page.
+        roles = page.get_role_options()
+        errors = page.get_org_role_error()
+        assert errors.text == ("Organization administrators cannot change" +
+                               " their own role in the organization.")
+
     def test_removing_a_member_from_an_organization(self):
         """An admin member can remove a member from an organization."""
 
@@ -82,6 +106,21 @@ def test_removing_a_member_from_an_organization(self):
         members = page.get_table_row().text
         assert "Test User" not in members
 
+    def test_removing_an_admin_member_from_an_organization(self):
+        """An admin member can remove a member from an organization."""
+
+        LoginPage(self).login('admin_user', 'password')
+        page = OrganizationMemberPage(self)
+        page.go_to()
+        OrganizationPage(self).go_to_organization_page()
+        OrganizationMemberListPage(self).go_to_member_list_page()
+        page.go_to_admin_member_page()
+
+        assert page.get_member_title() == "MEMBER: Admin User"
+
+        page.click_disabled_remove_button()
+        assert page.get_member_title() == "MEMBER: Admin User"
+
     def test_changing_member_project_permissions(self):
         """An admin user can change a member's permissions
         on individual projects."""
@@ -121,4 +160,4 @@ def test_admin_creation_when_adding_organization(self):
         page.go_to_testuser_member_page()
 
         roles = page.get_role_options()
-        assert roles["admin"].text == roles["selected"].text
+        assert roles['selected'].text == "Administrator"