Merge pull request #14408 from edx/clintonb/automated-refunds

Automating Refund Approvals

common/djangoapps/student/models.py

@@ -49,7 +49,6 @@ import request_cache
 from certificates.models import GeneratedCertificate
 from course_modes.models import CourseMode
 from enrollment.api import _default_course_mode
-from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, ECOMMERCE_DATE_FORMAT
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.xmodule_django.models import CourseKeyField, NoneToEmptyManager
@@ -1537,6 +1536,9 @@ class CourseEnrollment(models.Model):
 
     def refund_cutoff_date(self):
         """ Calculate and return the refund window end date. """
+        # NOTE: This is here to avoid circular references
+        from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, ECOMMERCE_DATE_FORMAT
+
         try:
             attribute = self.attributes.get(namespace='order', name='order_number')
         except ObjectDoesNotExist:

common/djangoapps/student/tests/test_refunds.py

@@ -30,7 +30,6 @@ from config_models.models import cache
 
 log = logging.getLogger(__name__)
 TEST_API_URL = 'http://www-internal.example.com/api'
-TEST_API_SIGNING_KEY = 'edx'
 JSON = 'application/json'
 
 
@@ -131,7 +130,7 @@ class RefundableTest(SharedModuleStoreTestCase):
     )
     @ddt.unpack
     @httpretty.activate
-    @override_settings(ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY, ECOMMERCE_API_URL=TEST_API_URL)
+    @override_settings(ECOMMERCE_API_URL=TEST_API_URL)
     def test_refund_cutoff_date(self, order_date_delta, course_start_delta, expected_date_delta, days):
         """
         Assert that the later date is used with the configurable refund period in calculating the returned cutoff date.
@@ -172,7 +171,7 @@ class RefundableTest(SharedModuleStoreTestCase):
         self.assertIsNone(self.enrollment.refund_cutoff_date())
 
     @httpretty.activate
-    @override_settings(ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY, ECOMMERCE_API_URL=TEST_API_URL)
+    @override_settings(ECOMMERCE_API_URL=TEST_API_URL)
     def test_multiple_refunds_dashbaord_page_error(self):
         """ Order with mutiple refunds will not throw 500 error when dashboard page will access."""
         now = datetime.now(pytz.UTC).replace(microsecond=0)

common/djangoapps/third_party_auth/tests/specs/base.py

@@ -18,7 +18,7 @@ from social import actions, exceptions
 from social.apps.django_app import utils as social_utils
 from social.apps.django_app import views as social_views
 
-from lms.djangoapps.commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY
+from lms.djangoapps.commerce.tests import TEST_API_URL
 from student import models as student_models
 from student import views as student_views
 from student.tests.factories import UserFactory
@@ -911,7 +911,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
 
 
 # pylint: disable=test-inherits-tests, abstract-method
-@django_utils.override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
+@django_utils.override_settings(ECOMMERCE_API_URL=TEST_API_URL)
 class Oauth2IntegrationTest(IntegrationTest):
     """Base test case for integration tests of Oauth2 providers."""
 

lms/djangoapps/commerce/api/v0/tests/test_views.py

@@ -17,7 +17,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
 from commerce.constants import Messages
-from commerce.tests import TEST_BASKET_ID, TEST_ORDER_NUMBER, TEST_PAYMENT_DATA, TEST_API_URL, TEST_API_SIGNING_KEY
+from commerce.tests import TEST_BASKET_ID, TEST_ORDER_NUMBER, TEST_PAYMENT_DATA
 from commerce.tests.mocks import mock_basket_order, mock_create_basket
 from commerce.tests.test_views import UserMixin
 from course_modes.models import CourseMode
@@ -39,7 +39,6 @@ UTM_COOKIE_CONTENTS = {
 
 @attr(shard=1)
 @ddt.ddt
-@override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
 class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase):
     """
     Tests for the commerce orders view.
@@ -276,7 +275,7 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
         # We should be enrolled in honor mode
         self._test_course_without_sku(enrollment_mode=CourseMode.HONOR)
 
-    @override_settings(ECOMMERCE_API_URL=None, ECOMMERCE_API_SIGNING_KEY=None)
+    @override_settings(ECOMMERCE_API_URL=None)
     def test_ecommerce_service_not_configured(self):
         """
         If the E-Commerce Service is not configured, the view should enroll the user.
@@ -313,7 +312,7 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
         """ Verifies that the view behaves appropriately when the course only has a professional mode. """
         self.assertProfessionalModeBypassed()
 
-    @override_settings(ECOMMERCE_API_URL=None, ECOMMERCE_API_SIGNING_KEY=None)
+    @override_settings(ECOMMERCE_API_URL=None)
     def test_professional_mode_only_and_ecommerce_service_not_configured(self):
         """
         Verifies that the view behaves appropriately when the course only has a professional mode and
@@ -390,7 +389,6 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
 
 
 @attr(shard=1)
-@override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
 class BasketOrderViewTests(UserMixin, TestCase):
     """ Tests for the basket order view. """
     view_name = 'commerce_api:v0:baskets:retrieve_order'

lms/djangoapps/commerce/api/v1/tests/test_views.py

@@ -17,7 +17,6 @@ from rest_framework.utils.encoders import JSONEncoder
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
-from commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY
 from commerce.tests.mocks import mock_order_endpoint
 from commerce.tests.test_views import UserMixin
 from course_modes.models import CourseMode
@@ -391,7 +390,6 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase)
 
 
 @attr(shard=1)
