From 1dfe57d570d1ffdb79179a3dd044440914beebf7 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Sat, 6 Mar 2021 13:13:26 +0300 Subject: [PATCH 1/6] create_devstack_site: no need for the debug hack on staging trial now works on staging, so this command is dangerous to have on production when we get there. --- .../sites/management/commands/create_devstack_site.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py b/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py index 348f816224cb..6021707c8071 100644 --- a/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py +++ b/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py @@ -56,9 +56,8 @@ def congrats(self, **kwargs): )) def handle(self, *args, **options): - # TODO: Uncomment after fixing the AMC trial site - # if not settings.DEBUG: - # raise CommandError('This only works on devstack.') + if not settings.DEBUG: + raise CommandError('This only works on devstack.') name = options['name'][0].lower() try: From 1937cb797209143c0c84d2f491f779b44bb5e924 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Sat, 6 Mar 2021 13:37:51 +0300 Subject: [PATCH 2/6] fix login issue after create_devstack_site UserOrganizationMapping was missing --- .../sites/management/commands/create_devstack_site.py | 2 +- openedx/core/djangoapps/appsembler/sites/serializers.py | 2 +- .../core/djangoapps/appsembler/sites/tests/test_commands.py | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py b/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py index 6021707c8071..30a606e531f7 100644 --- a/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py +++ b/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py @@ -89,7 +89,7 @@ def handle(self, *args, **options): 'domain': site_name, 'name': site_name, }, - 'user_email': user.email, + 'username': user.username, 'organization': { 'name': name, 'short_name': name, diff --git a/openedx/core/djangoapps/appsembler/sites/serializers.py b/openedx/core/djangoapps/appsembler/sites/serializers.py index 5af6b5a0bec8..1d3acae357cb 100644 --- a/openedx/core/djangoapps/appsembler/sites/serializers.py +++ b/openedx/core/djangoapps/appsembler/sites/serializers.py @@ -75,7 +75,7 @@ class Meta: def create(self, validated_data): site = super(SiteSerializer, self).create(validated_data) - organization, site, user = bootstrap_site(site) + _organization, site, _user = bootstrap_site(site) return site def custom_domain_status(self, obj): diff --git a/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py b/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py index 8a397cb2748a..0d699c76dd34 100644 --- a/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py +++ b/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py @@ -44,7 +44,7 @@ UserStandingFactory, ) -from organizations.models import Organization, OrganizationCourse +from organizations.models import Organization, OrganizationCourse, UserOrganizationMapping from oauth2_provider.models import AccessToken, RefreshToken, Application @@ -97,6 +97,7 @@ def test_create_devstack_site(self): user = get_user_model().objects.get() assert user.check_password(self.name) assert user.profile.name == self.name + assert UserOrganizationMapping.objects.get(organization__name=self.name, user=user) assert CourseCreatorRole().has_user(user), 'User should be a course creator' From 59a1252bd571ee041f39e926b47b83a5b34a1c06 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Sat, 6 Mar 2021 13:47:20 +0300 Subject: [PATCH 3/6] py3 fixes for appsembler/sites/tests/test_views.py response.content is binary in python3 --- .../core/djangoapps/appsembler/sites/tests/test_views.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/appsembler/sites/tests/test_views.py b/openedx/core/djangoapps/appsembler/sites/tests/test_views.py index 388c8af1b71d..b0d5beeca14e 100644 --- a/openedx/core/djangoapps/appsembler/sites/tests/test_views.py +++ b/openedx/core/djangoapps/appsembler/sites/tests/test_views.py @@ -47,7 +47,7 @@ def test_find_username(self): create_org_user(red_org, email=email, username=username) response = self.get_username(email, red_org.name) - assert response.status_code == status.HTTP_200_OK, response.content + assert response.status_code == status.HTTP_200_OK, response.content.decode('utf-8') assert response.json()['username'] == username def test_organization_separation(self): @@ -64,11 +64,11 @@ def test_organization_separation(self): pass response = self.get_username(email, blue_org.name) - assert response.status_code == status.HTTP_404_NOT_FOUND, response.content # Should keep sites separated + assert response.status_code == status.HTTP_404_NOT_FOUND, response.content.decode('utf-8') # Should keep sites separated blue_user = create_org_user(blue_org, email=email) response = self.get_username(email, blue_org.name) - assert response.status_code == status.HTTP_200_OK, response.content + assert response.status_code == status.HTTP_200_OK, response.content.decode('utf-8') assert response.json()['username'] == blue_user.username def test_not_found(self): @@ -79,4 +79,4 @@ def test_not_found(self): pass response = self.get_username('nobody@example.com', red_org.name) - assert response.status_code == status.HTTP_404_NOT_FOUND, response.content + assert response.status_code == status.HTTP_404_NOT_FOUND, response.content.decode('utf-8') From 4245c12719b71638da49a2e3a92872948ba00494 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Sun, 7 Mar 2021 11:15:03 +0300 Subject: [PATCH 4/6] Fix Django 2 queryset requirement to use __in ValueError: The QuerySet value for an exact lookup must be limited to one result using slicing. --- .../multi_tenant_emails/tests/test_utils.py | 4 +-- .../core/djangoapps/appsembler/sites/api.py | 2 +- .../appsembler/sites/tests/test_views.py | 35 +++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/appsembler/multi_tenant_emails/tests/test_utils.py b/openedx/core/djangoapps/appsembler/multi_tenant_emails/tests/test_utils.py index 7f7dbce32d79..5e810735efea 100644 --- a/openedx/core/djangoapps/appsembler/multi_tenant_emails/tests/test_utils.py +++ b/openedx/core/djangoapps/appsembler/multi_tenant_emails/tests/test_utils.py @@ -48,12 +48,12 @@ def with_organization_context(site_color, configs=None): yield org -def create_org_user(organization, **kwargs): +def create_org_user(organization, is_amc_admin=False, **kwargs): """ Create one user and save it to the database. """ user = UserFactory.create(**kwargs) - UserOrganizationMapping.objects.create(user=user, organization=organization) + UserOrganizationMapping.objects.create(user=user, organization=organization, is_amc_admin=is_amc_admin) return user diff --git a/openedx/core/djangoapps/appsembler/sites/api.py b/openedx/core/djangoapps/appsembler/sites/api.py index 5b4c44544578..258e91c95361 100644 --- a/openedx/core/djangoapps/appsembler/sites/api.py +++ b/openedx/core/djangoapps/appsembler/sites/api.py @@ -47,7 +47,7 @@ def get_queryset(self): queryset = Site.objects.exclude(id=settings.SITE_ID) user = self.request.user if not user.is_superuser: - queryset = queryset.filter(organizations=user.organizations.all()) + queryset = queryset.filter(organizations__in=user.organizations.all()) return queryset diff --git a/openedx/core/djangoapps/appsembler/sites/tests/test_views.py b/openedx/core/djangoapps/appsembler/sites/tests/test_views.py index b0d5beeca14e..a860c168bc0b 100644 --- a/openedx/core/djangoapps/appsembler/sites/tests/test_views.py +++ b/openedx/core/djangoapps/appsembler/sites/tests/test_views.py @@ -4,8 +4,10 @@ from mock import patch from django.urls import reverse +from openedx.core.djangoapps.appsembler.sites.utils import make_amc_admin from rest_framework import status from rest_framework.test import APITestCase +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from openedx.core.djangolib.testing.utils import skip_unless_lms from openedx.core.djangoapps.appsembler.multi_tenant_emails.tests.test_utils import ( @@ -80,3 +82,36 @@ def test_not_found(self): response = self.get_username('nobody@example.com', red_org.name) assert response.status_code == status.HTTP_404_NOT_FOUND, response.content.decode('utf-8') + + +@skip_unless_lms +@patch( + 'openedx.core.djangoapps.appsembler.sites.api.SiteViewSet.authentication_classes', + [SessionAuthenticationAllowInactiveUser] +) +class TestSiteViewSet(APITestCase): + """ + Tests for the SiteViewSet AMC API. + """ + def setUp(self): + super(TestSiteViewSet, self).setUp() + self.url = reverse('site-list') + self.color = 'red' + with with_organization_context(site_color=self.color) as red_org: + self.red_org = red_org + self.admin = create_org_user( + organization=red_org, + is_amc_admin=True, + email='red@example.com', + username=self.color, + password=self.color + ) + + def test_list_sites(self): + with with_organization_context(site_color=self.color): + assert self.client.login(username=self.admin.username, password=self.color) + response = self.client.get( + self.url, + ) + content = response.content.decode('utf-8') + assert response.status_code == status.HTTP_200_OK, content From b18da4fcf3354479e93e8e66c5ed80dd60f1f3d6 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Sun, 7 Mar 2021 11:33:05 +0300 Subject: [PATCH 5/6] django 2 fix: ensure admin form button shows up correctly --- openedx/core/djangoapps/appsembler/sites/admin.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/appsembler/sites/admin.py b/openedx/core/djangoapps/appsembler/sites/admin.py index d7b5a79913d2..59664620f828 100644 --- a/openedx/core/djangoapps/appsembler/sites/admin.py +++ b/openedx/core/djangoapps/appsembler/sites/admin.py @@ -5,11 +5,11 @@ from django.conf.urls import url from django.conf import settings from django.template.response import TemplateResponse +from django.utils.html import format_html from hijack_admin.admin import HijackUserAdminMixin from ratelimitbackend import admin from student.admin import UserAdmin -from openedx.core.djangolib.markup import HTML, Text from openedx.core.djangoapps.appsembler.sites.models import AlternativeDomain from organizations.models import UserOrganizationMapping @@ -24,7 +24,7 @@ class TahoeUserAdmin(UserAdmin, HijackUserAdminMixin): list_display = UserAdmin.list_display + ( 'hijack_field', - 'make_amc_admin_action', + 'amc_actions', ) def get_urls(self): @@ -36,12 +36,11 @@ def get_urls(self): ), ] + super(UserAdmin, self).get_urls() - def make_amc_admin_action(self, obj): - return HTML('AMC Admin Form ').format( - href=Text(reverse('admin:make-amc-admin', args=[obj.id])), + def amc_actions(self, obj): + return format_html( + 'AMC Admin Form ', + href=reverse('admin:make-amc-admin', args=[obj.id]) ) - make_amc_admin_action.short_description = 'AMC Actions' - make_amc_admin_action.allow_tags = True def process_make_amc_admin(self, request, user_id, *args, **kwargs): user = self.get_object(request, user_id) From 0582c6fde4d05216bd1d64938829474c0759e171 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Sun, 7 Mar 2021 11:36:44 +0300 Subject: [PATCH 6/6] use BearerAuthenticationAllowInactiveUser on Tahoe APIs OAuth2AuthenticationAllowInactiveUser has been deprecated --- openedx/core/djangoapps/appsembler/sites/api.py | 6 +++--- openedx/core/djangoapps/appsembler/tpa_admin/api.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/appsembler/sites/api.py b/openedx/core/djangoapps/appsembler/sites/api.py index 258e91c95361..a63eb407262e 100644 --- a/openedx/core/djangoapps/appsembler/sites/api.py +++ b/openedx/core/djangoapps/appsembler/sites/api.py @@ -17,7 +17,7 @@ from rest_framework.views import APIView from openedx.core.lib.api.permissions import ApiKeyHeaderPermission from openedx.core.lib.api.authentication import ( - OAuth2AuthenticationAllowInactiveUser, + BearerAuthenticationAllowInactiveUser, ) from openedx.core.djangoapps.appsembler.sites.models import AlternativeDomain from openedx.core.djangoapps.appsembler.sites.permissions import AMCAdminPermission @@ -40,7 +40,7 @@ class SiteViewSet(viewsets.ReadOnlyModelViewSet): queryset = Site.objects.all() serializer_class = SiteSerializer - authentication_classes = (OAuth2AuthenticationAllowInactiveUser,) + authentication_classes = (BearerAuthenticationAllowInactiveUser,) permission_classes = (IsAuthenticated, AMCAdminPermission) def get_queryset(self): @@ -56,7 +56,7 @@ class SiteConfigurationViewSet(viewsets.ModelViewSet): serializer_class = SiteConfigurationSerializer list_serializer_class = SiteConfigurationListSerializer create_serializer_class = SiteSerializer - authentication_classes = (OAuth2AuthenticationAllowInactiveUser,) + authentication_classes = (BearerAuthenticationAllowInactiveUser,) permission_classes = (IsAuthenticated, AMCAdminPermission) def get_serializer_class(self): diff --git a/openedx/core/djangoapps/appsembler/tpa_admin/api.py b/openedx/core/djangoapps/appsembler/tpa_admin/api.py index 4e44dc445282..23756a34f068 100644 --- a/openedx/core/djangoapps/appsembler/tpa_admin/api.py +++ b/openedx/core/djangoapps/appsembler/tpa_admin/api.py @@ -8,7 +8,7 @@ from openedx.core.djangoapps.appsembler.sites.permissions import AMCAdminPermission from openedx.core.lib.api.authentication import ( - OAuth2AuthenticationAllowInactiveUser, + BearerAuthenticationAllowInactiveUser, ) from third_party_auth.models import SAMLConfiguration, SAMLProviderConfig from openedx.core.djangoapps.appsembler.tpa_admin.serializers import ( @@ -25,7 +25,7 @@ class SAMLConfigurationViewSet(viewsets.ModelViewSet): model = SAMLConfiguration queryset = SAMLConfiguration.objects.current_set().order_by('id') serializer_class = SAMLConfigurationSerializer - authentication_classes = (OAuth2AuthenticationAllowInactiveUser,) + authentication_classes = (BearerAuthenticationAllowInactiveUser,) permission_classes = (IsAuthenticated, AMCAdminPermission) filterset_class = SAMLConfigurationFilter @@ -50,7 +50,7 @@ def get_queryset(self): class SAMLProviderConfigViewSet(viewsets.ModelViewSet): queryset = SAMLProviderConfig.objects.current_set().order_by('id') serializer_class = SAMLProviderConfigSerializer - authentication_classes = (OAuth2AuthenticationAllowInactiveUser,) + authentication_classes = (BearerAuthenticationAllowInactiveUser,) permission_classes = (IsAuthenticated, AMCAdminPermission) filterset_class = SAMLProviderConfigFilter