From b70d96796b8da6c0e4bfb58d569e923c99088c50 Mon Sep 17 00:00:00 2001 From: Stefan Fleckenstein Date: Mon, 15 Apr 2024 12:36:41 +0200 Subject: [PATCH] fix: revoke of product_api_token didn't work properly (#1388) * fix: revoke of product_api_token didn't work properly * chore: pylint --- .../services/product_api_token.py | 22 +++++-- .../services/test_product_api_token.py | 57 +++++++++++++------ 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/backend/application/access_control/services/product_api_token.py b/backend/application/access_control/services/product_api_token.py index cc8274caf..813d5091e 100644 --- a/backend/application/access_control/services/product_api_token.py +++ b/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 diff --git a/backend/unittests/access_control/services/test_product_api_token.py b/backend/unittests/access_control/services/test_product_api_token.py index 611ea2df8..3dc781e67 100644 --- a/backend/unittests/access_control/services/test_product_api_token.py +++ b/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):