-@override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
 class OrderViewTests(UserMixin, TestCase):
     """ Tests for the basket order view. """
     view_name = 'commerce_api:v1:orders:detail'

lms/djangoapps/commerce/migrations/0005_commerceconfiguration_enable_automatic_refund_approval.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('commerce', '0004_auto_20160531_0950'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='commerceconfiguration',
+            name='enable_automatic_refund_approval',
+            field=models.BooleanField(default=True, help_text='Automatically approve valid refund requests, without manual processing'),
+        ),
+    ]

lms/djangoapps/commerce/models.py

@@ -39,6 +39,10 @@ class CommerceConfiguration(ConfigurationModel):
         default=DEFAULT_RECEIPT_PAGE_URL,
         help_text=_('Path to order receipt page.')
     )
+    enable_automatic_refund_approval = models.BooleanField(
+        default=True,
+        help_text=_('Automatically approve valid refund requests, without manual processing')
+    )
 
     def __unicode__(self):
         return "Commerce configuration"

lms/djangoapps/commerce/signals.py

@@ -9,23 +9,24 @@ from urlparse import urljoin
 
 import requests
 from django.conf import settings
+from django.contrib.auth import get_user_model
 from django.contrib.auth.models import AnonymousUser
 from django.dispatch import receiver
 from django.utils.translation import ugettext as _
-from edx_rest_api_client.exceptions import HttpClientError
-from request_cache.middleware import RequestCache
-from student.models import UNENROLL_DONE
 
+from commerce.models import CommerceConfiguration
 from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, is_commerce_service_configured
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.theming import helpers as theming_helpers
+from request_cache.middleware import RequestCache
+from student.models import UNENROLL_DONE
 
 log = logging.getLogger(__name__)
 
 
+# pylint: disable=unused-argument
 @receiver(UNENROLL_DONE)
-def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False,
-                         **kwargs):  # pylint: disable=unused-argument
+def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, **kwargs):
     """
     Signal receiver for unenrollments, used to automatically initiate refunds
     when applicable.
@@ -40,12 +41,12 @@ def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False,
             request_user = get_request_user() or course_enrollment.user
             if isinstance(request_user, AnonymousUser):
                 # Assume the request was initiated via server-to-server
-                # api call (presumably Otto).  In this case we cannot
+                # API call (presumably Otto).  In this case we cannot
                 # construct a client to call Otto back anyway, because
                 # the client does not work anonymously, and furthermore,
                 # there's certainly no need to inform Otto about this request.
                 return
-            refund_seat(course_enrollment, request_user)
+            refund_seat(course_enrollment)
         except:  # pylint: disable=bare-except
             # don't assume the signal was fired with `send_robust`.
             # avoid blowing up other signal handlers by gracefully
@@ -69,57 +70,56 @@ def get_request_user():
     return getattr(request, 'user', None)
 
 
-def refund_seat(course_enrollment, request_user):
+def refund_seat(course_enrollment):
     """
-    Attempt to initiate a refund for any orders associated with the seat being
-    unenrolled, using the commerce service.
+    Attempt to initiate a refund for any orders associated with the seat being unenrolled, using the commerce service.
 
     Arguments:
         course_enrollment (CourseEnrollment): a student enrollment
-        request_user: the user as whom to authenticate to the commerce service
-            when attempting to initiate the refund.
 
     Returns:
         A list of the external service's IDs for any refunds that were initiated
             (may be empty).
 
     Raises:
