Using shared secret for JWTs sent to Credentials API

This change brings the Credentials API calls in line with those of other services. The change also makes it easier for the future switch to an asymmetric signing key.

LEARNER-629

lms/djangoapps/student_account/views.py

@@ -338,7 +338,7 @@ def get_user_orders(user):
     cache_key = commerce_configuration.CACHE_KEY + '.' + str(user.id) if use_cache else None
     api = ecommerce_api_client(user)
     commerce_user_orders = get_edx_api_data(
-        commerce_configuration, user, 'orders', api=api, querystring=user_query, cache_key=cache_key
+        commerce_configuration, 'orders', api=api, querystring=user_query, cache_key=cache_key
     )
 
     for order in commerce_user_orders:

openedx/core/djangoapps/catalog/tests/test_utils.py

@@ -198,7 +198,7 @@ class TestGetCourseRuns(CatalogIntegrationMixin, TestCase):
         """
         args, kwargs = call_args
 
-        for arg in (self.catalog_integration, self.user, 'course_runs'):
+        for arg in (self.catalog_integration, 'course_runs'):
             self.assertIn(arg, args)
 
         self.assertEqual(kwargs['api']._store['base_url'], self.catalog_integration.internal_api_url)  # pylint: disable=protected-access

openedx/core/djangoapps/catalog/utils.py

@@ -2,7 +2,6 @@
 import copy
 import logging
 
-import waffle
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from edx_rest_api_client.client import EdxRestApiClient
@@ -11,7 +10,6 @@ from openedx.core.djangoapps.catalog.models import CatalogIntegration
 from openedx.core.lib.edx_api_utils import get_edx_api_data
 from openedx.core.lib.token_utils import JwtBuilder
 
-
 log = logging.getLogger(__name__)
 
 User = get_user_model()  # pylint: disable=invalid-name
@@ -67,11 +65,10 @@ def get_programs(uuid=None, types=None):  # pylint: disable=redefined-builtin
 
         return get_edx_api_data(
             catalog_integration,
-            user,
             'programs',
+            api=api,
             resource_id=uuid,
             cache_key=cache_key if catalog_integration.is_cache_enabled else None,
-            api=api,
             querystring=querystring,
         )
     else:
@@ -98,13 +95,8 @@ def get_program_types(name=None):
         api = create_catalog_api_client(user, catalog_integration)
         cache_key = '{base}.program_types'.format(base=catalog_integration.CACHE_KEY)
 
-        data = get_edx_api_data(
-            catalog_integration,
-            user,
-            'program_types',
-            cache_key=cache_key if catalog_integration.is_cache_enabled else None,
-            api=api
-        )
+        data = get_edx_api_data(catalog_integration, 'program_types', api=api,
+                                cache_key=cache_key if catalog_integration.is_cache_enabled else None)
 
         # Filter by name if a name was provided
         if name:
@@ -169,12 +161,6 @@ def get_course_runs():
             'exclude_utm': 1,
         }
 
-        course_runs = get_edx_api_data(
-            catalog_integration,
-            user,
-            'course_runs',
-            api=api,
-            querystring=querystring,
-        )
+        course_runs = get_edx_api_data(catalog_integration, 'course_runs', api=api, querystring=querystring)
 
     return course_runs

openedx/core/djangoapps/credentials/utils.py

 """Helper functions for working with Credentials."""
 from __future__ import unicode_literals
+
 import logging
 
+from django.conf import settings
+from edx_rest_api_client.client import EdxRestApiClient
+
 from openedx.core.djangoapps.catalog.utils import get_programs
 from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
 from openedx.core.lib.edx_api_utils import get_edx_api_data
-
+from openedx.core.lib.token_utils import JwtBuilder
 
 log = logging.getLogger(__name__)
 
 
+def get_credentials_api_client(user):
+    """ Returns an authenticated Credentials API client. """
+
+    scopes = ['email', 'profile']
+    expires_in = settings.OAUTH_ID_TOKEN_EXPIRATION
+    jwt = JwtBuilder(user).build_token(scopes, expires_in)
+    return EdxRestApiClient(CredentialsApiConfig.current().internal_api_url, jwt=jwt)
+
+
 def get_credentials(user, program_uuid=None):
     """
     Given a user, get credentials earned from the credentials service.
