[Patchew-devel] [PATCH] test: more testcases around authorization
Paolo Bonzini
pbonzini at redhat.com
Thu May 17 13:36:47 UTC 2018
On 17/05/2018 15:19, 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
> ---
> 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
This can use request.user.project_set.exists() or something like that,
but see below...
> +
> 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)
>
> def has_permission(self, request, view):
> return self.has_generic_permission(request, view) or \
If you go this way, the extra check should be added only to
MessagesViewSet, perhaps by subclassing ImportPermission.
But there is another possibility: just use IsAuthenticatedOrReadOnly for
the MessagesViewSet. If a user is authenticated but not a maintainer,
simply the create() message will not recognize them, and it will
"import" the message into no project at all.
> @@ -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)]
You should keep both tests: maintainer _and_ recognized, exactly as in
the code that is there now. But actually I realized now that this
change is not needed, because superusers are maintainers of all
projects! So it's already working as Fam intended, and it's also being
tested already.
Paolo
> 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):
> 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)
> +
> def test_message(self):
> series = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
> self.p.id, '20160628014747.20971-1-famz at redhat.com')
>
More information about the Patchew-devel
mailing list