-        exceptions.SlumberBaseException: for any unhandled HTTP error during
-            communication with the commerce service.
-        exceptions.Timeout: if the attempt to reach the commerce service timed
-            out.
-
+        exceptions.SlumberBaseException: for any unhandled HTTP error during communication with the E-Commerce Service.
+        exceptions.Timeout: if the attempt to reach the commerce service timed out.
     """
+    User = get_user_model()  # pylint:disable=invalid-name
     course_key_str = unicode(course_enrollment.course_id)
-    unenrolled_user = course_enrollment.user
+    enrollee = course_enrollment.user
 
-    try:
-        refund_ids = ecommerce_api_client(request_user or unenrolled_user).refunds.post(
-            {'course_id': course_key_str, 'username': unenrolled_user.username}
-        )
-    except HttpClientError, exc:
-        if exc.response.status_code == 403 and request_user != unenrolled_user:
-            # this is a known limitation; commerce service does not presently
-            # support the case of a non-superusers initiating a refund on
-            # behalf of another user.
-            log.warning("User [%s] was not authorized to initiate a refund for user [%s] "
-                        "upon unenrollment from course [%s]", request_user.id, unenrolled_user.id, course_key_str)
-            return []
-        else:
-            # no other error is anticipated, so re-raise the Exception
-            raise exc
+    service_user = User.objects.get(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME)
+    api_client = ecommerce_api_client(service_user)
+
+    log.info('Attempting to create a refund for user [%s], course [%s]...', enrollee.id, course_key_str)
+
+    refund_ids = api_client.refunds.post({'course_id': course_key_str, 'username': enrollee.username})
 
     if refund_ids:
-        # at least one refundable order was found.
-        log.info(
-            "Refund successfully opened for user [%s], course [%s]: %r",
-            unenrolled_user.id,
-            course_key_str,
-            refund_ids,
-        )
+        log.info('Refund successfully opened for user [%s], course [%s]: %r', enrollee.id, course_key_str, refund_ids)
+
+        config = CommerceConfiguration.current()
+
+        if config.enable_automatic_refund_approval:
+            refunds_requiring_approval = []
+
+            for refund_id in refund_ids:
+                try:
+                    # NOTE: Approve payment only because the user has already been unenrolled. Additionally, this
+                    # ensures we don't tie up an additional web worker when the E-Commerce Service tries to unenroll
+                    # the learner
+                    api_client.refunds(refund_id).process.put({'action': 'approve_payment_only'})
+                    log.info('Refund [%d] successfully approved.', refund_id)
+                except:  # pylint: disable=bare-except
+                    log.exception('Failed to automatically approve refund [%d]!', refund_id)
+                    refunds_requiring_approval.append(refund_id)
+        else:
+            refunds_requiring_approval = refund_ids
 
+        if refunds_requiring_approval:
             # XCOM-371: this is a temporary measure to suppress refund-related email
-        # notifications to students and support@) for free enrollments.  This
+            # notifications to students and support for free enrollments.  This
             # condition should be removed when the CourseEnrollment.refundable() logic
             # is updated to be more correct, or when we implement better handling (and
             # notifications) in Otto for handling reversal of $0 transactions.
@@ -127,20 +127,19 @@ def refund_seat(course_enrollment, request_user):
                 # 'verified' is the only enrollment mode that should presently
                 # result in opening a refund request.
                 log.info(
-                "Skipping refund email notification for non-verified mode for user [%s], course [%s], mode: [%s]",
+                    'Skipping refund email notification for non-verified mode for user [%s], course [%s], mode: [%s]',
                     course_enrollment.user.id,
                     course_enrollment.course_id,
                     course_enrollment.mode,
                 )
             else:
                 try:
-                send_refund_notification(course_enrollment, refund_ids)
+                    send_refund_notification(course_enrollment, refunds_requiring_approval)
                 except:  # pylint: disable=bare-except
                     # don't break, just log a warning
-                log.warning("Could not send email notification for refund.", exc_info=True)
+                    log.warning('Could not send email notification for refund.', exc_info=True)
     else:
-        # no refundable orders were found.
-        log.debug("No refund opened for user [%s], course [%s]", unenrolled_user.id, course_key_str)
+        log.info('No refund opened for user [%s], course [%s]', enrollee.id, course_key_str)
 
     return refund_ids
 

lms/djangoapps/commerce/tests/__init__.py

 # -*- coding: utf-8 -*-
 """ Commerce app tests package. """
-import datetime
-import json
 
+import httpretty
+import mock
 from django.conf import settings
-from django.contrib.auth.models import User
 from django.test import TestCase
-from django.test.utils import override_settings
 from freezegun import freeze_time
-import httpretty
-import jwt
-import mock
 
-from edx_rest_api_client import auth
 from openedx.core.djangoapps.commerce.utils import ecommerce_api_client
+from openedx.core.lib.token_utils import JwtBuilder
 from student.tests.factories import UserFactory
 
 JSON = 'application/json'
 TEST_PUBLIC_URL_ROOT = 'http://www.example.com'
 TEST_API_URL = 'http://www-internal.example.com/api'
-TEST_API_SIGNING_KEY = 'edx'
 TEST_BASKET_ID = 7
 TEST_ORDER_NUMBER = '100004'
 TEST_PAYMENT_DATA = {
@@ -29,33 +23,27 @@ TEST_PAYMENT_DATA = {
 }
 
 
-@override_settings(ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY, ECOMMERCE_API_URL=TEST_API_URL)
 class EdxRestApiClientTest(TestCase):
     """ Tests to ensure the client is initialized properly. """
 
-    TEST_USER_EMAIL = 'test@example.com'
     TEST_CLIENT_ID = 'test-client-id'
 
     def setUp(self):
         super(EdxRestApiClientTest, self).setUp()
-
         self.user = UserFactory()
-        self.user.email = self.TEST_USER_EMAIL
-        self.user.save()  # pylint: disable=no-member
 
     @httpretty.activate
     @freeze_time('2015-7-2')
-    @override_settings(JWT_AUTH={'JWT_ISSUER': 'http://example.com/oauth', 'JWT_EXPIRATION': 30})
     def test_tracking_context(self):
         """
         Ensure the tracking context is set up in the api client correctly and
         automatically.
         """
 
-        # fake an ecommerce api request.
+        # fake an E-Commerce API request.
         httpretty.register_uri(
             httpretty.POST,
-            '{}/baskets/1/'.format(TEST_API_URL),
+            '{}/baskets/1/'.format(settings.ECOMMERCE_API_URL.strip('/')),
             status=200, body='{}',
             adding_headers={'Content-Type': JSON}
         )
@@ -65,23 +53,18 @@ class EdxRestApiClientTest(TestCase):
         with mock.patch('openedx.core.djangoapps.commerce.utils.tracker.get_tracker', return_value=mock_tracker):
             ecommerce_api_client(self.user).baskets(1).post()
 
-        # make sure the request's JWT token payload included correct tracking context values.
+        # Verify the JWT includes the tracking context for the user
         actual_header = httpretty.last_request().headers['Authorization']
-        expected_payload = {
-            'username': self.user.username,
-            'full_name': self.user.profile.name,
-            'email': self.user.email,
-            'iss': settings.JWT_AUTH['JWT_ISSUER'],
-            'iat': datetime.datetime.utcnow(),
-            'exp': datetime.datetime.utcnow() + datetime.timedelta(seconds=settings.JWT_AUTH['JWT_EXPIRATION']),
+
+        claims = {
             'tracking_context': {
                 'lms_user_id': self.user.id,  # pylint: disable=no-member
                 'lms_client_id': self.TEST_CLIENT_ID,
                 'lms_ip': '127.0.0.1',
-            },
             }
-
-        expected_header = 'JWT {}'.format(jwt.encode(expected_payload, TEST_API_SIGNING_KEY))
+        }
+        expected_jwt = JwtBuilder(self.user).build_token(['email', 'profile'], additional_claims=claims)
+        expected_header = 'JWT {}'.format(expected_jwt)
         self.assertEqual(actual_header, expected_header)
 
     @httpretty.activate
@@ -95,19 +78,9 @@ class EdxRestApiClientTest(TestCase):
         expected_content = '{"result": "Préparatoire"}'
         httpretty.register_uri(
             httpretty.GET,
-            '{}/baskets/1/order/'.format(TEST_API_URL),
+            '{}/baskets/1/order/'.format(settings.ECOMMERCE_API_URL.strip('/')),
             status=200, body=expected_content,
             adding_headers={'Content-Type': JSON},
         )
         actual_object = ecommerce_api_client(self.user).baskets(1).order.get()
         self.assertEqual(actual_object, {u"result": u"Préparatoire"})
-
-    def test_client_with_user_without_profile(self):
-        """
-        Verify client initialize successfully for users having no profile.
-        """
-        worker = User.objects.create_user(username='test_worker', email='test@example.com')
-        api_client = ecommerce_api_client(worker)
-
-        self.assertEqual(api_client._store['session'].auth.__dict__['username'], worker.username)  # pylint: disable=protected-access
-        self.assertIsNone(api_client._store['session'].auth.__dict__['full_name'])  # pylint: disable=protected-access

lms/djangoapps/commerce/tests/mocks.py

@@ -2,11 +2,14 @@
 import json
 
 import httpretty
+from django.conf import settings
 
-from commerce.tests import TEST_API_URL, factories
+from commerce.tests import factories
 
 
-class mock_ecommerce_api_endpoint(object):  # pylint: disable=invalid-name
+# pylint: disable=invalid-name
+
+class mock_ecommerce_api_endpoint(object):
     """
     Base class for contextmanagers used to mock calls to api endpoints.
 
