Merge pull request #5459 from nyaruka/api_token_cleanup_pt2

API tokens should use the current role of the user

temba/api/models.py

@@ -74,13 +74,13 @@ def has_permission(self, request, view):
         org = request.org
 
         if request.auth:
-            # check that user is still allowed to use the token's role
-            if not request.auth.is_valid():
-                return False
+            # auth token was used
+            role = org.get_user_role(request.auth.user)
 
-            role = OrgRole.from_group(request.auth.role)
+            # only editors and administrators can use API tokens
+            if role not in (OrgRole.EDITOR, OrgRole.ADMINISTRATOR):
+                return False
         elif org:
-            # user may not have used token authentication
             role = org.get_user_role(request.user)
         else:
             return False
@@ -260,14 +260,6 @@ def get_default_role(cls, org, user):
 
         return role
 
-    def is_valid(self) -> bool:
-        """
-        A user's role in an org can change so this return whether this token is still valid.
-        """
-        role = self.org.get_user_role(self.user)
-        roles_allowed_this_perm_group = self.GROUP_GRANTED_TO.get(self.role.name, ())
-        return role and role in roles_allowed_this_perm_group
-
     def record_used(self):
         r = get_redis_connection()
         r.sadd("api_tokens_used", self.key)

temba/api/tests.py

@@ -48,17 +48,6 @@ def test_get_or_create(self):
         self.assertRaises(ValueError, APIToken.get_or_create, self.org, self.admin, role=OrgRole.VIEWER)
         self.assertRaises(ValueError, APIToken.get_or_create, self.org, self.user)
 
-    def test_is_valid(self):
-        token1 = APIToken.get_or_create(self.org, self.admin, role=OrgRole.ADMINISTRATOR)
-        token2 = APIToken.get_or_create(self.org, self.admin, role=OrgRole.EDITOR)
-
-        # demote admin to an editor
-        self.org.add_user(self.admin, OrgRole.EDITOR)
-        self.admin.refresh_from_db()
-
-        self.assertFalse(token1.is_valid())
-        self.assertTrue(token2.is_valid())
-
     def test_get_default_role(self):
         self.assertEqual(APIToken.get_default_role(self.org, self.admin), OrgRole.ADMINISTRATOR)
         self.assertEqual(APIToken.get_default_role(self.org, self.editor), OrgRole.EDITOR)

temba/api/v2/tests.py

@@ -40,7 +40,7 @@
 from .serializers import format_datetime, normalize_extra
 
 NUM_BASE_SESSION_QUERIES = 4  # number of queries required for any request using session auth
-NUM_BASE_TOKEN_QUERIES = 3  # number of queries required for any request using token auth
+NUM_BASE_TOKEN_QUERIES = 2  # number of queries required for any request using token auth
 
 
 class APITest(APITestMixin, TembaTest):
@@ -536,15 +536,14 @@ def request_by_session(endpoint, user):
         response = request_by_token(fields_url, token1.key)
         self.assertEqual(response.status_code, 429)
 
-        # if user loses access to the token's role, don't allow the request
-        self.org.add_user(self.admin, OrgRole.EDITOR)
+        # if user is demoted to a role that can't use tokens, tokens shouldn't work for them
+        self.org.add_user(self.admin, OrgRole.VIEWER)
 
         self.assertEqual(request_by_token(campaigns_url, token1.key).status_code, 403)
         self.assertEqual(request_by_basic_auth(campaigns_url, self.admin.username, token1.key).status_code, 403)
-        self.assertEqual(request_by_token(contacts_url, token2.key).status_code, 200)  # other token unaffected
-        self.assertEqual(request_by_basic_auth(contacts_url, self.editor.username, token2.key).status_code, 200)
 
         # and if user is inactive, disallow the request
+        self.org.add_user(self.admin, OrgRole.ADMINISTRATOR)
         self.admin.is_active = False
         self.admin.save()
 

temba/orgs/tests.py

@@ -1656,11 +1656,6 @@ def test_manage_accounts(self):
         self.assertEqual(set(), set(self.org.get_users(roles=[OrgRole.VIEWER])))
         self.assertEqual({self.agent}, set(self.org.get_users(roles=[OrgRole.AGENT])))
 
-        # our second editors API token should be deleted
-        self.assertEqual(self.admin.api_tokens.filter(is_active=True).count(), 1)
-        self.assertEqual(self.editor.api_tokens.filter(is_active=True).count(), 1)
-        self.assertEqual(editor2.api_tokens.filter(is_active=True).count(), 0)
-
         # pretend our first invite was acted on
         invitation1.release()
 
@@ -1724,14 +1719,6 @@ def test_manage_accounts(self):
         self.assertEqual(set(), set(self.org.get_users(roles=[OrgRole.VIEWER])))
         self.assertEqual({self.editor}, set(self.org.get_users(roles=[OrgRole.AGENT])))
 
-        # editor will have lost their API tokens
-        self.editor.refresh_from_db()
-        self.assertEqual(0, self.editor.api_tokens.filter(is_active=True).count())
-
-        # and all our API tokens for the admin are deleted
-        self.admin.refresh_from_db()
-        self.assertEqual(self.admin.api_tokens.filter(is_active=True).count(), 0)
-
     def test_manage_children(self):
         children_url = reverse("orgs.org_sub_orgs")
 

temba/orgs/views.py

@@ -2134,11 +2134,6 @@ def post_save(self, obj):
                 elif org.get_user_role(user) != new_role:
                     org.add_user(user, new_role)
 
-                # when a user's role changes, delete any API tokens they're no longer allowed to have
-                for token in APIToken.objects.filter(org=org, user=user):
-                    if not token.is_valid():
-                        token.release()
-
             return obj
 
         def get_context_data(self, **kwargs):