diff --git a/taiga/base/filters.py b/taiga/base/filters.py index a0b994a1..b8589afd 100644 --- a/taiga/base/filters.py +++ b/taiga/base/filters.py @@ -17,12 +17,14 @@ import operator from functools import reduce from django.db.models import Q -from django.db.models.sql.where import ExtraWhere, OR +from django.db.models.sql.where import ExtraWhere, OR, AND from rest_framework import filters from taiga.base import tags +from taiga.users.models import Role + class QueryParamsFilterMixin(filters.BaseFilterBackend): _special_values_dict = { @@ -89,19 +91,31 @@ class PermissionBasedFilterBackend(FilterBackend): permission = None def filter_queryset(self, request, queryset, view): - # TODO: Make permissions aware of members permissions, now only check membership. + project_id = None + if hasattr(view, "filter_fields") and "project" in view.filter_fields: + project_id = request.QUERY_PARAMS.get("project", None) + qs = queryset if request.user.is_authenticated() and request.user.is_superuser: qs = qs elif request.user.is_authenticated(): - qs = qs.filter(Q(project__owner=request.user) | - Q(project__members=request.user) | - Q(project__is_private=False)) + roles_qs = Role.objects.filter(memberships__user=request.user) + roles_qs = roles_qs.extra(where=["users_role.permissions @> ARRAY['{}']".format(self.permission)]) + if project_id: + roles_qs = roles_qs.filter(project_id=project_id) + projects_list = [role.project_id for role in roles_qs] + + if len(projects_list) == 0: + qs = qs.filter(Q(project__owner=request.user)) + elif len(projects_list) == 1: + qs = qs.filter(Q(project__owner=request.user) | Q(project=projects_list[0])) + else: + qs = qs.filter(Q(project__owner=request.user) | Q(project__in=projects_list)) qs.query.where.add(ExtraWhere(["projects_project.public_permissions @> ARRAY['{}']".format(self.permission)], []), OR) else: - qs = qs.filter(project__is_private=False) - qs.query.where.add(ExtraWhere(["projects_project.anon_permissions @> ARRAY['{}']".format(self.permission)], []), OR) + qs = qs.exclude(project__owner=-1) + qs.query.where.add(ExtraWhere(["projects_project.anon_permissions @> ARRAY['{}']".format(self.permission)], []), AND) return super().filter_queryset(request, qs.distinct(), view) @@ -133,28 +147,15 @@ class CanViewWikiLinksFilterBackend(PermissionBasedFilterBackend): class CanViewMilestonesFilterBackend(PermissionBasedFilterBackend): permission = "view_milestones" -class PermissionBasedAttachmentFilterBackend(FilterBackend): + +class PermissionBasedAttachmentFilterBackend(PermissionBasedFilterBackend): permission = None def filter_queryset(self, request, queryset, view): - # TODO: Make permissions aware of members permissions, now only check membership. - qs = queryset - - if request.user.is_authenticated() and request.user.is_superuser: - qs = qs - elif request.user.is_authenticated(): - qs = qs.filter(Q(project__owner=request.user) | - Q(project__members=request.user) | - Q(project__is_private=False)) - qs.query.where.add(ExtraWhere(["projects_project.public_permissions @> ARRAY['{}']".format(self.permission)], []), OR) - else: - qs = qs.filter(project__is_private=False) - qs.query.where.add(ExtraWhere(["projects_project.anon_permissions @> ARRAY['{}']".format(self.permission)], []), OR) + qs = super().filter_queryset(request, queryset, view) ct = view.get_content_type() - qs = qs.filter(content_type=ct) - - return super().filter_queryset(request, qs.distinct(), view) + return qs.filter(content_type=ct) class CanViewUserStoryAttachmentFilterBackend(PermissionBasedAttachmentFilterBackend): @@ -175,18 +176,30 @@ class CanViewWikiAttachmentFilterBackend(PermissionBasedAttachmentFilterBackend) class CanViewProjectObjFilterBackend(FilterBackend): def filter_queryset(self, request, queryset, view): + project_id = None + if hasattr(view, "filter_fields") and "project" in view.filter_fields: + project_id = request.QUERY_PARAMS.get("project", None) + qs = queryset if request.user.is_authenticated() and request.user.is_superuser: qs = qs elif request.user.is_authenticated(): - qs = qs.filter(Q(owner=request.user) | - Q(members=request.user) | - Q(is_private=False)) + roles_qs = Role.objects.filter(memberships__user=request.user) + roles_qs = roles_qs.extra(where=["users_role.permissions @> ARRAY['view_project']"]) + if project_id: + roles_qs = roles_qs.filter(project_id=project_id) + projects_list = [role.project_id for role in roles_qs] + + if len(projects_list) == 0: + qs = qs.filter(Q(owner=request.user)) + elif len(projects_list) == 1: + qs = qs.filter(Q(owner=request.user) | Q(id=projects_list[0])) + else: + qs = qs.filter(Q(owner=request.user) | Q(id__in=projects_list)) qs.query.where.add(ExtraWhere(["projects_project.public_permissions @> ARRAY['view_project']"], []), OR) else: - qs = qs.filter(is_private=False) - qs.query.where.add(ExtraWhere(["projects_project.anon_permissions @> ARRAY['view_project']"], []), OR) + qs.query.where.add(ExtraWhere(["projects_project.anon_permissions @> ARRAY['view_project']"], []), AND) return super().filter_queryset(request, qs.distinct(), view) diff --git a/taiga/projects/serializers.py b/taiga/projects/serializers.py index 56bff2b3..511b7144 100644 --- a/taiga/projects/serializers.py +++ b/taiga/projects/serializers.py @@ -152,10 +152,14 @@ class ProjectSerializer(ModelSerializer): return getattr(obj, "stars_count", 0) def get_my_permissions(self, obj): - return get_user_project_permissions(self.context['request'].user, obj) + if "request" in self.context: + return get_user_project_permissions(self.context["request"].user, obj) + return [] def get_i_am_owner(self, obj): - return is_project_owner(self.context['request'].user, obj) + if "request" in self.context: + return is_project_owner(self.context["request"].user, obj) + return False class ProjectDetailSerializer(ProjectSerializer): diff --git a/tests/factories.py b/tests/factories.py index 223cb8df..344ea69e 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -71,7 +71,7 @@ class RoleFactory(Factory): FACTORY_FOR = get_model("users", "Role") name = "Tester" - slug = "tester" + slug = factory.Sequence(lambda n: "test-role-{}".format(n)) project = factory.SubFactory("tests.factories.ProjectFactory") diff --git a/tests/integration/resources_permissions/test_attachment_resources.py b/tests/integration/resources_permissions/test_attachment_resources.py index 86b80a51..bfc6843f 100644 --- a/tests/integration/resources_permissions/test_attachment_resources.py +++ b/tests/integration/resources_permissions/test_attachment_resources.py @@ -546,7 +546,7 @@ def test_user_story_attachment_list(client, data): ] results = helper_test_http_method_and_count(client, 'get', url, None, users) - assert results == [(200, 2), (200, 2), (200, 3), (200, 3), (200, 3)] + assert results == [(200, 2), (200, 2), (200, 2), (200, 3), (200, 3)] def test_task_attachment_list(client, data): @@ -561,7 +561,7 @@ def test_task_attachment_list(client, data): ] results = helper_test_http_method_and_count(client, 'get', url, None, users) - assert results == [(200, 2), (200, 2), (200, 3), (200, 3), (200, 3)] + assert results == [(200, 2), (200, 2), (200, 2), (200, 3), (200, 3)] def test_issue_attachment_list(client, data): @@ -576,7 +576,7 @@ def test_issue_attachment_list(client, data): ] results = helper_test_http_method_and_count(client, 'get', url, None, users) - assert results == [(200, 2), (200, 2), (200, 3), (200, 3), (200, 3)] + assert results == [(200, 2), (200, 2), (200, 2), (200, 3), (200, 3)] def test_wiki_attachment_list(client, data): @@ -591,4 +591,4 @@ def test_wiki_attachment_list(client, data): ] results = helper_test_http_method_and_count(client, 'get', url, None, users) - assert results == [(200, 2), (200, 2), (200, 3), (200, 3), (200, 3)] + assert results == [(200, 2), (200, 2), (200, 2), (200, 3), (200, 3)] diff --git a/tests/integration/resources_permissions/test_milestones_resources.py b/tests/integration/resources_permissions/test_milestones_resources.py index deedf10f..6f5c1957 100644 --- a/tests/integration/resources_permissions/test_milestones_resources.py +++ b/tests/integration/resources_permissions/test_milestones_resources.py @@ -146,29 +146,6 @@ def test_milestone_delete(client, data): assert results == [401, 403, 403, 204] -def test_milestone_delete_last_milestone(client, data): - url_not_last_milestone = reverse('milestones-detail', kwargs={"pk": data.public_milestone.pk}) - - data.public_milestone2 = f.MilestoneFactory(project=data.public_project) - url_last_milestone = reverse('milestones-detail', kwargs={"pk": data.public_milestone2.pk}) - - data.public_membership.role.permissions = list(filter(lambda x: x != "delete_milestone", data.public_membership.role.permissions)) - data.public_membership.role.save() - - users = [ - None, - data.registered_user, - data.project_member_without_perms, - data.project_member_with_perms, - ] - results = helper_test_http_method(client, 'delete', url_not_last_milestone, None, users) - assert results == [401, 403, 403, 403] - results = helper_test_http_method(client, 'delete', url_last_milestone, None, users) - assert results == [401, 403, 403, 204] - results = helper_test_http_method(client, 'delete', url_not_last_milestone, None, users) - assert results == [401, 403, 403, 204] - - def test_milestone_list(client, data): url = reverse('milestones-list') diff --git a/tests/integration/resources_permissions/test_projects_choices_resources.py b/tests/integration/resources_permissions/test_projects_choices_resources.py index b4b1f54d..00f43078 100644 --- a/tests/integration/resources_permissions/test_projects_choices_resources.py +++ b/tests/integration/resources_permissions/test_projects_choices_resources.py @@ -182,7 +182,7 @@ def test_roles_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 5 + assert len(projects_data) == 3 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -310,7 +310,7 @@ def test_points_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 3 + assert len(projects_data) == 2 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -471,7 +471,7 @@ def test_user_story_status_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 3 + assert len(projects_data) == 2 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -632,7 +632,7 @@ def test_task_status_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 3 + assert len(projects_data) == 2 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -793,7 +793,7 @@ def test_issue_status_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 3 + assert len(projects_data) == 2 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -954,7 +954,7 @@ def test_issue_type_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 3 + assert len(projects_data) == 2 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -1115,7 +1115,7 @@ def test_priority_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 3 + assert len(projects_data) == 2 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -1276,7 +1276,7 @@ def test_severity_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 3 + assert len(projects_data) == 2 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -1437,7 +1437,7 @@ def test_membership_list(client, data): client.login(data.project_member_without_perms) response = client.get(url) projects_data = json.loads(response.content.decode('utf-8')) - assert len(projects_data) == 5 + assert len(projects_data) == 3 assert response.status_code == 200 client.login(data.project_member_with_perms) @@ -1575,7 +1575,7 @@ def test_membership_action_resend_invitation(client, data): assert results == [401, 403, 403, 403, 204] results = helper_test_http_method(client, 'post', private2_url, None, users) - assert results == [404, 404, 403, 403, 204] + assert results == [404, 404, 404, 403, 204] def test_project_template_retrieve(client, data): diff --git a/tests/integration/resources_permissions/test_projects_resource.py b/tests/integration/resources_permissions/test_projects_resource.py index 95ddc11a..bc91ed74 100644 --- a/tests/integration/resources_permissions/test_projects_resource.py +++ b/tests/integration/resources_permissions/test_projects_resource.py @@ -302,7 +302,7 @@ def test_project_action_fans(client, data): results = helper_test_http_method_and_count(client, 'get', private1_url, None, users) assert results == [(200, 2), (200, 2), (200, 2), (200, 2), (200, 2)] results = helper_test_http_method_and_count(client, 'get', private2_url, None, users) - assert results == [(404, 1), (404, 1), (403, 2), (200, 2), (200, 2)] + assert results == [(404, 1), (404, 1), (404, 1), (200, 2), (200, 2)] def test_user_action_starred(client, data): @@ -350,7 +350,7 @@ def test_project_action_create_template(client, data): results = helper_test_http_method(client, 'post', private1_url, template_data, users) assert results == [401, 403, 403, 403, 403, 201] results = helper_test_http_method(client, 'post', private2_url, template_data, users) - assert results == [404, 404, 403, 403, 403, 201] + assert results == [404, 404, 404, 403, 403, 201] def test_invitations_list(client, data): diff --git a/tests/integration/test_stars.py b/tests/integration/test_stars.py index ce62aed4..27293eb1 100644 --- a/tests/integration/test_stars.py +++ b/tests/integration/test_stars.py @@ -48,7 +48,7 @@ def test_project_owner_unstar_project(client): def test_project_member_star_project(client): user = f.UserFactory.create() project = f.ProjectFactory.create() - role = f.RoleFactory.create(project=project) + role = f.RoleFactory.create(project=project, permissions=["view_project"]) f.MembershipFactory.create(project=project, user=user, role=role) url = reverse("projects-star", args=(project.id,)) @@ -61,7 +61,7 @@ def test_project_member_star_project(client): def test_project_member_unstar_project(client): user = f.UserFactory.create() project = f.ProjectFactory.create() - role = f.RoleFactory.create(project=project) + role = f.RoleFactory.create(project=project, permissions=["view_project"]) f.MembershipFactory.create(project=project, user=user, role=role) url = reverse("projects-unstar", args=(project.id,)) diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py index 1f817dae..04cdc358 100644 --- a/tests/integration/test_users.py +++ b/tests/integration/test_users.py @@ -85,7 +85,7 @@ def test_api_user_action_change_email_no_token(client): response = client.post(url, json.dumps(data), content_type="application/json") assert response.status_code == 400 - assert response.data['_error_message'] == 'Token is invalid' + assert response.data['_error_message'] == 'Invalid, are you sure the token is correct and you didn\'t use it before?' def test_api_user_action_change_email_invalid_token(client): @@ -97,4 +97,4 @@ def test_api_user_action_change_email_invalid_token(client): response = client.post(url, json.dumps(data), content_type="application/json") assert response.status_code == 400 - assert response.data['_error_message'] == 'Token is invalid' + assert response.data['_error_message'] == 'Invalid, are you sure the token is correct and you didn\'t use it before?'