Merge pull request #6173 from mirumee/SALEOR-681-update-required-perm…

…s-for-apps-management

Update required perms for apps management

CHANGELOG.md

@@ -59,6 +59,7 @@ All notable, unreleased changes to this project will be documented in this file.
 - Fix crashing system when avalara is improperly configured - #6117 by @IKarbowiak
 - Fix for failing finalising draft order - #6133 by @korycins
 - Remove corresponding draft order lines when variant is removing - #6119 by @IKarbowiak
+- Update required perms for apps management - #6173 by @IKarbowiak
 
 ## 2.10.2
 

saleor/graphql/app/mutations.py

@@ -257,21 +257,9 @@ class Meta:
         error_type_class = AppError
         error_type_field = "app_errors"
 
-    @classmethod
-    def clean_instance(cls, info, app: App):
-        requestor = get_user_or_app_from_context(info.context)
-        permissions = app.permissions.all()
-        if not requestor_is_superuser(requestor) and not requestor.has_perms(
-            permissions
-        ):
-            msg = "You don't have enough permission to perform this action."
-            code = AppErrorCode.OUT_OF_SCOPE_APP.value
-            raise ValidationError({"id": ValidationError(msg, code=code)})
-
     @classmethod
     def perform_mutation(cls, root, info, **data):
         app = cls.get_instance(info, **data)
-        cls.clean_instance(info, app)
         app.is_active = True
         cls.save(info, app, cleaned_input=None)
         return cls.success_response(app)
@@ -288,18 +276,9 @@ class Meta:
         error_type_class = AppError
         error_type_field = "app_errors"
 
-    @classmethod
-    def clean_instance(cls, info, app: App):
-        requestor = get_user_or_app_from_context(info.context)
-        if not requestor_is_superuser(requestor) and not can_manage_app(requestor, app):
-            msg = "You don't have enough permission to perform this action."
-            code = AppErrorCode.OUT_OF_SCOPE_APP.value
-            raise ValidationError({"id": ValidationError(msg, code=code)})
-
     @classmethod
     def perform_mutation(cls, root, info, **data):
         app = cls.get_instance(info, **data)
-        cls.clean_instance(info, app)
         app.is_active = False
         cls.save(info, app, cleaned_input=None)
         return cls.success_response(app)
@@ -320,15 +299,6 @@ class Meta:
 
     @classmethod
     def clean_instance(cls, info, instance):
-        requestor = get_user_or_app_from_context(info.context)
-        permissions = instance.permissions.all()
-        if not requestor_is_superuser(requestor) and not requestor.has_perms(
-            permissions
-        ):
-            msg = "You don't have enough permission to perform this action."
-            code = AppErrorCode.OUT_OF_SCOPE_APP.value
-            raise ValidationError({"id": ValidationError(msg, code=code)})
-
         if instance.status != JobStatus.FAILED:
             msg = "Cannot delete installation with different status than failed."
             code = AppErrorCode.INVALID_STATUS.value
@@ -358,15 +328,6 @@ def save(cls, info, instance, cleaned_input):
 
     @classmethod
     def clean_instance(cls, info, instance):
-        requestor = get_user_or_app_from_context(info.context)
-        permissions = instance.permissions.all()
-        if not requestor_is_superuser(requestor) and not requestor.has_perms(
-            permissions
-        ):
-            msg = "You don't have enough permission to perform this action."
-            code = AppErrorCode.OUT_OF_SCOPE_APP.value
-            raise ValidationError({"id": ValidationError(msg, code=code)})
-
         if instance.status != JobStatus.FAILED:
             msg = "Cannot retry installation with different status than failed."
             code = AppErrorCode.INVALID_STATUS.value

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

@@ -1,7 +1,6 @@
 import graphene
 
 from .....app.models import App
