From dbe8ae1fbb0dd45eaadadf6153c903cef2c5b849 Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Wed, 28 Jul 2021 13:48:23 -0400 Subject: [PATCH] refactor!: Remove unused callback endpoints used for PDF cert generation/updates [MICROBA-1087] [DEPR-155] * Removed callback functions that were originally used to update certificates (and example certificates) post xqueue processing --- .../certificates/tests/test_views.py | 151 ----------------- lms/djangoapps/certificates/views/xqueue.py | 153 +----------------- lms/urls.py | 4 - 3 files changed, 1 insertion(+), 307 deletions(-) diff --git a/lms/djangoapps/certificates/tests/test_views.py b/lms/djangoapps/certificates/tests/test_views.py index 7d862cb2f0..062d252d62 100644 --- a/lms/djangoapps/certificates/tests/test_views.py +++ b/lms/djangoapps/certificates/tests/test_views.py @@ -2,28 +2,20 @@ import datetime -import json from uuid import uuid4 -import ddt from django.conf import settings -from django.core.cache import cache from django.test.client import Client from django.test.utils import override_settings -from django.urls import reverse -from opaque_keys.edx.locator import CourseLocator from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.certificates.models import ( CertificateHtmlViewConfiguration, CertificateStatuses, - ExampleCertificate, - ExampleCertificateSet ) from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.certificates.utils import get_certificate_url from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -39,149 +31,6 @@ FEATURES_WITH_CUSTOM_CERTS_ENABLED = { FEATURES_WITH_CUSTOM_CERTS_ENABLED.update(FEATURES_WITH_CERTS_ENABLED) -@ddt.ddt -class UpdateExampleCertificateViewTest(CacheIsolationTestCase): - """Tests for the XQueue callback that updates example certificates. """ - - COURSE_KEY = CourseLocator(org='test', course='test', run='test') - - DESCRIPTION = 'test' - TEMPLATE = 'test.pdf' - DOWNLOAD_URL = 'http://www.example.com' - ERROR_REASON = 'Kaboom!' - - ENABLED_CACHES = ['default'] - - def setUp(self): - super().setUp() - self.cert_set = ExampleCertificateSet.objects.create(course_key=self.COURSE_KEY) - self.cert = ExampleCertificate.objects.create( - example_cert_set=self.cert_set, - description=self.DESCRIPTION, - template=self.TEMPLATE, - ) - self.url = reverse('update_example_certificate') - - # Since rate limit counts are cached, we need to clear - # this before each test. - cache.clear() - - def test_update_example_certificate_success(self): - response = self._post_to_view(self.cert, download_url=self.DOWNLOAD_URL) - self._assert_response(response) - - self.cert = ExampleCertificate.objects.get() - assert self.cert.status == ExampleCertificate.STATUS_SUCCESS - assert self.cert.download_url == self.DOWNLOAD_URL - - def test_update_example_certificate_invalid_key(self): - payload = { - 'xqueue_header': json.dumps({ - 'lms_key': 'invalid' - }), - 'xqueue_body': json.dumps({ - 'username': self.cert.uuid, - 'url': self.DOWNLOAD_URL - }) - } - response = self.client.post(self.url, data=payload) - assert response.status_code == 404 - - def test_update_example_certificate_error(self): - response = self._post_to_view(self.cert, error_reason=self.ERROR_REASON) - self._assert_response(response) - - self.cert = ExampleCertificate.objects.get() - assert self.cert.status == ExampleCertificate.STATUS_ERROR - assert self.cert.error_reason == self.ERROR_REASON - - @ddt.data('xqueue_header', 'xqueue_body') - def test_update_example_certificate_invalid_params(self, missing_param): - payload = { - 'xqueue_header': json.dumps({ - 'lms_key': self.cert.access_key - }), - 'xqueue_body': json.dumps({ - 'username': self.cert.uuid, - 'url': self.DOWNLOAD_URL - }) - } - del payload[missing_param] - - response = self.client.post(self.url, data=payload) - assert response.status_code == 400 - - def test_update_example_certificate_missing_download_url(self): - payload = { - 'xqueue_header': json.dumps({ - 'lms_key': self.cert.access_key - }), - 'xqueue_body': json.dumps({ - 'username': self.cert.uuid - }) - } - response = self.client.post(self.url, data=payload) - assert response.status_code == 400 - - def test_update_example_certificate_non_json_param(self): - payload = { - 'xqueue_header': '{/invalid', - 'xqueue_body': '{/invalid' - } - response = self.client.post(self.url, data=payload) - assert response.status_code == 400 - - def test_unsupported_http_method(self): - response = self.client.get(self.url) - assert response.status_code == 405 - - def test_bad_request_rate_limiting(self): - payload = { - 'xqueue_header': json.dumps({ - 'lms_key': 'invalid' - }), - 'xqueue_body': json.dumps({ - 'username': self.cert.uuid, - 'url': self.DOWNLOAD_URL - }) - } - - # Exceed the rate limit for invalid requests - # (simulate a DDOS with invalid keys) - for _ in range(100): - response = self.client.post(self.url, data=payload) - if response.status_code == 403: - break - - # The final status code should indicate that the rate - # limit was exceeded. - assert response.status_code == 403 - - def _post_to_view(self, cert, download_url=None, error_reason=None): - """Simulate a callback from the XQueue to the example certificate end-point. """ - header = {'lms_key': cert.access_key} - body = {'username': cert.uuid} - - if download_url is not None: - body['url'] = download_url - - if error_reason is not None: - body['error'] = 'error' - body['error_reason'] = self.ERROR_REASON - - payload = { - 'xqueue_header': json.dumps(header), - 'xqueue_body': json.dumps(body) - } - return self.client.post(self.url, data=payload) - - def _assert_response(self, response): - """Check the response from the callback end-point. """ - content = json.loads(response.content.decode('utf-8')) - assert response.status_code == 200 - assert content['return_code'] == 0 - - class CertificatesViewsSiteTests(ModuleStoreTestCase): """ Tests for the certificates web/html views diff --git a/lms/djangoapps/certificates/views/xqueue.py b/lms/djangoapps/certificates/views/xqueue.py index f13f481dbc..ec66cf1658 100644 --- a/lms/djangoapps/certificates/views/xqueue.py +++ b/lms/djangoapps/certificates/views/xqueue.py @@ -8,18 +8,11 @@ import logging from django.contrib.auth import get_user_model from django.db import transaction -from django.http import Http404, HttpResponse, HttpResponseForbidden +from django.http import HttpResponse from django.views.decorators.csrf import csrf_exempt -from django.views.decorators.http import require_POST from opaque_keys.edx.keys import CourseKey -from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest -from common.djangoapps.util.request_rate_limiter import BadRequestRateLimiter from lms.djangoapps.certificates.api import generate_certificate_task -from lms.djangoapps.certificates.models import ( - ExampleCertificate, - GeneratedCertificate, -) from lms.djangoapps.certificates.utils import certificate_status_for_student log = logging.getLogger(__name__) @@ -49,147 +42,3 @@ def request_certificate(request): generate_certificate_task(student, course_key) return HttpResponse(json.dumps({'add_status': status}), content_type='application/json') # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps return HttpResponse(json.dumps({'add_status': 'ERRORANONYMOUSUSER'}), content_type='application/json') # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps - - -@csrf_exempt -def update_certificate(request): - """ - Will update GeneratedCertificate for a new certificate or - modify an existing certificate entry. - - This view should only ever be accessed by the xqueue server - """ - - if request.method == "POST": - - xqueue_body = json.loads(request.POST.get('xqueue_body')) - xqueue_header = json.loads(request.POST.get('xqueue_header')) - - try: - course_key = CourseKey.from_string(xqueue_body['course_id']) - - cert = GeneratedCertificate.eligible_certificates.get( - user__username=xqueue_body['username'], - course_id=course_key, - key=xqueue_header['lms_key']) - - except GeneratedCertificate.DoesNotExist: - log.critical( - 'Unable to lookup certificate\n' - 'xqueue_body: %s\n' - 'xqueue_header: %s', - xqueue_body, - xqueue_header - ) - - return HttpResponse(json.dumps({ # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps - 'return_code': 1, - 'content': 'unable to lookup key' - }), content_type='application/json') - - user = cert.user - log.warning(f'{course_key} is using V2 certificates. Request to update the certificate for user {user.id} will ' - f'be ignored.') - return HttpResponse( # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps - json.dumps({ - 'return_code': 1, - 'content': 'allowlist certificate' - }), - content_type='application/json' - ) - - -@csrf_exempt -@require_POST -def update_example_certificate(request): - """Callback from the XQueue that updates example certificates. - - Example certificates are used to verify that certificate - generation is configured correctly for a course. - - Unlike other certificates, example certificates - are not associated with a particular user or displayed - to students. - - For this reason, we need a different end-point to update - the status of generated example certificates. - - Arguments: - request (HttpRequest) - - Returns: - HttpResponse (200): Status was updated successfully. - HttpResponse (400): Invalid parameters. - HttpResponse (403): Rate limit exceeded for bad requests. - HttpResponse (404): Invalid certificate identifier or access key. - - """ - log.info("Received response for example certificate from XQueue.") - - rate_limiter = BadRequestRateLimiter() - - # Check the parameters and rate limits - # If these are invalid, return an error response. - if rate_limiter.is_rate_limit_exceeded(request): - log.info("Bad request rate limit exceeded for update example certificate end-point.") - return HttpResponseForbidden("Rate limit exceeded") - - if 'xqueue_body' not in request.POST: - log.info("Missing parameter 'xqueue_body' for update example certificate end-point") - rate_limiter.tick_request_counter(request) - return JsonResponseBadRequest("Parameter 'xqueue_body' is required.") - - if 'xqueue_header' not in request.POST: - log.info("Missing parameter 'xqueue_header' for update example certificate end-point") - rate_limiter.tick_request_counter(request) - return JsonResponseBadRequest("Parameter 'xqueue_header' is required.") - - try: - xqueue_body = json.loads(request.POST['xqueue_body']) - xqueue_header = json.loads(request.POST['xqueue_header']) - except (ValueError, TypeError): - log.info("Could not decode params to example certificate end-point as JSON.") - rate_limiter.tick_request_counter(request) - return JsonResponseBadRequest("Parameters must be JSON-serialized.") - - # Attempt to retrieve the example certificate record - # so we can update the status. - try: - uuid = xqueue_body.get('username') - access_key = xqueue_header.get('lms_key') - cert = ExampleCertificate.objects.get(uuid=uuid, access_key=access_key) - except ExampleCertificate.DoesNotExist as e: - # If we are unable to retrieve the record, it means the uuid or access key - # were not valid. This most likely means that the request is NOT coming - # from the XQueue. Return a 404 and increase the bad request counter - # to protect against a DDOS attack. - log.info("Could not find example certificate with uuid '%s' and access key '%s'", uuid, access_key) - rate_limiter.tick_request_counter(request) - raise Http404 from e - - if 'error' in xqueue_body: - # If an error occurs, save the error message so we can fix the issue. - error_reason = xqueue_body.get('error_reason') - cert.update_status(ExampleCertificate.STATUS_ERROR, error_reason=error_reason) - log.warning( - ( - "Error occurred during example certificate generation for uuid '%s'. " - "The error response was '%s'." - ), uuid, error_reason - ) - else: - # If the certificate generated successfully, save the download URL - # so we can display the example certificate. - download_url = xqueue_body.get('url') - if download_url is None: - rate_limiter.tick_request_counter(request) - log.warning("No download URL provided for example certificate with uuid '%s'.", uuid) - return JsonResponseBadRequest( - "Parameter 'download_url' is required for successfully generated certificates." - ) - else: - cert.update_status(ExampleCertificate.STATUS_SUCCESS, download_url=download_url) - log.info("Successfully updated example certificate with uuid '%s'.", uuid) - - # Let the XQueue know that we handled the response - return JsonResponse({'return_code': 0}) diff --git a/lms/urls.py b/lms/urls.py index bb433b0c1d..243b1be33a 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -889,10 +889,6 @@ if settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER'): urlpatterns += [ url(r'^certificates/', include('lms.djangoapps.certificates.urls')), - # Backwards compatibility with XQueue, which uses URLs that are not prefixed with /certificates/ - url(r'^update_certificate$', certificates_views.update_certificate, name='update_certificate'), - url(r'^update_example_certificate$', certificates_views.update_example_certificate, - name='update_example_certificate'), url(r'^request_certificate$', certificates_views.request_certificate, name='request_certificate'),