@@ -21,7 +24,9 @@ class mock_ecommerce_api_endpoint(object):  # pylint: disable=invalid-name
     # override this in subclasses, using one of httpretty's method constants
     method = None
 
-    def __init__(self, response=None, status=200, expect_called=True, exception=None):
+    host = settings.ECOMMERCE_API_URL.strip('/')
+
+    def __init__(self, response=None, status=200, expect_called=True, exception=None, reset_on_exit=True):
         """
         Keyword Arguments:
             response: a JSON-serializable Python type representing the desired response body.
@@ -29,17 +34,28 @@ class mock_ecommerce_api_endpoint(object):  # pylint: disable=invalid-name
             expect_called: a boolean indicating whether an API request was expected; set
                 to False if we should ensure that no request arrived.
             exception: raise this exception instead of returning an HTTP response when called.
+            reset_on_exit (bool): Indicates if `httpretty` should be reset after the decorator exits.
         """
         self.response = response or self.default_response
         self.status = status
         self.expect_called = expect_called
         self.exception = exception
+        self.reset_on_exit = reset_on_exit
 
     def get_uri(self):
         """
-        Return the uri to register with httpretty for this contextmanager.
+        Returns the uri to register with httpretty for this contextmanager.
+        """
+        return self.host + '/' + self.get_path().lstrip('/')
+
+    def get_path(self):
+        """
+        Returns the path of the URI to register with httpretty for this contextmanager.
 
         Subclasses must override this method.
+
+        Returns:
+            str
         """
         raise NotImplementedError
 
@@ -48,7 +64,6 @@ class mock_ecommerce_api_endpoint(object):  # pylint: disable=invalid-name
         raise self.exception  # pylint: disable=raising-bad-type
 
     def __enter__(self):
