Merge pull request #8 from ziadhany/add-permission

Add missing permissions check on some activities like Update, Delete.

Author: pombredanne

fedcode/activitypub.py

@@ -23,7 +23,7 @@
 from federatedcode.settings import FEDERATED_CODE_DOMAIN
 from federatedcode.settings import FEDERATED_CODE_GIT_PATH
 
-from .models import Follow, FederateRequest
+from .models import Follow, FederateRequest, SyncRequest
 from .models import Note
 from .models import Person
 from .models import Package
@@ -72,10 +72,10 @@
 URL_MAPPER = {
     "user-ap-profile": "username",
     "purl-ap-profile": "purl_string",
-    "review-page": "uuid",
-    "repository-page": "uuid",
-    "note-page": "uuid",
-    "vulnerability-page": "str",
+    "review-page": "review_id",
+    "repository-page": 'repository_id',
+    "note-page": "note_id",
+    "vulnerability-page": "vulnerability_id",
 }
 
 logger = logging.getLogger(__name__)
@@ -107,7 +107,7 @@ def add_ap_target(response):
 
 def has_valid_header(view):
     """
-    check if the request header in the AP_VALID_HEADERS if yes return view else return HttpResponseForbidden
+    check if the request header in the AP_VALID_HEADERS if yes return view else return
     """
 
     def wrapper(request, *args, **kwargs):
@@ -153,6 +153,43 @@ def federate(cls, targets, body, key_id):
                 except Exception as e:
                     logger.error(f"{e}")
 
+    @classmethod
+    def get_actor_permissions(cls, actor, object):
+        """get the actor permission to do some activity on the object"""
+        permissions = {
+            Person: {
+                Note: lambda: {
+                    CreateActivity,
+                    UpdateActivity if object.acct == actor.acct else None,
+                    DeleteActivity if object.acct == actor.acct else None
+                },
+
+                Review: lambda: {
+                    CreateActivity,
+                    UpdateActivity if object.author == actor else None,
+                    DeleteActivity if object.author == actor else None
+                },
+            },
+            Service: {
+                Repository: lambda: {
+                    CreateActivity,
+                    SyncActivity if object.admin == actor else None,
+                    UpdateActivity if object.admin == actor else None,
+                    DeleteActivity if object.admin == actor else None
+                }
+            },
+            Package: {
+                Note: lambda: {
+                    CreateActivity,
+                    UpdateActivity if object.acct == actor.acct else None,
+                    DeleteActivity if object.acct == actor.acct else None
+                },
+            }
+        }
+
+        # Return the permissions for the specific actor and object type
+        return permissions.get(type(actor), {}).get(type(object), lambda: {})
+
 
 @dataclass
 class ApActor:
@@ -430,13 +467,13 @@ def save(self):
                 (isinstance(actor, Person) and self.object.type in ["Note", "Review"])
                 or (isinstance(actor, Service) and self.object.type == "Repository")
                 or (isinstance(actor, Package) and self.object.type == "Note")
-        ):
+        ) and UpdateActivity in Activity.get_actor_permissions(actor, old_obj)():
             for key, value in updated_param[self.object.type].items():
                 if value:
                     setattr(old_obj, key, value)
             old_obj.save()
 
-        Activity.federate(targets=self.to, body=self.to_ap(), key_id=actor.key_id)
+            Activity.federate(targets=self.to, body=self.to_ap(), key_id=actor.key_id)
         return self.succeeded_ap_rs(old_obj.to_ap)
 
     def succeeded_ap_rs(self, update_obj):
@@ -480,11 +517,12 @@ def save(self):
                 or (type(actor) is Service and self.object.type == ["Repository", "Package"])
         ):
             instance = self.object.get()
