From 452a5562c459d93d87663eeb307e5509a1d1f822 Mon Sep 17 00:00:00 2001 From: James Tanner Date: Fri, 13 Oct 2023 08:43:44 -0400 Subject: [PATCH 01/12] Enable social auth users to see other users. Issue: AAH-2781 Signed-off-by: James Tanner --- .../access_control/statements/standalone.py | 4 +-- .../test_community_namespace_rbac.py | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/galaxy_ng/app/access_control/statements/standalone.py b/galaxy_ng/app/access_control/statements/standalone.py index 70e0da14a6..40576d1958 100644 --- a/galaxy_ng/app/access_control/statements/standalone.py +++ b/galaxy_ng/app/access_control/statements/standalone.py @@ -198,13 +198,13 @@ "action": ["list"], "principal": "authenticated", "effect": "allow", - "condition": "has_model_perms:galaxy.view_user" + # "condition": "has_model_perms:galaxy.view_user" }, { "action": ["retrieve"], "principal": "authenticated", "effect": "allow", - "condition": "has_model_perms:galaxy.view_user" + # "condition": "has_model_perms:galaxy.view_user" }, { "action": "destroy", diff --git a/galaxy_ng/tests/integration/community/test_community_namespace_rbac.py b/galaxy_ng/tests/integration/community/test_community_namespace_rbac.py index a939c53e1d..7a6b765638 100644 --- a/galaxy_ng/tests/integration/community/test_community_namespace_rbac.py +++ b/galaxy_ng/tests/integration/community/test_community_namespace_rbac.py @@ -570,3 +570,28 @@ def test_community_social_v3_namespace_sorting(ansible_config): # https://issues.redhat.com/browse/AAH-2729 # social auth code was trying to sort namespaces ... pass + + +@pytest.mark.deployment_community +def test_social_auth_access_api_ui_v1_users(ansible_config): + # https://issues.redhat.com/browse/AAH-2781 + + username = "foo1234" + default_cfg = extract_default_config(ansible_config) + + ga = GithubAdminClient() + ga.delete_user(login=username) + + user_c = ga.create_user(login=username, email="foo1234@gmail.com") + user_c.update(default_cfg) + user_c['username'] = username + + with SocialGithubClient(config=user_c) as client: + users_resp = client.get('_ui/v1/users/') + assert users_resp.status_code == 200 + + # try to fetch each user .. + for udata in users_resp.json()['data']: + uid = udata['id'] + user_resp = client.get(f'_ui/v1/users/{uid}/') + assert user_resp.status_code == 200 From cc13a4018a438f68e75c7029bb381aece9a7883b Mon Sep 17 00:00:00 2001 From: James Tanner Date: Fri, 13 Oct 2023 08:57:22 -0400 Subject: [PATCH 02/12] Add changelog. Issue: AAH-2781 Signed-off-by: James Tanner --- CHANGES/2781.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGES/2781.bugfix diff --git a/CHANGES/2781.bugfix b/CHANGES/2781.bugfix new file mode 100644 index 0000000000..a278665c60 --- /dev/null +++ b/CHANGES/2781.bugfix @@ -0,0 +1 @@ +Allow all authenticated users to list and retrieve other users. From d02ad6fe20e75a696db543bd51d88627403d9dc0 Mon Sep 17 00:00:00 2001 From: James Tanner Date: Fri, 13 Oct 2023 10:30:39 -0400 Subject: [PATCH 03/12] Fix unit tests? Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py index 1f5e7df182..1b5ff44293 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py @@ -151,16 +151,16 @@ def _test_user_list(): def test_user_get(self): def _test_user_get(): - # Check test user cannot view themselves on the users/ api + # Check test user can* view themselves on the users/ api self.client.force_authenticate(user=self.user) url = "{}{}/".format(self.user_url, self.user.id) response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_200_OK) - # Check test user cannot view other users + # Check test user can* view other users url = "{}{}/".format(self.user_url, self.admin_user.id) response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_200_OK) # Check admin user can view others self.client.force_authenticate(user=self.admin_user) From 9e4d706440b9f33dec0fd826f04c2832a5deabab Mon Sep 17 00:00:00 2001 From: James Tanner Date: Fri, 13 Oct 2023 11:27:53 -0400 Subject: [PATCH 04/12] Fix unit tests betterer? Issue: AAH-2781 Signed-off-by: James Tanner --- .../unit/api/test_api_ui_user_viewsets.py | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py index 1b5ff44293..55f93f7759 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py @@ -130,13 +130,15 @@ def test_user_can_create_users_with_right_perms(self): self.assertEqual(response.status_code, status.HTTP_201_CREATED) def test_user_list(self): - def _test_user_list(): + def _test_user_list(expected=None): + # Check test user can[not] view other users self.client.force_authenticate(user=self.user) log.debug("self.client: %s", self.client) log.debug("self.client.__dict__: %s", self.client.__dict__) response = self.client.get(self.user_url) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, expected) + # Check admin user can -always- view others self.client.force_authenticate(user=self.admin_user) response = self.client.get(self.user_url) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -144,29 +146,29 @@ def _test_user_list(): self.assertEqual(len(data), auth_models.User.objects.all().count()) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.STANDALONE.value): - _test_user_list() + _test_user_list(expected=status.HTTP_200_OK) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.INSIGHTS.value): - _test_user_list() + _test_user_list(expected=status.HTTP_403_FORBIDDEN) def test_user_get(self): - def _test_user_get(): - # Check test user can* view themselves on the users/ api + def _test_user_get(expected=None): + # Check test user can[not] view themselves on the users/ api self.client.force_authenticate(user=self.user) url = "{}{}/".format(self.user_url, self.user.id) response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, expected) - # Check test user can* view other users + # Check test user can[not] view other users url = "{}{}/".format(self.user_url, self.admin_user.id) response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, expected) - # Check admin user can view others + # Check admin user can -always- view others self.client.force_authenticate(user=self.admin_user) url = "{}{}/".format(self.user_url, self.user.id) response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, expected) data = response.data self.assertEqual(data["email"], self.user.email) self.assertEqual(data["first_name"], self.user.first_name) @@ -175,10 +177,10 @@ def _test_user_get(): self.assertTrue(self.user.groups.exists(id=group["id"])) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.STANDALONE.value): - _test_user_get() + _test_user_get(expected=status.HTTP_200_OK) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.INSIGHTS.value): - _test_user_get() + _test_user_get(expected=status.HTTP_403_FORBIDDEN) def _test_create_or_update(self, method_call, url, new_user_data, crud_status, auth_user): self.client.force_authenticate(user=auth_user) From 649f75e0f0f97ee83422eaff220c843f4eb61d8c Mon Sep 17 00:00:00 2001 From: James Tanner Date: Fri, 13 Oct 2023 13:44:13 -0400 Subject: [PATCH 05/12] Fix tests super good? Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py index 55f93f7759..9ab7aa9c25 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py @@ -1,5 +1,6 @@ import logging +from django.conf import settings as django_settings from django.test import override_settings from rest_framework import status from rest_framework.test import APIClient @@ -168,7 +169,7 @@ def _test_user_get(expected=None): self.client.force_authenticate(user=self.admin_user) url = "{}{}/".format(self.user_url, self.user.id) response = self.client.get(url) - self.assertEqual(response.status_code, expected) + self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.data self.assertEqual(data["email"], self.user.email) self.assertEqual(data["first_name"], self.user.first_name) From 8cdb48a145c8d2f3a04b2030969303257588f284 Mon Sep 17 00:00:00 2001 From: James Tanner Date: Fri, 13 Oct 2023 13:52:48 -0400 Subject: [PATCH 06/12] Unused import. Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py | 1 - 1 file changed, 1 deletion(-) diff --git a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py index 9ab7aa9c25..c5aab8bb16 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py @@ -1,6 +1,5 @@ import logging -from django.conf import settings as django_settings from django.test import override_settings from rest_framework import status from rest_framework.test import APIClient From b702778d99877a2e3928384b36d0a0e30202f8ea Mon Sep 17 00:00:00 2001 From: James Tanner Date: Sat, 14 Oct 2023 12:37:02 -0400 Subject: [PATCH 07/12] Fix integration tests. Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/tests/integration/api/test_rbac_roles.py | 1 + 1 file changed, 1 insertion(+) diff --git a/galaxy_ng/tests/integration/api/test_rbac_roles.py b/galaxy_ng/tests/integration/api/test_rbac_roles.py index 2d1652ea33..fc7790f586 100644 --- a/galaxy_ng/tests/integration/api/test_rbac_roles.py +++ b/galaxy_ng/tests/integration/api/test_rbac_roles.py @@ -613,6 +613,7 @@ view_tasks, view_role, view_pulp_groups, + view_users, } DENIED_FOR_ALL_USERS = { From eeaeb186fea67eb1f0225de88384e9f6cbbb2c49 Mon Sep 17 00:00:00 2001 From: James Tanner Date: Mon, 16 Oct 2023 11:00:40 -0400 Subject: [PATCH 08/12] Refactor to custom rbac condition. Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/app/access_control/access_policy.py | 20 +++++++++++++++++++ .../access_control/statements/standalone.py | 8 ++++++-- .../tests/integration/api/test_rbac_roles.py | 1 - 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/galaxy_ng/app/access_control/access_policy.py b/galaxy_ng/app/access_control/access_policy.py index d82013684c..bc3a740e83 100644 --- a/galaxy_ng/app/access_control/access_policy.py +++ b/galaxy_ng/app/access_control/access_policy.py @@ -81,6 +81,9 @@ def galaxy_statements(self): return self._STATEMENTS def _get_statements(self): + print(f'_GET_STATEMENTS settings.GALAXY_DEPLOYMENT_MODE:{settings.GALAXY_DEPLOYMENT_MODE}') + res = self.galaxy_statements[settings.GALAXY_DEPLOYMENT_MODE] + print(res) return self.galaxy_statements[settings.GALAXY_DEPLOYMENT_MODE] def get_pulp_access_policy(self, name, default=None): @@ -281,6 +284,23 @@ def v3_can_destroy_collections(self, request, view, action): return True return False + def v3_can_view_users(self, request, view, action): + """ + Community galaxy users need to be able to see one-another, + so that they can grant eachother access to their namespaces. + """ + SOCIAL_AUTH_GITHUB_KEY = settings.get("SOCIAL_AUTH_GITHUB_KEY", default=None) + SOCIAL_AUTH_GITHUB_SECRET = settings.get("SOCIAL_AUTH_GITHUB_SECRET", default=None) + is_github_social_auth = all([SOCIAL_AUTH_GITHUB_KEY, SOCIAL_AUTH_GITHUB_SECRET]) + + if is_github_social_auth: + return True + + if request.user.has_perm('galaxy.view_user'): + return True + + return False + def has_ansible_repo_perms(self, request, view, action, permission): """ Check if the user has model or object-level permissions diff --git a/galaxy_ng/app/access_control/statements/standalone.py b/galaxy_ng/app/access_control/statements/standalone.py index 40576d1958..163bc9c0ff 100644 --- a/galaxy_ng/app/access_control/statements/standalone.py +++ b/galaxy_ng/app/access_control/statements/standalone.py @@ -198,13 +198,17 @@ "action": ["list"], "principal": "authenticated", "effect": "allow", - # "condition": "has_model_perms:galaxy.view_user" + # "condition": "has_model_perms:galaxy.view_user", + #"condition": ["has_model_perms:galaxy.view_user", "v3_can_view_users"], + "condition": ["v3_can_view_users"], }, { "action": ["retrieve"], "principal": "authenticated", "effect": "allow", - # "condition": "has_model_perms:galaxy.view_user" + # "condition": "has_model_perms:galaxy.view_user", + #"condition": ["has_model_perms:galaxy.view_user", "v3_can_view_users"], + "condition": ["v3_can_view_users"], }, { "action": "destroy", diff --git a/galaxy_ng/tests/integration/api/test_rbac_roles.py b/galaxy_ng/tests/integration/api/test_rbac_roles.py index fc7790f586..2d1652ea33 100644 --- a/galaxy_ng/tests/integration/api/test_rbac_roles.py +++ b/galaxy_ng/tests/integration/api/test_rbac_roles.py @@ -613,7 +613,6 @@ view_tasks, view_role, view_pulp_groups, - view_users, } DENIED_FOR_ALL_USERS = { From 0e53627a57f73dd7418515e710307326feb36b00 Mon Sep 17 00:00:00 2001 From: James Tanner Date: Mon, 16 Oct 2023 11:18:21 -0400 Subject: [PATCH 09/12] Fixup tests. Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/app/access_control/access_policy.py | 3 --- .../unit/api/test_api_ui_user_viewsets.py | 23 +++++++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/galaxy_ng/app/access_control/access_policy.py b/galaxy_ng/app/access_control/access_policy.py index bc3a740e83..b2d91c4443 100644 --- a/galaxy_ng/app/access_control/access_policy.py +++ b/galaxy_ng/app/access_control/access_policy.py @@ -81,9 +81,6 @@ def galaxy_statements(self): return self._STATEMENTS def _get_statements(self): - print(f'_GET_STATEMENTS settings.GALAXY_DEPLOYMENT_MODE:{settings.GALAXY_DEPLOYMENT_MODE}') - res = self.galaxy_statements[settings.GALAXY_DEPLOYMENT_MODE] - print(res) return self.galaxy_statements[settings.GALAXY_DEPLOYMENT_MODE] def get_pulp_access_policy(self, name, default=None): diff --git a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py index c5aab8bb16..9c149fcc8c 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py @@ -146,11 +146,20 @@ def _test_user_list(expected=None): self.assertEqual(len(data), auth_models.User.objects.all().count()) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.STANDALONE.value): - _test_user_list(expected=status.HTTP_200_OK) + _test_user_list(expected=status.HTTP_403_FORBIDDEN) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.INSIGHTS.value): _test_user_list(expected=status.HTTP_403_FORBIDDEN) + # community + kwargs = { + 'GALAXY_DEPLOYMENT_MODE': DeploymentMode.STANDALONE.value, + 'SOCIAL_AUTH_GITHUB_KEY': '1234', + 'SOCIAL_AUTH_GITHUB_SECRET': '1234' + } + with self.settings(**kwargs): + _test_user_list(expected=status.HTTP_200_OK) + def test_user_get(self): def _test_user_get(expected=None): # Check test user can[not] view themselves on the users/ api @@ -177,11 +186,21 @@ def _test_user_get(expected=None): self.assertTrue(self.user.groups.exists(id=group["id"])) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.STANDALONE.value): - _test_user_get(expected=status.HTTP_200_OK) + _test_user_get(expected=status.HTTP_403_FORBIDDEN) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.INSIGHTS.value): _test_user_get(expected=status.HTTP_403_FORBIDDEN) + # community + kwargs = { + 'GALAXY_DEPLOYMENT_MODE': DeploymentMode.STANDALONE.value, + 'SOCIAL_AUTH_GITHUB_KEY': '1234', + 'SOCIAL_AUTH_GITHUB_SECRET': '1234' + } + with self.settings(**kwargs): + _test_user_get(expected=status.HTTP_200_OK) + + def _test_create_or_update(self, method_call, url, new_user_data, crud_status, auth_user): self.client.force_authenticate(user=auth_user) # set user with invalid password From 550e0440f9ed0c99581ce7abd84aedc150fd34a0 Mon Sep 17 00:00:00 2001 From: James Tanner Date: Mon, 16 Oct 2023 11:38:48 -0400 Subject: [PATCH 10/12] Cleanup. Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/app/access_control/statements/standalone.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/galaxy_ng/app/access_control/statements/standalone.py b/galaxy_ng/app/access_control/statements/standalone.py index 163bc9c0ff..58fb4053eb 100644 --- a/galaxy_ng/app/access_control/statements/standalone.py +++ b/galaxy_ng/app/access_control/statements/standalone.py @@ -198,16 +198,12 @@ "action": ["list"], "principal": "authenticated", "effect": "allow", - # "condition": "has_model_perms:galaxy.view_user", - #"condition": ["has_model_perms:galaxy.view_user", "v3_can_view_users"], "condition": ["v3_can_view_users"], }, { "action": ["retrieve"], "principal": "authenticated", "effect": "allow", - # "condition": "has_model_perms:galaxy.view_user", - #"condition": ["has_model_perms:galaxy.view_user", "v3_can_view_users"], "condition": ["v3_can_view_users"], }, { From f2a0e6cb792adac67ac0861005da15a5dc3f5411 Mon Sep 17 00:00:00 2001 From: James Tanner Date: Mon, 16 Oct 2023 11:39:28 -0400 Subject: [PATCH 11/12] Update changelog note. Issue: AAH-2781 Signed-off-by: James Tanner --- CHANGES/2781.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/2781.bugfix b/CHANGES/2781.bugfix index a278665c60..efee5117af 100644 --- a/CHANGES/2781.bugfix +++ b/CHANGES/2781.bugfix @@ -1 +1 @@ -Allow all authenticated users to list and retrieve other users. +Allow all authenticated users to list and retrieve other users when using github social auth. From f36065dd50ecae3c72d980278f5e2afac8f3770f Mon Sep 17 00:00:00 2001 From: James Tanner Date: Mon, 16 Oct 2023 11:42:58 -0400 Subject: [PATCH 12/12] Fix lint error. Issue: AAH-2781 Signed-off-by: James Tanner --- galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py | 1 - 1 file changed, 1 deletion(-) diff --git a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py index 9c149fcc8c..f87250b35f 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py @@ -200,7 +200,6 @@ def _test_user_get(expected=None): with self.settings(**kwargs): _test_user_get(expected=status.HTTP_200_OK) - def _test_create_or_update(self, method_call, url, new_user_data, crud_status, auth_user): self.client.force_authenticate(user=auth_user) # set user with invalid password