From 5c7249297b5dc6eb87acefb04005090159f11e45 Mon Sep 17 00:00:00 2001 From: Daniel Rodowicz Date: Thu, 12 Oct 2023 10:15:59 -0400 Subject: [PATCH 1/9] set proper CONTENT_ORIGIN for ephemeral pr checks (#1932) Issue: AAH-2775 --- CHANGES/2775.misc | 1 + dev/ephemeral/patch_ephemeral.sh | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 CHANGES/2775.misc diff --git a/CHANGES/2775.misc b/CHANGES/2775.misc new file mode 100644 index 0000000000..93947d835b --- /dev/null +++ b/CHANGES/2775.misc @@ -0,0 +1 @@ +set proper CONTENT_ORIGIN for ephemeral pr checks diff --git a/dev/ephemeral/patch_ephemeral.sh b/dev/ephemeral/patch_ephemeral.sh index 0839f535da..4a2aa16b78 100755 --- a/dev/ephemeral/patch_ephemeral.sh +++ b/dev/ephemeral/patch_ephemeral.sh @@ -5,9 +5,14 @@ FE_ROUTE=$(oc get routes | awk '{print $1}' | grep front-end-aggregator) if [ -n "${FE_ROUTE}" ]; then CONTENT_ORIGIN=$(oc get route front-end-aggregator -o jsonpath='https://{.spec.host}{"\n"}') else - CONTENT_ORIGIN=$(bonfire namespace describe ${NAMESPACE} | grep "Frontend route" | awk '{print $3}') + CONTENT_ORIGIN=$(bonfire namespace describe ${NAMESPACE} | grep "Gateway route" | awk '{print $3}') +fi +if [ -z "${CONTENT_ORIGIN}" ]; then + echo "ERROR: unable to determine CONTENT_ORIGIN" + exit 1 +else + echo "CONTENT_ORIGIN = ${CONTENT_ORIGIN}" fi -echo "CONTENT_ORIGIN = ${CONTENT_ORIGIN}" oc patch clowdapp automation-hub --type=json \ -p '[{"op": "replace", "path": "/spec/deployments/1/podSpec/env/1/value", From 6c7b3f999fb55d2dd8134a0dcdae19b6cfaf6919 Mon Sep 17 00:00:00 2001 From: jctanner Date: Thu, 12 Oct 2023 12:32:00 -0400 Subject: [PATCH 2/9] Enable using users in addition to groups for RBAC. (#1899) No-Issue Signed-off-by: James Tanner --- galaxy_ng/app/access_control/fields.py | 46 ++++++++ galaxy_ng/app/access_control/mixins.py | 51 +++++++++ galaxy_ng/app/api/v3/serializers/namespace.py | 11 +- galaxy_ng/app/models/namespace.py | 6 +- .../api/test_namespace_management.py | 105 ++++++++++++++++++ requirements/requirements.common.txt | 4 +- requirements/requirements.insights.txt | 4 +- requirements/requirements.standalone.txt | 4 +- setup.py | 2 +- 9 files changed, 223 insertions(+), 10 deletions(-) diff --git a/galaxy_ng/app/access_control/fields.py b/galaxy_ng/app/access_control/fields.py index d898dccf9c..1404deb850 100644 --- a/galaxy_ng/app/access_control/fields.py +++ b/galaxy_ng/app/access_control/fields.py @@ -1,4 +1,5 @@ from django.utils.translation import gettext_lazy as _ +from django.contrib.auth import get_user_model from rest_framework import serializers from rest_framework.exceptions import ValidationError @@ -10,6 +11,9 @@ from galaxy_ng.app.models import auth as auth_models +User = get_user_model() + + class GroupPermissionField(serializers.Field): def _validate_group(self, group_data): if 'object_roles' not in group_data: @@ -74,6 +78,48 @@ def to_internal_value(self, data): return internal +class UserPermissionField(serializers.Field): + + def _validate_user(self, user_data): + # FIXME - fill this in ... + pass + + def to_representation(self, value): + rep = [] + for user in value: + rep.append({ + 'id': user.id, + 'name': user.username, + 'object_roles': value[user] + }) + return rep + + def to_internal_value(self, data): + if not isinstance(data, list): + raise ValidationError(detail={ + 'users': _('Users must be a list of user objects') + }) + + internal = {} + for user_data in data: + self._validate_user(user_data) + user_filter = {} + for field in user_data: + if field in ('id', 'username'): + user_filter[field] = user_data[field] + + user = User.objects.filter(**user_filter).first() + if not user: + raise ValidationError(detail={'user': _('Invalid user username or ID')}) + + if 'object_permissions' in user_data: + internal[user] = user_data['object_permissions'] + if 'object_roles' in user_data: + internal[user] = user_data['object_roles'] + + return internal + + class MyPermissionsField(serializers.Serializer): def to_representation(self, original_obj): request = self.context.get('request', None) diff --git a/galaxy_ng/app/access_control/mixins.py b/galaxy_ng/app/access_control/mixins.py index 52e37a01e8..49c3582d60 100644 --- a/galaxy_ng/app/access_control/mixins.py +++ b/galaxy_ng/app/access_control/mixins.py @@ -8,6 +8,7 @@ assign_role, remove_role, get_groups_with_perms_attached_roles, + get_users_with_perms_attached_roles, ) from django_lifecycle import hook @@ -61,3 +62,53 @@ def _set_groups(self, groups): def set_object_groups(self): if self._groups: self._set_groups(self._groups) + + +class UserModelPermissionsMixin: + _users = None + + @property + def users(self): + return get_users_with_perms_attached_roles( + self, include_model_permissions=False, for_concrete_model=True) + + @users.setter + def users(self, users): + self._set_users(users) + + @transaction.atomic + def _set_users(self, users): + if self._state.adding: + self._users = users + else: + obj = self + + # If the model is a proxy model, get the original model since pulp + # doesn't allow us to assign permissions to proxied models. + if self._meta.proxy: + obj = self._meta.concrete_model.objects.get(pk=self.pk) + + current_users = get_users_with_perms_attached_roles( + obj, include_model_permissions=False) + for user in current_users: + for perm in current_users[user]: + remove_role(perm, user, obj) + + for user in users: + for role in users[user]: + try: + assign_role(role, user, obj) + except BadRequest: + raise ValidationError( + detail={ + 'users': _( + 'Role {role} does not exist or does not ' + 'have any permissions related to this object.' + ).format(role=role) + } + ) + + @hook('after_save') + def set_object_users(self): + if self._users: + self._set_users(self._users) diff --git a/galaxy_ng/app/api/v3/serializers/namespace.py b/galaxy_ng/app/api/v3/serializers/namespace.py index 0742d3debc..d76c3ba335 100644 --- a/galaxy_ng/app/api/v3/serializers/namespace.py +++ b/galaxy_ng/app/api/v3/serializers/namespace.py @@ -13,7 +13,11 @@ from galaxy_ng.app import models from galaxy_ng.app.tasks import dispatch_create_pulp_namespace_metadata -from galaxy_ng.app.access_control.fields import GroupPermissionField, MyPermissionsField +from galaxy_ng.app.access_control.fields import ( + GroupPermissionField, + UserPermissionField, + MyPermissionsField +) from galaxy_ng.app.api.base import RelatedFieldsBaseSerializer log = logging.getLogger(__name__) @@ -73,7 +77,8 @@ def validate_url(self, url): class NamespaceSerializer(serializers.ModelSerializer): links = NamespaceLinkSerializer(many=True, required=False) - groups = GroupPermissionField() + groups = GroupPermissionField(required=False) + users = UserPermissionField(required=False) related_fields = NamespaceRelatedFieldSerializer(source="*") avatar_url = fields.URLField(required=False, allow_blank=True) avatar_sha256 = serializers.SerializerMethodField() @@ -93,6 +98,7 @@ class Meta: 'description', 'links', 'groups', + 'users', 'resources', 'related_fields', 'metadata_sha256', @@ -178,6 +184,7 @@ class Meta: 'avatar_url', 'description', 'groups', + 'users', 'related_fields', 'metadata_sha256', 'avatar_sha256' diff --git a/galaxy_ng/app/models/namespace.py b/galaxy_ng/app/models/namespace.py index 2b7dbdcd38..b724d0d347 100644 --- a/galaxy_ng/app/models/namespace.py +++ b/galaxy_ng/app/models/namespace.py @@ -16,7 +16,11 @@ __all__ = ("Namespace", "NamespaceLink") -class Namespace(LifecycleModel, mixins.GroupModelPermissionsMixin): +class Namespace( + LifecycleModel, + mixins.GroupModelPermissionsMixin, + mixins.UserModelPermissionsMixin +): """ A model representing Ansible content namespace. diff --git a/galaxy_ng/tests/integration/api/test_namespace_management.py b/galaxy_ng/tests/integration/api/test_namespace_management.py index 7405af12ec..f47cf9ccb9 100644 --- a/galaxy_ng/tests/integration/api/test_namespace_management.py +++ b/galaxy_ng/tests/integration/api/test_namespace_management.py @@ -56,3 +56,108 @@ def test_namespace_create_and_delete(ansible_config, api_version): existing3 = get_all_namespaces(api_client=api_client, api_version=api_version) existing3 = dict((x['name'], x) for x in existing3) assert new_namespace not in existing3 + + +@pytest.mark.galaxyapi_smoke +@pytest.mark.namespace +@pytest.mark.all +@pytest.mark.parametrize( + "user_property", + [ + 'id', + 'username' + ] +) +def test_namespace_create_with_user(ansible_config, user_property): + config = ansible_config("partner_engineer") + api_client = get_client(config, request_token=True, require_auth=True) + api_prefix = config.get("api_prefix").rstrip("/") + + # find this client's user info... + me = api_client(f'{api_prefix}/_ui/v1/me/') + username = me['username'] + + new_namespace = generate_unused_namespace(api_client=api_client) + + # make a namespace with a user and without defining groups ... + object_roles = [ + 'galaxy.collection_namespace_owner', + 'galaxy.collection_publisher' + ] + payload = { + 'name': new_namespace, + 'users': [ + { + user_property: me.get(user_property), + 'object_roles': object_roles, + } + ] + } + resp = api_client(f'{api_prefix}/_ui/v1/my-namespaces/', args=payload, method='POST') + + # should have the right results ... + assert resp['name'] == new_namespace + assert resp['groups'] == [] + assert resp['users'] != [] + assert username in [x['name'] for x in resp['users']] + assert sorted(resp['users'][0]['object_roles']) == sorted(object_roles) + + +@pytest.mark.galaxyapi_smoke +@pytest.mark.namespace +@pytest.mark.all +@pytest.mark.parametrize( + "user_property", + [ + 'id', + 'username' + ] +) +def test_namespace_edit_with_user(ansible_config, user_property): + config = ansible_config("partner_engineer") + api_client = get_client(config, request_token=True, require_auth=True) + api_prefix = config.get("api_prefix").rstrip("/") + + # find this client's user info... + me = api_client(f'{api_prefix}/_ui/v1/me/') + username = me['username'] + + new_namespace = generate_unused_namespace(api_client=api_client) + + # make a namespace without users and without groups ... + payload = { + 'name': new_namespace, + } + resp = api_client(f'{api_prefix}/_ui/v1/my-namespaces/', args=payload, method='POST') + + # should have the right results ... + assert resp['name'] == new_namespace + assert resp['groups'] == [] + assert resp['users'] == [] + + # now edit the namespace to add the user + object_roles = [ + 'galaxy.collection_namespace_owner', + 'galaxy.collection_publisher' + ] + payload = { + 'name': new_namespace, + 'users': [ + { + user_property: me.get(user_property), + 'object_roles': object_roles, + } + ] + } + resp = api_client( + f'{api_prefix}/_ui/v1/my-namespaces/{new_namespace}/', + args=payload, + method='PUT' + ) + + # should have the right results ... + assert resp['name'] == new_namespace + assert resp['groups'] == [] + assert resp['users'] != [] + assert username in [x['name'] for x in resp['users']] + assert sorted(resp['users'][0]['object_roles']) == sorted(object_roles) diff --git a/requirements/requirements.common.txt b/requirements/requirements.common.txt index f8368aff01..bb1a67807f 100644 --- a/requirements/requirements.common.txt +++ b/requirements/requirements.common.txt @@ -314,13 +314,13 @@ psycopg[binary]==3.1.9 # via pulpcore psycopg-binary==3.1.9 # via psycopg -pulp-ansible==0.19.0 +pulp-ansible==0.20.1 # via galaxy-ng (setup.py) pulp-container==2.15.2 # via galaxy-ng (setup.py) pulp-glue==0.19.5 # via pulpcore -pulpcore==3.28.12 +pulpcore==3.28.17 # via # galaxy-ng (setup.py) # pulp-ansible diff --git a/requirements/requirements.insights.txt b/requirements/requirements.insights.txt index 56f3dec035..a70be64fd8 100644 --- a/requirements/requirements.insights.txt +++ b/requirements/requirements.insights.txt @@ -325,13 +325,13 @@ psycopg[binary]==3.1.9 # via pulpcore psycopg-binary==3.1.9 # via psycopg -pulp-ansible==0.19.0 +pulp-ansible==0.20.1 # via galaxy-ng (setup.py) pulp-container==2.15.2 # via galaxy-ng (setup.py) pulp-glue==0.19.5 # via pulpcore -pulpcore==3.28.12 +pulpcore==3.28.17 # via # galaxy-ng (setup.py) # pulp-ansible diff --git a/requirements/requirements.standalone.txt b/requirements/requirements.standalone.txt index 90fe78935d..d0739f5e60 100644 --- a/requirements/requirements.standalone.txt +++ b/requirements/requirements.standalone.txt @@ -314,13 +314,13 @@ psycopg[binary]==3.1.9 # via pulpcore psycopg-binary==3.1.9 # via psycopg -pulp-ansible==0.19.0 +pulp-ansible==0.20.1 # via galaxy-ng (setup.py) pulp-container==2.15.2 # via galaxy-ng (setup.py) pulp-glue==0.19.5 # via pulpcore -pulpcore==3.28.12 +pulpcore==3.28.17 # via # galaxy-ng (setup.py) # pulp-ansible diff --git a/setup.py b/setup.py index c7d0658bb3..ecfe74af94 100644 --- a/setup.py +++ b/setup.py @@ -113,7 +113,7 @@ def _format_pulp_requirement(plugin, specifier=None, ref=None, gh_namespace="pul requirements = [ "galaxy-importer>=0.4.13,<0.5.0", "pulpcore>=3.28.12,<3.29.0", - "pulp_ansible>=0.19.0,<0.20.0", + "pulp_ansible>=0.20.0,<0.21.0", "django-prometheus>=2.0.0", "drf-spectacular", "pulp-container>=2.15.0,<2.16.0", From 9f1cb9daddcdc6ab8d6a6939e67d422c311810f1 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 12 Oct 2023 20:04:54 +0200 Subject: [PATCH 3/9] nginx: add entry for /api, use client_max_body_size (#1924) No-Issue --- galaxy_ng/app/webserver_snippets/nginx.conf | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/galaxy_ng/app/webserver_snippets/nginx.conf b/galaxy_ng/app/webserver_snippets/nginx.conf index 05f6e57647..1a9011cd50 100644 --- a/galaxy_ng/app/webserver_snippets/nginx.conf +++ b/galaxy_ng/app/webserver_snippets/nginx.conf @@ -8,3 +8,14 @@ location /ui/ { proxy_redirect off; proxy_pass http://pulp-api/static/galaxy_ng/index.html; } + +location /api/ { + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header Host $http_host; + # we don't want nginx trying to do something clever with + # redirects, we set the Host: header above already. + proxy_redirect off; + proxy_pass http://pulp-api; + client_max_body_size 0; +} From 82fde1758eaad9ffa42f1a09cf29a27470292b39 Mon Sep 17 00:00:00 2001 From: jctanner Date: Thu, 12 Oct 2023 14:09:32 -0400 Subject: [PATCH 4/9] Bump galaxy-importer to 0.4.14 (#1930) No-Issue Signed-off-by: James Tanner --- requirements/requirements.common.txt | 2 +- requirements/requirements.insights.txt | 2 +- requirements/requirements.standalone.txt | 2 +- setup.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/requirements.common.txt b/requirements/requirements.common.txt index bb1a67807f..aa6ba2d399 100644 --- a/requirements/requirements.common.txt +++ b/requirements/requirements.common.txt @@ -154,7 +154,7 @@ frozenlist==1.4.0 # aiosignal future==0.18.3 # via pyjwkest -galaxy-importer==0.4.13 +galaxy-importer==0.4.14 # via # galaxy-ng (setup.py) # pulp-ansible diff --git a/requirements/requirements.insights.txt b/requirements/requirements.insights.txt index a70be64fd8..2e2cd3b320 100644 --- a/requirements/requirements.insights.txt +++ b/requirements/requirements.insights.txt @@ -163,7 +163,7 @@ frozenlist==1.4.0 # aiosignal future==0.18.3 # via pyjwkest -galaxy-importer==0.4.13 +galaxy-importer==0.4.14 # via # galaxy-ng (setup.py) # pulp-ansible diff --git a/requirements/requirements.standalone.txt b/requirements/requirements.standalone.txt index d0739f5e60..be837f5138 100644 --- a/requirements/requirements.standalone.txt +++ b/requirements/requirements.standalone.txt @@ -154,7 +154,7 @@ frozenlist==1.4.0 # aiosignal future==0.18.3 # via pyjwkest -galaxy-importer==0.4.13 +galaxy-importer==0.4.14 # via # galaxy-ng (setup.py) # pulp-ansible diff --git a/setup.py b/setup.py index ecfe74af94..fde3e92e4c 100644 --- a/setup.py +++ b/setup.py @@ -111,7 +111,7 @@ def _format_pulp_requirement(plugin, specifier=None, ref=None, gh_namespace="pul requirements = [ - "galaxy-importer>=0.4.13,<0.5.0", + "galaxy-importer>=0.4.14,<0.5.0", "pulpcore>=3.28.12,<3.29.0", "pulp_ansible>=0.20.0,<0.21.0", "django-prometheus>=2.0.0", From 67bf324c47940aac3a4000d15b1a47fd1c2a52d8 Mon Sep 17 00:00:00 2001 From: jctanner Date: Mon, 16 Oct 2023 13:27:51 -0400 Subject: [PATCH 5/9] Enable social auth users to see other users. (#1934) Add a custom condition function to detect if using github social auth and then always allow user list and retrieve. Issue: AAH-2781 Signed-off-by: James Tanner --- CHANGES/2781.bugfix | 1 + galaxy_ng/app/access_control/access_policy.py | 17 +++++++ .../access_control/statements/standalone.py | 4 +- .../test_community_namespace_rbac.py | 25 +++++++++++ .../unit/api/test_api_ui_user_viewsets.py | 44 ++++++++++++++----- 5 files changed, 77 insertions(+), 14 deletions(-) create mode 100644 CHANGES/2781.bugfix diff --git a/CHANGES/2781.bugfix b/CHANGES/2781.bugfix new file mode 100644 index 0000000000..efee5117af --- /dev/null +++ b/CHANGES/2781.bugfix @@ -0,0 +1 @@ +Allow all authenticated users to list and retrieve other users when using github social auth. diff --git a/galaxy_ng/app/access_control/access_policy.py b/galaxy_ng/app/access_control/access_policy.py index d82013684c..b2d91c4443 100644 --- a/galaxy_ng/app/access_control/access_policy.py +++ b/galaxy_ng/app/access_control/access_policy.py @@ -281,6 +281,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 70e0da14a6..58fb4053eb 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": ["v3_can_view_users"], }, { "action": ["retrieve"], "principal": "authenticated", "effect": "allow", - "condition": "has_model_perms:galaxy.view_user" + "condition": ["v3_can_view_users"], }, { "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 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..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 @@ -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,25 +146,34 @@ 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_403_FORBIDDEN) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.INSIGHTS.value): - _test_user_list() + _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(): - # Check test user cannot 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_403_FORBIDDEN) + self.assertEqual(response.status_code, expected) - # Check test user cannot 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_403_FORBIDDEN) + 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) @@ -175,10 +186,19 @@ 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_403_FORBIDDEN) with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.INSIGHTS.value): - _test_user_get() + _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) From b10dd99441143631bb380b56896e4d9138db88b4 Mon Sep 17 00:00:00 2001 From: christian Date: Mon, 16 Oct 2023 22:31:12 +0200 Subject: [PATCH 6/9] skip test_social_auth_creates_group (#1936) No-Issue --- galaxy_ng/tests/integration/api/test_galaxy_stage_ansible.py | 1 + 1 file changed, 1 insertion(+) diff --git a/galaxy_ng/tests/integration/api/test_galaxy_stage_ansible.py b/galaxy_ng/tests/integration/api/test_galaxy_stage_ansible.py index 6739386426..4017ed974e 100644 --- a/galaxy_ng/tests/integration/api/test_galaxy_stage_ansible.py +++ b/galaxy_ng/tests/integration/api/test_galaxy_stage_ansible.py @@ -87,6 +87,7 @@ def test_me_social_with_precreated_user(galaxy_client): assert uinfo['username'] == gc.username +@pytest.mark.skip(reason='no longer creating groups for social users') @pytest.mark.galaxy_stage_ansible def test_social_auth_creates_group(gh_user_1_pre): github_user_username = GALAXY_STAGE_ANSIBLE_PROFILES["github_user"]["username"] From 67df85772e8af9fd02b0e5f0469bb648a5408033 Mon Sep 17 00:00:00 2001 From: christian Date: Tue, 17 Oct 2023 14:13:51 +0200 Subject: [PATCH 7/9] skip namespace tests if version is lower than 4.9 (#1938) No-Issue --- galaxy_ng/tests/integration/api/test_namespace_management.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/galaxy_ng/tests/integration/api/test_namespace_management.py b/galaxy_ng/tests/integration/api/test_namespace_management.py index f47cf9ccb9..897907231a 100644 --- a/galaxy_ng/tests/integration/api/test_namespace_management.py +++ b/galaxy_ng/tests/integration/api/test_namespace_management.py @@ -68,6 +68,7 @@ def test_namespace_create_and_delete(ansible_config, api_version): 'username' ] ) +@pytest.mark.min_hub_version("4.9") def test_namespace_create_with_user(ansible_config, user_property): config = ansible_config("partner_engineer") api_client = get_client(config, request_token=True, require_auth=True) @@ -113,6 +114,7 @@ def test_namespace_create_with_user(ansible_config, user_property): 'username' ] ) +@pytest.mark.min_hub_version("4.9") def test_namespace_edit_with_user(ansible_config, user_property): config = ansible_config("partner_engineer") api_client = get_client(config, request_token=True, require_auth=True) From 913f7ab00c0486f3ef318f973a94931144fa1326 Mon Sep 17 00:00:00 2001 From: jctanner Date: Tue, 17 Oct 2023 11:47:54 -0400 Subject: [PATCH 8/9] fix social auth namespace creation (again) (#1937) The code was skipping the check of possible emails because github usually returns null for the user's email. No-Issue Signed-off-by: James Tanner --- .../oci_env_configs/community.compose.env | 4 +- galaxy_ng/social/pipeline/user.py | 105 +++++++----------- profiles/community/pulp_config.env | 2 + 3 files changed, 45 insertions(+), 66 deletions(-) diff --git a/dev/oci_env_integration/oci_env_configs/community.compose.env b/dev/oci_env_integration/oci_env_configs/community.compose.env index 0bc2cd7d6b..162c510e55 100644 --- a/dev/oci_env_integration/oci_env_configs/community.compose.env +++ b/dev/oci_env_integration/oci_env_configs/community.compose.env @@ -12,8 +12,8 @@ DJANGO_SUPERUSER_USERNAME=admin DJANGO_SUPERUSER_PASSWORD=admin ## Enable github social auth by setting these two values -SOCIAL_AUTH_GITHUB_KEY='abcd1234' -SOCIAL_AUTH_GITHUB_SECRET='abcd1234' +#SOCIAL_AUTH_GITHUB_KEY='abcd1234' +#SOCIAL_AUTH_GITHUB_SECRET='abcd1234' PULP_GALAXY_API_PATH_PREFIX=/api/ diff --git a/galaxy_ng/social/pipeline/user.py b/galaxy_ng/social/pipeline/user.py index 006a067019..3475b8a030 100644 --- a/galaxy_ng/social/pipeline/user.py +++ b/galaxy_ng/social/pipeline/user.py @@ -18,13 +18,17 @@ def get_username(strategy, details, backend, user=None, *args, **kwargs): def create_user(strategy, details, backend, user=None, *args, **kwargs): - logger.info(f'create_user(1): user-kwarg:{user}') + if user: + logger.info(f'create_user(1): user-kwarg:{user}:{user.id}') + else: + logger.info(f'create_user(1): user-kwarg:{user}') + logger.info(f'create_user(2): details:{details}') if user: if user.username != details.get('username'): user.username = details.get('username') user.save() - logger.info(f'create_user(2): returning user-kwarg {user}') + logger.info(f'create_user(2): returning user-kwarg {user}:{user.id}') return {'is_new': False} fields = dict( @@ -33,7 +37,7 @@ def create_user(strategy, details, backend, user=None, *args, **kwargs): ) if not fields: - logger.info(f'create_user(3): no fields for {user}') + logger.info(f'create_user(3): no fields for {user}:{user.id}') return # bypass the strange logic that can't find the user ... ? @@ -47,111 +51,84 @@ def create_user(strategy, details, backend, user=None, *args, **kwargs): ) # check for all possible emails ... - possible_emails = [generate_unverified_email(github_id), email] + possible_emails = [generate_unverified_email(github_id)] + if email: + possible_emails.append(email) + found_email = None for possible_email in possible_emails: # if the email is null maybe that causes the user hijacking? - if not email: + if not possible_email: continue found_email = User.objects.filter(email=possible_email).first() if found_email is not None: - logger.info(f'create_user(5): found user {found_email} via email {possible_email}') + logger.info( + f'create_user(5): found user {found_email}:{found_email.id}' + + f' via email {possible_email}' + ) break if found_email is not None: # fix the username if they've changed their login since last time if found_email.username != username: - logger.info(f'create_user(6): set found user {found_email} username to {username}') + logger.info( + f'create_user(6): set found user {found_email}:{found_email.id}' + + f' username to {username}' + ) found_email.username = username found_email.save() if found_email.email != email: - logger.info(f'create_user(7): set found user {found_email} email to {email}') + logger.info( + f'create_user(7): set found user {found_email}:{found_email.id}' + + f' email to {email}' + ) found_email.email = email found_email.save() logger.info( - f'create_user(8): returning found user {found_email} via email {possible_email}' + f'create_user(8): returning found user {found_email}:{found_email.id}' + + f' via email {possible_email}' ) return { 'is_new': False, 'user': found_email } + logger.info(f'create_user(9): did not find any users via matching emails {possible_emails}') + found_username = User.objects.filter(username=username).first() + if found_username: + logger.info( + f'create_user(10): found user:{found_username}:{found_username.id}:' + + f'{found_username.email}' + + f' by username:{username}' + ) + if found_username is not None and found_username.email: # we have an old user who's got the username but it's not the same person logging in ... # so change that username? The email should be unique right? - logger.info(f'create_user(9): set {found_username} username to {found_username.email}') + logger.info( + f'create_user(11): set {found_username}:{found_username.id}:{found_username.email}' + + f' username to {found_username.email}' + ) found_username.username = found_username.email found_username.save() found_username = User.objects.filter(username=username).first() if found_username is not None: - logger.info(f'create_user(10): {found_username}') + logger.info(f'create_user(12): {found_username}') return { 'is_new': False, 'user': found_username } new_user = strategy.create_user(**fields) - logger.info(f'create_user(11): {new_user}') + logger.info(f'create_user(13): {new_user}') return { 'is_new': True, 'user': new_user } - - -def user_details(strategy, details, backend, user=None, *args, **kwargs): - """Update user details using data from provider.""" - - if not user: - return - - changed = False # flag to track changes - - # Default protected user fields (username, id, pk and email) can be ignored - # by setting the SOCIAL_AUTH_NO_DEFAULT_PROTECTED_USER_FIELDS to True - if strategy.setting("NO_DEFAULT_PROTECTED_USER_FIELDS") is True: - protected = () - else: - protected = ( - "username", - "id", - "pk", - "email", - "password", - "is_active", - "is_staff", - "is_superuser", - ) - - protected = protected + tuple(strategy.setting("PROTECTED_USER_FIELDS", [])) - - # Update user model attributes with the new data sent by the current - # provider. Update on some attributes is disabled by default, for - # example username and id fields. It's also possible to disable update - # on fields defined in SOCIAL_AUTH_PROTECTED_USER_FIELDS. - field_mapping = strategy.setting("USER_FIELD_MAPPING", {}, backend) - for name, value in details.items(): - # Convert to existing user field if mapping exists - name = field_mapping.get(name, name) - if value is None or not hasattr(user, name) or name in protected: - continue - - current_value = getattr(user, name, None) - if current_value == value: - continue - - immutable_fields = tuple(strategy.setting("IMMUTABLE_USER_FIELDS", [])) - if name in immutable_fields and current_value: - continue - - changed = True - setattr(user, name, value) - - if changed: - strategy.storage.user.changed(user) diff --git a/profiles/community/pulp_config.env b/profiles/community/pulp_config.env index 2c16a0f999..158874200d 100644 --- a/profiles/community/pulp_config.env +++ b/profiles/community/pulp_config.env @@ -23,6 +23,8 @@ PULP_GALAXY_FEATURE_FLAGS__ai_deny_index=true SOCIAL_AUTH_GITHUB_BASE_URL='http://localhost:8082' SOCIAL_AUTH_GITHUB_API_URL='http://localhost:8082' +SOCIAL_AUTH_GITHUB_KEY='abcd1234' +SOCIAL_AUTH_GITHUB_SECRET='abcd1234' # Signing configs ENABLE_SIGNING=2 From 133f4c80eae85e4b748cd5250d2484c9d92dc699 Mon Sep 17 00:00:00 2001 From: christian Date: Tue, 17 Oct 2023 17:53:11 +0200 Subject: [PATCH 9/9] No-Issue (#1928) make lint test homogeneous for all versions --- galaxy_ng/tests/integration/api/test_artifact_upload.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/galaxy_ng/tests/integration/api/test_artifact_upload.py b/galaxy_ng/tests/integration/api/test_artifact_upload.py index 0dbb03db86..00de5375bd 100644 --- a/galaxy_ng/tests/integration/api/test_artifact_upload.py +++ b/galaxy_ng/tests/integration/api/test_artifact_upload.py @@ -416,10 +416,7 @@ def test_ansible_lint_exception(ansible_config, upload_artifact, hub_version): log_messages = [item["message"] for item in resp["messages"]] - pattern = "Linting collection via ansible-lint" # hub 4.8, galaxy-importer 0.4.11 - if parse_version(hub_version) < parse_version('4.7.0dev'): - pattern = "Linting role .* via ansible-lint" # 4.6, galaxy-importer 0.4.7 - + pattern = "Linting .* via ansible-lint" linting_re = re.compile(pattern) linting = [item for item in log_messages if linting_re.match(item)] assert len(linting) == 1 # linting occurred