-            instance.delete()
-            Activity.federate(targets=self.to, body=self.to_ap(), key_id=actor.key_id)
-            return self.succeeded_ap_rs()
-        else:
-            return self.failed_ap_rs()
+            if DeleteActivity in Activity.get_actor_permissions(actor, instance)():
+                instance.delete()
+                Activity.federate(targets=self.to, body=self.to_ap(), key_id=actor.key_id)
+                return self.succeeded_ap_rs()
+
+        return self.failed_ap_rs()
 
     def ap_rq(self):
         """Request for deleting object in activitypub format"""
@@ -571,9 +609,13 @@ def save(self):
         actor = self.actor.get()
         if not actor:
             return self.failed_ap_rs()
-        repo = self.object.get().git_repo_obj
-        repo.remotes.origin.pull()
-        return self.succeeded_ap_rs()
+        repo = self.object.get()
+
+        if SyncActivity in Activity.get_actor_permissions(actor, repo)():
+            SyncRequest.objects.create(repo=repo)
+            return self.succeeded_ap_rs()
+
+        return self.failed_ap_rs()
 
     def succeeded_ap_rs(self):
         """Response for successfully deleting the object"""
@@ -607,3 +649,4 @@ def check_remote_actor(key_id):
     obj_id, page_name = resolver.kwargs, resolver.url_name
     identity = URL_MAPPER[page_name]
     return webfinger_actor(parser.netloc, resolver.kwargs[identity])
+

fedcode/models.py

@@ -352,7 +352,7 @@ def __str__(self):
         return f"{self.person.user.username} - {self.package.purl}"
 
 
-class Repository(models.Model):  # TODO
+class Repository(models.Model):
     """
     A git repository used as a backing storage for Package and vulnerability data
     """
@@ -435,7 +435,6 @@ def to_ap(self):
 
 
 class Review(models.Model):
-    # TODO
     id = models.UUIDField(
         primary_key=True,
         editable=False,

federatedcode/urls.py

@@ -80,7 +80,7 @@
         redirect_vulnerability,
         name="vulnerability-page",
     ),
-    path("notes/<uuid:uuid>", NoteView.as_view(), name="note-page"),
+    path("notes/<uuid:note_id>", NoteView.as_view(), name="note-page"),
     path("api/v0/users/@<str:username>", UserProfile.as_view(), name="user-ap-profile"),
     path(
         "api/v0/purls/@<path:purl_string>/",

tests/test_activitypub.py

@@ -11,18 +11,27 @@
 import pytest
 
 from fedcode.activitypub import AP_CONTEXT
+from fedcode.activitypub import Activity
+from fedcode.activitypub import CreateActivity
+from fedcode.activitypub import DeleteActivity
+from fedcode.activitypub import SyncActivity
+from fedcode.activitypub import UpdateActivity
 from fedcode.activitypub import create_activity_obj
 from fedcode.models import Follow
 from fedcode.models import Note
 from fedcode.models import Repository
 from fedcode.models import Review
+from fedcode.models import SyncRequest
 
+from .test_models import fake_service
 from .test_models import follow
 from .test_models import mute_post_save_signal
 from .test_models import note
 from .test_models import package
 from .test_models import person
+from .test_models import pkg_note
 from .test_models import repo
+from .test_models import review
 from .test_models import service
 from .test_models import vulnerability
 
@@ -240,19 +249,55 @@ def test_person_unfollow_package(person, package, follow):
     assert Follow.objects.count() == 0
 
 
-# @pytest.mark.django_db
-# def test_person_sync_repo(service, repo):
-#     payload = json.dumps(
-#         {
-#             **AP_CONTEXT,
-#             "type": "Sync",
-#             "actor": f"https://127.0.0.1:8000/users/@{service.user.username}",
-#             "object": {
-#                 "type": "Repository",
-#                 "id": f"https://127.0.0.1:8000/repository/{repo.id}/",
-#             },
-#         }
-#     )
-#
-#     activity = create_activity_obj(payload)
-#     sync_activity = activity.handler()
+@pytest.mark.django_db
+def test_get_actor_permissions(
+    person, package, service, repo, note, review, pkg_note, fake_service
+):
+    assert Activity.get_actor_permissions(person, note)() == {
+        CreateActivity,
+        UpdateActivity,
+        DeleteActivity,
+    }
+    assert Activity.get_actor_permissions(person, review)() == {
+        CreateActivity,
+        UpdateActivity,
+        DeleteActivity,
+    }
+    assert Activity.get_actor_permissions(service, repo)() == {
+        CreateActivity,
+        UpdateActivity,
+        DeleteActivity,
+        SyncActivity,
+    }
+    assert Activity.get_actor_permissions(package, pkg_note)() == {
+        CreateActivity,
+        UpdateActivity,
+        DeleteActivity,
+    }
+
+    note.acct = "fake_person@127.0.0.2"
+    assert Activity.get_actor_permissions(person, note)() == {CreateActivity, None}
+
+    repo.admin = fake_service
+    assert Activity.get_actor_permissions(service, repo)() == {CreateActivity, None}
+
+    assert Activity.get_actor_permissions(package, note)() == {CreateActivity, None}
+
+
+@pytest.mark.django_db
+def test_service_sync_repo(service, repo):
+    payload = json.dumps(
+        {
+            **AP_CONTEXT,
+            "type": "Sync",
+            "actor": f"https://127.0.0.1:8000/api/v0/users/@{service.user.username}",
+            "object": {
+                "type": "Repository",
+                "id": f"https://127.0.0.1:8000/repository/{repo.id}/",
+            },
+        }
+    )
+
+    activity = create_activity_obj(payload)
+    sync_activity = activity.handler()
+    assert SyncRequest.objects.all().count() == 1

tests/test_ap_api.py

@@ -226,7 +226,18 @@ def test_get_user_outbox(person, vulnerability, review, note):
         format="json",
     )
     assert json.loads(response.content) == {
-        "notes": {"type": "OrderedCollection", "totalItems": 0, "orderedItems": []},
+        "notes": {
+            "type": "OrderedCollection",
+            "totalItems": 1,
+            "orderedItems": [
+                {
+                    "author": note.acct,
+                    "content": note.content,
+                    "id": f"https://127.0.0.1:8000/notes/{note.id}",
+                    "type": "Note",
+                }
+            ],
+        },
         "reviews": {
             "type": "OrderedCollection",
             "totalItems": 1,

tests/test_models.py

@@ -35,6 +35,18 @@ def service(db):
     )
 
 
+@pytest.fixture
+def fake_service(db):
+    user = User.objects.create(
+        username="fake_service",
+        email="vcio@nexb.com",
+        password="complex-password",
+    )
+    return Service.objects.create(
+        user=user,
+    )
+
+
 @pytest.fixture
 def package(db, service):
     return Package.objects.create(
@@ -116,11 +128,21 @@ def review(db, repo, person):
 @pytest.fixture
 def note(db):
     return Note.objects.create(
-        acct="ziad@vcio",
+        acct="ziad@127.0.0.1:8000",
         content="Comment #1",
     )
 
 
+@pytest.fixture
+def pkg_note(db, package):
+    return Note.objects.create(
+        acct=package.acct,
+        content="purl: "
+        "pkg:maven/org.apache.logging@2.23-r0?arch=aarch64&distroversion=edge&reponame=community\n"
+        "         affected_by_vulnerabilities: ....",
+    )
+
+
 @pytest.fixture
 def follow(db, package, person):
     return Follow.objects.create(package=package, person=person)

tests/test_utils.py

@@ -76,7 +76,7 @@ def test_full_reverse():
 
 def test_full_resolve():
     assert full_resolve(f"https://127.0.0.1:8000/notes/7e676ad1-995d-405c-a829-cb39813c74e5") == (
-        {"uuid": uuid.UUID("7e676ad1-995d-405c-a829-cb39813c74e5")},
+        {"note_id": uuid.UUID("7e676ad1-995d-405c-a829-cb39813c74e5")},
         "note-page",
     )