-        httpretty.reset()
         httpretty.enable()
         httpretty.register_uri(
             self.method,
@@ -61,9 +76,11 @@ class mock_ecommerce_api_endpoint(object):  # pylint: disable=invalid-name
     def __exit__(self, exc_type, exc_val, exc_tb):
         assert self.expect_called == (httpretty.last_request().headers != {})
         httpretty.disable()
+        if self.reset_on_exit:
+            httpretty.reset()
 
 
-class mock_create_basket(mock_ecommerce_api_endpoint):  # pylint: disable=invalid-name
+class mock_create_basket(mock_ecommerce_api_endpoint):
     """ Mocks calls to E-Commerce API client basket creation method. """
 
     default_response = {
@@ -77,11 +94,11 @@ class mock_create_basket(mock_ecommerce_api_endpoint):  # pylint: disable=invali
     }
     method = httpretty.POST
 
-    def get_uri(self):
-        return TEST_API_URL + '/baskets/'
+    def get_path(self):
+        return '/baskets/'
 
 
-class mock_basket_order(mock_ecommerce_api_endpoint):  # pylint: disable=invalid-name
+class mock_basket_order(mock_ecommerce_api_endpoint):
     """ Mocks calls to E-Commerce API client basket order method. """
 
     default_response = {'number': 1}
@@ -91,21 +108,35 @@ class mock_basket_order(mock_ecommerce_api_endpoint):  # pylint: disable=invalid
         super(mock_basket_order, self).__init__(**kwargs)
         self.basket_id = basket_id
 
-    def get_uri(self):
-        return TEST_API_URL + '/baskets/{}/order/'.format(self.basket_id)
+    def get_path(self):
+        return '/baskets/{}/order/'.format(self.basket_id)
 
 
-class mock_create_refund(mock_ecommerce_api_endpoint):  # pylint: disable=invalid-name
+class mock_create_refund(mock_ecommerce_api_endpoint):
     """ Mocks calls to E-Commerce API client refund creation method. """
 
     default_response = []
     method = httpretty.POST
 
-    def get_uri(self):
-        return TEST_API_URL + '/refunds/'
+    def get_path(self):
+        return '/refunds/'
+
+
+class mock_process_refund(mock_ecommerce_api_endpoint):
+    """ Mocks calls to E-Commerce API client refund process method. """
+
+    default_response = []
+    method = httpretty.PUT
 
+    def __init__(self, refund_id, **kwargs):
+        super(mock_process_refund, self).__init__(**kwargs)
+        self.refund_id = refund_id
 
-class mock_order_endpoint(mock_ecommerce_api_endpoint):  # pylint: disable=invalid-name
+    def get_path(self):
+        return '/refunds/{}/process/'.format(self.refund_id)
+
+
+class mock_order_endpoint(mock_ecommerce_api_endpoint):
     """ Mocks calls to E-Commerce API client basket order method. """
 
     default_response = {'number': 'EDX-100001'}
@@ -115,11 +146,11 @@ class mock_order_endpoint(mock_ecommerce_api_endpoint):  # pylint: disable=inval
         super(mock_order_endpoint, self).__init__(**kwargs)
         self.order_number = order_number
 
-    def get_uri(self):
-        return TEST_API_URL + '/orders/{}/'.format(self.order_number)
+    def get_path(self):
+        return '/orders/{}/'.format(self.order_number)
 
 
-class mock_get_orders(mock_ecommerce_api_endpoint):  # pylint: disable=invalid-name
+class mock_get_orders(mock_ecommerce_api_endpoint):
     """ Mocks calls to E-Commerce API client order get method. """
 
     default_response = {
@@ -138,5 +169,5 @@ class mock_get_orders(mock_ecommerce_api_endpoint):  # pylint: disable=invalid-n
     }
     method = httpretty.GET
 
-    def get_uri(self):
-        return TEST_API_URL + '/orders/'
+    def get_path(self):
+        return '/orders/'

lms/djangoapps/commerce/tests/test_signals.py

@@ -9,21 +9,22 @@ import json
 from urlparse import urljoin
 
 import ddt
+import httpretty
+import mock
+from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
 from django.test import TestCase
 from django.test.utils import override_settings
-import httpretty
-import mock
 from opaque_keys.edx.keys import CourseKey
 from requests import Timeout
 
+from commerce.models import CommerceConfiguration
+from commerce.signals import send_refund_notification, generate_refund_notification_body, create_zendesk_ticket
+from commerce.tests import JSON
+from commerce.tests.mocks import mock_create_refund, mock_process_refund
+from course_modes.models import CourseMode
 from student.models import UNENROLL_DONE
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
-from commerce.signals import (refund_seat, send_refund_notification, generate_refund_notification_body,
-                              create_zendesk_ticket)
-from commerce.tests import TEST_PUBLIC_URL_ROOT, TEST_API_URL, TEST_API_SIGNING_KEY, JSON
-from commerce.tests.mocks import mock_create_refund
-from course_modes.models import CourseMode
 
 ZENDESK_URL = 'http://zendesk.example.com/'
 ZENDESK_USER = 'test@example.com'
@@ -31,11 +32,7 @@ ZENDESK_API_KEY = 'abc123'
 
 
 @ddt.ddt
-@override_settings(
-    ECOMMERCE_PUBLIC_URL_ROOT=TEST_PUBLIC_URL_ROOT,
-    ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY,
-    ZENDESK_URL=ZENDESK_URL, ZENDESK_USER=ZENDESK_USER, ZENDESK_API_KEY=ZENDESK_API_KEY
-)
+@override_settings(ZENDESK_URL=ZENDESK_URL, ZENDESK_USER=ZENDESK_USER, ZENDESK_API_KEY=ZENDESK_API_KEY)
 class TestRefundSignal(TestCase):
     """
     Exercises logic triggered by the UNENROLL_DONE signal.
@@ -43,6 +40,10 @@ class TestRefundSignal(TestCase):
 
     def setUp(self):
         super(TestRefundSignal, self).setUp()
+
+        # Ensure the E-Commerce service user exists
+        UserFactory(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME, is_staff=True)
+
         self.requester = UserFactory(username="test-requester")
         self.student = UserFactory(
             username="test-student",
@@ -55,6 +56,10 @@ class TestRefundSignal(TestCase):
         )
         self.course_enrollment.refundable = mock.Mock(return_value=True)
 
+        self.config = CommerceConfiguration.current()
+        self.config.enable_automatic_refund_approval = True
+        self.config.save()
+
     def send_signal(self, skip_refund=False):
         """
         DRY helper: emit the UNENROLL_DONE signal, as is done in
@@ -65,7 +70,6 @@ class TestRefundSignal(TestCase):
     @override_settings(
         ECOMMERCE_PUBLIC_URL_ROOT=None,
         ECOMMERCE_API_URL=None,
-        ECOMMERCE_API_SIGNING_KEY=None,
     )
     def test_no_service(self):
         """
@@ -88,7 +92,7 @@ class TestRefundSignal(TestCase):
         """
         self.send_signal()
         self.assertTrue(mock_refund_seat.called)
-        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.student))
+        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
 
         # if skip_refund is set to True in the signal, we should not try to initiate a refund.
         mock_refund_seat.reset_mock()
