fix: revoke of product_api_token didn't work properly (#1388)

* fix: revoke of product_api_token didn't work properly

* chore: pylint

backend/application/access_control/services/product_api_token.py

@@ -14,13 +14,22 @@ def create_product_api_token(product: Product, role: Roles) -> str:
     product_user_name = _get_product_user_name(product)
     user = get_user_by_username(product_user_name)
     if user:
-        raise ValidationError("Only one API token per product is allowed.")
+        try:
+            user.api_token  # pylint: disable=pointless-statement
+            # This statement raises an exception if the user has no API token.
+            raise ValidationError("Only one API token per product is allowed.")
+        except API_Token.DoesNotExist:
+            pass
 
     api_token, api_token_hash = generate_api_token_hash()
 
-    user = User(username=product_user_name, is_active=True)
+    if user:
+        user.is_active = True
+    else:
+        user = User(username=product_user_name, is_active=True)
     user.set_unusable_password()
     user.save()
+
     Product_Member(product=product, user=user, role=role).save()
     API_Token(user=user, api_token_hash=api_token_hash).save()
 
@@ -33,15 +42,18 @@ def revoke_product_api_token(product: Product) -> None:
     if not user:
         return
 
-    api_tokens = API_Token.objects.filter(user=user)
-    for api_token in api_tokens:
+    try:
+        api_token = user.api_token
         api_token.delete()
+    except API_Token.DoesNotExist:
+        pass
 
     product_member = get_product_member(product, user)
     if product_member:
         product_member.delete()
 
-    user.delete()
+    user.is_active = False
+    user.save()
 
 
 @dataclass

backend/unittests/access_control/services/test_product_api_token.py

@@ -17,18 +17,46 @@
 class TestProductApiToken(BaseTestCase):
     @patch("application.access_control.services.product_api_token.get_user_by_username")
     def test_create_product_api_token_exists(self, mock):
-        mock.return_value = User()
+        user = User(username="username", full_name="full_name")
+        api_token = API_Token(user=user, api_token_hash="hash")
+        mock.return_value = user
 
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(ValidationError) as e:
             create_product_api_token(self.product_1, Roles.Upload)
             mock.assert_called_with("-product-None-api_token-")
+            self.assertEqual("Only one API token per product is allowed.", str(e))
 
     @patch("application.access_control.services.product_api_token.get_user_by_username")
     @patch("application.access_control.models.API_Token.save")
     @patch("application.access_control.models.User.save")
     @patch("application.core.models.Product_Member.save")
     @patch("application.access_control.models.User.set_unusable_password")
-    def test_create_product_api_token_new(
+    def test_create_product_api_token_with_user(
+        self,
+        set_unusable_password_mock,
+        product_member_save_mock,
+        user_save_mock,
+        api_token_save_mock,
+        user_mock,
+    ):
+        user_mock.return_value = User()
+
+        api_token = create_product_api_token(self.product_1, Roles.Upload)
+
+        self.assertEqual(42, len(api_token))
+
+        user_mock.assert_called_with("-product-None-api_token-")
+        api_token_save_mock.assert_called()
+        user_save_mock.assert_called()
+        product_member_save_mock.assert_called()
+        set_unusable_password_mock.assert_called()
+
+    @patch("application.access_control.services.product_api_token.get_user_by_username")
+    @patch("application.access_control.models.API_Token.save")
+    @patch("application.access_control.models.User.save")
+    @patch("application.core.models.Product_Member.save")
+    @patch("application.access_control.models.User.set_unusable_password")
+    def test_create_product_api_token_without_user(
         self,
         set_unusable_password_mock,
         product_member_save_mock,
@@ -58,38 +86,31 @@ def test_revoke_product_api_token_not_exists(self, filter_mock, user_mock):
         filter_mock.assert_not_called()
 
     @patch("application.access_control.services.product_api_token.get_user_by_username")
-    @patch("application.access_control.models.API_Token.objects.filter")
     @patch("application.access_control.models.API_Token.delete")
-    @patch("application.access_control.models.User.delete")
+    @patch("application.access_control.models.User.save")
     @patch("application.core.models.Product_Member.delete")
     @patch("application.access_control.services.product_api_token.get_product_member")
     def test_revoke_product_api_token(
         self,
         get_product_member_mock,
         product_member_delete_mock,
-        user_delete_mock,
+        user_save_mock,
         api_token_delete_mock,
-        filter_mock,
         user_mock,
     ):
-        user = User()
+        user = User(username="username", full_name="full_name")
+        api_token = API_Token(user=user, api_token_hash="hash")
         user_mock.return_value = user
 
-        none_qs = API_Token.objects.none()
-        api_token_1 = API_Token()
-        api_token_2 = API_Token()
-        qs = list(chain(none_qs, [api_token_1, api_token_2]))
-        filter_mock.return_value = qs
-
         get_product_member_mock.return_value = Product_Member()
 
         revoke_product_api_token(self.product_1)
 
         user_mock.assert_called_with("-product-None-api_token-")
-        filter_mock.assert_called_with(user=user)
-        self.assertEqual(2, api_token_delete_mock.call_count)
-        self.assertEqual(1, product_member_delete_mock.call_count)
-        user_delete_mock.assert_called()
+        api_token_delete_mock.assert_called()
+        get_product_member_mock.assert_called_with(self.product_1, user)
+        product_member_delete_mock.assert_called()
+        user_save_mock.assert_called()
 
     @patch("application.access_control.services.product_api_token.get_user_by_username")
     def test_get_product_api_tokens_no_user(self, user_mock):