Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

chore: rolling back unique entity ID validation on saml provider configs

Browse Source
This commit is contained in:
Alexander Sheehan
2022-07-21 13:27:26 -04:00
parent 90a5e18b27
commit 9b1e8dc515
4 changed files with 39 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
common/djangoapps/third_party_auth/models.py
View File

@@ -12,7 +12,7 @@ from config_models.models import ConfigurationModel, cache
from django.conf import settings
from django.contrib.sites.models import Site
from django.core.exceptions import ValidationError
from django.db import models, IntegrityError
from django.db import models
from django.utils import timezone
from django.utils.translation import gettext_lazy as _
from organizations.models import Organization
@@ -746,7 +746,10 @@ class SAMLProviderConfig(ProviderConfig):
# If any exist, raise an integrity error
if existing_provider_configs:
exc_str = f'Entity ID: {self.entity_id} already in use'
raise IntegrityError(exc_str)
# There are cases of preexisting configurations that share entity id's so we can't blow up if we
# encounter this issue. Instead just log for clarity.
# raise IntegrityError(exc_str)
log.warning(exc_str)
super().save(*args, **kwargs)
def get_url_params(self):

8
common/djangoapps/third_party_auth/samlproviderconfig/serializers.py
View File

@@ -2,10 +2,13 @@
Serializer for SAMLProviderConfig
"""
import logging
from rest_framework import serializers
from common.djangoapps.third_party_auth.models import SAMLProviderConfig, SAMLConfiguration
log = logging.getLogger(__name__)
class SAMLProviderConfigSerializer(serializers.ModelSerializer): # lint-amnesty, pylint: disable=missing-class-docstring
saml_config_id = serializers.IntegerField(required=False)
@@ -27,7 +30,10 @@ class SAMLProviderConfigSerializer(serializers.ModelSerializer): # lint-amnesty
entity_id=data['entity_id'],
archived=False,
).exclude(slug=data['slug']):
raise serializers.ValidationError(f"Entity ID: {data['entity_id']} already taken")
# There are cases of preexisting configurations that share entity id's so we can't blow up if we
# encounter this issue. Instead just log for clarity.
# raise serializers.ValidationError(f"Entity ID: {data['entity_id']} already taken")
log.warning(f"Entity ID: {data['entity_id']} already taken")
return data
def create(self, validated_data):

20
common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py
View File

@@ -265,16 +265,20 @@ class SAMLProviderConfigTests(APITestCase):
Test that a config cannot be created with an entity ID if another config already exists with that entity ID and
a different slug
"""
url = reverse('saml_provider_config-list')
data = copy.copy(SINGLE_PROVIDER_CONFIG)
data['enterprise_customer_uuid'] = ENTERPRISE_ID
data['slug'] = 'some-other-slug'
with self.assertLogs() as ctx:
url = reverse('saml_provider_config-list')
data = copy.copy(SINGLE_PROVIDER_CONFIG)
data['enterprise_customer_uuid'] = ENTERPRISE_ID
data['slug'] = 'some-other-slug'
response = self.client.post(url, data)
response = self.client.post(url, data)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert len(SAMLProviderConfig.objects.all()) == 1
assert str(response.data.get('non_field_errors')[0]) == f"Entity ID: {data['entity_id']} already taken"
# 7/21/22 : Disabling the exception on duplicate entity ID's because of existing data.
assert ctx.records[-2].msg == f"Entity ID: {data['entity_id']} already taken"
assert response.status_code == status.HTTP_201_CREATED
# assert response.status_code == status.HTTP_400_BAD_REQUEST
# assert len(SAMLProviderConfig.objects.all()) == 1
# assert str(response.data.get('non_field_errors')[0]) == f"Entity ID: {data['entity_id']} already taken"
def test_unique_entity_id_constraint_with_same_slug(self):
"""

29
common/djangoapps/third_party_auth/tests/test_models.py
View File

@@ -1,15 +1,14 @@
"""
Tests for third_party_auth/models.py.
"""
import pytest
import unittest
from django.test import TestCase
from django.db.utils import IntegrityError
from .factories import SAMLProviderConfigFactory
from ..models import SAMLProviderConfig
class TestSamlProviderConfigModel(TestCase):
class TestSamlProviderConfigModel(TestCase, unittest.TestCase):
"""
Test model operations for the saml provider config model.
"""
@@ -22,19 +21,21 @@ class TestSamlProviderConfigModel(TestCase):
"""
Test that the unique entity ID enforcement does not apply to noncurrent configs
"""
assert len(SAMLProviderConfig.objects.all()) == 1
old_entity_id = self.saml_provider_config.entity_id
self.saml_provider_config.entity_id = f'{self.saml_provider_config.entity_id}-ayylmao'
self.saml_provider_config.save()
with self.assertLogs() as ctx:
assert len(SAMLProviderConfig.objects.all()) == 1
old_entity_id = self.saml_provider_config.entity_id
self.saml_provider_config.entity_id = f'{self.saml_provider_config.entity_id}-ayylmao'
self.saml_provider_config.save()
# check that we now have two records, one non-current
assert len(SAMLProviderConfig.objects.all()) == 2
assert len(SAMLProviderConfig.objects.current_set()) == 1
# check that we now have two records, one non-current
assert len(SAMLProviderConfig.objects.all()) == 2
assert len(SAMLProviderConfig.objects.current_set()) == 1
# Make sure we can use that old entity id
SAMLProviderConfigFactory(entity_id=old_entity_id)
# Make sure we can use that old entity id
SAMLProviderConfigFactory(entity_id=old_entity_id)
# Now if we try and create a new model using a current entity ID then it should throw the integrity error
with pytest.raises(IntegrityError):
# 7/21/22 : Disabling the exception on duplicate entity ID's because of existing data.
# with pytest.raises(IntegrityError):
bad_config = SAMLProviderConfig(entity_id=self.saml_provider_config.entity_id)
bad_config.save()
assert ctx.records[0].msg == f'Entity ID: {self.saml_provider_config.entity_id} already in use'