@@ -110,21 +114,21 @@ class TestRefundSignal(TestCase):
         # no HTTP request/user: auth to commerce service as the unenrolled student.
         self.send_signal()
         self.assertTrue(mock_refund_seat.called)
-        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.student))
+        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
 
         # HTTP user is the student: auth to commerce service as the unenrolled student.
         mock_get_request_user.return_value = self.student
         mock_refund_seat.reset_mock()
         self.send_signal()
         self.assertTrue(mock_refund_seat.called)
-        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.student))
+        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
 
         # HTTP user is another user: auth to commerce service as the requester.
         mock_get_request_user.return_value = self.requester
         mock_refund_seat.reset_mock()
         self.send_signal()
         self.assertTrue(mock_refund_seat.called)
-        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.requester))
+        self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
 
         # HTTP user is another server (AnonymousUser): do not try to initiate a refund at all.
         mock_get_request_user.return_value = AnonymousUser()
@@ -132,15 +136,6 @@ class TestRefundSignal(TestCase):
         self.send_signal()
         self.assertFalse(mock_refund_seat.called)
 
-    @mock.patch('commerce.signals.log.warning')
-    def test_not_authorized_warning(self, mock_log_warning):
-        """
-        Ensure that expected authorization issues are logged as warnings.
-        """
-        with mock_create_refund(status=403):
-            refund_seat(self.course_enrollment, UserFactory())
-            self.assertTrue(mock_log_warning.called)
-
     @mock.patch('commerce.signals.log.exception')
     def test_error_logging(self, mock_log_exception):
         """
@@ -152,14 +147,48 @@ class TestRefundSignal(TestCase):
             self.assertTrue(mock_log_exception.called)
 
     @mock.patch('commerce.signals.send_refund_notification')
-    def test_notification(self, mock_send_notification):
+    def test_notification_when_approval_fails(self, mock_send_notification):
         """
-        Ensure the notification function is triggered when refunds are
-        initiated
+        Ensure the notification function is triggered when refunds are initiated, and cannot be automatically approved.
         """
-        with mock_create_refund(status=200, response=[1, 2, 3]):
+        refund_id = 1
+        failed_refund_id = 2
+
+        with mock_create_refund(status=201, response=[refund_id, failed_refund_id]):
+            with mock_process_refund(refund_id, reset_on_exit=False):
+                with mock_process_refund(failed_refund_id, status=500, reset_on_exit=False):
                     self.send_signal()
                     self.assertTrue(mock_send_notification.called)
+                    mock_send_notification.assert_called_with(self.course_enrollment, [failed_refund_id])
+
+    @mock.patch('commerce.signals.send_refund_notification')
+    def test_notification_if_automatic_approval_disabled(self, mock_send_notification):
+        """
+        Ensure the notification is always sent if the automatic approval functionality is disabled.
+        """
+        refund_id = 1
+        self.config.enable_automatic_refund_approval = False
+        self.config.save()
+
+        with mock_create_refund(status=201, response=[refund_id]):
+            self.send_signal()
+            self.assertTrue(mock_send_notification.called)
+            mock_send_notification.assert_called_with(self.course_enrollment, [refund_id])
+
+    @mock.patch('commerce.signals.send_refund_notification')
+    def test_no_notification_after_approval(self, mock_send_notification):
+        """
+        Ensure the notification function is triggered when refunds are initiated, and cannot be automatically approved.
+        """
+        refund_id = 1
+
+        with mock_create_refund(status=201, response=[refund_id]):
+            with mock_process_refund(refund_id, reset_on_exit=False):
+                self.send_signal()
+                self.assertFalse(mock_send_notification.called)
+
+                last_request = httpretty.last_request()
+                self.assertDictEqual(json.loads(last_request.body), {'action': 'approve_payment_only'})
 
     @mock.patch('commerce.signals.send_refund_notification')
     def test_notification_no_refund(self, mock_send_notification):

lms/djangoapps/student_account/test/test_views.py

@@ -32,7 +32,7 @@ from provider.oauth2.models import (
 from testfixtures import LogCapture
 
 from commerce.models import CommerceConfiguration
-from commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY, factories
+from commerce.tests import factories
 from commerce.tests.mocks import mock_get_orders
 from course_modes.models import CourseMode
 from openedx.core.djangoapps.oauth_dispatch.tests import factories as dot_factories
@@ -506,7 +506,6 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
         })
 
 
-@override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
 class AccountSettingsViewTest(ThirdPartyAuthTestMixin, TestCase, ProgramsApiConfigMixin):
     """ Tests for the account settings view. """
 

lms/djangoapps/verify_student/tests/test_views.py

@@ -36,7 +36,7 @@ from course_modes.tests.factories import CourseModeFactory
 from courseware.url_helpers import get_redirect_url
 from common.test.utils import XssTestMixin
 from commerce.models import CommerceConfiguration
-from commerce.tests import TEST_PAYMENT_DATA, TEST_API_URL, TEST_API_SIGNING_KEY, TEST_PUBLIC_URL_ROOT
+from commerce.tests import TEST_PAYMENT_DATA, TEST_API_URL, TEST_PUBLIC_URL_ROOT
 from openedx.core.djangoapps.embargo.test_utils import restrict_course
 from openedx.core.djangoapps.user_api.accounts.api import get_account_settings
 from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme
@@ -140,7 +140,6 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase, XssTestMixin):
     @httpretty.activate
     @override_settings(
         ECOMMERCE_API_URL=TEST_API_URL,
-        ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY,
         ECOMMERCE_PUBLIC_URL_ROOT=TEST_PUBLIC_URL_ROOT
     )
     def test_start_flow_with_ecommerce(self):
@@ -1053,7 +1052,7 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase, XssTestMixin):
         self.assertEqual(response_dict['course_name'], mode_display_name)
 
     @httpretty.activate
-    @override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
+    @override_settings(ECOMMERCE_API_URL=TEST_API_URL)
     @ddt.data("verify_student_start_flow", "verify_student_begin_flow")
     def test_processors_api(self, payment_flow):
         """
