From 1314e2c7cc6a937b0646cf1f402b974e4a9800f7 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Mon, 2 Mar 2020 22:17:39 +0300 Subject: [PATCH 01/14] hide intercom widget from beta_tester users --- .../context_processors.py | 7 +-- .../intercom_integration/helpers.py | 31 +++++++++++ .../intercom_integration/tests/__init__.py | 0 .../tests/test_should_show_intercom_widget.py | 55 +++++++++++++++++++ 4 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 openedx/core/djangoapps/appsembler/intercom_integration/helpers.py create mode 100644 openedx/core/djangoapps/appsembler/intercom_integration/tests/__init__.py create mode 100644 openedx/core/djangoapps/appsembler/intercom_integration/tests/test_should_show_intercom_widget.py diff --git a/openedx/core/djangoapps/appsembler/intercom_integration/context_processors.py b/openedx/core/djangoapps/appsembler/intercom_integration/context_processors.py index 8a6afd7e4f3a..5dae0e6fd1e3 100644 --- a/openedx/core/djangoapps/appsembler/intercom_integration/context_processors.py +++ b/openedx/core/djangoapps/appsembler/intercom_integration/context_processors.py @@ -6,6 +6,8 @@ from student.models import CourseAccessRole from student.roles import CourseCreatorRole +from openedx.core.djangoapps.appsembler.intercom_integration.helpers import should_show_intercom_widget + def intercom(request): data = {'show_intercom_widget': False} @@ -15,10 +17,7 @@ def intercom(request): return data user = request.user - if user.is_authenticated() and ( - user_has_role(user, CourseCreatorRole()) # Course authors, which is given by default for AMC site admins - or CourseAccessRole.objects.filter(user=user).exists() # Course staff, of any type - ): + if should_show_intercom_widget(user): data['show_intercom_widget'] = True user_hash = hmac.new( str(settings.INTERCOM_APP_SECRET), diff --git a/openedx/core/djangoapps/appsembler/intercom_integration/helpers.py b/openedx/core/djangoapps/appsembler/intercom_integration/helpers.py new file mode 100644 index 000000000000..f0d5acd34bcc --- /dev/null +++ b/openedx/core/djangoapps/appsembler/intercom_integration/helpers.py @@ -0,0 +1,31 @@ +""" +Helpers for Intercom integration. +""" + +from student.auth import user_has_role +from student.models import CourseAccessRole +from student.roles import CourseCreatorRole, CourseInstructorRole, CourseStaffRole + + +def should_show_intercom_widget(user): + """ + Show or hide the Intercom chat widget to different users. + + The widget should be displayed only for course creators, course staff and course admins. + + :param user: User object to check for. + :return: bool + """ + if not user.is_authenticated: + return False + + if user.is_superuser: + return False + + if user_has_role(user, CourseCreatorRole()): + return True + + return CourseAccessRole.objects.filter(user=user, role__in=[ + CourseStaffRole.ROLE, + CourseInstructorRole.ROLE, + ]).exists() diff --git a/openedx/core/djangoapps/appsembler/intercom_integration/tests/__init__.py b/openedx/core/djangoapps/appsembler/intercom_integration/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/appsembler/intercom_integration/tests/test_should_show_intercom_widget.py b/openedx/core/djangoapps/appsembler/intercom_integration/tests/test_should_show_intercom_widget.py new file mode 100644 index 000000000000..a1557b809e93 --- /dev/null +++ b/openedx/core/djangoapps/appsembler/intercom_integration/tests/test_should_show_intercom_widget.py @@ -0,0 +1,55 @@ +""" +Tests for the intercom should_show_intercom_widget helper. +""" + +from mock import patch, Mock +import ddt +from django.test import TestCase +from django.conf import settings +from opaque_keys.edx.keys import CourseKey +from student.roles import CourseCreatorRole, CourseInstructorRole, CourseStaffRole, CourseBetaTesterRole +from student.tests.factories import UserFactory + +from openedx.core.djangoapps.appsembler.intercom_integration.helpers import should_show_intercom_widget + + +@ddt.ddt +@patch.dict(settings.FEATURES, ENABLE_CREATOR_GROUP=True) +class TestShouldShowIntercomWidgetHelperTestCase(TestCase): + """ + Tests for the `should_show_intercom_widget` helper. + """ + + def test_should_hide_for_non_authenticated(self): + anonymous_user = Mock(is_authenticated=False) + assert not should_show_intercom_widget(anonymous_user) + + def test_should_hide_for_superusers(self): + superuser = UserFactory.create(is_superuser=True) + assert not should_show_intercom_widget(superuser) + + def test_should_show_for_site_wide_staff(self): + staff = UserFactory.create(is_staff=True) + assert should_show_intercom_widget(staff) + + def test_should_show_for_course_creators(self): + course_creator = UserFactory.create() + CourseCreatorRole().add_users(course_creator) + assert should_show_intercom_widget(course_creator) + + @ddt.unpack + @ddt.data({ + 'course_role_class': CourseStaffRole, + 'should_show': True, + }, { + 'course_role_class': CourseInstructorRole, + 'should_show': True, + }, { + 'course_role_class': CourseBetaTesterRole, + 'should_show': False, + }) + def test_should_show_for_course_staff(self, course_role_class, should_show): + course_staff = UserFactory.create(is_staff=False, is_superuser=False) + course_key = CourseKey.from_string('course-v1:Demo+Course+2017') + course_role_class(course_key).add_users(course_staff) + assert should_show_intercom_widget(course_staff) == should_show From 4774de84e0d3f4947847417497b9b2f2fa986099 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Wed, 11 Mar 2020 08:02:43 +0300 Subject: [PATCH 02/14] Fix unregistered celery task: course_structures Mainly to fix KeyError exception when the task course_structures.tasks.update_course_structure is received. --- cms/envs/common.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cms/envs/common.py b/cms/envs/common.py index be038e08223a..f0ed2ab49917 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -886,6 +886,9 @@ 'cms.djangoapps.contentstore.tasks', 'openedx.core.djangoapps.bookmarks.tasks', 'openedx.core.djangoapps.ccxcon.tasks', + + # TODO: Remove after Hawthorn because the `course_structures` app will no longer exist. + 'openedx.core.djangoapps.content.course_structures.tasks', ) # Message configuration From a4db057d3f4c00b9dcbc684f07fa0058ace41f49 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Wed, 11 Mar 2020 07:50:58 +0300 Subject: [PATCH 03/14] Django command to create devstack demo site --- .../commands/create_devstack_site.py | 123 ++++++++++++++++++ .../appsembler/sites/tests/test_commands.py | 70 ++++++++++ .../core/djangoapps/appsembler/sites/utils.py | 8 +- 3 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py create mode 100644 openedx/core/djangoapps/appsembler/sites/tests/test_commands.py 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 new file mode 100644 index 000000000000..6de52f0f7240 --- /dev/null +++ b/openedx/core/djangoapps/appsembler/sites/management/commands/create_devstack_site.py @@ -0,0 +1,123 @@ +import hashlib +import inspect +import json +import uuid + +from django.core.management.base import BaseCommand, CommandError +from django.contrib.auth.models import User +from django.core.validators import validate_slug, ValidationError +from django.conf import settings + +from openedx.core.djangoapps.appsembler.sites.serializers import RegistrationSerializer +from openedx.core.djangoapps.appsembler.sites.utils import reset_amc_tokens +from student.models import UserProfile +from student.roles import CourseCreatorRole + + +class Command(BaseCommand): + """ + Create the demo something.localhost:18000 site for devstack. + + Needs the corresponding `create_devstack_site` AMC command to be run as well. + """ + + def add_arguments(self, parser): + parser.add_argument( + 'name', + help='A slug for the username and used as a site name prefix e.g. something.localhost:18000', + nargs=1, + type=str, + ) + + def congrats(self, **kwargs): + """ + Write a congrats message. + + :param kwargs: congrats message format keyword arguments. + """ + self.stdout.write(inspect.cleandoc( + """ + Congrats, Your site is ready! + + Username: "{name}" + Email: "{email}" + Password: "{password}" + + Site URL: "http://{site}/" + + Please add the following entry to your /etc/hosts file: + + 127.0.0.1 {domain} + + Remember to run the corresponding AMC command. + + Enjoy! + """.format(**kwargs) + )) + + def handle(self, *args, **options): + if not settings.DEBUG: + raise CommandError('This only works on devstack.') + + name = options['name'][0].lower() + try: + validate_slug(name) + except ValidationError: + raise CommandError('Please enter a valid slug') + + if User.objects.filter(username=name).exists(): + raise CommandError('User exists with the username: "{}". Please choose another name.'.format(name)) + + domain = '{}.localhost'.format(name) + site_name = '{}:18000'.format(domain) + + user = User.objects.create_user( + username=name, + email='{}@example.com'.format(name), + password=name, + ) + CourseCreatorRole().add_users(user) + UserProfile.objects.create(user=user, name=name) + + # Calculated access tokens to the AMC devstack can have them without needing to communicate with the LMS. + # Just making it easier to automate this without having cross-dependency in devstack + fake_token = hashlib.md5(user.username).hexdigest() + reset_amc_tokens(user, access_token=fake_token, refresh_token=fake_token) + + data = { + 'site': { + 'domain': site_name, + 'name': site_name, + }, + 'user_email': user.email, + 'organization': { + 'name': name, + 'short_name': name, + 'edx_uuid': uuid.uuid4(), + }, + 'initial_values': { + 'SITE_NAME': site_name, + 'platform_name': '{} Academy'.format(name), + 'logo_positive': None, + 'logo_negative': None, + 'font': 'Roboto', + 'accent-font': 'Delius Unicase', + 'primary_brand_color': '#F00', + 'base_text_color': '#000', + 'cta_button_bg': '#00F', + } + } + serializer = RegistrationSerializer(data=data) + if not serializer.is_valid(): + raise CommandError('Something went wrong with the process: \n{errors}'.format( + errors=json.dumps(serializer.errors, indent=4) + )) + serializer.save() + + self.congrats( + name=user.username, + email=user.email, + password=name, + site=site_name, + domain=domain, + ) diff --git a/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py b/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py new file mode 100644 index 000000000000..0ed21b9e53e2 --- /dev/null +++ b/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py @@ -0,0 +1,70 @@ +import hashlib +from mock import patch + +from django.test import override_settings, TestCase +from django.conf import settings +from django.contrib.auth import get_user_model +from django.contrib.sites.models import Site +from django.core.management import call_command + +from openedx.core.djangoapps.appsembler.sites.management.commands.create_devstack_site import Command +from organizations.models import Organization +from provider.constants import CONFIDENTIAL +from provider.oauth2.models import AccessToken, RefreshToken, Client +from student.roles import CourseCreatorRole + + +@override_settings( + DEBUG=True, + DEFAULT_SITE_THEME='edx-theme-codebase', + FEATURES={ + 'AMC_APP_URL': 'http://localhost:13000', + "DISABLE_COURSE_CREATION": False, + "ENABLE_CREATOR_GROUP": True, + }, + COMPREHENSIVE_THEME_DIRS=[settings.REPO_ROOT / 'common/test/appsembler'], +) +class CreateDevstackSiteCommandTestCase(TestCase): + """ + Test ./manage.py lms create_devstack_site mydevstack + """ + name = 'mydevstack' # Used for both username, email and domain prefix. + site_name = '{}.localhost:18000'.format(name) + + def setUp(self): + assert settings.ENABLE_COMPREHENSIVE_THEMING + Client.objects.create(url=settings.FEATURES['AMC_APP_URL'], client_type=CONFIDENTIAL) + + def test_no_sites(self): + """ + Ensure nothing exists prior to the site creation. + + If something exists, it means Open edX have changed something in the sites so this + test needs to be refactored. + """ + assert not Site.objects.filter(domain=self.site_name).count() + assert not Organization.objects.count() + assert not get_user_model().objects.count() + + def test_create_devstack_site(self): + """ + Test that `create_devstack_site` and creates the required objects. + """ + with patch.object(Command, 'congrats') as mock_congrats: + call_command('create_devstack_site', self.name) + + mock_congrats.assert_called_once() # Ensure that congrats message is printed + + # Ensure objects are created correctly. + assert Site.objects.get(domain=self.site_name) + assert Organization.objects.get(name=self.name) + user = get_user_model().objects.get() + assert user.check_password(self.name) + assert user.profile.name == self.name + + assert CourseCreatorRole().has_user(user), 'User should be a course creator' + + fake_token = hashlib.md5(user.username).hexdigest() # Using a fake token so AMC devstack can guess it + assert fake_token == '80bfa968ffad007c79bfc603f3670c99', 'Ensure hash is identical to AMC' + assert AccessToken.objects.get(user=user).token == fake_token, 'Access token is needed' + assert RefreshToken.objects.get(user=user).token == fake_token, 'Refresh token is needed' diff --git a/openedx/core/djangoapps/appsembler/sites/utils.py b/openedx/core/djangoapps/appsembler/sites/utils.py index 3d835eebbd7b..3dd172e435d5 100644 --- a/openedx/core/djangoapps/appsembler/sites/utils.py +++ b/openedx/core/djangoapps/appsembler/sites/utils.py @@ -80,7 +80,7 @@ def get_amc_tokens(user): return tokens -def reset_amc_tokens(user): +def reset_amc_tokens(user, access_token=None, refresh_token=None): """ Create and return new tokens, or extend existing ones to one year in the future. """ @@ -94,6 +94,8 @@ def reset_amc_tokens(user): ) access.expires = access.client.get_default_token_expiry() + if access_token: + access.token = access_token access.save() try: @@ -105,7 +107,9 @@ def reset_amc_tokens(user): access_token=access, ) - refresh.expired = True + if refresh_token: + refresh.token = refresh_token + refresh.expired = False refresh.save() return get_amc_tokens(user) From a59c348d29bd097b33747d3e68f12acbb69c64b3 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Mon, 16 Mar 2020 14:27:05 +0300 Subject: [PATCH 04/14] Move access control backends into courseware Because it depends on AccessResponse class --- lms/djangoapps/courseware/access.py | 2 +- .../courseware}/access_control_backends.py | 5 +- .../tests/test_access_control_backends.py | 63 +++++++++++++++++-- ...est_access_control_backends_integration.py | 50 --------------- tox.ini | 3 +- 5 files changed, 62 insertions(+), 61 deletions(-) rename lms/{lib => djangoapps/courseware}/access_control_backends.py (92%) rename lms/{lib => djangoapps/courseware}/tests/test_access_control_backends.py (72%) delete mode 100644 lms/djangoapps/courseware/tests/test_access_control_backends_integration.py diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 615e45490667..95a883a7c2e9 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -37,7 +37,7 @@ from courseware.masquerade import get_masquerade_role, is_masquerading_as_student from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException from lms.djangoapps.ccx.models import CustomCourseForEdX -from lms.lib.access_control_backends import access_control_backends +from lms.djangoapps.courseware.access_control_backends import access_control_backends from mobile_api.models import IgnoreMobileAvailableFlagConfig from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.external_auth.models import ExternalAuthMap diff --git a/lms/lib/access_control_backends.py b/lms/djangoapps/courseware/access_control_backends.py similarity index 92% rename from lms/lib/access_control_backends.py rename to lms/djangoapps/courseware/access_control_backends.py index d494a0851f4b..ca421ccbb65a 100644 --- a/lms/lib/access_control_backends.py +++ b/lms/djangoapps/courseware/access_control_backends.py @@ -76,8 +76,9 @@ def query(self, action, user, resource, default_has_access): :param action: currently supporting the course access actions in SUPPORTED_ACTIONS. :param user: The User model object. :param resource: The course/resource ID. - :param default_has_access: True/False What's the default Open edX access control. - :return: True/False whether the `user` can perform the `action` on the `resource` or not. + :param default_has_access: AccessResponse The default access response object by Open edX. + :return: AccessResponse: ACCESS_GRANTED or ACCESS_DENIED whether the + `user` can perform the `action` on the `resource` or not. """ if action not in self.SUPPORTED_ACTIONS: raise NotImplementedError(self.UNSUPPORTED_ERROR_FMT(action=action)) diff --git a/lms/lib/tests/test_access_control_backends.py b/lms/djangoapps/courseware/tests/test_access_control_backends.py similarity index 72% rename from lms/lib/tests/test_access_control_backends.py rename to lms/djangoapps/courseware/tests/test_access_control_backends.py index bdeff0843c3b..1df7a9ebd90e 100644 --- a/lms/lib/tests/test_access_control_backends.py +++ b/lms/djangoapps/courseware/tests/test_access_control_backends.py @@ -1,15 +1,28 @@ """ Test cases for the pluggable access control system. """ +import datetime import ddt +from mock import patch, Mock import pytest +import pytz from django.conf import settings from django.test.utils import override_settings from django.test import TestCase -from mock import patch, Mock +from opaque_keys.edx.locator import CourseLocator -from lms.lib.access_control_backends import AccessControlBackends +from lms.djangoapps.courseware.access_utils import ( + ACCESS_DENIED, + ACCESS_GRANTED, +) +import lms.djangoapps.courseware.access as access +from lms.djangoapps.courseware.access_control_backends import ( + access_control_backends, + AccessControlBackends, +) +from student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @ddt.ddt @@ -93,7 +106,7 @@ def test_settings_with_options(self, mock_backend): 'NAME': 'lms.lib:see_in_catalog_backend', } }) - @patch('lms.lib.access_control_backends.log') + @patch('lms.djangoapps.courseware.access_control_backends.log') def test_settings_with_missing_function(self, mock_log): """ Check that the system fails explicitly on a missing function. @@ -159,11 +172,14 @@ def test_query_missing_backend(self, default_has_access): @override_settings(ACCESS_CONTROL_BACKENDS={ 'course.load': { - 'NAME': 'lms.lib:load_backend', + 'NAME': 'lms.djangoapps.courseware.access_control_backends:load_backend', } }) - @patch('lms.lib.load_backend', Mock(side_effect=ArithmeticError('Dividing by zero!')), create=True) - @patch('lms.lib.access_control_backends.log') + @patch('lms.djangoapps.courseware.access_control_backends.load_backend', Mock( + side_effect=ArithmeticError('Dividing by zero!')), + create=True, + ) + @patch('lms.djangoapps.courseware.access_control_backends.log') def test_query_broken_backend(self, mock_log): """ Ensure a broken backend fails explicitly. @@ -175,3 +191,38 @@ def test_query_broken_backend(self, mock_log): mock_log.exception.assert_called_once_with( 'Something went wrong in querying the access control backend for `course.load`.' ) + + +@ddt.ddt +class AccessWithACLBackendsTestCase(ModuleStoreTestCase): + """ + Integration tests for `access._has_access_course`. + """ + + def setUp(self): + """ + Set up tests environment. + """ + tomorrow = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) + self.user = UserFactory.create() + self.course = Mock( + enrollment_domain='', + enrollment_end=tomorrow, + enrollment_start=tomorrow, + id=CourseLocator('edX', 'test', '2012_Fall'), + ) + CourseEnrollmentAllowedFactory(email=self.user.email, course_id=self.course.id) + + def test_has_access_with_no_acl_backends(self): + """ + Ensure that the `access._has_access_course` queries the Access Control Backends. + """ + assert access._has_access_course(self.user, 'enroll', self.course).has_access + + @ddt.data(ACCESS_GRANTED, ACCESS_DENIED) + def test_has_access_with_acl_backends(self, backend_access): + """ + Ensure that the `access._has_access_course` queries the Access Control Backends. + """ + with patch.object(access_control_backends, 'query', Mock(return_value=backend_access)): + assert access._has_access_course(self.user, 'enroll', self.course) == backend_access diff --git a/lms/djangoapps/courseware/tests/test_access_control_backends_integration.py b/lms/djangoapps/courseware/tests/test_access_control_backends_integration.py deleted file mode 100644 index 9aea877bf212..000000000000 --- a/lms/djangoapps/courseware/tests/test_access_control_backends_integration.py +++ /dev/null @@ -1,50 +0,0 @@ -# -*- coding: utf-8 -*- -""" -Integration tests for the access control framework with the Access Control Backends plugins. -""" -import datetime - -import ddt -import pytz -from mock import patch, Mock -from opaque_keys.edx.locator import CourseLocator - -import courseware.access as access -from lms.lib.access_control_backends import access_control_backends -from student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase - - -@ddt.ddt -class AccessWithACLBackendsTestCase(ModuleStoreTestCase): - """ - Integration tests for `access._has_access_course`. - """ - - def setUp(self): - """ - Set up tests environment. - """ - tomorrow = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) - self.user = UserFactory.create() - self.course = Mock( - enrollment_domain='', - enrollment_end=tomorrow, - enrollment_start=tomorrow, - id=CourseLocator('edX', 'test', '2012_Fall'), - ) - CourseEnrollmentAllowedFactory(email=self.user.email, course_id=self.course.id) - - def test_has_access_with_no_acl_backends(self): - """ - Ensure that the `access._has_access_course` queries the Access Control Backends. - """ - assert access._has_access_course(self.user, 'enroll', self.course) - - @ddt.data(False, True) - def test_has_access_with_acl_backends(self, backend_access): - """ - Ensure that the `access._has_access_course` queries the Access Control Backends. - """ - with patch.object(access_control_backends, 'query', Mock(return_value=backend_access)): - assert backend_access == access._has_access_course(self.user, 'enroll', self.course) diff --git a/tox.ini b/tox.ini index 79de0948c9e5..fc6740e320b6 100644 --- a/tox.ini +++ b/tox.ini @@ -121,11 +121,10 @@ commands = lms/djangoapps/course_api/ \ lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py \ lms/djangoapps/courseware/tests/test_access.py \ - lms/djangoapps/courseware/tests/test_access_control_backends_integration.py \ + lms/djangoapps/courseware/tests/test_access_control_backends.py \ lms/djangoapps/grades/tests/integration/test_events.py \ lms/djangoapps/instructor/tests/test_certificates.py::CertificatesInstructorApiTest \ lms/djangoapps/verify_student/tests/test_services.py \ - lms/lib/tests/test_access_control_backends.py \ openedx/core/djangoapps/appsembler \ openedx/core/djangoapps/site_configuration/tests/test_tahoe_changes.py \ openedx/core/djangoapps/user_api/accounts/tests/test_utils.py::CompletionUtilsTestCase From 2cf9d6876c03535dbcf1f6494f995bfd941ef608 Mon Sep 17 00:00:00 2001 From: melvinsoft Date: Wed, 18 Mar 2020 22:06:16 -0300 Subject: [PATCH 05/14] Disable mongo debug pannel on django debug toolbar --- lms/envs/devstack.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index acbe7b735a3b..626a7e4ff093 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -70,7 +70,11 @@ 'debug_toolbar.panels.sql.SQLPanel', 'debug_toolbar.panels.signals.SignalsPanel', 'debug_toolbar.panels.logging.LoggingPanel', - 'debug_toolbar_mongo.panel.MongoDebugPanel', + + # MongoDebugPanel has been intentionally disabled by maxi@appsembler.com + # since it was breaking the mongo connections. Probably it's because we + # upgraded pymongo and DjangoDebugToolbar is some versions behind. + # ProfilingPanel has been intentionally removed for default devstack.py # runtimes for performance reasons. If you wish to re-enable it in your # local development environment, please create a new settings file From a8853dd47e74e1c9637a61e6ec580ce92012ea99 Mon Sep 17 00:00:00 2001 From: Ahmed Jazzar Date: Tue, 9 Jul 2019 11:16:20 -0700 Subject: [PATCH 06/14] Working delete site function --- openedx/core/djangoapps/appsembler/sites/api.py | 11 +++++++++++ openedx/core/djangoapps/appsembler/sites/urls.py | 3 +++ openedx/core/djangoapps/appsembler/sites/utils.py | 6 +++--- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/appsembler/sites/api.py b/openedx/core/djangoapps/appsembler/sites/api.py index dc3b776f1460..66a168f9c397 100644 --- a/openedx/core/djangoapps/appsembler/sites/api.py +++ b/openedx/core/djangoapps/appsembler/sites/api.py @@ -66,6 +66,17 @@ def get_serializer_class(self): return self.create_serializer_class return super(SiteConfigurationViewSet, self).get_serializer_class() + def perform_destroy(self, instance): + delete_site(instance.site) + + +class OffboardOrganizationAPIView(generics.DestroyAPIView): + authentication_classes = (OAuth2AuthenticationAllowInactiveUser,) + permission_classes = (IsAuthenticated, AMCAdminPermission) + queryset = Site.objects.all() + serializer_class = SiteSerializer + lookup_field = 'domain' + def perform_destroy(self, instance): delete_site(instance) diff --git a/openedx/core/djangoapps/appsembler/sites/urls.py b/openedx/core/djangoapps/appsembler/sites/urls.py index f647e9520b05..75da6678045d 100644 --- a/openedx/core/djangoapps/appsembler/sites/urls.py +++ b/openedx/core/djangoapps/appsembler/sites/urls.py @@ -1,12 +1,14 @@ from django.conf import settings from django.conf.urls import url, include from rest_framework.routers import DefaultRouter + from openedx.core.djangoapps.appsembler.sites.api import ( CustomDomainView, DomainAvailabilityView, DomainSwitchView, HostFilesView, FileUploadView, + OffboardOrganizationAPIView, SiteConfigurationViewSet, SiteCreateView, SiteViewSet, @@ -27,6 +29,7 @@ url(r'^custom_domain/', CustomDomainView.as_view()), url(r'^domain_switch/', DomainSwitchView.as_view()), url(r'^register/', SiteCreateView.as_view()), + url(r'^offboard/(?P\w+(\.\w+)*(:[0-9]+)?\/?)/', OffboardOrganizationAPIView.as_view()), url(r'^', include(router.urls)), ] diff --git a/openedx/core/djangoapps/appsembler/sites/utils.py b/openedx/core/djangoapps/appsembler/sites/utils.py index 3dd172e435d5..bc26710e7e57 100644 --- a/openedx/core/djangoapps/appsembler/sites/utils.py +++ b/openedx/core/djangoapps/appsembler/sites/utils.py @@ -306,10 +306,10 @@ def bootstrap_site(site, org_data=None, user_email=None): return organization, site, user -def delete_site(site_id): - site = Site.objects.get(id=site_id) +def delete_site(site): site.configuration.delete() - site.themes.delete() + site.themes.all().delete() + site.delete() From ba7aa44637f997531e39e3efc94589b0151a90cb Mon Sep 17 00:00:00 2001 From: Maximiliano Fernandez Date: Thu, 19 Mar 2020 09:26:02 -0300 Subject: [PATCH 07/14] Update lms/envs/devstack.py Co-Authored-By: Omar Al-Ithawi --- lms/envs/devstack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 626a7e4ff093..622ac7b5644e 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -71,7 +71,7 @@ 'debug_toolbar.panels.signals.SignalsPanel', 'debug_toolbar.panels.logging.LoggingPanel', - # MongoDebugPanel has been intentionally disabled by maxi@appsembler.com + # Appsembler: MongoDebugPanel has been intentionally disabled by maxi@appsembler.com # since it was breaking the mongo connections. Probably it's because we # upgraded pymongo and DjangoDebugToolbar is some versions behind. From 80f7a84d049b31e33887b38ccded080a9318f222 Mon Sep 17 00:00:00 2001 From: Anders Pearson Date: Tue, 24 Mar 2020 14:43:29 +0000 Subject: [PATCH 08/14] upgrade django-storages to 1.6.5 gets us support for boto3 and AWS v4 signatures so we can use newer region S3 buckets --- requirements/edx/base.in | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/edx/base.in b/requirements/edx/base.in index a4bffa02c15e..493ddbc44499 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -58,7 +58,7 @@ django-ses==0.8.4 django-simple-history django-splash django-statici18n==1.4.0 -django-storages==1.4.1 +django-storages==1.6.5 django-user-tasks django-waffle==0.12.0 django-webpack-loader # Used to wire webpack bundles into the django asset pipeline diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3d2e1d1b7fb7..8320b96b6c6e 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -96,7 +96,7 @@ django-ses==0.8.4 django-simple-history==2.1.1 django-splash==0.2.2 django-statici18n==1.4.0 -django-storages==1.4.1 +django-storages==1.6.5 django-user-tasks==0.1.5 django-waffle==0.12.0 django-webpack-loader==0.6.0 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 1a84b45094f3..7a2721c62846 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -116,7 +116,7 @@ django-ses==0.8.4 django-simple-history==2.1.1 django-splash==0.2.2 django-statici18n==1.4.0 -django-storages==1.4.1 +django-storages==1.6.5 django-user-tasks==0.1.5 django-waffle==0.12.0 django-webpack-loader==0.6.0 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 05a6037e105e..3e6de865b4f1 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -112,7 +112,7 @@ django-ses==0.8.4 django-simple-history==2.1.1 django-splash==0.2.2 django-statici18n==1.4.0 -django-storages==1.4.1 +django-storages==1.6.5 django-user-tasks==0.1.5 django-waffle==0.12.0 django-webpack-loader==0.6.0 From a9e3b3fb58d6af40f5484c1dbc27c16ac2dea267 Mon Sep 17 00:00:00 2001 From: Anders Pearson Date: Tue, 24 Mar 2020 14:48:47 +0000 Subject: [PATCH 09/14] default to AWS v4 sigs This *should* enable `boto` to connect to S3 buckets in newer regions. This can be removed once we've verified that everything works correctly with `boto3` though. --- cms/envs/aws.py | 3 +++ lms/envs/aws.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index eaa9b514100f..dbc9798bf9a1 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -22,6 +22,9 @@ from path import Path as path from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed +# force S3 v4 (temporary until we can upgrade to django-storages 1.9) +S3_USE_SIGV4 = True + # SERVICE_VARIANT specifies name of the variant used, which decides what JSON # configuration files are read during startup. SERVICE_VARIANT = os.environ.get('SERVICE_VARIANT', None) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index e5fa8ae11895..a2f9f558c3cc 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -30,6 +30,9 @@ from path import Path as path from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed +# force S3 v4 (temporary until we can upgrade to django-storages 1.9) +S3_USE_SIGV4 = True + # SERVICE_VARIANT specifies name of the variant used, which decides what JSON # configuration files are read during startup. SERVICE_VARIANT = os.environ.get('SERVICE_VARIANT', None) From 68fb3500d37abcd94602631030fe4a5cc25b5cbe Mon Sep 17 00:00:00 2001 From: Ahmed Jazzar Date: Thu, 19 Mar 2020 10:52:33 +0000 Subject: [PATCH 10/14] Remove Site management command Update docs Fix tests Updating tests Remove the white space Check for theme count Renaming --- .../core/djangoapps/appsembler/sites/api.py | 11 ----- .../sites/management/commands/remove_site.py | 41 +++++++++++++++++ .../appsembler/sites/tests/test_commands.py | 46 +++++++++++++++++++ .../core/djangoapps/appsembler/sites/urls.py | 2 - 4 files changed, 87 insertions(+), 13 deletions(-) create mode 100644 openedx/core/djangoapps/appsembler/sites/management/commands/remove_site.py diff --git a/openedx/core/djangoapps/appsembler/sites/api.py b/openedx/core/djangoapps/appsembler/sites/api.py index 66a168f9c397..dae005ae4911 100644 --- a/openedx/core/djangoapps/appsembler/sites/api.py +++ b/openedx/core/djangoapps/appsembler/sites/api.py @@ -70,17 +70,6 @@ def perform_destroy(self, instance): delete_site(instance.site) -class OffboardOrganizationAPIView(generics.DestroyAPIView): - authentication_classes = (OAuth2AuthenticationAllowInactiveUser,) - permission_classes = (IsAuthenticated, AMCAdminPermission) - queryset = Site.objects.all() - serializer_class = SiteSerializer - lookup_field = 'domain' - - def perform_destroy(self, instance): - delete_site(instance) - - class FileUploadView(views.APIView): parser_classes = (MultiPartParser,) # TODO: oauth token isn't present after step 3 in signup, fix later diff --git a/openedx/core/djangoapps/appsembler/sites/management/commands/remove_site.py b/openedx/core/djangoapps/appsembler/sites/management/commands/remove_site.py new file mode 100644 index 000000000000..81caac89edf9 --- /dev/null +++ b/openedx/core/djangoapps/appsembler/sites/management/commands/remove_site.py @@ -0,0 +1,41 @@ +from django.core.management.base import BaseCommand, CommandError +from django.contrib.sites.models import Site + +from openedx.core.djangoapps.appsembler.sites.utils import delete_site + + +class Command(BaseCommand): + """ + Remove a Tahoe website from LMS records. + + Must be used `remove_site` on AMC to avoid any errors there. + """ + + def add_arguments(self, parser): + parser.add_argument( + 'domain', + help='The domain of the organization to be deleted.', + type=str, + ) + + def handle(self, *args, **options): + organization_domain = options['domain'] + self.stdout.write(self.style.WARNING('Same command must be ran on the connected AMC instance')) + + self.stdout.write('Removing "%s" in progress...' % organization_domain) + organization = self._get_site(organization_domain) + + delete_site(organization) + self.stdout.write(self.style.SUCCESS('Successfully removed site "%s"' % organization_domain)) + + def _get_site(self, domain): + """ + Locates the site to be deleted and return its instance. + + :param domain: The domain of the site to be returned. + :return: Returns the site object that has the given domain. + """ + try: + return Site.objects.get(domain=domain) + except Site.DoesNotExist: + raise CommandError('Cannot find "%s" in Sites!' % domain) diff --git a/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py b/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py index 0ed21b9e53e2..f5faa54191ea 100644 --- a/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py +++ b/openedx/core/djangoapps/appsembler/sites/tests/test_commands.py @@ -8,6 +8,8 @@ from django.core.management import call_command from openedx.core.djangoapps.appsembler.sites.management.commands.create_devstack_site import Command +from openedx.core.djangoapps.site_configuration.models import SiteConfiguration +from openedx.core.djangoapps.theming.models import SiteTheme from organizations.models import Organization from provider.constants import CONFIDENTIAL from provider.oauth2.models import AccessToken, RefreshToken, Client @@ -68,3 +70,47 @@ def test_create_devstack_site(self): assert fake_token == '80bfa968ffad007c79bfc603f3670c99', 'Ensure hash is identical to AMC' assert AccessToken.objects.get(user=user).token == fake_token, 'Access token is needed' assert RefreshToken.objects.get(user=user).token == fake_token, 'Refresh token is needed' + + +@override_settings( + DEBUG=True, + DEFAULT_SITE_THEME='edx-theme-codebase', + FEATURES={ + 'AMC_APP_URL': 'http://localhost:13000', + "DISABLE_COURSE_CREATION": False, + "ENABLE_CREATOR_GROUP": True, + }, + COMPREHENSIVE_THEME_DIRS=[settings.REPO_ROOT / 'common/test/appsembler'], +) +class RemoveSiteCommandTestCase(TestCase): + """ + Test ./manage.py lms remove_site mysite + """ + def setUp(self): + assert settings.ENABLE_COMPREHENSIVE_THEMING + Client.objects.create(url=settings.FEATURES['AMC_APP_URL'], client_type=CONFIDENTIAL) + + self.to_be_deleted = 'delete' + self.shall_remain = 'keep' + + # This command should be tested above + call_command('create_devstack_site', self.to_be_deleted) + call_command('create_devstack_site', self.shall_remain) + + def test_create_devstack_site(self): + """ + Test that `create_devstack_site` and creates the required objects. + """ + call_command('remove_site', '{}.localhost:18000'.format(self.to_be_deleted)) + + # Ensure objects are removed correctly. + deleted_domain = '{}.localhost:18000'.format(self.to_be_deleted) + remained_domain = '{}.localhost:18000'.format(self.shall_remain) + + assert not Site.objects.filter(domain=deleted_domain).exists() + site = Site.objects.get(domain=remained_domain) + + assert SiteConfiguration.objects.count() == 1 + assert SiteConfiguration.objects.get(site=site) + + assert SiteTheme.objects.filter(site=site).count() == site.themes.count() diff --git a/openedx/core/djangoapps/appsembler/sites/urls.py b/openedx/core/djangoapps/appsembler/sites/urls.py index 75da6678045d..64214d103a71 100644 --- a/openedx/core/djangoapps/appsembler/sites/urls.py +++ b/openedx/core/djangoapps/appsembler/sites/urls.py @@ -8,7 +8,6 @@ DomainSwitchView, HostFilesView, FileUploadView, - OffboardOrganizationAPIView, SiteConfigurationViewSet, SiteCreateView, SiteViewSet, @@ -29,7 +28,6 @@ url(r'^custom_domain/', CustomDomainView.as_view()), url(r'^domain_switch/', DomainSwitchView.as_view()), url(r'^register/', SiteCreateView.as_view()), - url(r'^offboard/(?P\w+(\.\w+)*(:[0-9]+)?\/?)/', OffboardOrganizationAPIView.as_view()), url(r'^', include(router.urls)), ] From 3311d09e0feccf63a164c08eb62fcb398ec6068e Mon Sep 17 00:00:00 2001 From: melvinsoft Date: Wed, 25 Mar 2020 11:43:58 -0300 Subject: [PATCH 11/14] use default storage setting in site creation process --- openedx/core/djangoapps/site_configuration/models.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/site_configuration/models.py b/openedx/core/djangoapps/site_configuration/models.py index e432d097b061..ac409ebfdf98 100644 --- a/openedx/core/djangoapps/site_configuration/models.py +++ b/openedx/core/djangoapps/site_configuration/models.py @@ -6,6 +6,7 @@ import os from django.conf import settings +from django.core.files.storage import get_storage_class from django.contrib.sites.models import Site from django.contrib.staticfiles.templatetags.staticfiles import static from django.core.files.storage import get_storage_class @@ -15,7 +16,6 @@ from jsonfield.fields import JSONField from model_utils.models import TimeStampedModel -from storages.backends.s3boto import S3BotoStorage logger = getLogger(__name__) # pylint: disable=invalid-name @@ -171,7 +171,8 @@ def compile_microsite_sass(self): css_output = compile_sass('main.scss', custom_branding=self._sass_var_override) file_name = self.get_value('css_overrides_file') if settings.USE_S3_FOR_CUSTOMER_THEMES: - storage = S3BotoStorage( + storage_class = get_storage_class(settings.DEFAULT_FILE_STORAGE) + storage = storage_class( location="customer_themes", ) with storage.open(file_name, 'w') as f: From 50927efc95469a06c7d7feced1a98ed464f72dd5 Mon Sep 17 00:00:00 2001 From: melvinsoft Date: Wed, 25 Mar 2020 13:23:24 -0300 Subject: [PATCH 12/14] update to boto3 in cms --- cms/djangoapps/contentstore/storage.py | 4 ++-- cms/djangoapps/contentstore/views/import_export.py | 4 ++-- openedx/core/storage.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/storage.py b/cms/djangoapps/contentstore/storage.py index 76bd5b54bdc8..4340582a64c5 100644 --- a/cms/djangoapps/contentstore/storage.py +++ b/cms/djangoapps/contentstore/storage.py @@ -5,11 +5,11 @@ from django.conf import settings from django.core.files.storage import get_storage_class -from storages.backends.s3boto import S3BotoStorage +from storages.backends.s3boto3 import S3Boto3Storage from storages.utils import setting -class ImportExportS3Storage(S3BotoStorage): # pylint: disable=abstract-method +class ImportExportS3Storage(S3Boto3Storage): # pylint: disable=abstract-method """ S3 backend for course import and export OLX files. """ diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 4bee4b776da4..067411b15105 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -23,7 +23,7 @@ from opaque_keys.edx.locator import LibraryLocator from path import Path as path from six import text_type -from storages.backends.s3boto import S3BotoStorage +from storages.backends.s3boto3 import S3Boto3Storage from user_tasks.conf import settings as user_tasks_settings from user_tasks.models import UserTaskArtifact, UserTaskStatus from wsgiref.util import FileWrapper @@ -373,7 +373,7 @@ def export_status_handler(request, course_key_string): artifact = UserTaskArtifact.objects.get(status=task_status, name='Output') if isinstance(artifact.file.storage, FileSystemStorage): output_url = reverse_course_url('export_output_handler', course_key) - elif isinstance(artifact.file.storage, S3BotoStorage): + elif isinstance(artifact.file.storage, S3Boto3Storage): filename = os.path.basename(artifact.file.name).encode('utf-8') disposition = 'attachment; filename="{}"'.format(filename) output_url = artifact.file.storage.url(artifact.file.name, response_headers={ diff --git a/openedx/core/storage.py b/openedx/core/storage.py index b7683ce162ea..ff9938094ff9 100644 --- a/openedx/core/storage.py +++ b/openedx/core/storage.py @@ -6,7 +6,7 @@ from django.utils.lru_cache import lru_cache from pipeline.storage import NonPackagingMixin, PipelineCachedStorage from require.storage import OptimizedFilesMixin -from storages.backends.s3boto import S3BotoStorage +from storages.backends.s3boto3 import S3Boto3Storage from openedx.core.djangoapps.theming.storage import ThemeCachedFilesMixin, ThemePipelineMixin, ThemeStorage @@ -55,7 +55,7 @@ class DevelopmentStorage( pass -class S3ReportStorage(S3BotoStorage): # pylint: disable=abstract-method +class S3ReportStorage(S3Boto3Storage): # pylint: disable=abstract-method """ Storage for reports. """ @@ -68,7 +68,7 @@ def __init__(self, acl=None, bucket=None, custom_domain=None, **settings): acl: content policy for the uploads i.e. private, public etc. bucket: Name of S3 bucket to use for storing and/or retrieving content custom_domain: custom domain to use for generating file urls - **settings: additional settings to be passed in to S3BotoStorage, + **settings: additional settings to be passed in to S3Boto3Storage, Returns: From a9ae4c57e5ffcb151e9a59aae9170c20358d5b4f Mon Sep 17 00:00:00 2001 From: melvinsoft Date: Wed, 25 Mar 2020 16:16:28 -0300 Subject: [PATCH 13/14] removed obsolete parameter in boto3 --- cms/djangoapps/contentstore/views/import_export.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 067411b15105..f8422586ea91 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -376,11 +376,7 @@ def export_status_handler(request, course_key_string): elif isinstance(artifact.file.storage, S3Boto3Storage): filename = os.path.basename(artifact.file.name).encode('utf-8') disposition = 'attachment; filename="{}"'.format(filename) - output_url = artifact.file.storage.url(artifact.file.name, response_headers={ - 'response-content-disposition': disposition, - 'response-content-encoding': 'application/octet-stream', - 'response-content-type': 'application/x-tgz' - }) + output_url = artifact.file.storage.url(artifact.file.name) else: output_url = artifact.file.storage.url(artifact.file.name) elif task_status.state in (UserTaskStatus.FAILED, UserTaskStatus.CANCELED): From 65db8a5f043587158c6c5895ba9172fb927bc6c8 Mon Sep 17 00:00:00 2001 From: melvinsoft Date: Wed, 25 Mar 2020 17:13:09 -0300 Subject: [PATCH 14/14] remove unused vars --- cms/djangoapps/contentstore/views/import_export.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index f8422586ea91..f29267bec7ad 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -374,8 +374,6 @@ def export_status_handler(request, course_key_string): if isinstance(artifact.file.storage, FileSystemStorage): output_url = reverse_course_url('export_output_handler', course_key) elif isinstance(artifact.file.storage, S3Boto3Storage): - filename = os.path.basename(artifact.file.name).encode('utf-8') - disposition = 'attachment; filename="{}"'.format(filename) output_url = artifact.file.storage.url(artifact.file.name) else: output_url = artifact.file.storage.url(artifact.file.name)