From ec18bbd761992766ee8bab6d613f3389372d0034 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Thu, 9 Jan 2025 13:42:29 -0800 Subject: [PATCH 1/4] fix: Fix deactivated enterprise user when no plan activated users --- graphql_api/types/query/query.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graphql_api/types/query/query.py b/graphql_api/types/query/query.py index e193dc4ca8..05687eb5fc 100644 --- a/graphql_api/types/query/query.py +++ b/graphql_api/types/query/query.py @@ -51,9 +51,9 @@ async def resolve_owner( if settings.IS_ENTERPRISE and settings.GUEST_ACCESS is False: if not user or not user.is_authenticated: raise UnauthorizedGuestAccess() - - target = await get_owner(service, username) - if user.ownerid not in target.plan_activated_users: + target_owner = await get_owner(service, username) + has_plan_activated_users = target_owner and target_owner.plan_activated_users is not None + if has_plan_activated_users and user.ownerid not in target_owner.plan_activated_users: raise UnauthorizedGuestAccess() return await get_owner(service, username) From 1b03cc3a8faf7b94910a6216732ec5f4f44e63a1 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Thu, 9 Jan 2025 14:30:32 -0800 Subject: [PATCH 2/4] add test --- graphql_api/tests/test_owner.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/graphql_api/tests/test_owner.py b/graphql_api/tests/test_owner.py index 25a5b3ae23..b626a07da6 100644 --- a/graphql_api/tests/test_owner.py +++ b/graphql_api/tests/test_owner.py @@ -806,6 +806,26 @@ def test_fetch_owner_on_unauthenticated_enteprise_guest_access_not_activated(sel assert e.message == UnauthorizedGuestAccess.message assert e.extensions["code"] == UnauthorizedGuestAccess.code + @override_settings(IS_ENTERPRISE=True, GUEST_ACCESS=False) + def test_fetch_owner_plan_activated_users_is_none(self): + """ + This test is when Enterprise guest access is disabled, and you are + trying to view an org that does not track plan activated users (e.g., historic data) + """ + user = OwnerFactory(username="sample-user") + owner = OwnerFactory(username="sample-owner", plan_activated_users=None) + user.save() + owner.save() + query = """{ + owner(username: "%s") { + username + } + } + """ % (owner.username) + + data = self.gql_request(query, owner=user) + assert data["owner"]["username"] == "sample-owner" + def test_fetch_current_user_is_okta_authenticated(self): account = AccountFactory() owner = OwnerFactory(username="sample-owner", service="github", account=account) From 79341b5be0d3aa777e2406820321bbed0e0d29c5 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Thu, 9 Jan 2025 14:52:53 -0800 Subject: [PATCH 3/4] appease linter --- graphql_api/types/query/query.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/graphql_api/types/query/query.py b/graphql_api/types/query/query.py index 05687eb5fc..db4309c227 100644 --- a/graphql_api/types/query/query.py +++ b/graphql_api/types/query/query.py @@ -52,8 +52,13 @@ async def resolve_owner( if not user or not user.is_authenticated: raise UnauthorizedGuestAccess() target_owner = await get_owner(service, username) - has_plan_activated_users = target_owner and target_owner.plan_activated_users is not None - if has_plan_activated_users and user.ownerid not in target_owner.plan_activated_users: + has_plan_activated_users = ( + target_owner and target_owner.plan_activated_users is not None + ) + if ( + has_plan_activated_users + and user.ownerid not in target_owner.plan_activated_users + ): raise UnauthorizedGuestAccess() return await get_owner(service, username) From 422043023e4848b1da0522078e2a5f7333fca361 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Fri, 10 Jan 2025 09:30:11 -0800 Subject: [PATCH 4/4] account for empty list --- graphql_api/types/query/query.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/graphql_api/types/query/query.py b/graphql_api/types/query/query.py index db4309c227..ab4064130d 100644 --- a/graphql_api/types/query/query.py +++ b/graphql_api/types/query/query.py @@ -51,9 +51,13 @@ async def resolve_owner( if settings.IS_ENTERPRISE and settings.GUEST_ACCESS is False: if not user or not user.is_authenticated: raise UnauthorizedGuestAccess() + + # if the owner tracks plan activated users, check if the user is in the list target_owner = await get_owner(service, username) has_plan_activated_users = ( - target_owner and target_owner.plan_activated_users is not None + target_owner + and target_owner.plan_activated_users is not None + and len(target_owner.plan_activated_users) > 0 ) if ( has_plan_activated_users