@@ -35,9 +48,10 @@ def get_credentials(user, program_uuid=None):
     # want to see them displayed immediately.
     use_cache = credential_configuration.is_cache_enabled and not user.is_staff
     cache_key = credential_configuration.CACHE_KEY + '.' + user.username if use_cache else None
+    api = get_credentials_api_client(user)
 
     return get_edx_api_data(
-        credential_configuration, user, 'credentials', querystring=querystring, cache_key=cache_key
+        credential_configuration, 'credentials', api=api, querystring=querystring, cache_key=cache_key
     )
 
 

openedx/core/lib/edx_api_utils.py

@@ -14,15 +14,14 @@ from openedx.core.lib.token_utils import JwtBuilder
 log = logging.getLogger(__name__)
 
 
-def get_edx_api_data(api_config, user, resource, api=None, resource_id=None,
-                     querystring=None, cache_key=None, many=True, traverse_pagination=True):
+def get_edx_api_data(api_config, resource, api, resource_id=None, querystring=None, cache_key=None, many=True,
+                     traverse_pagination=True):
     """GET data from an edX REST API.
 
     DRY utility for handling caching and pagination.
 
     Arguments:
         api_config (ConfigurationModel): The configuration model governing interaction with the API.
-        user (User): The user to authenticate as when requesting data.
         resource (str): Name of the API resource being requested.
 
     Keyword Arguments:
@@ -53,27 +52,6 @@ def get_edx_api_data(api_config, user, resource, api=None, resource_id=None,
             return cached
 
     try:
-        if not api:
-            # TODO: Use the system's JWT_AUDIENCE and JWT_SECRET_KEY instead of client ID and name.
-            client_name = api_config.OAUTH2_CLIENT_NAME
-
-            try:
-                client = Client.objects.get(name=client_name)
-            except Client.DoesNotExist:
-                raise ImproperlyConfigured(
-                    'OAuth2 Client with name [{}] does not exist.'.format(client_name)
-                )
-
-            scopes = ['email', 'profile']
-            expires_in = settings.OAUTH_ID_TOKEN_EXPIRATION
-            jwt = JwtBuilder(user, secret=client.client_secret).build_token(scopes, expires_in, aud=client.client_id)
-
-            api = EdxRestApiClient(api_config.internal_api_url, jwt=jwt)
-    except:  # pylint: disable=bare-except
-        log.exception('Failed to initialize the %s API client.', api_config.API_NAME)
-        return no_data
-
-    try:
         endpoint = getattr(api, resource)
         querystring = querystring if querystring else {}
         response = endpoint(resource_id).get(**querystring)

openedx/core/lib/tests/test_edx_api_utils.py

@@ -65,7 +65,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
         )
 
         with mock.patch('openedx.core.lib.edx_api_utils.EdxRestApiClient.__init__') as mock_init:
-            actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api)
+            actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api)
 
             # Verify that the helper function didn't initialize its own client.
             self.assertFalse(mock_init.called)
@@ -96,7 +96,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
 
         self._mock_catalog_api(responses)
 
-        actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api)
+        actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api)
         self.assertEqual(actual_collection, expected_collection)
 
         self._assert_num_requests(len(expected_collection))
@@ -125,9 +125,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
             [httpretty.Response(body=json.dumps(body), content_type='application/json') for body in responses]
         )
 
-        actual_collection = get_edx_api_data(
-            catalog_integration, self.user, 'programs', api=api, traverse_pagination=False,
-        )
+        actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api, traverse_pagination=False)
         self.assertEqual(actual_collection, expected_response)
         self._assert_num_requests(1)
 
@@ -149,7 +147,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
             url=url
         )
 
-        actual_resource = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, resource_id=resource_id)
+        actual_resource = get_edx_api_data(catalog_integration, 'programs', api=api, resource_id=resource_id)
         self.assertEqual(actual_resource, expected_resource)
 
         self._assert_num_requests(1)
@@ -178,7 +176,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
             url=url
         )
 
-        actual_resource = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, resource_id=resource_id)
+        actual_resource = get_edx_api_data(catalog_integration, 'programs', api=api, resource_id=resource_id)
         self.assertEqual(actual_resource, expected_resource)
 
         self._assert_num_requests(1)
@@ -214,17 +212,15 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
         cache_key = CatalogIntegration.current().CACHE_KEY
 
         # Warm up the cache.
-        get_edx_api_data(catalog_integration, self.user, 'programs', api=api, cache_key=cache_key)
-        get_edx_api_data(
-            catalog_integration, self.user, 'programs', api=api, resource_id=resource_id, cache_key=cache_key
-        )
+        get_edx_api_data(catalog_integration, 'programs', api=api, cache_key=cache_key)
+        get_edx_api_data(catalog_integration, 'programs', api=api, resource_id=resource_id, cache_key=cache_key)
 
         # Hit the cache.
-        actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, cache_key=cache_key)
+        actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api, cache_key=cache_key)
         self.assertEqual(actual_collection, expected_collection)
 
         actual_resource = get_edx_api_data(
-            catalog_integration, self.user, 'programs', api=api, resource_id=resource_id, cache_key=cache_key
+            catalog_integration, 'programs', api=api, resource_id=resource_id, cache_key=cache_key
         )
         self.assertEqual(actual_resource, expected_resource)
 
@@ -236,24 +232,11 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
         """Verify that no data is retrieved if the provided config model is disabled."""
         catalog_integration = self.create_catalog_integration(enabled=False)
 
-        actual = get_edx_api_data(catalog_integration, self.user, 'programs')
+        actual = get_edx_api_data(catalog_integration, 'programs', api=None)
 
         self.assertTrue(mock_warning.called)
         self.assertEqual(actual, [])
 
-    @mock.patch('edx_rest_api_client.client.EdxRestApiClient.__init__')
-    @mock.patch(UTILITY_MODULE + '.log.exception')
-    def test_client_initialization_failure(self, mock_exception, mock_init):
-        """Verify that an exception is logged when the API client fails to initialize."""
-        mock_init.side_effect = Exception
-
-        catalog_integration = self.create_catalog_integration()
-
-        actual = get_edx_api_data(catalog_integration, self.user, 'programs')
-
-        self.assertTrue(mock_exception.called)
-        self.assertEqual(actual, [])
-
     @mock.patch(UTILITY_MODULE + '.log.exception')
     def test_data_retrieval_failure(self, mock_exception):
         """Verify that an exception is logged when data can't be retrieved."""
