Merge pull request #9095 from OpenMined/bschell/fix-multiple-request-…

…repr

Remove default ALL_READ permission for requests

packages/syft/src/syft/service/request/request_service.py

@@ -8,8 +8,6 @@
 from ...store.linked_obj import LinkedObject
 from ...types.uid import UID
 from ...util.telemetry import instrument
-from ..action.action_permissions import ActionObjectPermission
-from ..action.action_permissions import ActionPermission
 from ..context import AuthedServiceContext
 from ..notification.email_templates import RequestEmailTemplate
 from ..notification.email_templates import RequestUpdateEmailTemplate
@@ -58,15 +56,7 @@ def submit(
         """Submit a Request"""
         try:
             req = request.to(Request, context=context)
-            result = self.stash.set(
-                context.credentials,
-                req,
-                add_permissions=[
-                    ActionObjectPermission(
-                        uid=req.id, permission=ActionPermission.ALL_READ
-                    ),
-                ],
-            )
+            result = self.stash.set(context.credentials, req)
             if result.is_ok():
                 request = result.ok()
                 link = LinkedObject.with_context(request, context=context)

packages/syft/tests/conftest.py

@@ -189,6 +189,27 @@ def guest_datasite_client(root_datasite_client) -> DatasiteClient:
     yield root_datasite_client.guest()
 
 
+@pytest.fixture
+def ds_client(
+    faker: Faker, root_datasite_client: DatasiteClient, guest_client: DatasiteClient
+):
+    guest_email = faker.email()
+    password = "mysecretpassword"
+    root_datasite_client.register(
+        name=faker.name(),
+        email=guest_email,
+        password=password,
+        password_verify=password,
+    )
+    ds_client = guest_client.login(email=guest_email, password=password)
+    yield ds_client
+
+
+@pytest.fixture
+def ds_verify_key(ds_client: DatasiteClient):
+    yield ds_client.credentials.verify_key
+
+
 @pytest.fixture
 def document_store(worker):
     yield worker.document_store

packages/syft/tests/syft/request/fixtures.py

@@ -7,6 +7,7 @@
 from syft.server.credentials import SyftVerifyKey
 from syft.server.worker import Worker
 from syft.service.context import AuthedServiceContext
+from syft.service.request.request_service import RequestService
 from syft.service.request.request_stash import RequestStash
 from syft.store.document_store import DocumentStore
 
@@ -22,3 +23,8 @@ def authed_context_guest_datasite_client(
 ) -> AuthedServiceContext:
     verify_key: SyftVerifyKey = guest_datasite_client.credentials.verify_key
     yield AuthedServiceContext(credentials=verify_key, server=worker)
+
+
+@pytest.fixture
+def request_service(document_store: DocumentStore):
+    yield RequestService(store=document_store)

packages/syft/tests/syft/request/request_code_accept_deny_test.py

@@ -1,7 +1,3 @@
-# third party
-from faker import Faker
-import pytest
-
 # syft absolute
 import syft
 from syft.client.client import SyftClient
@@ -15,33 +11,12 @@
 from syft.service.request.request import ObjectMutation
 from syft.service.request.request import RequestStatus
 from syft.service.request.request import UserCodeStatusChange
-from syft.service.request.request_service import RequestService
 from syft.service.response import SyftError
 from syft.service.response import SyftSuccess
 from syft.service.settings.settings_service import SettingsService
-from syft.store.document_store import DocumentStore
 from syft.store.linked_obj import LinkedObject
 
 
-@pytest.fixture
-def request_service(document_store: DocumentStore):
-    yield RequestService(store=document_store)
-
-
-def get_ds_client(faker: Faker, root_client: SyftClient, guest_client: SyftClient):
-    guest_email = faker.email()
-    password = "mysecretpassword"
-    result = root_client.register(
-        name=faker.name(),
-        email=guest_email,
-        password=password,
-        password_verify=password,
-    )
-    assert isinstance(result, SyftSuccess)
-    ds_client = guest_client.login(email=guest_email, password=password)
-    return ds_client
-
-
 def test_object_mutation(worker: Worker):
     root_client = worker.root_client
     setting = root_client.api.services.settings.get()
@@ -76,16 +51,14 @@ def test_object_mutation(worker: Worker):
     assert setting.organization == original_name
 
 
-def test_action_store_change(faker: Faker, worker: Worker):
+def test_action_store_change(worker: Worker, ds_client: SyftClient):
     root_client = worker.root_client
     dummy_data = [1, 2, 3]
     data = ActionObject.from_obj(dummy_data)
     action_obj = data.send(root_client)
 
     assert action_obj.get() == dummy_data
 
-    ds_client = get_ds_client(faker, root_client, worker.guest_client)
-
     action_object_link = LinkedObject.from_obj(
         action_obj, server_uid=action_obj.syft_server_uid
     )
@@ -116,14 +89,12 @@ def test_action_store_change(faker: Faker, worker: Worker):
     assert isinstance(result, SyftError)
 
 
-def test_user_code_status_change(faker: Faker, worker: Worker):
+def test_user_code_status_change(worker: Worker, ds_client: SyftClient):
     root_client = worker.root_client
     dummy_data = [1, 2, 3]
     data = ActionObject.from_obj(dummy_data)
     action_obj = data.send(root_client)
 
-    ds_client = get_ds_client(faker, root_client, worker.guest_client)
-
     @syft.syft_function(
         input_policy=syft.ExactMatch(data=action_obj),
         output_policy=syft.SingleExecutionExactOutput(),
@@ -164,14 +135,12 @@ def simple_function(data):
     assert not user_code.status.approved
 
 
-def test_code_accept_deny(faker: Faker, worker: Worker):
+def test_code_accept_deny(worker: Worker, ds_client: SyftClient):
     root_client = worker.root_client
     dummy_data = [1, 2, 3]
     data = ActionObject.from_obj(dummy_data)
     action_obj = data.send(root_client)
 
-    ds_client = get_ds_client(faker, root_client, worker.guest_client)
-
     @syft.syft_function(
         input_policy=syft.ExactMatch(data=action_obj),
         output_policy=syft.SingleExecutionExactOutput(),

packages/syft/tests/syft/request/request_code_permissions_test.py

@@ -0,0 +1,69 @@
+# syft absolute
+import syft
+from syft.client.client import SyftClient
+from syft.server.worker import Worker
+from syft.service.action.action_object import ActionObject
+from syft.service.code.user_code import UserCode
+from syft.service.response import SyftSuccess
+
+
+def test_code_request_submitted_by_admin_only_admin_can_view(
+    worker: Worker, ds_client: SyftClient
+):
+    root_client = worker.root_client
+    dummy_data = [1, 2, 3]
+    data = ActionObject.from_obj(dummy_data)
+    action_obj = data.send(root_client)
+
+    @syft.syft_function(
+        input_policy=syft.ExactMatch(data=action_obj),
+        output_policy=syft.SingleExecutionExactOutput(),
+    )
+    def simple_function(data):
+        return sum(data)
+
+    project = syft.Project(name="test", members=[root_client])
+
+    result = project.create_code_request(simple_function, root_client)
+    assert isinstance(result, SyftSuccess)
+
+    # only root should be able to see request and access code
+    ds_request_all = ds_client.requests.get_all()
+    assert len(ds_request_all) == 0
+
+    root_request_all = root_client.requests.get_all()
+    assert len(root_request_all) == 1
+    root_code_access = root_request_all[0].code
+    assert isinstance(root_code_access, UserCode)
+
+
+def test_code_request_submitted_by_ds_root_and_ds_can_view(
+    worker: Worker, ds_client: SyftClient
+):
+    root_client = worker.root_client
+    dummy_data = [1, 2, 3]
+    data = ActionObject.from_obj(dummy_data)
+    action_obj = data.send(root_client)
+
+    @syft.syft_function(
+        input_policy=syft.ExactMatch(data=action_obj),
+        output_policy=syft.SingleExecutionExactOutput(),
+    )
+    def simple_function(data):
+        return sum(data)
+
+    project = syft.Project(name="test", members=[ds_client])
+
+    result = project.create_code_request(simple_function, ds_client)
+    assert isinstance(result, SyftSuccess)
+
+    # both root and ds should be able to see request and access code
+    ds_request_all = ds_client.requests.get_all()
+    assert len(ds_request_all) == 1
+    ds_code_access = ds_request_all[0].code
+    assert isinstance(ds_code_access, UserCode)
+
+    root_request_all = root_client.requests.get_all()
+    assert len(root_request_all) == 1
+    root_code_access = root_request_all[0].code
+    assert isinstance(root_code_access, UserCode)

packages/syft/tests/syft/worker_pool/worker_pool_service_test.py

@@ -1,9 +1,9 @@
 # third party
-from faker import Faker
 import pytest
 
 # syft absolute
 import syft as sy
+from syft.client.client import SyftClient
 from syft.custom_worker.config import DockerWorkerConfig
 from syft.custom_worker.config import PrebuiltWorkerConfig
 from syft.custom_worker.config import WorkerConfig
@@ -13,9 +13,6 @@
 from syft.service.worker.worker_image import SyftWorkerImage
 from syft.service.worker.worker_pool import WorkerPool
 
-# relative
-from ..request.request_code_accept_deny_test import get_ds_client
-
 PREBUILT_IMAGE_TAG = f"docker.io/openmined/syft-backend:{sy.__version__}"
 
 CUSTOM_DOCKERFILE = f"""
@@ -43,15 +40,17 @@
 
 @pytest.mark.parametrize("docker_tag,worker_config", WORKER_CONFIG_TEST_CASES)
 def test_create_image_and_pool_request_accept(
-    faker: Faker, worker: Worker, docker_tag: str, worker_config: WorkerConfig
+    worker: Worker,
+    docker_tag: str,
+    worker_config: WorkerConfig,
+    ds_client: SyftClient,
 ) -> None:
     """
     Test the functionality of `SyftWorkerPoolService.create_image_and_pool_request`
     when the request is accepted
     """
     # construct a root client and data scientist client for a datasite
     root_client = worker.root_client
-    ds_client = get_ds_client(faker, root_client, worker.guest_client)
     assert root_client.credentials != ds_client.credentials
 
     # the DS makes a request to create an image and a pool based on the image
@@ -94,19 +93,18 @@ def test_create_image_and_pool_request_accept(
     WORKER_CONFIG_TEST_CASES_WITH_N_IMAGES,
 )
 def test_create_pool_request_accept(
-    faker: Faker,
     worker: Worker,
     docker_tag: str,
     worker_config: WorkerConfig,
     n_images: int,
+    ds_client: SyftClient,
 ) -> None:
     """
     Test the functionality of `SyftWorkerPoolService.create_pool_request`
     when the request is accepted
     """
     # construct a root client and data scientist client for a datasite
     root_client = worker.root_client
-    ds_client = get_ds_client(faker, root_client, worker.guest_client)
     assert root_client.credentials != ds_client.credentials
 
     # the DO submits the docker config to build an image