From c46ebf7d44ad5e84b57121d9aceb5610317c2d2d Mon Sep 17 00:00:00 2001 From: David Newswanger Date: Tue, 4 Jun 2024 10:01:06 -0600 Subject: [PATCH 1/4] Add option to disable creating, update and deleting users and groups. No-Issue --- galaxy_ng/app/access_control/access_policy.py | 29 ++-- .../app/access_control/statements/pulp.py | 28 +++- .../access_control/statements/standalone.py | 124 +++++++++------ galaxy_ng/app/settings.py | 5 +- .../api/test_disable_shared_resources.py | 148 ++++++++++++++++++ profiles/dab/pulp_config.env | 4 + profiles/dab_jwt/pulp_config.env | 3 + 7 files changed, 278 insertions(+), 63 deletions(-) create mode 100644 galaxy_ng/tests/integration/api/test_disable_shared_resources.py diff --git a/galaxy_ng/app/access_control/access_policy.py b/galaxy_ng/app/access_control/access_policy.py index b2d91c4443..448e804df6 100644 --- a/galaxy_ng/app/access_control/access_policy.py +++ b/galaxy_ng/app/access_control/access_policy.py @@ -573,6 +573,22 @@ def require_requirements_yaml(self, request, view, action): }) return True + def is_direct_shared_resource_management_disabled(self, request, view, action): + return not settings.DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED + + def user_is_superuser(self, request, view, action): + if getattr(self, "swagger_fake_view", False): + # If OpenAPI schema is requested, don't check for superuser + return False + user = view.get_object() + return user.is_superuser + + def is_current_user(self, request, view, action): + if getattr(self, "swagger_fake_view", False): + # If OpenAPI schema is requested, don't check for current user + return False + return request.user == view.get_object() + class AIDenyIndexAccessPolicy(AccessPolicyBase): NAME = "AIDenyIndexView" @@ -613,19 +629,6 @@ class CollectionRemoteAccessPolicy(AccessPolicyBase): class UserAccessPolicy(AccessPolicyBase): NAME = "UserViewSet" - def user_is_superuser(self, request, view, action): - if getattr(self, "swagger_fake_view", False): - # If OpenAPI schema is requested, don't check for superuser - return False - user = view.get_object() - return user.is_superuser - - def is_current_user(self, request, view, action): - if getattr(self, "swagger_fake_view", False): - # If OpenAPI schema is requested, don't check for current user - return False - return request.user == view.get_object() - class MyUserAccessPolicy(AccessPolicyBase): NAME = "MyUserViewSet" diff --git a/galaxy_ng/app/access_control/statements/pulp.py b/galaxy_ng/app/access_control/statements/pulp.py index 42b1878999..cb18b3629a 100644 --- a/galaxy_ng/app/access_control/statements/pulp.py +++ b/galaxy_ng/app/access_control/statements/pulp.py @@ -1,8 +1,10 @@ from galaxy_ng.app.access_control.statements.standalone import ( _collection_statements as _galaxy_collection_statements, _group_statements as _galaxy_group_statements, + _group_role_statements as _galaxy_group_role_statements ) +from galaxy_ng.app.access_control.statements.standalone import _user_statements _collection_statements = {"statements": _galaxy_collection_statements} @@ -10,6 +12,9 @@ _group_statements = {"statements": _galaxy_group_statements} +_group_role_statements = {"statements": _galaxy_group_role_statements} + + _deny_all = { "statements": [ {"principal": "*", "action": "*", "effect": "deny"}, @@ -565,8 +570,29 @@ PULP_CORE_VIEWSETS = { - "groups/roles": _group_statements, + "groups/roles": _group_role_statements, "groups": _group_statements, + "groups/users": {"statements": [ + # We didn't have an access policy here before 4.10. The default pulp access policy + # checks core.group permissions, rather than galaxy.group permissions, which isn't + # used in our system. The end result should be that only admins can make modifications + # on this endpoint. This should be changed to match the validation we use for the + # ui apis (https://github.com/ansible/galaxy_ng/blob/7e6b335326fd1d1f366e3c5dd81b3f6e + # 75da9e1e/galaxy_ng/app/api/ui/serializers/user.py#L62), but given that we're looking + # at adopting DAB RBAC, I'm going to leave this as is for now. + { + "action": "*", + "principal": "admin", + "effect": "allow" + }, + { + "action": ["create", "destroy"], + "principal": "*", + "effect": "deny", + "condition": "is_direct_shared_resource_management_disabled" + }, + ]}, + "users": {"statements": _user_statements}, "roles": { "statements": [ { diff --git a/galaxy_ng/app/access_control/statements/standalone.py b/galaxy_ng/app/access_control/statements/standalone.py index c77a7ada3a..8e4cb1ea11 100644 --- a/galaxy_ng/app/access_control/statements/standalone.py +++ b/galaxy_ng/app/access_control/statements/standalone.py @@ -76,7 +76,7 @@ } ] -_group_statements = [ +_group_role_statements = [ { "action": ["list", "retrieve"], "principal": "authenticated", @@ -86,22 +86,87 @@ "action": "destroy", "principal": "authenticated", "effect": "allow", - "condition": "has_model_perms:galaxy.delete_group" + "condition": [ + "has_model_perms:galaxy.delete_group", + ] }, { "action": "create", "principal": "authenticated", "effect": "allow", - "condition": "has_model_perms:galaxy.add_group" + "condition": [ + "has_model_perms:galaxy.add_group", + ] }, { "action": ["update", "partial_update"], "principal": "authenticated", "effect": "allow", - "condition": "has_model_perms:galaxy.update_group" + "condition": [ + "has_model_perms:galaxy.update_group", + ] }, ] +_group_statements = _group_role_statements + [ + { + "action": ["create", "destroy", "update", "partial_update"], + "principal": "*", + "effect": "deny", + "condition": "is_direct_shared_resource_management_disabled" + }, +] + +_user_statements = [ + { + "action": ["list"], + "principal": "authenticated", + "effect": "allow", + "condition": ["v3_can_view_users"], + }, + { + "action": ["retrieve"], + "principal": "authenticated", + "effect": "allow", + "condition": ["v3_can_view_users"], + }, + { + "action": "destroy", + "principal": "*", + "effect": "deny", + "condition": ["user_is_superuser"] + }, + { + "action": "destroy", + "principal": "*", + "effect": "deny", + "condition": ["is_current_user"] + }, + { + "action": "destroy", + "principal": "*", + "effect": "allow", + "condition": "has_model_perms:galaxy.delete_user" + }, + { + "action": "create", + "principal": "authenticated", + "effect": "allow", + "condition": "has_model_perms:galaxy.add_user" + }, + { + "action": ["update", "partial_update"], + "principal": "authenticated", + "effect": "allow", + "condition": "has_model_perms:galaxy.change_user" + }, + { + "action": ["create", "destroy", "update", "partial_update"], + "principal": "*", + "effect": "deny", + "condition": "is_direct_shared_resource_management_disabled" + }, +] _deny_all = [ { "principal": "*", @@ -187,50 +252,7 @@ "condition": "has_model_perms:ansible.change_collectionremote" } ], - 'UserViewSet': [ - { - "action": ["list"], - "principal": "authenticated", - "effect": "allow", - "condition": ["v3_can_view_users"], - }, - { - "action": ["retrieve"], - "principal": "authenticated", - "effect": "allow", - "condition": ["v3_can_view_users"], - }, - { - "action": "destroy", - "principal": "*", - "effect": "deny", - "condition": ["user_is_superuser"] - }, - { - "action": "destroy", - "principal": "*", - "effect": "deny", - "condition": ["is_current_user"] - }, - { - "action": "destroy", - "principal": "*", - "effect": "allow", - "condition": "has_model_perms:galaxy.delete_user" - }, - { - "action": "create", - "principal": "authenticated", - "effect": "allow", - "condition": "has_model_perms:galaxy.add_user" - }, - { - "action": ["update", "partial_update"], - "principal": "authenticated", - "effect": "allow", - "condition": "has_model_perms:galaxy.change_user" - }, - ], + 'UserViewSet': _user_statements, 'MyUserViewSet': [ { "action": ["retrieve"], @@ -244,6 +266,12 @@ "effect": "allow", "condition": "is_current_user" }, + { + "action": ["create", "destroy", "update", "partial_update"], + "principal": "*", + "effect": "deny", + "condition": "is_direct_shared_resource_management_disabled" + }, ], # disable synclists for on prem installations 'SyncListViewSet': _deny_all, diff --git a/galaxy_ng/app/settings.py b/galaxy_ng/app/settings.py index b700c41c3c..e63566cca9 100644 --- a/galaxy_ng/app/settings.py +++ b/galaxy_ng/app/settings.py @@ -77,7 +77,7 @@ # Galaxy authentication classes are used to set REST_FRAMEWORK__DEFAULT_AUTHENTICATION_CLASSES GALAXY_AUTHENTICATION_CLASSES = [ - "galaxy_ng.app.auth.session.SessionAuthentication", + "rest_framework.authentication.SessionAuthentication", "rest_framework.authentication.TokenAuthentication", "rest_framework.authentication.BasicAuthentication", "ansible_base.jwt_consumer.hub.auth.HubJWTAuth", @@ -308,3 +308,6 @@ # WARNING: This setting is used in database migrations to create a default organization. DEFAULT_ORGANIZATION_NAME = "Default" + +# Disables editing and managing users and groups. +DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED = True diff --git a/galaxy_ng/tests/integration/api/test_disable_shared_resources.py b/galaxy_ng/tests/integration/api/test_disable_shared_resources.py new file mode 100644 index 0000000000..021e44e057 --- /dev/null +++ b/galaxy_ng/tests/integration/api/test_disable_shared_resources.py @@ -0,0 +1,148 @@ +import os +import pytest +from galaxykit.utils import GalaxyClientError +import uuid + + +@pytest.fixture +def test_group(galaxy_client): + gc = galaxy_client("admin") + + return gc.get("_ui/v1/groups/?name=ns_group_for_tests")["data"][0] + + +@pytest.fixture +def test_user(galaxy_client): + gc = galaxy_client("admin") + + return gc.get("_ui/v1/users/?username=admin")["data"][0] + + +@pytest.mark.parametrize( + 'url', + [ + "_ui/v1/groups/", + "pulp/api/v3/groups/", + ] +) +@pytest.mark.deployment_standalone +@pytest.mark.skipif( + not os.getenv("ENABLE_DAB_TESTS"), + reason="Skipping test because ENABLE_DAB_TESTS is not set" +) +def test_dab_groups_are_read_only(galaxy_client, url, test_group): + gc = galaxy_client("admin") + + group_pk = test_group["id"] + + with pytest.raises(GalaxyClientError) as ctx: + gc.post(url, body={"name": str(uuid.uuid4())}) + + assert ctx.value.args[0] == 403 + + # Apparently we don't support updating on the ui api? + if "pulp/api" in url: + detail = url + f"{group_pk}/" + with pytest.raises(GalaxyClientError) as ctx: + gc.patch(detail, body={"name": str(uuid.uuid4())}) + + assert ctx.value.args[0] == 403 + + detail = url + f"{group_pk}/" + with pytest.raises(GalaxyClientError) as ctx: + gc.put(detail, body={"name": str(uuid.uuid4())}) + + assert ctx.value.args[0] == 403 + + detail = url + f"{group_pk}/" + with pytest.raises(GalaxyClientError) as ctx: + gc.delete(detail) + + assert ctx.value.args[0] == 403 + + +@pytest.mark.parametrize( + 'url', + [ + "_ui/v1/users/", + "pulp/api/v3/users/", + ] +) +@pytest.mark.deployment_standalone +@pytest.mark.skipif( + not os.getenv("ENABLE_DAB_TESTS"), + reason="Skipping test because ENABLE_DAB_TESTS is not set" +) +def test_dab_users_are_read_only(galaxy_client, url, test_user): + gc = galaxy_client("admin") + + user_pk = test_user["id"] + + with pytest.raises(GalaxyClientError) as ctx: + gc.post(url, body={"username": str(uuid.uuid4())}) + + assert ctx.value.args[0] == 403 + + detail = url + f"{user_pk}/" + with pytest.raises(GalaxyClientError) as ctx: + gc.patch(detail, body={"username": str(uuid.uuid4())}) + + assert ctx.value.args[0] == 403 + + detail = url + f"{user_pk}/" + with pytest.raises(GalaxyClientError) as ctx: + gc.put(detail, body={"username": str(uuid.uuid4())}) + + assert ctx.value.args[0] == 403 + + detail = url + f"{user_pk}/" + with pytest.raises(GalaxyClientError) as ctx: + gc.delete(detail) + + assert ctx.value.args[0] == 403 + + +@pytest.mark.deployment_standalone +@pytest.mark.skipif( + not os.getenv("ENABLE_DAB_TESTS"), + reason="Skipping test because ENABLE_DAB_TESTS is not set" +) +def test_dab_cant_modify_group_memberships(galaxy_client, test_user, test_group): + gc = galaxy_client("admin") + + hub_user_detail = f"_ui/v1/users/{test_user['id']}/" + with pytest.raises(GalaxyClientError) as ctx: + gc.patch(hub_user_detail, body={ + "groups": [{ + "id": test_group["id"], + "name": test_group["name"], + }] + }) + + assert ctx.value.args[0] == 403 + + pulp_group_users = f"pulp/api/v3/groups/{test_group['id']}/users/" + + with pytest.raises(GalaxyClientError) as ctx: + gc.post(pulp_group_users, body={"username": test_user["username"]}) + + assert ctx.value.args[0] == 403 + + +@pytest.mark.deployment_standalone +@pytest.mark.skipif( + not os.getenv("ENABLE_DAB_TESTS"), + reason="Skipping test because ENABLE_DAB_TESTS is not set" +) +def test_dab_can_modify_roles(galaxy_client, test_user, test_group): + gc = galaxy_client("admin") + + gc.post(f"pulp/api/v3/groups/{test_group['id']}/roles/", body={ + "content_object": None, + "role": "galaxy.content_admin", + }) + + gc.post(f"pulp/api/v3/users/{test_user['id']}/roles/", body={ + "content_object": None, + "role": "galaxy.content_admin", + }) diff --git a/profiles/dab/pulp_config.env b/profiles/dab/pulp_config.env index 99482d7ae0..334e029644 100644 --- a/profiles/dab/pulp_config.env +++ b/profiles/dab/pulp_config.env @@ -19,3 +19,7 @@ PULP_GALAXY_CONTAINER_SIGNING_SERVICE=container-default # dynamic download urls PULP_DYNACONF_AFTER_GET_HOOKS=["read_settings_from_cache_or_db", "alter_hostname_settings"] +PULP_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED=false + +# disable user/group modifications +PULP_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED=false diff --git a/profiles/dab_jwt/pulp_config.env b/profiles/dab_jwt/pulp_config.env index ea987b2885..3f663864f8 100644 --- a/profiles/dab_jwt/pulp_config.env +++ b/profiles/dab_jwt/pulp_config.env @@ -19,3 +19,6 @@ PULP_GALAXY_CONTAINER_SIGNING_SERVICE=container-default # dynamic download urls PULP_DYNACONF_AFTER_GET_HOOKS=["read_settings_from_cache_or_db", "alter_hostname_settings"] + +# disable user/group modifications +PULP_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED=false From 216f71dc6727d40dbe7d3749012cf57106b61d37 Mon Sep 17 00:00:00 2001 From: David Newswanger Date: Thu, 6 Jun 2024 09:55:59 -0600 Subject: [PATCH 2/4] fix tests and ui --- galaxy_ng/app/api/ui/serializers/user.py | 3 +- .../tests/integration/api/test_groups.py | 5 + .../integration/api/test_ui_paths_gateway.py | 98 ------------------- 3 files changed, 7 insertions(+), 99 deletions(-) diff --git a/galaxy_ng/app/api/ui/serializers/user.py b/galaxy_ng/app/api/ui/serializers/user.py index 97d99d08ae..5607e20c6d 100644 --- a/galaxy_ng/app/api/ui/serializers/user.py +++ b/galaxy_ng/app/api/ui/serializers/user.py @@ -175,10 +175,11 @@ class Meta(UserSerializer.Meta): @extend_schema_field(OpenApiTypes.OBJECT) def get_model_permissions(self, obj): permissions = {} + allow_group_user_edits = settings.get("DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED", True) for i, j in PERMISSIONS.items(): permissions[i] = j permissions[i]["has_model_permission"] = obj.has_perm(i) - if settings.get("SOCIAL_AUTH_KEYCLOAK_KEY"): + if settings.get("SOCIAL_AUTH_KEYCLOAK_KEY") or not allow_group_user_edits: permissions["galaxy.delete_user"]['has_model_permission'] = False permissions["galaxy.change_user"]['has_model_permission'] = False permissions["galaxy.add_user"]['has_model_permission'] = False diff --git a/galaxy_ng/tests/integration/api/test_groups.py b/galaxy_ng/tests/integration/api/test_groups.py index ba9a9b5e08..6aa0030a14 100644 --- a/galaxy_ng/tests/integration/api/test_groups.py +++ b/galaxy_ng/tests/integration/api/test_groups.py @@ -6,6 +6,7 @@ import random import string import uuid +import os import pytest from requests import HTTPError @@ -34,6 +35,10 @@ @pytest.mark.pulp_api @pytest.mark.deployment_standalone @pytest.mark.min_hub_version("4.6dev") +@pytest.mark.skipif( + os.getenv("ENABLE_DAB_TESTS"), + reason="Group creation is disabled in the DAB test profile." +) def test_gw_group_role_listing(galaxy_client, test_data): """Tests ability to list roles assigned to a namespace.""" diff --git a/galaxy_ng/tests/integration/api/test_ui_paths_gateway.py b/galaxy_ng/tests/integration/api/test_ui_paths_gateway.py index 77e1c7e91b..a7e3bb0239 100644 --- a/galaxy_ng/tests/integration/api/test_ui_paths_gateway.py +++ b/galaxy_ng/tests/integration/api/test_ui_paths_gateway.py @@ -246,28 +246,6 @@ def test_gw_api_ui_v1_feature_flags(galaxy_client): assert ds['legacy_roles'] is False -@pytest.mark.deployment_standalone -@pytest.mark.api_ui -@pytest.mark.skipif(not aap_gateway(), reason="This test only runs if AAP Gateway is deployed") -def test_gw_api_ui_v1_groups(galaxy_client): - - gc = galaxy_client('partner_engineer') - # get the response - ds = gc.get('_ui/v1/groups/') - validate_json(instance=ds, schema=schema_objectlist) - - for grp in ds['data']: - validate_json(instance=grp, schema=schema_group) - - # try to make a group - suffix = random.choice(range(0, 1000)) - payload = {'name': f'foobar{suffix}'} - ds = gc.post('_ui/v1/groups/', body=payload) - validate_json(instance=ds, schema=schema_group) - assert ds['name'] == payload['name'] - assert ds['pulp_href'].endswith(f"/{ds['id']}/") - - @pytest.mark.deployment_standalone @pytest.mark.api_ui @pytest.mark.skipif(not aap_gateway(), reason="This test only runs if AAP Gateway is deployed") @@ -291,51 +269,6 @@ def test_gw_api_ui_v1_groups_users(galaxy_client): assert "jdoe" in [x["username"] for x in users_ds["data"]] -@pytest.mark.deployment_standalone -@pytest.mark.api_ui -@pytest.mark.skipif(not aap_gateway(), reason="This test only runs if AAP Gateway is deployed") -def test_gw_api_ui_v1_groups_users_add_delete(galaxy_client): - - gc = galaxy_client('partner_engineer') - suffix = random.choice(range(0, 1000)) - group_name = f'group{suffix}' - user_name = f'user{suffix}' - - # make the group - group_ds = gc.post('_ui/v1/groups/', body={'name': group_name}) - validate_json(instance=group_ds, schema=schema_group) - group_id = group_ds['id'] - - # make the user - user_ds = gc.post( - '_ui/v1/users/', - body={ - 'username': user_name, - 'first_name': 'foo', - 'last_name': 'bar', - 'email': 'foo@barz.com', - 'groups': [group_ds], - 'password': 'abcdefghijklmnopqrstuvwxyz1234567890!@#$%^&*()-+', - 'is_superuser': False - } - ) - validate_json(instance=user_ds, schema=schema_user) - - # validate the new user is in the group's userlist - users_ds = gc.get(f'_ui/v1/groups/{group_id}/users/') - validate_json(instance=users_ds, schema=schema_objectlist) - assert user_name in [x['username'] for x in users_ds['data']] - - # remove the user from the group - user_id = user_ds['id'] - gc.delete(f'_ui/v1/groups/{group_id}/users/{user_id}/', parse_json=False) - - # validate the new user is NOT in the group's userlist - users_ds = gc.get(f'_ui/v1/groups/{group_id}/users/') - validate_json(instance=users_ds, schema=schema_objectlist) - assert user_name not in [x['username'] for x in users_ds['data']] - - @pytest.mark.deployment_standalone @pytest.mark.api_ui @pytest.mark.skipif(not aap_gateway(), reason="This test only runs if AAP Gateway is deployed") @@ -545,37 +478,6 @@ def test_gw_api_ui_v1_tags(galaxy_client): # FIXME - ui tags api does not support POST? -@pytest.mark.deployment_standalone -@pytest.mark.api_ui -@pytest.mark.skipif(not aap_gateway(), reason="This test only runs if AAP Gateway is deployed") -def test_gw_api_ui_v1_users(galaxy_client): - gc = galaxy_client('partner_engineer') - # get the response - ds = gc.get('_ui/v1/users/') - validate_json(instance=ds, schema=schema_objectlist) - - assert len(ds['data']) >= 1 - for user in ds['data']: - validate_json(instance=user, schema=schema_user) - - # try creating a user - suffix = random.choice(range(0, 9999)) - payload = { - 'username': f'foobar{suffix}', - 'first_name': 'foobar', - 'last_name': f'{suffix}' - } - ds = gc.post('_ui/v1/users/', body=payload) - validate_json(instance=ds, schema=schema_user) - - # should NOT be superuser by default - assert not ds['is_superuser'] - - assert ds['username'] == payload['username'] - assert ds['first_name'] == payload['first_name'] - assert ds['last_name'] == payload['last_name'] - - @pytest.mark.deployment_standalone @pytest.mark.api_ui @pytest.mark.skipif(not aap_gateway(), reason="This test only runs if AAP Gateway is deployed") From 3708e63b34240a7f1c09194dbe57f18868ddcad6 Mon Sep 17 00:00:00 2001 From: David Newswanger Date: Thu, 6 Jun 2024 10:09:50 -0600 Subject: [PATCH 3/4] fix auth class blunder --- galaxy_ng/app/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/galaxy_ng/app/settings.py b/galaxy_ng/app/settings.py index e63566cca9..74ae10563e 100644 --- a/galaxy_ng/app/settings.py +++ b/galaxy_ng/app/settings.py @@ -77,7 +77,7 @@ # Galaxy authentication classes are used to set REST_FRAMEWORK__DEFAULT_AUTHENTICATION_CLASSES GALAXY_AUTHENTICATION_CLASSES = [ - "rest_framework.authentication.SessionAuthentication", + "galaxy_ng.app.auth.session.SessionAuthentication", "rest_framework.authentication.TokenAuthentication", "rest_framework.authentication.BasicAuthentication", "ansible_base.jwt_consumer.hub.auth.HubJWTAuth", From 34683a2ee68d92d0b3d6decb4dc6cafaeeca0469 Mon Sep 17 00:00:00 2001 From: David Newswanger Date: Mon, 10 Jun 2024 09:33:47 -0600 Subject: [PATCH 4/4] remove duplicate field --- profiles/dab/pulp_config.env | 1 - 1 file changed, 1 deletion(-) diff --git a/profiles/dab/pulp_config.env b/profiles/dab/pulp_config.env index 334e029644..1a5bddcd99 100644 --- a/profiles/dab/pulp_config.env +++ b/profiles/dab/pulp_config.env @@ -19,7 +19,6 @@ PULP_GALAXY_CONTAINER_SIGNING_SERVICE=container-default # dynamic download urls PULP_DYNACONF_AFTER_GET_HOOKS=["read_settings_from_cache_or_db", "alter_hostname_settings"] -PULP_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED=false # disable user/group modifications PULP_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED=false