Allow to access to app's resources when app is an owner (#9425)

* Allow to access to metadata when app is owner

* Fix typing

* Update description for AppCreate and AppInstall mutations

* Fix typing

* Update changelog

Co-authored-by: Marcin Gębala <5421321+maarcingebala@users.noreply.github.com>

CHANGELOG.md

@@ -18,6 +18,7 @@ All notable, unreleased changes to this project will be documented in this file.
 - Fix failing `checkoutCustomerAttach` mutation - #9401 by @IKarbowiak
 - Add new mutation `orderCreateFromCheckout` - #9343 by @korycins
 - Add `language_code` field to webhook payload for `Order`, `Checkout` and `Customer` - #9433 by @rafalp
+- Fix access to own resources by App - #9425 by @korycins
 
 
 # 3.1.7

saleor/graphql/account/utils.py

@@ -464,7 +464,7 @@ def look_for_permission_in_users_with_manage_staff(
 
 
 def requestor_has_access(
-    requestor: Union["User", "App"], owner: Optional["User"], *perms
+    requestor: Union["User", "App"], owner: Optional[Union["User", "App"]], *perms
 ) -> bool:
     """Check if requestor can access data.
 

saleor/graphql/app/mutations.py

@@ -16,6 +16,7 @@
 from ..core.enums import PermissionEnum
 from ..core.mutations import BaseMutation, ModelDeleteMutation, ModelMutation
 from ..core.types import AppError, NonNullList
+from ..decorators import staff_member_required
 from ..utils import get_user_or_app_from_context, requestor_is_superuser
 from .types import App, AppInstallation, AppToken, Manifest
 from .utils import ensure_can_manage_permissions
@@ -137,7 +138,11 @@ class Arguments:
         )
 
     class Meta:
-        description = "Creates a new app."
+        auto_permission_message = False
+        description = (
+            "Creates a new app. Requires the following "
+            "permissions: AUTHENTICATED_STAFF_USER and MANAGE_APPS."
+        )
         model = models.App
         object_type = App
         permissions = (AppPermission.MANAGE_APPS,)
@@ -155,6 +160,11 @@ def clean_input(cls, info, instance, data, input_cls=None):
             ensure_can_manage_permissions(requestor, permissions)
         return cleaned_input
 
+    @classmethod
+    @staff_member_required
+    def perform_mutation(cls, root, info, **data):
+        return super().perform_mutation(root, info, **data)
+
     @classmethod
     def save(cls, info, instance, cleaned_input):
         instance.save()
@@ -349,7 +359,11 @@ class Arguments:
         )
 
     class Meta:
-        description = "Install new app by using app manifest."
+        auto_permission_message = False
+        description = (
+            "Install new app by using app manifest. Requires the following "
+            "permissions: AUTHENTICATED_STAFF_USER and MANAGE_APPS."
+        )
         model = models.AppInstallation
         object_type = AppInstallation
         permissions = (AppPermission.MANAGE_APPS,)
@@ -371,6 +385,11 @@ def clean_input(cls, info, instance, data, input_cls=None):
             ensure_can_manage_permissions(requestor, permissions)
         return cleaned_input
 
+    @classmethod
+    @staff_member_required
+    def perform_mutation(cls, root, info, **data):
+        return super().perform_mutation(root, info, **data)
+
     @classmethod
     def post_save_action(cls, info, instance, cleaned_input):
         activate_after_installation = cleaned_input["activate_after_installation"]

saleor/graphql/app/tests/mutations/test_app_create.py

@@ -60,12 +60,13 @@ def test_app_create_mutation(
     assert default_token == app.tokens.get().auth_token
 
 
-def test_app_create_mutation_for_app(
+def test_app_is_not_allowed_to_call_create_mutation_for_app(
     permission_manage_apps,
     permission_manage_products,
     app_api_client,
     staff_user,
 ):
+    # given
     query = APP_CREATE_MUTATION
     requestor = app_api_client.app
     requestor.permissions.add(permission_manage_apps, permission_manage_products)
@@ -74,15 +75,12 @@ def test_app_create_mutation_for_app(
         "name": "New integration",
         "permissions": [PermissionEnum.MANAGE_PRODUCTS.name],
     }
+
+    # when
     response = app_api_client.post_graphql(query, variables=variables)
-    content = get_graphql_content(response)
-    app_data = content["data"]["appCreate"]["app"]
-    default_token = content["data"]["appCreate"]["authToken"]
-    app = App.objects.exclude(pk=requestor.pk).get()
-    assert app_data["isActive"] == app.is_active
-    assert app_data["name"] == app.name
-    assert list(app.permissions.all()) == [permission_manage_products]
-    assert default_token == app.tokens.get().auth_token
+
+    # then
+    assert_no_permission(response)
 
 
 def test_app_create_mutation_out_of_scope_permissions(
@@ -140,33 +138,6 @@ def test_app_create_mutation_superuser_can_create_app_with_any_perms(
     assert default_token == app.tokens.get().auth_token
 
 
-def test_app_create_mutation_for_app_out_of_scope_permissions(
-    permission_manage_apps,
-    permission_manage_products,
-    app_api_client,
-    staff_user,
-):
-    query = APP_CREATE_MUTATION
-
-    variables = {
-        "name": "New integration",
-        "permissions": [PermissionEnum.MANAGE_PRODUCTS.name],
-    }
-    response = app_api_client.post_graphql(
-        query, variables=variables, permissions=(permission_manage_apps,)
-    )
-    content = get_graphql_content(response)
-    data = content["data"]["appCreate"]
-
-    errors = data["errors"]
-    assert not data["app"]
-    assert len(errors) == 1
-    error = errors[0]
-    assert error["field"] == "permissions"
-    assert error["code"] == AppErrorCode.OUT_OF_SCOPE_PERMISSION.name
-    assert error["permissions"] == [PermissionEnum.MANAGE_PRODUCTS.name]
-
-
 def test_app_create_mutation_no_permissions(
     permission_manage_apps,
     permission_manage_products,

saleor/graphql/app/tests/mutations/test_app_install.py

@@ -5,7 +5,7 @@
 from .....app.models import AppInstallation
 from .....core import JobStatus
 from ....core.enums import AppErrorCode, PermissionEnum
-from ....tests.utils import get_graphql_content
+from ....tests.utils import assert_no_permission, get_graphql_content
 
 INSTALL_APP_MUTATION = """
     mutation AppInstall(
@@ -62,13 +62,10 @@ def test_install_app_mutation(
     mocked_task.assert_called_with(app_installation.pk, True)
 
 
-def test_install_app_mutation_by_app(
+def test_app_is_not_allowed_to_install_app(
     permission_manage_apps, permission_manage_orders, app_api_client, monkeypatch
 ):
-    mocked_task = Mock()
-    monkeypatch.setattr(
-        "saleor.graphql.app.mutations.install_app_task.delay", mocked_task
-    )
+    # given
     query = INSTALL_APP_MUTATION
     app_api_client.app.permissions.set(
         [permission_manage_apps, permission_manage_orders]
@@ -78,18 +75,15 @@ def test_install_app_mutation_by_app(
         "manifest_url": "http://localhost:3000/manifest",
         "permissions": [PermissionEnum.MANAGE_ORDERS.name],
     }
+
+    # when
     response = app_api_client.post_graphql(
         query,
         variables=variables,
     )
-    content = get_graphql_content(response)
-    app_installation = AppInstallation.objects.get()
-    app_installation_data = content["data"]["appInstall"]["appInstallation"]
-    _, app_id = graphene.Node.from_global_id(app_installation_data["id"])
-    assert int(app_id) == app_installation.id
-    assert app_installation_data["status"] == JobStatus.PENDING.upper()
-    assert app_installation_data["manifestUrl"] == app_installation.manifest_url
-    mocked_task.assert_called_with(app_installation.pk, True)
+
+    # then
+    assert_no_permission(response)
 
 
 def test_app_install_mutation_out_of_scope_permissions(
@@ -116,30 +110,3 @@ def test_app_install_mutation_out_of_scope_permissions(
     assert error["field"] == "permissions"
     assert error["code"] == AppErrorCode.OUT_OF_SCOPE_PERMISSION.name
     assert error["permissions"] == [PermissionEnum.MANAGE_ORDERS.name]
-
-
-def test_install_app_mutation_by_app_out_of_scope_permissions(
-    permission_manage_apps, app_api_client
-):
-    query = INSTALL_APP_MUTATION
-    app_api_client.app.permissions.set([permission_manage_apps])
-    variables = {
-        "app_name": "New external integration",
-        "manifest_url": "http://localhost:3000/manifest",
-        "permissions": [PermissionEnum.MANAGE_ORDERS.name],
-    }
-    response = app_api_client.post_graphql(
-        query,
-        variables=variables,
-    )
-
-    content = get_graphql_content(response)
-    data = content["data"]["appInstall"]
-
-    errors = data["errors"]
-    assert not data["appInstallation"]
-    assert len(errors) == 1
-    error = errors[0]
-    assert error["field"] == "permissions"
-    assert error["code"] == AppErrorCode.OUT_OF_SCOPE_PERMISSION.name
-    assert error["permissions"] == [PermissionEnum.MANAGE_ORDERS.name]

saleor/graphql/app/tests/queries/test_app.py

@@ -40,6 +40,14 @@
                     code
                 }
             }
+            metadata{
+                key
+                value
+            }
+            privateMetadata{
+                key
+                value
+            }
         }
     }
     """
@@ -154,14 +162,20 @@ def test_app_without_id_as_staff(
 def test_own_app_without_id(
     app_api_client,
     app,
-    permission_manage_orders,
     order_with_lines,
     webhook,
-    permission_manage_apps,
 ):
+    # given
+    app.store_value_in_metadata({"public": "metadata"})
+    app.store_value_in_private_metadata({"private": "metadata"})
+    app.save()
+
+    # when
     response = app_api_client.post_graphql(
-        QUERY_APP, permissions=[permission_manage_apps]
+        QUERY_APP,
     )
+
+    # then
     content = get_graphql_content(response)
 
     tokens = app.tokens.all()
@@ -181,6 +195,10 @@ def test_own_app_without_id(
     assert app_data["supportUrl"] == app.support_url
     assert app_data["configurationUrl"] == app.configuration_url
     assert app_data["appUrl"] == app.app_url
+    assert app_data["metadata"][0]["key"] == "public"
+    assert app_data["metadata"][0]["value"] == "metadata"
+    assert app_data["privateMetadata"][0]["key"] == "private"
+    assert app_data["privateMetadata"][0]["value"] == "metadata"
 
 
 def test_app_query_without_permission(

saleor/graphql/app/types.py

@@ -6,11 +6,11 @@
 from ...app.types import AppExtensionTarget
 from ...core.exceptions import PermissionDenied
 from ...core.permissions import AppPermission, AuthorizationFilters
+from ..account.utils import requestor_has_access
 from ..core.connection import CountableConnection
 from ..core.descriptions import ADDED_IN_31, PREVIEW_FEATURE
 from ..core.federation import federated_entity, resolve_federation_references
 from ..core.types import Job, ModelObjectType, NonNullList, Permission
-from ..decorators import permission_required
 from ..meta.types import ObjectWithMetadata
 from ..utils import format_permissions_for_display, get_user_or_app_from_context
 from ..webhook.types import Webhook
@@ -23,6 +23,14 @@
 )
 
 
+def has_required_permission(app: models.App, context):
+    requester = get_user_or_app_from_context(context)
+    if not requestor_has_access(requester, app, AppPermission.MANAGE_APPS):
+        raise PermissionDenied(
+            permissions=[AppPermission.MANAGE_APPS, AuthorizationFilters.OWNER]
+        )
+
+
 class AppManifestExtension(graphene.ObjectType):
     permissions = NonNullList(
         Permission,
@@ -217,18 +225,18 @@ def resolve_permissions(root: models.App, _info, **_kwargs):
         return format_permissions_for_display(permissions)
 
     @staticmethod
-    @permission_required(AppPermission.MANAGE_APPS)
-    def resolve_tokens(root: models.App, _info, **_kwargs):
-        return root.tokens.all()  # type: ignore
+    def resolve_tokens(root: models.App, info, **_kwargs):
+        has_required_permission(root, info.context)
+        return root.tokens.all()
 
     @staticmethod
-    @permission_required(AppPermission.MANAGE_APPS)
-    def resolve_webhooks(root: models.App, _info):
+    def resolve_webhooks(root: models.App, info):
+        has_required_permission(root, info.context)
         return root.webhooks.all()
 
     @staticmethod
-    @permission_required(AppPermission.MANAGE_APPS)
     def resolve_access_token(root: models.App, info):
+        has_required_permission(root, info.context)
         return resolve_access_token_for_app(info, root)
 
     @staticmethod

saleor/graphql/meta/permissions.py

@@ -82,7 +82,10 @@ def menu_permissions(_info, _object_pk: Any) -> List[BasePermissionEnum]:
     return [MenuPermissions.MANAGE_MENUS]
 
 
-def app_permissions(_info, _object_pk: int) -> List[BasePermissionEnum]:
+def app_permissions(info, object_pk: str) -> List[BasePermissionEnum]:
+    app = info.context.app
+    if app and app.pk == int(object_pk):
+        return []
     return [AppPermission.MANAGE_APPS]
 
 

saleor/graphql/meta/resolvers.py

@@ -92,9 +92,9 @@ def resolve_private_metadata(root: ModelWithMetadata, info):
     if not get_required_permission:
         raise PermissionDenied()
 
-    required_permissions = get_required_permission(info, item_id)
+    required_permissions = get_required_permission(info, item_id)  # type: ignore
 
-    if not required_permissions:
+    if not isinstance(required_permissions, list):
         raise PermissionDenied()
 
     requester = get_user_or_app_from_context(info.context)