[Patchew-devel] [PATCH] test: more testcases around authorization
Fam Zheng
famz at redhat.com
Thu May 17 13:38:31 UTC 2018
On Thu, 05/17 18:49, Shubham Jain wrote:
> The test check for:
> - user that is not a maintainer of any project should not result in any message being imported
> - user that is not a maintainer of a project, but is in the importer groups, should result in the message being imported to all recognized projects
> - user that is a maintainer of a project and is not in the importer group, should result in the message being imported to recognized & maintained projects
The commit message is misleading. You want to mention the code change as well.
> ---
> api/rest.py | 19 +++++++++++++++----
> tests/test_rest.py | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 65 insertions(+), 8 deletions(-)
>
> diff --git a/api/rest.py b/api/rest.py
> index 94b3162..3499250 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -48,6 +48,12 @@ class PatchewPermission(permissions.BasePermission):
> def has_project_permission(self, request, view, obj):
> return obj.maintained_by(request.user)
>
> + def has_maintainer_permission(self, request, view):
> + for p in Project.objects.all():
> + if p.maintained_by(request.user):
> + return True
> + return False
> +
> def has_message_permission(self, request, view, obj):
> return obj.project.maintained_by(request.user)
>
> @@ -60,7 +66,8 @@ class PatchewPermission(permissions.BasePermission):
> def has_generic_permission(self, request, view):
> return (request.method in permissions.SAFE_METHODS) or \
> self.is_superuser(request) or \
> - self.has_group_permission(request, view)
> + self.has_group_permission(request, view) or \
> + self.has_maintainer_permission(request, view)
Does this do what we want? If the user maintains ANY project, it gets the
permission, regardless of the project in request, which may not be the same one.
>
> def has_permission(self, request, view):
> return self.has_generic_permission(request, view) or \
> @@ -394,9 +401,13 @@ class MessagesViewSet(BaseMessageViewSet):
> parser_classes = (JSONParser, MessagePlainTextParser, )
>
> def create(self, request, *args, **kwargs):
> - projects = [p for p in Project.objects.all() if p.recognizes(MboxMessage(self.request.data['mbox']))]
> - if 'importers' not in self.request.user.groups.all():
> - projects = set(projects) & set([p for p in Project.objects.all() if p.maintained_by(self.request.user)])
> + m = MboxMessage(self.request.data['mbox'])
> + user_groups = request.user.groups.all()
> + user_group_names = [grp.name for grp in user_groups]
> + if request.user.is_superuser or 'importers' in user_group_names:
> + projects = [p for p in Project.objects.all() if p.recognizes(m)]
> + else:
> + projects = [p for p in Project.objects.all() if p.maintained_by(self.request.user)]
I think the else branch should still union the p.recognizes() filtering.
> results = []
> for project in projects:
> serializer = self.get_serializer(data=request.data)
> diff --git a/tests/test_rest.py b/tests/test_rest.py
> index 2da5459..af21c7b 100755
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -274,7 +274,7 @@ class RestTest(PatchewTestCase):
> self.assertEqual(resp_after.status_code, 404)
> self.assertEqual(resp_reply_after.status_code, 404)
>
> - def test_create_message(self):
> + def test_json_create_project_message(self):
Why the renames? Should they belong to a separate patch?
> dp = self.get_data_path("0022-another-simple-patch.json.gz")
> with open(dp, "r") as f:
> data = f.read()
> @@ -286,7 +286,7 @@ class RestTest(PatchewTestCase):
> self.assertEqual(resp.data['subject'], "[Qemu-devel] [PATCH v2 10/27] imx_fec: Reserve full 4K "
> "page for the register file")
>
> - def test_create_text_message(self):
> + def test_text_create_project_message(self):
> dp = self.get_data_path("0004-multiple-patch-reviewed.mbox.gz")
> with open(dp, "r") as f:
> data = f.read()
> @@ -297,7 +297,7 @@ class RestTest(PatchewTestCase):
> self.assertEqual(resp_get.status_code, 200)
> self.assertEqual(resp.data['subject'], "[Qemu-devel] [PATCH v4 0/2] Report format specific info for LUKS block driver")
>
> - def test_create_message_without_project_pk(self):
> + def test_json_create_message(self):
> dp = self.get_data_path("0024-multiple-project-patch.json.gz")
> with open(dp, "r") as f:
> data = f.read()
> @@ -311,7 +311,7 @@ class RestTest(PatchewTestCase):
> resp_get2 = self.api_client.get(self.PROJECT_BASE_2 + "messages/20180223132311.26555-2-marcandre.lureau at redhat.com/")
> self.assertEqual(resp_get2.status_code, 200)
>
> - def test_create_text_message_without_project_pk(self):
> + def test_text_create_message(self):
> dp = self.get_data_path("0023-multiple-project-patch.mbox.gz")
> with open(dp, "r") as f:
> data = f.read()
> @@ -325,6 +325,52 @@ class RestTest(PatchewTestCase):
> resp_get2 = self.api_client.get(self.PROJECT_BASE_2 + "messages/20180223132311.26555-2-marcandre.lureau at redhat.com/")
> self.assertEqual(resp_get2.status_code, 200)
>
> + def test_without_login_create_message(self):
> + dp = self.get_data_path("0022-another-simple-patch.json.gz")
> + with open(dp, "r") as f:
> + data = f.read()
> + resp = self.api_client.post(self.PROJECT_BASE + "messages/", data, content_type='message/rfc822')
> + self.assertEqual(resp.status_code, 403)
> +
> + def test_non_maintainer_create_message(self):
> + self.create_user(username="test", password="userpass")
> + self.api_client.login(username="test", password="userpass")
> + dp = self.get_data_path("0023-multiple-project-patch.mbox.gz")
> + with open(dp, "r") as f:
> + data = f.read()
> + resp = self.api_client.post(self.REST_BASE + "messages/", data, content_type='message/rfc822')
> + self.assertEqual(resp.status_code, 403)
> +
> + def test_maintainer_create_message(self):
> + test = self.create_user(username="test", password="userpass")
> + self.api_client.login(username="test", password="userpass")
> + self.p.maintainers = (test, )
> + dp = self.get_data_path("0023-multiple-project-patch.mbox.gz")
> + with open(dp, "r") as f:
> + data = f.read()
> + resp = self.api_client.post(self.REST_BASE + "messages/", data, content_type='message/rfc822')
> + self.assertEqual(resp.status_code, 201)
> + self.assertEqual(resp.data['count'], 1)
> + resp_get = self.api_client.get(self.PROJECT_BASE + "messages/20180223132311.26555-2-marcandre.lureau at redhat.com/")
> + self.assertEqual(resp_get.status_code, 200)
> + resp_get2 = self.api_client.get(self.PROJECT_BASE_2 + "messages/20180223132311.26555-2-marcandre.lureau at redhat.com/")
> + self.assertEqual(resp_get2.status_code, 404)
> +
> + def test_importer_create_message(self):
> + dp = self.get_data_path("0023-multiple-project-patch.mbox.gz")
> + with open(dp, "r") as f:
> + data = f.read()
> + test = self.create_user(username="test", password="userpass", groups=['importers'])
> + self.api_client.login(username="test", password="userpass")
> + resp = self.api_client.post(self.REST_BASE + "messages/", data, content_type='message/rfc822')
> + self.assertEqual(resp.status_code, 201)
> + self.assertEqual(resp.data['count'], 2)
> + resp_get = self.api_client.get(self.PROJECT_BASE + "messages/20180223132311.26555-2-marcandre.lureau at redhat.com/")
> + self.assertEqual(resp_get.status_code, 200)
> + self.assertEqual(resp_get.data['subject'], "[Qemu-devel] [PATCH 1/7] SecurityPkg/Tcg2Pei: drop Tcg2PhysicalPresenceLib dependency")
> + resp_get2 = self.api_client.get(self.PROJECT_BASE_2 + "messages/20180223132311.26555-2-marcandre.lureau at redhat.com/")
> + self.assertEqual(resp_get2.status_code, 200)
Please also add a "test_maintainer_of_project_x_import_a_patch_to_project_y()"
case.
Fam
> +
> def test_message(self):
> series = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
> self.p.id, '20160628014747.20971-1-famz at redhat.com')
> --
> 2.14.3 (Apple Git-98)
>
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel
More information about the Patchew-devel
mailing list