-from ....core.enums import AppErrorCode
 from ....tests.utils import assert_no_permission, get_graphql_content
 
 APP_ACTIVATE_MUTATION = """
@@ -22,31 +21,41 @@
 
 
 def test_activate_app(app, staff_api_client, permission_manage_apps):
+    # given
     app.is_active = False
     app.save()
     query = APP_ACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
+
+    # when
     response = staff_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_apps,)
     )
+
+    # then
     get_graphql_content(response)
 
     app.refresh_from_db()
     assert app.is_active
 
 
 def test_activate_app_by_app(app, app_api_client, permission_manage_apps):
+    # given
     app = App.objects.create(name="Sample app objects", is_active=False)
     query = APP_ACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
     app_api_client.app.permissions.set([permission_manage_apps])
+
+    # when
     response = app_api_client.post_graphql(query, variables=variables)
+
+    # then
     get_graphql_content(response)
 
     app.refresh_from_db()
@@ -56,16 +65,21 @@ def test_activate_app_by_app(app, app_api_client, permission_manage_apps):
 def test_activate_app_missing_permission(
     app, staff_api_client, permission_manage_orders
 ):
+    # given
     app.is_active = False
     app.save()
     query = APP_ACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
+
+    # when
     response = staff_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_orders,)
     )
+
+    # then
     assert_no_permission(response)
 
     app.refresh_from_db()
@@ -75,14 +89,19 @@ def test_activate_app_missing_permission(
 def test_activate_app_by_app_missing_permission(
     app, app_api_client, permission_manage_orders
 ):
+    # given
     app = App.objects.create(name="Sample app objects", is_active=False)
     query = APP_ACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
     app_api_client.app.permissions.set([permission_manage_orders])
+
+    # when
     response = app_api_client.post_graphql(query, variables=variables)
+
+    # then
     assert_no_permission(response)
 
     assert not app.is_active
@@ -91,6 +110,7 @@ def test_activate_app_by_app_missing_permission(
 def test_app_has_more_permission_than_user_requestor(
     app, staff_api_client, permission_manage_orders, permission_manage_apps
 ):
+    # given
     app.permissions.add(permission_manage_orders)
     app.is_active = False
     app.save()
@@ -100,24 +120,27 @@ def test_app_has_more_permission_than_user_requestor(
     variables = {
         "id": id,
     }
+
+    # when
     response = staff_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_apps,)
     )
+
+    # then
     content = get_graphql_content(response)
     app_data = content["data"]["appActivate"]["app"]
     app_errors = content["data"]["appActivate"]["appErrors"]
     app.refresh_from_db()
 
-    assert not app_data
-    assert len(app_errors) == 1
-    assert app_errors[0]["field"] == "id"
-    assert app_errors[0]["code"] == AppErrorCode.OUT_OF_SCOPE_APP.name
-    assert not app.is_active
+    assert not app_errors
+    assert app_data["isActive"] is True
+    assert app.is_active
 
 
 def test_app_has_more_permission_than_app_requestor(
     app_api_client, permission_manage_orders, permission_manage_apps
 ):
+    # given
     app = App.objects.create(name="Sample app objects", is_active=False)
     app.permissions.add(permission_manage_orders)
 
@@ -126,16 +149,18 @@ def test_app_has_more_permission_than_app_requestor(
     variables = {
         "id": id,
     }
+
+    # when
     response = app_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_apps,)
     )
+
+    # then
     content = get_graphql_content(response)
     app_data = content["data"]["appActivate"]["app"]
     app_errors = content["data"]["appActivate"]["appErrors"]
     app.refresh_from_db()
 
-    assert not app_data
-    assert len(app_errors) == 1
-    assert app_errors[0]["field"] == "id"
-    assert app_errors[0]["code"] == AppErrorCode.OUT_OF_SCOPE_APP.name
-    assert not app.is_active
+    assert not app_errors
+    assert app_data["isActive"] is True
+    assert app.is_active

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

@@ -1,7 +1,6 @@
 import graphene
 
 from .....app.models import App
-from ....core.enums import AppErrorCode
 from ....tests.utils import assert_no_permission, get_graphql_content
 
 APP_DEACTIVATE_MUTATION = """
