Merge pull request #18272 from edx/ziafazal/WL-1575
WL-1575: code refactor to avoid creation of duplicate oauth clients
This commit is contained in:
@@ -63,33 +63,34 @@ class Command(BaseCommand):
|
||||
help="Use devstack config, otherwise sandbox config is assumed",
|
||||
)
|
||||
|
||||
def _create_oauth2_client(self, url, site_name, is_discovery=True):
|
||||
def _create_oauth2_client(self, url, site_name, service_name, service_user):
|
||||
"""
|
||||
Creates the oauth2 client and add it in trusted clients.
|
||||
"""
|
||||
|
||||
client, _ = Client.objects.get_or_create(
|
||||
redirect_uri="{url}complete/edx-oidc/".format(url=url),
|
||||
client_id = "{service_name}-key{site_name}".format(
|
||||
service_name=service_name,
|
||||
site_name="" if site_name == "edx" else "-{}".format(site_name)
|
||||
)
|
||||
client, created = Client.objects.update_or_create(
|
||||
client_id=client_id,
|
||||
defaults={
|
||||
"user": self.discovery_user if is_discovery else self.ecommerce_user,
|
||||
"name": "{site_name}_{client_type}_client".format(
|
||||
"user": service_user,
|
||||
"name": "{site_name}_{service_name}_client".format(
|
||||
site_name=site_name,
|
||||
client_type="discovery" if is_discovery else "ecommerce",
|
||||
service_name=service_name,
|
||||
),
|
||||
"url": url,
|
||||
"client_id": "{client_type}-key{site_name}".format(
|
||||
client_type="discovery" if is_discovery else "ecommerce",
|
||||
site_name="" if site_name == "edx" else "-{}".format(site_name)
|
||||
),
|
||||
"client_secret": "{client_type}-secret".format(
|
||||
client_type="discovery" if is_discovery else "ecommerce"
|
||||
"client_secret": "{service_name}-secret".format(
|
||||
service_name=service_name
|
||||
),
|
||||
"client_type": CONFIDENTIAL,
|
||||
"redirect_uri": "{url}complete/edx-oidc/".format(url=url),
|
||||
"logout_uri": "{url}logout/".format(url=url)
|
||||
}
|
||||
)
|
||||
LOG.info("Adding {client} oauth2 client as trusted client".format(client=client.name))
|
||||
TrustedClient.objects.get_or_create(client=client)
|
||||
if created:
|
||||
LOG.info("Adding {client} oauth2 client as trusted client".format(client=client.name))
|
||||
TrustedClient.objects.get_or_create(client=client)
|
||||
|
||||
def _create_sites(self, site_domain, theme_dir_name, site_configuration):
|
||||
"""
|
||||
@@ -229,9 +230,9 @@ class Command(BaseCommand):
|
||||
self._create_sites(site_domain, site_data['theme_dir_name'], site_data['configuration'])
|
||||
|
||||
LOG.info("Creating discovery oauth2 client for '{site_name}' site".format(site_name=site_name))
|
||||
self._create_oauth2_client(discovery_url, site_name, is_discovery=True)
|
||||
self._create_oauth2_client(discovery_url, site_name, 'discovery', self.discovery_user)
|
||||
|
||||
LOG.info("Creating ecommerce oauth2 client for '{site_name}' site".format(site_name=site_name))
|
||||
self._create_oauth2_client(ecommerce_url, site_name, is_discovery=False)
|
||||
self._create_oauth2_client(ecommerce_url, site_name, 'ecommerce', self.ecommerce_user)
|
||||
|
||||
self._enable_commerce_configuration()
|
||||
|
||||
@@ -63,9 +63,8 @@ class TestCreateSiteAndConfiguration(TestCase):
|
||||
"""
|
||||
Checks that data of all sites is valid
|
||||
"""
|
||||
sites = Site.objects.all()
|
||||
# there is an extra default site.
|
||||
self.assertEqual(len(sites), len(SITES) + 1)
|
||||
sites = Site.objects.filter(domain__contains=self.dns_name)
|
||||
self.assertEqual(len(sites), len(SITES))
|
||||
for site in sites:
|
||||
if site.name in SITES:
|
||||
site_theme = SiteTheme.objects.get(site=site)
|
||||
@@ -203,6 +202,18 @@ class TestCreateSiteAndConfiguration(TestCase):
|
||||
self._assert_discovery_clients_are_valid()
|
||||
self._assert_ecommerce_clients_are_valid()
|
||||
|
||||
self.dns_name = "new-dns"
|
||||
mock_get_sites.return_value = _get_sites(self.dns_name)
|
||||
call_command(
|
||||
"create_sites_and_configurations",
|
||||
"--dns-name", self.dns_name,
|
||||
"--theme-path", self.theme_path
|
||||
)
|
||||
# if we run command with different dns existing oauth2 clients are updated with new dns
|
||||
self._assert_sites_are_valid()
|
||||
self._assert_discovery_clients_are_valid()
|
||||
self._assert_ecommerce_clients_are_valid()
|
||||
|
||||
@mock.patch(MANAGEMENT_COMMAND_PATH + "Command._enable_commerce_configuration")
|
||||
@mock.patch(MANAGEMENT_COMMAND_PATH + "Command._get_sites_data")
|
||||
def test_with_devstack_and_dns(self, mock_get_sites, mock_commerce):
|
||||
|
||||
Reference in New Issue
Block a user