From 93f4a5f46c7a81aaa3d5e31fb9e34c631adc524e Mon Sep 17 00:00:00 2001 From: chasegawa Date: Fri, 18 Jun 2021 11:02:26 -0700 Subject: [PATCH] SHIBUI-1783 Removed added code around custom entity attributes and filters that ended up being un-needed. Fixed bug (NPE) on creating attribute from custom entity attribute --- .../CustomEntityAttributeDefinition.java | 6 + .../CustomEntityAttributeFilterValue.java | 43 ----- .../filters/EntityAttributesFilter.java | 12 +- ...mEntityAttributeFilterValueRepository.java | 16 -- ...EntityAttributesDefinitionServiceImpl.java | 12 -- .../CustomPropertiesConfigurationTests.groovy | 1 - .../ui/configuration/TestConfiguration.groovy | 7 +- .../EntityDescriptorRepositoryTest.groovy | 4 - .../repository/FilterRepositoryTests.groovy | 178 ------------------ 9 files changed, 8 insertions(+), 271 deletions(-) delete mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/CustomEntityAttributeFilterValue.java delete mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/CustomEntityAttributeFilterValueRepository.java diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/CustomEntityAttributeDefinition.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/CustomEntityAttributeDefinition.java index d0522c0d6..f4d5d1054 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/CustomEntityAttributeDefinition.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/CustomEntityAttributeDefinition.java @@ -85,6 +85,12 @@ public String getDisplayType() { return attributeType.name().toLowerCase(); } + @Override + public String getAttributeFriendlyName() { + // This is here only to ensure proper functionality works until the full definition is revised with all the fields + return attributeFriendlyName == null ? name : attributeFriendlyName; + } + @Override public Boolean getFromConfigFile() { return Boolean.FALSE; diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/CustomEntityAttributeFilterValue.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/CustomEntityAttributeFilterValue.java deleted file mode 100644 index ed6715631..000000000 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/CustomEntityAttributeFilterValue.java +++ /dev/null @@ -1,43 +0,0 @@ -package edu.internet2.tier.shibboleth.admin.ui.domain.filters; - -import javax.persistence.Column; -import javax.persistence.Entity; -import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; -import javax.persistence.Id; -import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; -import javax.persistence.Table; -import javax.persistence.UniqueConstraint; - -import org.hibernate.envers.Audited; - -import edu.internet2.tier.shibboleth.admin.ui.domain.CustomEntityAttributeDefinition; -import lombok.Getter; -import lombok.Setter; - - -@Entity(name = "custom_entity_attr_filter_value") -@Table(uniqueConstraints = { @UniqueConstraint(columnNames = { "filter_id", "custom_entity_attribute_name" }) }) -@Audited -// NOTE: lombok's toString and equals cause an infinite loop somewhere that causes stack overflows, so if we need impls, -// do it manually. Do not replace the Getter and Setter with @Data... -@Getter -@Setter -public class CustomEntityAttributeFilterValue { - @Id - @GeneratedValue(strategy = GenerationType.AUTO) - @Column(name = "generated_id") - private Integer id; - - @ManyToOne - @JoinColumn(name = "filter_id", nullable = false) - EntityAttributesFilter entityAttributesFilter; - - @ManyToOne - @JoinColumn(name = "custom_entity_attribute_name", referencedColumnName = "name", nullable = false) - CustomEntityAttributeDefinition customEntityAttributeDefinition; - - @Column(name = "value", nullable = false) - String value; -} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java index 28926358f..7643f483d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java @@ -53,16 +53,7 @@ public EntityAttributesFilter() { @Transient private List attributeRelease = new ArrayList<>(); - - @JsonIgnore - @OneToMany(cascade = CascadeType.ALL, mappedBy = "entityAttributesFilter", orphanRemoval = true) - private Set customEntityAttributes = new HashSet<>(); - - public void setCustomEntityAttributes (Set newValues) { - customEntityAttributes.clear(); - customEntityAttributes.addAll(newValues); - } - + public void setAttributeRelease(List attributeRelease) { this.attributeRelease = attributeRelease; this.rebuildAttributes(); @@ -95,7 +86,6 @@ private EntityAttributesFilter updateConcreteFilterTypeData(EntityAttributesFilt filterToBeUpdated.setEntityAttributesFilterTarget(getEntityAttributesFilterTarget()); filterToBeUpdated.setRelyingPartyOverrides(getRelyingPartyOverrides()); filterToBeUpdated.setAttributeRelease(getAttributeRelease()); - filterToBeUpdated.setCustomEntityAttributes(customEntityAttributes); return filterToBeUpdated; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/CustomEntityAttributeFilterValueRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/CustomEntityAttributeFilterValueRepository.java deleted file mode 100644 index a6a7be164..000000000 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/CustomEntityAttributeFilterValueRepository.java +++ /dev/null @@ -1,16 +0,0 @@ -package edu.internet2.tier.shibboleth.admin.ui.repository; - -import java.util.List; - -import org.springframework.data.jpa.repository.JpaRepository; - -import edu.internet2.tier.shibboleth.admin.ui.domain.CustomEntityAttributeDefinition; -import edu.internet2.tier.shibboleth.admin.ui.domain.filters.CustomEntityAttributeFilterValue; -import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter; - -public interface CustomEntityAttributeFilterValueRepository extends JpaRepository { - // Not entirely sure this is needed for the core, but it did make validation/unit testing a whole lot easier - CustomEntityAttributeFilterValue findByEntityAttributesFilterAndCustomEntityAttributeDefinition(EntityAttributesFilter eaf , CustomEntityAttributeDefinition cead); - - List findAllByCustomEntityAttributeDefinition(CustomEntityAttributeDefinition definition); -} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/CustomEntityAttributesDefinitionServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/CustomEntityAttributesDefinitionServiceImpl.java index 90438164a..a7d1c46dc 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/CustomEntityAttributesDefinitionServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/CustomEntityAttributesDefinitionServiceImpl.java @@ -10,9 +10,7 @@ import org.springframework.transaction.annotation.Transactional; import edu.internet2.tier.shibboleth.admin.ui.domain.CustomEntityAttributeDefinition; -import edu.internet2.tier.shibboleth.admin.ui.domain.filters.CustomEntityAttributeFilterValue; import edu.internet2.tier.shibboleth.admin.ui.repository.CustomEntityAttributeDefinitionRepository; -import edu.internet2.tier.shibboleth.admin.ui.repository.CustomEntityAttributeFilterValueRepository; import edu.internet2.tier.shibboleth.admin.ui.service.events.CustomEntityAttributeDefinitionChangeEvent; @Service @@ -20,9 +18,6 @@ public class CustomEntityAttributesDefinitionServiceImpl implements CustomEntity @Autowired private ApplicationEventPublisher applicationEventPublisher; - @Autowired - CustomEntityAttributeFilterValueRepository customEntityAttributeFilterValueRepository; - @Autowired EntityManager entityManager; @@ -40,13 +35,6 @@ public CustomEntityAttributeDefinition createOrUpdateDefinition(CustomEntityAttr @Override @Transactional public void deleteDefinition(CustomEntityAttributeDefinition definition) { - // must remove any CustomEntityAttributeFilterValues first to avoid integrity constraint issues - List customEntityValues = customEntityAttributeFilterValueRepository.findAllByCustomEntityAttributeDefinition(definition); - customEntityValues.forEach(value -> { - value.getEntityAttributesFilter().getCustomEntityAttributes().remove(value); - entityManager.remove(value); - customEntityAttributeFilterValueRepository.delete(value); - }); CustomEntityAttributeDefinition entityToRemove = repository.findByName(definition.getName()); repository.delete(entityToRemove); notifyListeners(); diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/CustomPropertiesConfigurationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/CustomPropertiesConfigurationTests.groovy index e90e7974e..74ecb9796 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/CustomPropertiesConfigurationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/CustomPropertiesConfigurationTests.groovy @@ -6,7 +6,6 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.CustomEntityAttributeDefinition import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects import edu.internet2.tier.shibboleth.admin.ui.repository.CustomEntityAttributeDefinitionRepository -import edu.internet2.tier.shibboleth.admin.ui.repository.CustomEntityAttributeFilterValueRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository import edu.internet2.tier.shibboleth.admin.ui.service.CustomEntityAttributesDefinitionService diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/TestConfiguration.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/TestConfiguration.groovy index 775103ac0..7f12a050d 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/TestConfiguration.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/TestConfiguration.groovy @@ -3,7 +3,6 @@ package edu.internet2.tier.shibboleth.admin.ui.configuration import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects import edu.internet2.tier.shibboleth.admin.ui.repository.CustomEntityAttributeDefinitionRepository -import edu.internet2.tier.shibboleth.admin.ui.repository.CustomEntityAttributeFilterValueRepository import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository import edu.internet2.tier.shibboleth.admin.ui.security.DefaultAuditorAware import edu.internet2.tier.shibboleth.admin.ui.service.CustomEntityAttributesDefinitionServiceImpl @@ -46,10 +45,7 @@ class TestConfiguration { @Autowired private CustomEntityAttributeDefinitionRepository repository; - - @Autowired - CustomEntityAttributeFilterValueRepository customEntityAttributeFilterValueRepository; - + @Autowired EntityManager entityManager @@ -63,7 +59,6 @@ class TestConfiguration { new CustomEntityAttributesDefinitionServiceImpl().with { it.entityManager = entityManager it.repository = repository - it.customEntityAttributeFilterValueRepository = customEntityAttributeFilterValueRepository return it } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy index 282961b79..70331ffe6 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy @@ -41,9 +41,6 @@ class EntityDescriptorRepositoryTest extends Specification { @Autowired private CustomEntityAttributeDefinitionRepository repository; - @Autowired - CustomEntityAttributeFilterValueRepository customEntityAttributeFilterValueRepository; - @Autowired EntityManager entityManager @@ -113,7 +110,6 @@ class EntityDescriptorRepositoryTest extends Specification { new CustomEntityAttributesDefinitionServiceImpl().with { it.entityManager = entityManager it.repository = repository - it.customEntityAttributeFilterValueRepository = customEntityAttributeFilterValueRepository return it } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy index 48a042e5b..f233684fd 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy @@ -5,7 +5,6 @@ import javax.persistence.EntityManager import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.domain.EntityScan import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest -import org.springframework.context.annotation.ComponentScan import org.springframework.data.jpa.repository.config.EnableJpaRepositories import org.springframework.test.context.ContextConfiguration @@ -16,9 +15,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.Internationalization import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.TestConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.CustomEntityAttributeDefinition -import edu.internet2.tier.shibboleth.admin.ui.domain.filters.CustomEntityAttributeFilterValue import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter -import edu.internet2.tier.shibboleth.admin.ui.service.CustomEntityAttributesDefinitionService import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator import spock.lang.Specification @@ -26,18 +23,11 @@ import spock.lang.Specification @ContextConfiguration(classes=[CoreShibUiConfiguration, SearchConfiguration, TestConfiguration, InternationalizationConfiguration]) @EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) @EntityScan("edu.internet2.tier.shibboleth.admin.ui") -@ComponentScan("edu.internet2.tier.shibboleth.admin.ui.service") class FilterRepositoryTests extends Specification { @Autowired FilterRepository repositoryUnderTest - @Autowired - CustomEntityAttributesDefinitionService ceadService - - @Autowired - CustomEntityAttributeFilterValueRepository ceafvRepo - @Autowired EntityManager entityManager @@ -96,172 +86,4 @@ class FilterRepositoryTests extends Specification { persistedFilter.audId > 0L persistedFilter.formats.size() == 1 } - - def "FilterRepository + EntityAttributesFilter CRUD ops with custom entity attributes correctly"(){ - given: - def ca = new CustomEntityAttributeDefinition().with { - it.name = "ca-name" - it.attributeType = "STRING" - it.defaultValue = "foo" - it - } - ceadService.createOrUpdateDefinition(ca) - entityManager.flush() - entityManager.clear() - - def entityAttributesFilterJson = '''{ - "name": "EntityAttributes", - "resourceId": "29a5d409-562a-41cd-acee-e9b3d7098d05", - "filterEnabled": false, - "entityAttributesFilterTarget": { - "entityAttributesFilterTargetType": "CONDITION_SCRIPT", - "value": [ - "TwUuSOz5O6" - ] - }, - "attributeRelease": [ - "WbkhLQNI3m" - ], - "relyingPartyOverrides": { - "signAssertion": true, - "dontSignResponse": true, - "turnOffEncryption": true, - "useSha": true, - "ignoreAuthenticationMethod": false, - "omitNotBefore": true, - "responderId": null, - "nameIdFormats": [ - "xLenUFmCLn" - ], - "authenticationMethods": [] - }, - "@type": "EntityAttributes" - }''' - def filter = new ObjectMapper().readValue(entityAttributesFilterJson.bytes, EntityAttributesFilter.class) - def persistedFilter = repositoryUnderTest.save(filter) - entityManager.flush() - - def savedFilter = repositoryUnderTest.findByResourceId(persistedFilter.resourceId) - def saveEAD = ceadService.find("ca-name") - - def ceafv = new CustomEntityAttributeFilterValue().with { - it.entityAttributesFilter = savedFilter - it.customEntityAttributeDefinition = saveEAD - it.value = "bar" - it - } - - def customEntityAttributes = new HashSet() - - when: - customEntityAttributes.add(ceafv) // nothing to do yet, just here to let us verify nothing in the CEAFV table in 'then' block - - then: - ((Set)ceafvRepo.findAll()).size() == 0 //nothing yet - ((EntityAttributesFilter)savedFilter).setCustomEntityAttributes(customEntityAttributes) - repositoryUnderTest.save(savedFilter) - entityManager.flush() - - then: - def listOfceafv = ceafvRepo.findAll() - listOfceafv.size() == 1 - - def ceafvFromDb = listOfceafv.get(0).asType(CustomEntityAttributeFilterValue) - ceafvFromDb.getEntityAttributesFilter().getResourceId().equals("29a5d409-562a-41cd-acee-e9b3d7098d05") - - def filterFromDb = (EntityAttributesFilter) repositoryUnderTest.findByResourceId("29a5d409-562a-41cd-acee-e9b3d7098d05") - filterFromDb.getCustomEntityAttributes().size() == 1 - - // now remove all - def emptySet = new HashSet() - filterFromDb.setCustomEntityAttributes(emptySet) - repositoryUnderTest.save(filterFromDb) - entityManager.flush() - - ceafvRepo.findAll().size() == 0 - } - - def "Delete custom entity attributes definition removes entries from filter correctly"(){ - given: - def ca = new CustomEntityAttributeDefinition().with { - it.name = "ca-name" - it.attributeType = "STRING" - it.defaultValue = "foo" - it - } - ceadService.createOrUpdateDefinition(ca) - entityManager.flush() - entityManager.clear() - - def entityAttributesFilterJson = '''{ - "name": "EntityAttributes", - "resourceId": "29a5d409-562a-41cd-acee-e9b3d7098d05", - "filterEnabled": false, - "entityAttributesFilterTarget": { - "entityAttributesFilterTargetType": "CONDITION_SCRIPT", - "value": [ - "TwUuSOz5O6" - ] - }, - "attributeRelease": [ - "WbkhLQNI3m" - ], - "relyingPartyOverrides": { - "signAssertion": true, - "dontSignResponse": true, - "turnOffEncryption": true, - "useSha": true, - "ignoreAuthenticationMethod": false, - "omitNotBefore": true, - "responderId": null, - "nameIdFormats": [ - "xLenUFmCLn" - ], - "authenticationMethods": [] - }, - "@type": "EntityAttributes" - }''' - def filter = new ObjectMapper().readValue(entityAttributesFilterJson.bytes, EntityAttributesFilter.class) - def persistedFilter = repositoryUnderTest.save(filter) - entityManager.flush() - - def savedFilter = repositoryUnderTest.findByResourceId(persistedFilter.resourceId) - def saveEAD = ceadService.find("ca-name"); - - def ceafv = new CustomEntityAttributeFilterValue().with { - it.entityAttributesFilter = savedFilter - it.customEntityAttributeDefinition = saveEAD - it.value = "bar" - it - } - - def customEntityAttributes = new HashSet() - - when: - customEntityAttributes.add(ceafv) // nothing to do yet, just here to let us verify nothing in the CEAFV table in 'then' block - - then: - ((Set)ceafvRepo.findAll()).size() == 0 //nothing yet - ((EntityAttributesFilter)savedFilter).setCustomEntityAttributes(customEntityAttributes) - repositoryUnderTest.save(savedFilter) - entityManager.flush() - - then: - def listOfceafv = ceafvRepo.findAll() - listOfceafv.size() == 1 - - def ceafvFromDb = listOfceafv.get(0).asType(CustomEntityAttributeFilterValue) - ceafvFromDb.getEntityAttributesFilter().getResourceId().equals("29a5d409-562a-41cd-acee-e9b3d7098d05") - - def filterFromDb = (EntityAttributesFilter) repositoryUnderTest.findByResourceId("29a5d409-562a-41cd-acee-e9b3d7098d05") - filterFromDb.getCustomEntityAttributes().size() == 1 - - // now remove the definition - ceadService.deleteDefinition(saveEAD) - entityManager.flush() - entityManager.clear() - - def filterFromDb2 = (EntityAttributesFilter)repositoryUnderTest.findByResourceId("29a5d409-562a-41cd-acee-e9b3d7098d05") - filterFromDb2.getCustomEntityAttributes().size() == 0 - } }