From nobody Wed May 1 13:09:16 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=patchew-devel-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=patchew-devel-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1525950341813954.3714964848654; Thu, 10 May 2018 04:05:41 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CDFD8356F3; Thu, 10 May 2018 11:05:40 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A9DEC608EE; Thu, 10 May 2018 11:05:40 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 991A74BB78; Thu, 10 May 2018 11:05:40 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w4AB5dp2022198 for ; Thu, 10 May 2018 07:05:39 -0400 Received: by smtp.corp.redhat.com (Postfix) id 3D6942026E03; Thu, 10 May 2018 11:05:39 +0000 (UTC) Received: from donizetti.redhat.com (ovpn-116-206.ams2.redhat.com [10.36.116.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 850D02026DEF; Thu, 10 May 2018 11:05:38 +0000 (UTC) From: Paolo Bonzini To: patchew-devel@redhat.com Date: Thu, 10 May 2018 13:05:36 +0200 Message-Id: <20180510110537.1279-1-pbonzini@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-loop: patchew-devel@redhat.com Subject: [Patchew-devel] [PATCH v2] rest: introduce generic permission framework X-BeenThere: patchew-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Patchew development and discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: patchew-devel-bounces@redhat.com Errors-To: patchew-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 10 May 2018 11:05:40 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Until now, the REST API didn't really have a good permission system. In particular, it did not know about user groups. We will need to allow the "importer" group to add messages to any project, so it's time to improve the permissions. Similar to the old API, all the knowledge of permissions is encapsulated in one class, in this case a DRF Permission subclass. The new class replaces the old IsAdminUserOrReadOnly and IsMaintainerUserOrReadOnly permissions. Reviewed-by: Fam Zheng --- v1->v2: introduce has_message_permission; check obj class before invoking has_{project,message}_permission; check presence of view.project instead of kwarg api/rest.py | 78 +++++++++++++++++++++++++++++++++++----------- tests/test_rest.py | 11 +++++++ 2 files changed, 70 insertions(+), 19 deletions(-) diff --git a/api/rest.py b/api/rest.py index 9e280d4..a6f9deb 100644 --- a/api/rest.py +++ b/api/rest.py @@ -30,23 +30,53 @@ SEARCH_PARAM =3D 'q' =20 # patchew-specific permission classes =20 -class IsAdminUserOrReadOnly(permissions.BasePermission): +class PatchewPermission(permissions.BasePermission): """ - Allows access only to admin users. + Generic code to lookup for permissions based on message and project + objects. If the view also has a "project" property, it should + returns an api.models.Project, and has_permission will check that + property too. + + Subclasses can override the methods, or specify a set of groups that + are granted authorization independent of object permissions. """ + + allowed_groups =3D () + + def is_superuser(self, request): + return request.user and request.user.is_superuser + + def has_project_permission(self, request, view, obj): + return obj.maintained_by(request.user) + + def has_message_permission(self, request, view, obj): + return obj.project.maintained_by(request.user) + + def has_group_permission(self, request, view): + for grp in request.user.groups.all(): + if grp.name in self.allowed_groups: + return True + return False + + 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) + def has_permission(self, request, view): - return request.method in permissions.SAFE_METHODS or \ - (request.user and request.user.is_superuser) + return self.has_generic_permission(request, view) or \ + (hasattr(view, 'project') and view.project and \ + self.has_project_permission(request, view, view.project)) =20 -class IsMaintainerOrReadOnly(permissions.BasePermission): - """ - Allows access only to admin users or maintainers. - """ def has_object_permission(self, request, view, obj): - if isinstance(obj, Message): - obj =3D obj.project - return request.method in permissions.SAFE_METHODS or \ - obj.maintained_by(request.user) + return self.has_generic_permission(request, view) or \ + (isinstance(obj, Message) and \ + self.has_message_permission(request, view, obj)) or \ + (isinstance(obj, Project) and \ + self.has_project_permission(request, view, obj)) + +class ImportPermission(PatchewPermission): + allowed_groups =3D ('importers',) =20 # pluggable field for plugin support =20 @@ -87,7 +117,7 @@ class UserSerializer(serializers.HyperlinkedModelSeriali= zer): class UsersViewSet(viewsets.ModelViewSet): queryset =3D User.objects.all().order_by('id') serializer_class =3D UserSerializer - permission_classes =3D (IsAdminUserOrReadOnly,) + permission_classes =3D (PatchewPermission,) =20 # Projects =20 @@ -110,7 +140,7 @@ class ProjectSerializer(serializers.HyperlinkedModelSer= ializer): class ProjectsViewSet(viewsets.ModelViewSet): queryset =3D Project.objects.all().order_by('id') serializer_class =3D ProjectSerializer - permission_classes =3D (IsMaintainerOrReadOnly,) + permission_classes =3D (PatchewPermission,) =20 # Common classes for series and messages =20 @@ -152,7 +182,7 @@ class BaseMessageSerializer(serializers.ModelSerializer= ): class BaseMessageViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): serializer_class =3D BaseMessageSerializer queryset =3D Message.objects.all() - permission_classes =3D () + permission_classes =3D (ImportPermission,) lookup_field =3D 'message_id' lookup_value_regex =3D '[^/]+' =20 @@ -161,11 +191,21 @@ class ProjectMessagesViewSetMixin(mixins.RetrieveMode= lMixin): def get_queryset(self): return self.queryset.filter(project=3Dself.kwargs['projects_pk']) =20 - def get_serializer_context(self): + @property + def project(self): + if hasattr(self, '__project'): + return self.__project try: - return {'project': Project.objects.get(id=3Dself.kwargs['proje= cts_pk']), 'request': self.request} - except:=20 + self.__project =3D Project.objects.get(id=3Dself.kwargs['proje= cts_pk']) + except: + self.__project =3D None + return self.__project + + def get_serializer_context(self): + if self.project is None: return Http404 + return {'project': self.project, 'request': self.request} + # Series =20 class ReplySerializer(BaseMessageSerializer): @@ -248,7 +288,6 @@ class SeriesViewSet(BaseMessageViewSet): queryset =3D Message.objects.filter(is_series_head=3DTrue).order_by('-= last_reply_date') filter_backends =3D (PatchewSearchFilter,) search_fields =3D (SEARCH_PARAM,) - permission_classes =3D (IsMaintainerOrReadOnly,) =20 =20 class ProjectSeriesViewSet(ProjectMessagesViewSetMixin, @@ -376,6 +415,7 @@ class ResultSerializerFull(ResultSerializer): class ResultsViewSet(viewsets.ViewSet, generics.GenericAPIView): lookup_field =3D 'name' lookup_value_regex =3D '[^/]+' + permission_classes =3D (PatchewPermission,) =20 def get_serializer_class(self, *args, **kwargs): if self.lookup_field in self.kwargs: diff --git a/tests/test_rest.py b/tests/test_rest.py index 6a9c332..7896e8a 100755 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -84,10 +84,18 @@ class RestTest(PatchewTestCase): self.assertEquals(resp.data['mailing_list'], "qemu-block@nongnu.or= g") self.assertEquals(resp.data['parent_project'], self.PROJECT_BASE) =20 + def test_project_post_no_login(self): + data =3D { + 'name': 'keycodemapdb', + } + resp =3D self.api_client.post(self.REST_BASE + 'projects/', data= =3Ddata) + self.assertEquals(resp.status_code, 403) + def test_project_post_minimal(self): data =3D { 'name': 'keycodemapdb', } + self.api_client.login(username=3Dself.user, password=3Dself.passwo= rd) resp =3D self.api_client.post(self.REST_BASE + 'projects/', data= =3Ddata) self.assertEquals(resp.status_code, 201) self.assertEquals(resp.data['resource_uri'].startswith(self.REST_B= ASE + 'projects/'), True) @@ -97,6 +105,7 @@ class RestTest(PatchewTestCase): self.assertEquals(resp.data['name'], data['name']) =20 def test_project_post(self): + self.api_client.login(username=3Dself.user, password=3Dself.passwo= rd) data =3D { 'name': 'keycodemapdb', 'mailing_list': 'qemu-devel@nongnu.org', @@ -267,6 +276,7 @@ class RestTest(PatchewTestCase): dp =3D self.get_data_path("0022-another-simple-patch.json.gz") with open(dp, "r") as f: data =3D f.read() + self.api_client.login(username=3Dself.user, password=3Dself.passwo= rd) resp =3D self.api_client.post(self.PROJECT_BASE + "messages/", dat= a, content_type=3D'application/json') self.assertEqual(resp.status_code, 201) resp_get =3D self.api_client.get(self.PROJECT_BASE + "messages/201= 71023201055.21973-11-andrew.smirnov@gmail.com/") @@ -278,6 +288,7 @@ class RestTest(PatchewTestCase): dp =3D self.get_data_path("0004-multiple-patch-reviewed.mbox.gz") with open(dp, "r") as f: data =3D f.read() + self.api_client.login(username=3Dself.user, password=3Dself.passwo= rd) resp =3D self.api_client.post(self.PROJECT_BASE + "messages/", dat= a, content_type=3D'message/rfc822') self.assertEqual(resp.status_code, 201) resp_get =3D self.api_client.get(self.PROJECT_BASE + "messages/146= 9192015-16487-1-git-send-email-berrange@redhat.com/") --=20 2.17.0 _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel