From ec072b88ff7ed7a8a138fe0aa18474d6cd7aa410 Mon Sep 17 00:00:00 2001 From: Demid Date: Thu, 28 Nov 2024 15:51:19 +0200 Subject: [PATCH 1/5] build: enable CI for pull requests (#1978) Co-authored-by: Braden MacDonald Co-authored-by: Arunmozhi --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5d46dcb7..00e6d02d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,6 @@ on: push: branches: [master] pull_request: - branches: [master] concurrency: group: ci-${{ github.event.pull_request.number || github.ref }} From 4d5ba22917cdffbb4e25bb26bbb7715226a3b166 Mon Sep 17 00:00:00 2001 From: Saleem Latif Date: Wed, 27 Nov 2024 16:35:31 +0500 Subject: [PATCH 2/5] refactor: Removed API endpoint and related views from enterprise API. --- CHANGELOG.rst | 4 ++ enterprise/__init__.py | 2 +- enterprise/api/v1/urls.py | 6 --- enterprise/api/v1/views/plotly_auth.py | 62 ------------------------- tests/test_enterprise/api/test_views.py | 62 ------------------------- 5 files changed, 5 insertions(+), 131 deletions(-) delete mode 100644 enterprise/api/v1/views/plotly_auth.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a44618d31..b6572c0b7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,10 @@ Unreleased ---------- * nothing unreleased +[5.0.0] +-------- +* refactor: Removed `plotly_token/` API endpoint and related views from enterprise API. + [4.33.1] -------- * feat: Creating enterprise customer members endpoint for admin portal diff --git a/enterprise/__init__.py b/enterprise/__init__.py index 5c7d5784a..dee0e04d4 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "4.33.1" +__version__ = "5.0.0" diff --git a/enterprise/api/v1/urls.py b/enterprise/api/v1/urls.py index 3d276bd93..f46d5b55d 100644 --- a/enterprise/api/v1/urls.py +++ b/enterprise/api/v1/urls.py @@ -27,7 +27,6 @@ notifications, pending_enterprise_customer_admin_user, pending_enterprise_customer_user, - plotly_auth, ) router = DefaultRouter() @@ -131,11 +130,6 @@ coupon_codes.CouponCodesView.as_view(), name='request-codes' ), - re_path( - r'^plotly_token/(?P[A-Za-z0-9-]+)$', - plotly_auth.PlotlyAuthView.as_view(), - name='plotly-token' - ), re_path( r'^enterprise_report_types/(?P[A-Za-z0-9-]+)$', enterprise_customer_reporting.EnterpriseCustomerReportTypesView.as_view(), diff --git a/enterprise/api/v1/views/plotly_auth.py b/enterprise/api/v1/views/plotly_auth.py deleted file mode 100644 index 2f9005704..000000000 --- a/enterprise/api/v1/views/plotly_auth.py +++ /dev/null @@ -1,62 +0,0 @@ -""" -Views for Plotly auth. -""" - -from time import time - -import jwt -from edx_rbac.decorators import permission_required -from rest_framework import generics -from rest_framework.permissions import IsAuthenticated - -from django.conf import settings -from django.http import JsonResponse - -from enterprise.models import EnterpriseCustomer - - -class PlotlyAuthView(generics.GenericAPIView): - """ - API to generate a signed token for an enterprise admin to use Plotly analytics. - """ - permission_classes = (IsAuthenticated,) - - @permission_required( - 'enterprise.can_access_admin_dashboard', - fn=lambda request, enterprise_uuid: enterprise_uuid - ) - def get(self, request, enterprise_uuid): - """ - Generate auth token for plotly. - """ - # This is a new secret key and will be only shared between LMS and our Plotly server. - secret_key = settings.ENTERPRISE_PLOTLY_SECRET - - now = int(time()) - expires_in = 3600 # time in seconds after which token will be expired - exp = now + expires_in - - CLAIMS = { - "exp": exp, - "iat": now - } - - jwt_payload = dict({ - 'enterprise_uuid': enterprise_uuid, - 'audit_data_reporting_enabled': self._is_audit_data_reporting_enabled(enterprise_uuid), - }, **CLAIMS) - - token = jwt.encode(jwt_payload, secret_key, algorithm='HS512') - json_payload = {'token': token} - return JsonResponse(json_payload) - - @staticmethod - def _is_audit_data_reporting_enabled(enterprise_uuid): - """ - Check if audit data reporting is enabled for the enterprise. - - Args: - enterprise_uuid (str): UUID of the enterprise. - """ - enterprise = EnterpriseCustomer.objects.filter(uuid=enterprise_uuid).first() - return getattr(enterprise, 'enable_audit_data_reporting', False) diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index 9391f4cff..7c474989d 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -14,7 +14,6 @@ from urllib.parse import parse_qs, urlencode, urljoin, urlsplit, urlunsplit import ddt -import jwt import pytz import responses from edx_toggles.toggles.testutils import override_waffle_flag @@ -7755,67 +7754,6 @@ def test_same_enable_universal_link(self): self.assertEqual(response['detail'], 'No changes') -@mark.django_db -class TestPlotlyAuthView(APITest): - """ - Test PlotlyAuthView - """ - - PLOTLY_TOKEN_ENDPOINT = 'plotly-token' - - def setUp(self): - """ - Common setup for all tests. - """ - super().setUp() - self.client.login(username=self.user.username, password=TEST_PASSWORD) - self.enterprise_uuid = fake.uuid4() - self.enterprise_uuid2 = fake.uuid4() - self.url = settings.TEST_SERVER + reverse( - self.PLOTLY_TOKEN_ENDPOINT, kwargs={'enterprise_uuid': self.enterprise_uuid} - ) - - def test_view_with_normal_user(self): - """ - Verify that a user without having `enterprise.can_access_admin_dashboard` role can't access the view. - """ - response = self.client.get(self.url) - assert response.status_code == status.HTTP_403_FORBIDDEN - assert response.json() == {'detail': 'Missing: enterprise.can_access_admin_dashboard'} - - def test_view_with_admin_user(self): - """ - Verify that an enterprise admin user having `enterprise.can_access_admin_dashboard` role can access the view. - """ - EnterpriseCustomerFactory.create(uuid=self.enterprise_uuid, enable_audit_data_reporting=True) - self.set_jwt_cookie(ENTERPRISE_ADMIN_ROLE, self.enterprise_uuid) - - self.client.login(username=self.user.username, password=TEST_PASSWORD) - - response = self.client.get(self.url) - assert response.status_code == status.HTTP_200_OK - assert 'token' in response.json() - token = response.json().get('token') - decoded_jwt = jwt.decode(token, settings.ENTERPRISE_PLOTLY_SECRET, algorithms=['HS512']) - assert decoded_jwt['audit_data_reporting_enabled'] is True - - def test_view_with_admin_user_tries(self): - """ - Verify that an enterprise admin can create token for enterprise uuid present in jwt roles only. - """ - self.set_jwt_cookie(ENTERPRISE_ADMIN_ROLE, self.enterprise_uuid) - - url = settings.TEST_SERVER + reverse( - self.PLOTLY_TOKEN_ENDPOINT, kwargs={'enterprise_uuid': self.enterprise_uuid2} - ) - - self.client.login(username=self.user.username, password=TEST_PASSWORD) - - response = self.client.get(url) - assert response.status_code == status.HTTP_403_FORBIDDEN - assert response.json() == {'detail': 'Missing: enterprise.can_access_admin_dashboard'} - - @mark.django_db class TestAnalyticsSummaryView(APITest): """ From 0b70eead0341a0a93c13f1b867bc18a371bd97c2 Mon Sep 17 00:00:00 2001 From: Katrina Nguyen <71999631+katrinan029@users.noreply.github.com> Date: Mon, 2 Dec 2024 14:15:18 -0800 Subject: [PATCH 3/5] feat: update EnterpriseGroupMembershipSerializer to include enrollment count (#2286) * feat: update EnterpriseGroupMembershipSerializer to include learner course enrollment count and updated query to filter by name --- CHANGELOG.rst | 5 ++ enterprise/__init__.py | 2 +- enterprise/api/v1/serializers.py | 13 ++++ .../v1/views/enterprise_customer_members.py | 2 +- enterprise/models.py | 14 +++- tests/test_enterprise/api/test_views.py | 78 +------------------ 6 files changed, 32 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b6572c0b7..4606fec71 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,11 @@ Unreleased ---------- * nothing unreleased +[5.1.0] +-------- +* feat: update EnterpriseGroupMembershipSerializer to include learner course enrollment count +* feat: updated learner query to filter by full name + [5.0.0] -------- * refactor: Removed `plotly_token/` API endpoint and related views from enterprise API. diff --git a/enterprise/__init__.py b/enterprise/__init__.py index dee0e04d4..1a5c8140b 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "5.0.0" +__version__ = "5.1.0" diff --git a/enterprise/api/v1/serializers.py b/enterprise/api/v1/serializers.py index 632206a07..69488030f 100644 --- a/enterprise/api/v1/serializers.py +++ b/enterprise/api/v1/serializers.py @@ -664,6 +664,7 @@ class EnterpriseGroupMembershipSerializer(serializers.ModelSerializer): member_details = serializers.SerializerMethodField() recent_action = serializers.SerializerMethodField() status = serializers.CharField(required=False) + enrollments = serializers.SerializerMethodField() class Meta: model = models.EnterpriseGroupMembership @@ -676,6 +677,7 @@ class Meta: 'recent_action', 'status', 'activated_at', + 'enrollments', ) def get_member_details(self, obj): @@ -698,6 +700,17 @@ def get_recent_action(self, obj): return f"Accepted: {obj.activated_at.strftime('%B %d, %Y')}" return f"Invited: {obj.created.strftime('%B %d, %Y')}" + def get_enrollments(self, obj): + """ + Fetch all of user's enterprise enrollments + """ + if user := obj.enterprise_customer_user: + enrollments = models.EnterpriseCourseEnrollment.objects.filter( + enterprise_customer_user=user.user_id, + ) + return len(enrollments) + return 0 + class EnterpriseCustomerUserReadOnlySerializer(serializers.ModelSerializer): """ diff --git a/enterprise/api/v1/views/enterprise_customer_members.py b/enterprise/api/v1/views/enterprise_customer_members.py index 90a884ebb..ca292aa4c 100644 --- a/enterprise/api/v1/views/enterprise_customer_members.py +++ b/enterprise/api/v1/views/enterprise_customer_members.py @@ -78,7 +78,7 @@ def get_members(self, request, *args, **kwargs): au.id, au.email, au.date_joined, - coalesce(NULLIF(aup.name, ''), concat(au.first_name, ' ', au.last_name)) as full_name + coalesce(NULLIF(aup.name, ''), (au.first_name || ' ' || au.last_name)) as full_name FROM enterprise_enterprisecustomeruser ecu INNER JOIN auth_user as au on ecu.user_id = au.id LEFT JOIN auth_userprofile as aup on au.id = aup.user_id diff --git a/enterprise/models.py b/enterprise/models.py index 2d477e8fd..f76655795 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -4692,13 +4692,19 @@ def _get_filtered_ecu_ids(self, user_query): # https://docs.djangoproject.com/en/5.0/topics/security/ var_q = f"%{user_query}%" sql_string = """ - select entcu.id from enterprise_enterprisecustomeruser entcu - join auth_user au on entcu.user_id = au.id - where entcu.enterprise_customer_id = %s and au.email like %s; + with users as ( + select ecu.id, + au.email, + coalesce(NULLIF(aup.name, ''), (au.first_name || ' ' || au.last_name)) as full_name + from enterprise_enterprisecustomeruser ecu + inner join auth_user au on ecu.user_id = au.id + left join auth_userprofile aup on au.id = aup.user_id + where ecu.enterprise_customer_id = %s + ) select id from users where email like %s or full_name like %s; """ # Raw sql is picky about uuid format customer_id = str(self.enterprise_customer.pk).replace("-", "") - ecus = EnterpriseCustomerUser.objects.raw(sql_string, (customer_id, var_q)) + ecus = EnterpriseCustomerUser.objects.raw(sql_string, (customer_id, var_q, var_q)) return [ecu.id for ecu in ecus] def _get_explicit_group_members(self, user_query=None, fetch_removed=False, pending_users_only=False,): diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index 7c474989d..1d4016719 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -8341,65 +8341,6 @@ def test_list_learners_bad_sort_by(self): assert response.status_code == 400 assert response.data.get('user_query') - def test_list_learners_filtered(self): - """ - Test that the list learners endpoint can be filtered by user details - """ - group = EnterpriseGroupFactory( - enterprise_customer=self.enterprise_customer, - ) - pending_user = PendingEnterpriseCustomerUserFactory( - user_email="foobar@example.com", - enterprise_customer=self.enterprise_customer, - ) - pending_user_query_string = f'?user_query={pending_user.user_email}' - url = settings.TEST_SERVER + reverse( - 'enterprise-group-learners', - kwargs={'group_uuid': group.uuid}, - ) + pending_user_query_string - response = self.client.get(url) - - assert response.json().get('count') == 0 - - group.save() - pending_membership = EnterpriseGroupMembershipFactory( - group=group, - pending_enterprise_customer_user=pending_user, - enterprise_customer_user=None, - ) - existing_membership = EnterpriseGroupMembershipFactory( - group=group, - pending_enterprise_customer_user=None, - enterprise_customer_user__enterprise_customer=self.enterprise_customer, - ) - existing_user = existing_membership.enterprise_customer_user.user - # Changing email to something that we know will be unique for collision purposes - existing_user.email = "ayylmao@example.com" - existing_user.save() - existing_user_query_string = '?user_query=ayylmao' - url = settings.TEST_SERVER + reverse( - 'enterprise-group-learners', - kwargs={'group_uuid': group.uuid}, - ) + existing_user_query_string - response = self.client.get(url) - - assert response.json().get('count') == 1 - assert response.json().get('results')[0].get( - 'enterprise_customer_user_id' - ) == existing_membership.enterprise_customer_user.id - - url = settings.TEST_SERVER + reverse( - 'enterprise-group-learners', - kwargs={'group_uuid': group.uuid}, - ) + pending_user_query_string - - response = self.client.get(url) - - assert response.json().get('count') == 1 - assert response.json().get('results')[0].get( - 'pending_enterprise_customer_user_id' - ) == pending_membership.pending_enterprise_customer_user.id - def test_list_removed_learners(self): group = EnterpriseGroupFactory( enterprise_customer=self.enterprise_customer, @@ -8500,6 +8441,7 @@ def test_successful_list_learners(self): }, 'recent_action': f'Accepted: {datetime.now().strftime("%B %d, %Y")}', 'status': 'pending', + 'enrollments': 0, }, ) expected_response = { @@ -8541,6 +8483,7 @@ def test_successful_list_learners(self): }, 'recent_action': f'Accepted: {datetime.now().strftime("%B %d, %Y")}', 'status': 'pending', + 'enrollments': 0, } ], } @@ -8632,23 +8575,6 @@ def test_successful_list_with_filters(self): assert len(enterprise_filtered_response.json().get('results')) == 1 assert learner_filtered_response.json().get('results')[0].get('uuid') == str(new_group.uuid) - def test_list_members_little_bobby_tables(self): - """ - Test that we properly sanitize member user query filters - https://xkcd.com/327/ - """ - # url: 'http://testserver/enterprise/api/v1/enterprise_group//learners/' - url = settings.TEST_SERVER + reverse( - 'enterprise-group-learners', - kwargs={'group_uuid': self.group_1.uuid}, - ) - # The problematic child - filter_query_param = "?user_query=Robert`); DROP TABLE enterprise_enterprisecustomeruser;--" - sql_injection_protected_response = self.client.get(url + filter_query_param) - assert sql_injection_protected_response.status_code == 200 - assert not sql_injection_protected_response.json().get('results') - assert EnterpriseCustomerUser.objects.all() - def test_successful_post_group(self): """ Test creating a new group record From 74d2eb2f0be0f3a105a2196829d8065df2cc2898 Mon Sep 17 00:00:00 2001 From: jajjibhai008 Date: Wed, 20 Nov 2024 14:55:04 +0500 Subject: [PATCH 4/5] feat: removed custom pagination for reporting configurations --- CHANGELOG.rst | 4 ++++ enterprise/__init__.py | 2 +- .../api/v1/views/enterprise_customer_reporting.py | 10 ---------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4606fec71..24f1d3d6f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,10 @@ Unreleased ---------- * nothing unreleased +[5.2.0] +-------- +* feat: removed custom pagination for reporting configurations. + [5.1.0] -------- * feat: update EnterpriseGroupMembershipSerializer to include learner course enrollment count diff --git a/enterprise/__init__.py b/enterprise/__init__.py index 1a5c8140b..cc84447ff 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "5.1.0" +__version__ = "5.2.0" diff --git a/enterprise/api/v1/views/enterprise_customer_reporting.py b/enterprise/api/v1/views/enterprise_customer_reporting.py index ff82aa48c..c35783cce 100644 --- a/enterprise/api/v1/views/enterprise_customer_reporting.py +++ b/enterprise/api/v1/views/enterprise_customer_reporting.py @@ -6,7 +6,6 @@ from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from rest_framework import permissions, status from rest_framework.authentication import SessionAuthentication -from rest_framework.pagination import PageNumberPagination from rest_framework.response import Response from rest_framework.status import HTTP_200_OK, HTTP_404_NOT_FOUND from rest_framework.views import APIView @@ -18,14 +17,6 @@ from enterprise.utils import get_enterprise_customer -class ExpandDefaultPageSize(PageNumberPagination): - """ - Expands page size for the API. - Used to populate large reporting configurations. - """ - page_size = 100 - - class EnterpriseCustomerReportingConfigurationViewSet(EnterpriseReadWriteModelViewSet): """ API views for the ``enterprise-customer-reporting`` API endpoint. @@ -35,7 +26,6 @@ class EnterpriseCustomerReportingConfigurationViewSet(EnterpriseReadWriteModelVi serializer_class = serializers.EnterpriseCustomerReportingConfigurationSerializer lookup_field = 'uuid' permission_classes = [permissions.IsAuthenticated] - pagination_class = ExpandDefaultPageSize USER_ID_FILTER = 'enterprise_customer__enterprise_customer_users__user_id' FIELDS = ( From a52ef73f7dc644d88ca9fa6083122c7338060291 Mon Sep 17 00:00:00 2001 From: Saleem Latif Date: Wed, 4 Dec 2024 14:54:30 +0500 Subject: [PATCH 5/5] refactor: Removed unused django setting. --- CHANGELOG.rst | 4 ++++ enterprise/__init__.py | 2 +- enterprise/settings/test.py | 1 - 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 24f1d3d6f..384ee791d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,10 @@ Unreleased ---------- * nothing unreleased +[5.3.0] +-------- +* refactor: Removed unused django setting. + [5.2.0] -------- * feat: removed custom pagination for reporting configurations. diff --git a/enterprise/__init__.py b/enterprise/__init__.py index cc84447ff..504cc10c3 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "5.2.0" +__version__ = "5.3.0" diff --git a/enterprise/settings/test.py b/enterprise/settings/test.py index dbc456b5d..30087edd1 100644 --- a/enterprise/settings/test.py +++ b/enterprise/settings/test.py @@ -359,7 +359,6 @@ def root(*args): 'facebook.com' ] -ENTERPRISE_PLOTLY_SECRET = "I am a secret" ENTERPRISE_MANUAL_REPORTING_CUSTOMER_UUIDS = ['12aacfee8ffa4cb3bed1059565a57f06',] EXEC_ED_LANDING_PAGE = 'https://www.edx-external.com/account'