@@ -264,7 +247,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
             [httpretty.Response(body='clunk', content_type='application/json', status_code=500)]
         )
 
-        actual = get_edx_api_data(catalog_integration, self.user, 'programs', api=api)
+        actual = get_edx_api_data(catalog_integration, 'programs', api=api)
 
         self.assertTrue(mock_exception.called)
         self.assertEqual(actual, [])
@@ -276,33 +259,15 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
 
         actual = get_edx_api_data(
             catalog_integration,
-            self.user,
             'programs',
+            api=None,
             resource_id=100,
-            many=False,
+            many=False
         )
 
         self.assertTrue(mock_warning.called)
         self.assertEqual(actual, {})
 
-    @mock.patch('edx_rest_api_client.client.EdxRestApiClient.__init__')
-    @mock.patch(UTILITY_MODULE + '.log.exception')
-    def test_client_initialization_failure_with_id(self, mock_exception, mock_init):
-        """Verify that an exception is logged when the API client fails to initialize."""
-        mock_init.side_effect = Exception
-
-        catalog_integration = self.create_catalog_integration()
-
-        actual = get_edx_api_data(
-            catalog_integration,
-            self.user,
-            'programs',
-            resource_id=100,
-            many=False,
-        )
-        self.assertTrue(mock_exception.called)
-        self.assertEqual(actual, {})
-
     @mock.patch(UTILITY_MODULE + '.log.exception')
     def test_data_retrieval_failure_with_id(self, mock_exception):
         """Verify that an exception is logged when data can't be retrieved."""
@@ -315,21 +280,10 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
 
         actual = get_edx_api_data(
             catalog_integration,
-            self.user,
             'programs',
             api=api,
             resource_id=100,
-            many=False,
+            many=False
         )
         self.assertTrue(mock_exception.called)
         self.assertEqual(actual, {})
-
-    def test_api_client_not_provided(self):
-        """Verify that an API client doesn't need to be provided."""
-        ClientFactory(name=CredentialsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL)
-
-        credentials_api_config = self.create_credentials_config()
-
-        with mock.patch('openedx.core.lib.edx_api_utils.EdxRestApiClient.__init__') as mock_init:
-            get_edx_api_data(credentials_api_config, self.user, 'credentials')
-            self.assertTrue(mock_init.called)