@@ -1223,7 +1222,7 @@ class TestCreateOrderShoppingCart(CheckoutTestMixin, ModuleStoreTestCase):
 
 
 @attr(shard=2)
-@override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
+@override_settings(ECOMMERCE_API_URL=TEST_API_URL)
 @patch(
     'lms.djangoapps.verify_student.views.checkout_with_ecommerce_service',
     return_value=TEST_PAYMENT_DATA,
@@ -1248,7 +1247,7 @@ class TestCheckoutWithEcommerceService(ModuleStoreTestCase):
     """
 
     @httpretty.activate
-    @override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
+    @override_settings(ECOMMERCE_API_URL=TEST_API_URL)
     def test_create_basket(self):
         """
         Check that when working with a product being processed by the

lms/envs/aws.py

@@ -776,7 +776,6 @@ ONLOAD_BEACON_SAMPLE_RATE = ENV_TOKENS.get('ONLOAD_BEACON_SAMPLE_RATE', ONLOAD_B
 ##### ECOMMERCE API CONFIGURATION SETTINGS #####
 ECOMMERCE_PUBLIC_URL_ROOT = ENV_TOKENS.get('ECOMMERCE_PUBLIC_URL_ROOT', ECOMMERCE_PUBLIC_URL_ROOT)
 ECOMMERCE_API_URL = ENV_TOKENS.get('ECOMMERCE_API_URL', ECOMMERCE_API_URL)
-ECOMMERCE_API_SIGNING_KEY = AUTH_TOKENS.get('ECOMMERCE_API_SIGNING_KEY', ECOMMERCE_API_SIGNING_KEY)
 ECOMMERCE_API_TIMEOUT = ENV_TOKENS.get('ECOMMERCE_API_TIMEOUT', ECOMMERCE_API_TIMEOUT)
 
 COURSE_CATALOG_API_URL = ENV_TOKENS.get('COURSE_CATALOG_API_URL', COURSE_CATALOG_API_URL)

lms/envs/bok_choy.py

@@ -218,7 +218,6 @@ BADGING_BACKEND = 'lms.djangoapps.badges.backends.tests.dummy_backend.DummyBacke
 
 # Configure the LMS to use our stub eCommerce implementation
 ECOMMERCE_API_URL = 'http://localhost:8043/api/v2/'
-ECOMMERCE_API_SIGNING_KEY = 'ecommerce-key'
 
 LMS_ROOT_URL = "http://localhost:8000"
 DOC_LINK_BASE_URL = 'http://edx.readthedocs.io/projects/edx-guide-for-students'

lms/envs/common.py

@@ -2831,7 +2831,6 @@ ACCOUNT_VISIBILITY_CONFIGURATION = {
 # E-Commerce API Configuration
 ECOMMERCE_PUBLIC_URL_ROOT = None
 ECOMMERCE_API_URL = None
-ECOMMERCE_API_SIGNING_KEY = None
 ECOMMERCE_API_TIMEOUT = 5
 ECOMMERCE_SERVICE_WORKER_USERNAME = 'ecommerce_worker'
 ENTERPRISE_SERVICE_WORKER_USERNAME = 'enterprise_worker'

lms/envs/test.py

@@ -588,3 +588,5 @@ COMPREHENSIVE_THEME_DIRS = [REPO_ROOT / "themes", REPO_ROOT / "common/test"]
 COMPREHENSIVE_THEME_LOCALE_PATHS = [REPO_ROOT / "themes/conf/locale", ]
 
 LMS_ROOT_URL = "http://localhost:8000"
+
+ECOMMERCE_API_URL = 'https://ecommerce.example.com/api/v2/'

openedx/core/djangoapps/commerce/utils.py

@@ -4,13 +4,14 @@ from edx_rest_api_client.client import EdxRestApiClient
 from eventtracking import tracker
 
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
+from openedx.core.lib.token_utils import JwtBuilder
 
-ECOMMERCE_DATE_FORMAT = "%Y-%m-%dT%H:%M:%SZ"
+ECOMMERCE_DATE_FORMAT = '%Y-%m-%dT%H:%M:%SZ'
 
 
 def create_tracking_context(user):
     """ Assembles attributes from user and request objects to be sent along
-    in ecommerce api calls for tracking purposes. """
+    in E-Commerce API calls for tracking purposes. """
     context_tracker = tracker.get_tracker().resolve_context()
 
     return {
@@ -22,27 +23,19 @@ def create_tracking_context(user):
 
 def is_commerce_service_configured():
     """
-    Return a Boolean indicating whether or not configuration is present to use
-    the external commerce service.
+    Return a Boolean indicating whether or not configuration is present to use the external commerce service.
     """
-    ecommerce_api_url = configuration_helpers.get_value("ECOMMERCE_API_URL", settings.ECOMMERCE_API_URL)
-    ecommerce_api_signing_key = configuration_helpers.get_value(
-        "ECOMMERCE_API_SIGNING_KEY", settings.ECOMMERCE_API_SIGNING_KEY,
-    )
-    return bool(ecommerce_api_url and ecommerce_api_signing_key)
+    ecommerce_api_url = configuration_helpers.get_value('ECOMMERCE_API_URL', settings.ECOMMERCE_API_URL)
+    return bool(ecommerce_api_url)
 
 
-def ecommerce_api_client(user, session=None):
+def ecommerce_api_client(user, session=None, token_expiration=None):
     """ Returns an E-Commerce API client setup with authentication for the specified user. """
-    jwt_auth = configuration_helpers.get_value("JWT_AUTH", settings.JWT_AUTH)
+    claims = {'tracking_context': create_tracking_context(user)}
+    jwt = JwtBuilder(user).build_token(['email', 'profile'], expires_in=token_expiration, additional_claims=claims)
+
     return EdxRestApiClient(
-        configuration_helpers.get_value("ECOMMERCE_API_URL", settings.ECOMMERCE_API_URL),
-        configuration_helpers.get_value("ECOMMERCE_API_SIGNING_KEY", settings.ECOMMERCE_API_SIGNING_KEY),
-        user.username,
-        user.profile.name if hasattr(user, 'profile') else None,
-        user.email,
-        tracking_context=create_tracking_context(user),
-        issuer=jwt_auth['JWT_ISSUER'],
-        expires_in=jwt_auth['JWT_EXPIRATION'],
+        configuration_helpers.get_value('ECOMMERCE_API_URL', settings.ECOMMERCE_API_URL),
+        jwt=jwt,
         session=session
     )

openedx/core/djangoapps/credit/tests/test_api.py

@@ -11,7 +11,7 @@ from django.test.utils import override_settings
 from django.db import connection
 from nose.plugins.attrib import attr
 import httpretty
-from lms.djangoapps.commerce.tests import TEST_API_SIGNING_KEY, TEST_API_URL
+from lms.djangoapps.commerce.tests import TEST_API_URL
 import mock
 import pytz
 from opaque_keys.edx.keys import CourseKey
@@ -575,7 +575,6 @@ class CreditRequirementApiTests(CreditApiTestBase):
     @httpretty.activate
     @override_settings(
         ECOMMERCE_API_URL=TEST_API_URL,
-        ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY,
         ECOMMERCE_SERVICE_WORKER_USERNAME=TEST_ECOMMERCE_WORKER
     )
     def test_satisfy_all_requirements(self):
@@ -622,7 +621,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
         self.assertFalse(api.is_user_eligible_for_credit(user.username, self.course_key))
 
         # Satisfy the other requirement
-        with self.assertNumQueries(21):
+        with self.assertNumQueries(25):
             api.set_credit_requirement_status(
                 user,
                 self.course_key,
@@ -676,7 +675,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
         # Delete the eligibility entries and satisfy the user's eligibility
         # requirement again to trigger eligibility notification
         CreditEligibility.objects.all().delete()
-        with self.assertNumQueries(16):
+        with self.assertNumQueries(17):
             api.set_credit_requirement_status(
                 user,
                 self.course_key,
@@ -1167,7 +1166,6 @@ class CreditProviderIntegrationApiTests(CreditApiTestBase):
 @skip_unless_lms
 @override_settings(
     ECOMMERCE_API_URL=TEST_API_URL,
-    ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY,
     ECOMMERCE_SERVICE_WORKER_USERNAME=TEST_ECOMMERCE_WORKER
 )
 @ddt.ddt

openedx/core/lib/tests/test_edx_api_utils.py

@@ -19,7 +19,6 @@ from student.tests.factories import UserFactory
 
 UTILITY_MODULE = 'openedx.core.lib.edx_api_utils'
 TEST_API_URL = 'http://www-internal.example.com/api'
-TEST_API_SIGNING_KEY = 'edx'
 
 
 @skip_unless_lms
@@ -200,8 +199,7 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase):
         self.assertTrue(mock_exception.called)
         self.assertEqual(actual, [])
 
-    @override_settings(JWT_AUTH={'JWT_ISSUER': 'http://example.com/oauth', 'JWT_EXPIRATION': 30},
-                       ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY, ECOMMERCE_API_URL=TEST_API_URL)
+    @override_settings(ECOMMERCE_API_URL=TEST_API_URL)
     def test_client_passed(self):
         """ Verify that when API client is passed edx_rest_api_client is not
         used.

openedx/core/lib/token_utils.py

@@ -33,17 +33,22 @@ class JwtBuilder(object):
         self.secret = secret
         self.jwt_auth = configuration_helpers.get_value('JWT_AUTH', settings.JWT_AUTH)
 
-    def build_token(self, scopes, expires_in, aud=None):
+    def build_token(self, scopes, expires_in=None, aud=None, additional_claims=None):
         """Returns a JWT access token.
 
         Arguments:
             scopes (list): Scopes controlling which optional claims are included in the token.
-            expires_in (int): Time to token expiry, specified in seconds.
 
         Keyword Arguments:
+            expires_in (int): Time to token expiry, specified in seconds.
             aud (string): Overrides configured JWT audience claim.
+            additional_claims (dict): Additional claims to include in the token.
+
+        Returns:
+            str: Encoded JWT
         """
         now = int(time())
+        expires_in = expires_in or self.jwt_auth['JWT_EXPIRATION']
         payload = {
             'aud': aud if aud else self.jwt_auth['JWT_AUDIENCE'],
             'exp': now + expires_in,
@@ -54,6 +59,9 @@ class JwtBuilder(object):
             'sub': anonymous_id_for_user(self.user, None),
         }
 
+        if additional_claims:
+            payload.update(additional_claims)
+
         for scope in scopes:
             handler = self.claim_handlers.get(scope)