Updated CredentialsApiConfig to pull URLs from settings

Pulling URLs from settings allows us to rely on site configuration overrides in order to support multi-tenancy.

LEARNER-1103

lms/envs/aws.py

@@ -799,6 +799,9 @@ ECOMMERCE_API_TIMEOUT = ENV_TOKENS.get('ECOMMERCE_API_TIMEOUT', ECOMMERCE_API_TI
 
 COURSE_CATALOG_API_URL = ENV_TOKENS.get('COURSE_CATALOG_API_URL', COURSE_CATALOG_API_URL)
 
+CREDENTIALS_INTERNAL_SERVICE_URL = ENV_TOKENS.get('CREDENTIALS_INTERNAL_SERVICE_URL', CREDENTIALS_INTERNAL_SERVICE_URL)
+CREDENTIALS_PUBLIC_SERVICE_URL = ENV_TOKENS.get('CREDENTIALS_PUBLIC_SERVICE_URL', CREDENTIALS_PUBLIC_SERVICE_URL)
+
 ##### Custom Courses for EdX #####
 if FEATURES.get('CUSTOM_COURSES_EDX'):
     INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon')

lms/envs/common.py

@@ -2908,6 +2908,9 @@ ECOMMERCE_SERVICE_WORKER_USERNAME = 'ecommerce_worker'
 
 COURSE_CATALOG_API_URL = None
 
+CREDENTIALS_INTERNAL_SERVICE_URL = None
+CREDENTIALS_PUBLIC_SERVICE_URL = None
+
 # Reverification checkpoint name pattern
 CHECKPOINT_PATTERN = r'(?P<checkpoint_name>[^/]+)'
 

lms/envs/devstack.py

@@ -219,6 +219,9 @@ if FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and 'third_party_auth.dummy.DummyBack
 ############## ECOMMERCE API CONFIGURATION SETTINGS ###############
 ECOMMERCE_PUBLIC_URL_ROOT = "http://localhost:8002"
 
+CREDENTIALS_INTERNAL_SERVICE_URL = 'http://localhost:8008'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:8008'
+
 ###################### Cross-domain requests ######################
 FEATURES['ENABLE_CORS_HEADERS'] = True
 CORS_ALLOW_CREDENTIALS = True

lms/envs/devstack_docker.py

@@ -15,8 +15,12 @@ LMS_ROOT_URL = 'http://{}'.format(HOST)
 
 ECOMMERCE_PUBLIC_URL_ROOT = 'http://localhost:18130'
 ECOMMERCE_API_URL = 'http://edx.devstack.ecommerce:18130/api/v2'
+
 ENTERPRISE_API_URL = '{}/enterprise/api/v1/'.format(LMS_ROOT_URL)
 
+CREDENTIALS_INTERNAL_SERVICE_URL = 'http://edx.devstack.credentials:18150'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:18150'
+
 OAUTH_OIDC_ISSUER = '{}/oauth2'.format(LMS_ROOT_URL)
 
 JWT_AUTH.update({

lms/envs/test.py

@@ -591,6 +591,9 @@ JWT_AUTH.update({
 
 COURSE_CATALOG_API_URL = 'https://catalog.example.com/api/v1'
 
+CREDENTIALS_INTERNAL_SERVICE_URL = 'https://credentials-internal.example.com'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'https://credentials.example.com'
+
 COMPREHENSIVE_THEME_DIRS = [REPO_ROOT / "themes", REPO_ROOT / "common/test"]
 COMPREHENSIVE_THEME_LOCALE_PATHS = [REPO_ROOT / "themes/conf/locale", ]
 

openedx/core/djangoapps/credentials/migrations/0003_auto_20170525_1109.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('credentials', '0002_auto_20160325_0631'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='credentialsapiconfig',
+            name='internal_service_url',
+            field=models.URLField(help_text=b'DEPRECATED: Use the setting CREDENTIALS_INTERNAL_SERVICE_URL.', verbose_name='Internal Service URL'),
+        ),
+        migrations.AlterField(
+            model_name='credentialsapiconfig',
+            name='public_service_url',
+            field=models.URLField(help_text=b'DEPRECATED: Use the setting CREDENTIALS_PUBLIC_SERVICE_URL.', verbose_name='Public Service URL'),
+        ),
+    ]

openedx/core/djangoapps/credentials/models.py

@@ -4,10 +4,12 @@ Models for credentials support for the LMS and Studio.
 
 from urlparse import urljoin
 
-from django.utils.translation import ugettext_lazy as _
+from config_models.models import ConfigurationModel
+from django.conf import settings
 from django.db import models
+from django.utils.translation import ugettext_lazy as _
 
-from config_models.models import ConfigurationModel
+from openedx.core.djangoapps.site_configuration import helpers
 
 API_VERSION = 'v2'
 
@@ -17,35 +19,42 @@ class CredentialsApiConfig(ConfigurationModel):
     Manages configuration for connecting to the Credential service and using its
     API.
     """
+
     class Meta(object):
-        app_label = "credentials"
+        app_label = 'credentials'
 
     OAUTH2_CLIENT_NAME = 'credentials'
     API_NAME = 'credentials'
     CACHE_KEY = 'credentials.api.data'
 
-    internal_service_url = models.URLField(verbose_name=_("Internal Service URL"))
-    public_service_url = models.URLField(verbose_name=_("Public Service URL"))
+    internal_service_url = models.URLField(
+        verbose_name=_('Internal Service URL'),
+        help_text='DEPRECATED: Use the setting CREDENTIALS_INTERNAL_SERVICE_URL.'
+    )
+    public_service_url = models.URLField(
+        verbose_name=_('Public Service URL'),
+        help_text='DEPRECATED: Use the setting CREDENTIALS_PUBLIC_SERVICE_URL.'
+    )
 
     enable_learner_issuance = models.BooleanField(
-        verbose_name=_("Enable Learner Issuance"),
+        verbose_name=_('Enable Learner Issuance'),
         default=False,
         help_text=_(
-            "Enable issuance of credentials via Credential Service."
+            'Enable issuance of credentials via Credential Service.'
         )
     )
     enable_studio_authoring = models.BooleanField(
-        verbose_name=_("Enable Authoring of Credential in Studio"),
+        verbose_name=_('Enable Authoring of Credential in Studio'),
         default=False,
         help_text=_(
-            "Enable authoring of Credential Service credentials in Studio."
+            'Enable authoring of Credential Service credentials in Studio.'
         )
     )
     cache_ttl = models.PositiveIntegerField(
-        verbose_name=_("Cache Time To Live"),
+        verbose_name=_('Cache Time To Live'),
         default=0,
         help_text=_(
-            "Specified in seconds. Enable caching by setting this to a value greater than 0."
+            'Specified in seconds. Enable caching by setting this to a value greater than 0.'
         )
     )
 
@@ -55,32 +64,26 @@ class CredentialsApiConfig(ConfigurationModel):
     @property
     def internal_api_url(self):
         """
-        Generate a URL based on internal service URL and API version number.
+        Internally-accessible API URL root.
         """
-        return urljoin(self.internal_service_url, '/api/{}/'.format(API_VERSION))
+        root = helpers.get_value('CREDENTIALS_INTERNAL_SERVICE_URL', settings.CREDENTIALS_INTERNAL_SERVICE_URL)
+        return urljoin(root, '/api/{}/'.format(API_VERSION))
 
     @property
     def public_api_url(self):
         """
-        Generate a URL based on public service URL and API version number.
+        Publicly-accessible API URL root.
         """
-        return urljoin(self.public_service_url, '/api/{}/'.format(API_VERSION))
+        root = helpers.get_value('CREDENTIALS_PUBLIC_SERVICE_URL', settings.CREDENTIALS_PUBLIC_SERVICE_URL)
+        return urljoin(root, '/api/{}/'.format(API_VERSION))
 
     @property
     def is_learner_issuance_enabled(self):
         """
-        Indicates whether the learner credential should be enabled or not.
+        Returns boolean indicating if credentials should be issued.
         """
         return self.enabled and self.enable_learner_issuance
 
-    @property
-    def is_studio_authoring_enabled(self):
-        """
-        Indicates whether Studio functionality related to Credential should
-        be enabled or not.
-        """
-        return self.enabled and self.enable_studio_authoring
-
     @property
     def is_cache_enabled(self):
         """Whether responses from the Credentials API will be cached."""

openedx/core/djangoapps/credentials/tests/test_models.py

 """Tests for models supporting Credentials-related functionality."""
 
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from nose.plugins.attrib import attr
 
+from openedx.core.djangoapps.credentials.models import API_VERSION
 from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 
+CREDENTIALS_INTERNAL_SERVICE_URL = 'https://credentials.example.com'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'https://credentials.example.com'
+
 
 @skip_unless_lms
 @attr(shard=2)
 class TestCredentialsApiConfig(CredentialsApiConfigMixin, TestCase):
     """Tests covering the CredentialsApiConfig model."""
+
+    @override_settings(
+        CREDENTIALS_INTERNAL_SERVICE_URL=CREDENTIALS_INTERNAL_SERVICE_URL,
+        CREDENTIALS_PUBLIC_SERVICE_URL=CREDENTIALS_PUBLIC_SERVICE_URL
+    )
     def test_url_construction(self):
         """Verify that URLs returned by the model are constructed correctly."""
         credentials_config = self.create_credentials_config()
 
-        self.assertEqual(
-            credentials_config.internal_api_url,
-            credentials_config.internal_service_url.strip('/') + '/api/v2/')
+        expected = '{root}/api/{version}/'.format(root=CREDENTIALS_INTERNAL_SERVICE_URL.strip('/'), version=API_VERSION)
+        self.assertEqual(credentials_config.internal_api_url, expected)
 
-        self.assertEqual(
-            credentials_config.public_api_url,
-            credentials_config.public_service_url.strip('/') + '/api/v2/')
+        expected = '{root}/api/{version}/'.format(root=CREDENTIALS_PUBLIC_SERVICE_URL.strip('/'), version=API_VERSION)
+        self.assertEqual(credentials_config.public_api_url, expected)
 
     def test_is_learner_issuance_enabled(self):
         """
@@ -36,17 +43,3 @@ class TestCredentialsApiConfig(CredentialsApiConfigMixin, TestCase):
 
         credentials_config = self.create_credentials_config()
         self.assertTrue(credentials_config.is_learner_issuance_enabled)
-
-    def test_is_studio_authoring_enabled(self):
-        """
-        Verify that the property controlling display in the Studio authoring is only True
-        when configuration is enabled and all required configuration is provided.
-        """
-        credentials_config = self.create_credentials_config(enabled=False)
-        self.assertFalse(credentials_config.is_studio_authoring_enabled)
-
-        credentials_config = self.create_credentials_config(enable_studio_authoring=False)
-        self.assertFalse(credentials_config.is_studio_authoring_enabled)
-
-        credentials_config = self.create_credentials_config()
-        self.assertTrue(credentials_config.is_studio_authoring_enabled)

openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py

@@ -3,15 +3,15 @@ Tests for programs celery tasks.
 """
 import json
 
-from celery.exceptions import MaxRetriesExceededError
 import ddt
+import httpretty
+import mock
+from celery.exceptions import MaxRetriesExceededError
 from django.conf import settings
 from django.test import override_settings, TestCase
+from edx_oauth2_provider.tests.factories import ClientFactory
 from edx_rest_api_client import exceptions
 from edx_rest_api_client.client import EdxRestApiClient
-from edx_oauth2_provider.tests.factories import ClientFactory
-import httpretty
-import mock
 
 from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
 from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
@@ -19,7 +19,7 @@ from openedx.core.djangoapps.programs.tasks.v1 import tasks
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from student.tests.factories import UserFactory
 
-
+CREDENTIALS_INTERNAL_SERVICE_URL = 'https://credentials.example.com'
 TASKS_MODULE = 'openedx.core.djangoapps.programs.tasks.v1.tasks'
 
 
@@ -29,6 +29,7 @@ class GetApiClientTestCase(CredentialsApiConfigMixin, TestCase):
     Test the get_api_client function
     """
 
+    @override_settings(CREDENTIALS_INTERNAL_SERVICE_URL=CREDENTIALS_INTERNAL_SERVICE_URL)
     @mock.patch(TASKS_MODULE + '.JwtBuilder.build_token')
     def test_get_api_client(self, mock_build_token):
         """
@@ -36,13 +37,12 @@ class GetApiClientTestCase(CredentialsApiConfigMixin, TestCase):
         """
         student = UserFactory()
         ClientFactory.create(name='credentials')
-        api_config = self.create_credentials_config(
-            internal_service_url='http://foo'
-        )
+        api_config = self.create_credentials_config()
         mock_build_token.return_value = 'test-token'
 
         api_client = tasks.get_api_client(api_config, student)
-        self.assertEqual(api_client._store['base_url'], 'http://foo/api/v2/')  # pylint: disable=protected-access
+        expected = CREDENTIALS_INTERNAL_SERVICE_URL.strip('/') + '/api/v2/'
+        self.assertEqual(api_client._store['base_url'], expected)  # pylint: disable=protected-access
         self.assertEqual(api_client._store['session'].auth.token, 'test-token')  # pylint: disable=protected-access
 
 
@@ -82,7 +82,7 @@ class GetAwardedCertificateProgramsTestCase(TestCase):
         ]
 
         result = tasks.get_certified_programs(student)
-        self.assertEqual(mock_get_credentials.call_args[0], (student, ))
+        self.assertEqual(mock_get_credentials.call_args[0], (student,))
         self.assertEqual(result, [1])
 
 
@@ -136,10 +136,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
         UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME)  # pylint: disable=no-member
 
     def test_completion_check(
-            self,
-            mock_get_completed_programs,
-            mock_get_certified_programs,  # pylint: disable=unused-argument
-            mock_award_program_certificate,  # pylint: disable=unused-argument
+        self,
+        mock_get_completed_programs,
+        mock_get_certified_programs,  # pylint: disable=unused-argument
+        mock_award_program_certificate,  # pylint: disable=unused-argument
     ):
         """
         Checks that the Programs API is used correctly to determine completed
@@ -155,12 +155,12 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
     )
     @ddt.unpack
     def test_awarding_certs(
-            self,
-            already_awarded_program_uuids,
-            expected_awarded_program_uuids,
-            mock_get_completed_programs,
-            mock_get_certified_programs,
-            mock_award_program_certificate,
+        self,
+        already_awarded_program_uuids,
+        expected_awarded_program_uuids,
+        mock_get_completed_programs,
+        mock_get_certified_programs,
+        mock_award_program_certificate,
     ):
         """
         Checks that the Credentials API is used to award certificates for
@@ -179,10 +179,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
     )
     @ddt.unpack
     def test_retry_if_config_disabled(
-            self,
-            disabled_config_type,
-            disabled_config_attribute,
-            *mock_helpers
+        self,
+        disabled_config_type,
+        disabled_config_attribute,
+        *mock_helpers
     ):
         """
         Checks that the task is aborted if any relevant api configs are
@@ -208,10 +208,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
             self.assertFalse(mock_helper.called)
 
     def test_abort_if_no_completed_programs(
-            self,
-            mock_get_completed_programs,
-            mock_get_certified_programs,
-            mock_award_program_certificate,
+        self,
+        mock_get_completed_programs,
+        mock_get_certified_programs,
+        mock_award_program_certificate,
     ):
         """
         Checks that the task will be aborted without further action if there
@@ -234,19 +234,21 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
             http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock.side_effect
 
         """
+
         def side_effect(*_a):  # pylint: disable=missing-docstring
             if side_effects:
                 exc = side_effects.pop(0)
                 if exc:
                     raise exc
             return mock.DEFAULT
+
         return side_effect
 
     def test_continue_awarding_certs_if_error(
-            self,
-            mock_get_completed_programs,
-            mock_get_certified_programs,
-            mock_award_program_certificate,
+        self,
+        mock_get_completed_programs,
+        mock_get_certified_programs,
+        mock_award_program_certificate,
     ):
         """
         Checks that a single failure to award one of several certificates
@@ -268,9 +270,9 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
         mock_info.assert_any_call(mock.ANY, 2, self.student.username)
 
     def test_retry_on_programs_api_errors(
-            self,
-            mock_get_completed_programs,
-            *_mock_helpers  # pylint: disable=unused-argument
+        self,
+        mock_get_completed_programs,
+        *_mock_helpers  # pylint: disable=unused-argument
     ):
         """
         Ensures that any otherwise-unhandled errors that arise while trying
@@ -283,10 +285,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
         self.assertEqual(mock_get_completed_programs.call_count, 2)
 
     def test_retry_on_credentials_api_errors(
-            self,
-            mock_get_completed_programs,
-            mock_get_certified_programs,
-            mock_award_program_certificate,
+        self,
+        mock_get_completed_programs,
+        mock_get_certified_programs,
+        mock_award_program_certificate,
     ):
         """
         Ensures that any otherwise-unhandled errors that arise while trying
@@ -302,10 +304,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
         self.assertEqual(mock_award_program_certificate.call_count, 1)
 
     def test_no_retry_on_credentials_api_not_found_errors(
-            self,
-            mock_get_completed_programs,
-            mock_get_certified_programs,
-            mock_award_program_certificate,
+        self,
+        mock_get_completed_programs,
+        mock_get_certified_programs,
+        mock_award_program_certificate,
     ):
         mock_get_completed_programs.return_value = [1, 2]
         mock_get_certified_programs.side_effect = [[], [2]]