From e15e00336dbbc5e3ab88aa351de43b062786ca7c Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 28 Jan 2016 12:33:55 +0100 Subject: [PATCH] When leaving a project or removing a membership the project must allways have a valid owner --- taiga/projects/api.py | 4 ++-- taiga/projects/permissions.py | 3 +-- taiga/projects/serializers.py | 4 ++-- taiga/projects/services/__init__.py | 2 +- taiga/projects/services/members.py | 14 +++++++---- tests/integration/test_projects.py | 36 ++++++++++++++++++++++++++--- 6 files changed, 48 insertions(+), 15 deletions(-) diff --git a/taiga/projects/api.py b/taiga/projects/api.py index cede5ec9..2ed19f97 100644 --- a/taiga/projects/api.py +++ b/taiga/projects/api.py @@ -489,7 +489,7 @@ class MembershipViewSet(ModelCrudViewSet): def get_serializer_class(self): use_admin_serializer = False - + if self.action == "create": use_admin_serializer = True @@ -544,7 +544,7 @@ class MembershipViewSet(ModelCrudViewSet): def pre_delete(self, obj): if obj.user is not None and not services.can_user_leave_project(obj.user, obj.project): - raise exc.BadRequest(_("At least one of the user must be an active admin")) + raise exc.BadRequest(_("The project must have an owner and at least one of the users must be an active admin")) def pre_save(self, obj): if not obj.token: diff --git a/taiga/projects/permissions.py b/taiga/projects/permissions.py index c1e017a7..ee96523f 100644 --- a/taiga/projects/permissions.py +++ b/taiga/projects/permissions.py @@ -37,8 +37,7 @@ class CanLeaveProject(PermissionComponent): try: if not services.can_user_leave_project(request.user, obj): - raise exc.PermissionDenied(_("You can't leave the project if there are no " - "more owners")) + raise exc.PermissionDenied(_("You can't leave the project if you are the owner or there are no more admins")) return True except Membership.DoesNotExist: return False diff --git a/taiga/projects/serializers.py b/taiga/projects/serializers.py index 6147c962..a313b8a6 100644 --- a/taiga/projects/serializers.py +++ b/taiga/projects/serializers.py @@ -257,8 +257,8 @@ class MembershipSerializer(serializers.ModelSerializer): project = self.object.project if (self.object and - not services.project_has_valid_owners(project, exclude_user=self.object.user)): - raise serializers.ValidationError(_("At least one of the user must be an active admin")) + not services.project_has_valid_admins(project, exclude_user=self.object.user)): + raise serializers.ValidationError(_("The project must have an owner and at least one of the users must be an active admin")) return attrs diff --git a/taiga/projects/services/__init__.py b/taiga/projects/services/__init__.py index d4940c37..b6a51e3e 100644 --- a/taiga/projects/services/__init__.py +++ b/taiga/projects/services/__init__.py @@ -37,7 +37,7 @@ from .logo import get_logo_big_thumbnail_url from .members import create_members_in_bulk from .members import get_members_from_bulk -from .members import remove_user_from_project, project_has_valid_owners, can_user_leave_project +from .members import remove_user_from_project, project_has_valid_admins, can_user_leave_project from .modules_config import get_modules_config diff --git a/taiga/projects/services/members.py b/taiga/projects/services/members.py index f4efc5e9..c883156c 100644 --- a/taiga/projects/services/members.py +++ b/taiga/projects/services/members.py @@ -36,15 +36,15 @@ def remove_user_from_project(user, project): models.Membership.objects.get(project=project, user=user).delete() -def project_has_valid_owners(project, exclude_user=None): +def project_has_valid_admins(project, exclude_user=None): """ Checks if the project has any owner membership with a user different than the specified """ - owner_memberships = project.memberships.filter(is_owner=True, user__is_active=True) + admin_memberships = project.memberships.filter(is_owner=True, user__is_active=True) if exclude_user: - owner_memberships = owner_memberships.exclude(user=exclude_user) + admin_memberships = admin_memberships.exclude(user=exclude_user) - return owner_memberships.count() > 0 + return admin_memberships.count() > 0 def can_user_leave_project(user, project): @@ -52,7 +52,11 @@ def can_user_leave_project(user, project): if not membership.is_owner: return True - if not project_has_valid_owners(project, exclude_user=user): + #The user can't leave if is the real owner of the project + if project.owner == user: + return False + + if not project_has_valid_admins(project, exclude_user=user): return False return True diff --git a/tests/integration/test_projects.py b/tests/integration/test_projects.py index 5349218c..8b079fea 100644 --- a/tests/integration/test_projects.py +++ b/tests/integration/test_projects.py @@ -249,7 +249,22 @@ def test_leave_project_valid_membership_only_owner(client): url = reverse("projects-leave", args=(project.id,)) response = client.post(url) assert response.status_code == 403 - assert response.data["_error_message"] == "You can't leave the project if there are no more owners" + assert response.data["_error_message"] == "You can't leave the project if you are the owner or there are no more admins" + + +def test_leave_project_valid_membership_real_owner(client): + owner_user = f.UserFactory.create() + member_user = f.UserFactory.create() + project = f.ProjectFactory.create(owner=owner_user) + role = f.RoleFactory.create(project=project, permissions=["view_project"]) + f.MembershipFactory.create(project=project, user=owner_user, role=role, is_owner=True) + f.MembershipFactory.create(project=project, user=member_user, role=role, is_owner=True) + + client.login(owner_user) + url = reverse("projects-leave", args=(project.id,)) + response = client.post(url) + assert response.status_code == 403 + assert response.data["_error_message"] == "You can't leave the project if you are the owner or there are no more admins" def test_leave_project_invalid_membership(client): @@ -286,7 +301,22 @@ def test_delete_membership_only_owner(client): url = reverse("memberships-detail", args=(membership.id,)) response = client.delete(url) assert response.status_code == 400 - assert response.data["_error_message"] == "At least one of the user must be an active admin" + assert response.data["_error_message"] == "The project must have an owner and at least one of the users must be an active admin" + + +def test_delete_membership_real_owner(client): + owner_user = f.UserFactory.create() + member_user = f.UserFactory.create() + project = f.ProjectFactory.create(owner=owner_user) + role = f.RoleFactory.create(project=project, permissions=["view_project"]) + owner_membership = f.MembershipFactory.create(project=project, user=owner_user, role=role, is_owner=True) + f.MembershipFactory.create(project=project, user=member_user, role=role, is_owner=True) + + client.login(owner_user) + url = reverse("memberships-detail", args=(owner_membership.id,)) + response = client.delete(url) + assert response.status_code == 400 + assert response.data["_error_message"] == "The project must have an owner and at least one of the users must be an active admin" def test_edit_membership_only_owner(client): @@ -301,7 +331,7 @@ def test_edit_membership_only_owner(client): url = reverse("memberships-detail", args=(membership.id,)) response = client.json.patch(url, json.dumps(data)) assert response.status_code == 400 - assert response.data["is_owner"][0] == "At least one of the user must be an active admin" + assert response.data["is_owner"][0] == "The project must have an owner and at least one of the users must be an active admin" def test_anon_permissions_generation_when_making_project_public(client):