@@ -22,31 +21,41 @@
 
 
 def test_deactivate_app(app, staff_api_client, permission_manage_apps):
+    # given
     app.is_active = True
     app.save()
     query = APP_DEACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
+
+    # when
     response = staff_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_apps,)
     )
+
+    # then
     get_graphql_content(response)
 
     app.refresh_from_db()
     assert not app.is_active
 
 
 def test_deactivate_app_by_app(app, app_api_client, permission_manage_apps):
+    # given
     app = App.objects.create(name="Sample app objects", is_active=True)
     query = APP_DEACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
     app_api_client.app.permissions.set([permission_manage_apps])
+
+    # when
     response = app_api_client.post_graphql(query, variables=variables)
+
+    # then
     get_graphql_content(response)
 
     app.refresh_from_db()
@@ -56,16 +65,21 @@ def test_deactivate_app_by_app(app, app_api_client, permission_manage_apps):
 def test_deactivate_app_missing_permission(
     app, staff_api_client, permission_manage_orders
 ):
+    # given
     app.is_active = True
     app.save()
     query = APP_DEACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
+
+    # when
     response = staff_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_orders,)
     )
+
+    # then
     assert_no_permission(response)
 
     app.refresh_from_db()
@@ -75,14 +89,19 @@ def test_deactivate_app_missing_permission(
 def test_activate_app_by_app_missing_permission(
     app, app_api_client, permission_manage_orders
 ):
+    # given
     app = App.objects.create(name="Sample app objects", is_active=True)
     query = APP_DEACTIVATE_MUTATION
     id = graphene.Node.to_global_id("App", app.id)
     variables = {
         "id": id,
     }
     app_api_client.app.permissions.set([permission_manage_orders])
+
+    # when
     response = app_api_client.post_graphql(query, variables=variables)
+
+    # then
     assert_no_permission(response)
 
     assert app.is_active
@@ -91,6 +110,7 @@ def test_activate_app_by_app_missing_permission(
 def test_app_has_more_permission_than_user_requestor(
     app, staff_api_client, permission_manage_orders, permission_manage_apps
 ):
+    # given
     app.permissions.add(permission_manage_orders)
     app.is_active = True
     app.save()
@@ -100,24 +120,27 @@ def test_app_has_more_permission_than_user_requestor(
     variables = {
         "id": id,
     }
+
+    # when
     response = staff_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_apps,)
     )
+
+    # then
     content = get_graphql_content(response)
     app_data = content["data"]["appDeactivate"]["app"]
     app_errors = content["data"]["appDeactivate"]["appErrors"]
     app.refresh_from_db()
 
-    assert not app_data
-    assert len(app_errors) == 1
-    assert app_errors[0]["field"] == "id"
-    assert app_errors[0]["code"] == AppErrorCode.OUT_OF_SCOPE_APP.name
-    assert app.is_active
+    assert not app_errors
+    assert not app.is_active
+    assert app_data["isActive"] is False
 
 
 def test_app_has_more_permission_than_app_requestor(
     app_api_client, permission_manage_orders, permission_manage_apps
 ):
+    # given
     app = App.objects.create(name="Sample app objects", is_active=True)
     app.permissions.add(permission_manage_orders)
 
@@ -126,16 +149,18 @@ def test_app_has_more_permission_than_app_requestor(
     variables = {
         "id": id,
     }
+
+    # when
     response = app_api_client.post_graphql(
         query, variables=variables, permissions=(permission_manage_apps,)
     )
+
+    # then
     content = get_graphql_content(response)
     app_data = content["data"]["appDeactivate"]["app"]
     app_errors = content["data"]["appDeactivate"]["appErrors"]
     app.refresh_from_db()
 
-    assert not app_data
-    assert len(app_errors) == 1
-    assert app_errors[0]["field"] == "id"
-    assert app_errors[0]["code"] == AppErrorCode.OUT_OF_SCOPE_APP.name
-    assert app.is_active
+    assert not app_errors
+    assert not app.is_active
+    assert app_data["isActive"] is False