From 8f5dc7a53a141732c8ed04d24106561dc041d6a2 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Wed, 28 Jul 2021 12:44:15 -0700 Subject: [PATCH] SHIBUI-2000 delete entity descriptor fixed --- .../EntityDescriptorController.java | 12 ++--- .../ui/security/service/GroupServiceImpl.java | 52 +++++++++++++------ .../ui/security/service/IGroupService.java | 3 ++ .../JPAEntityDescriptorServiceImpl.java | 42 ++------------- .../EntityDescriptorControllerTests.groovy | 26 +++++++++- 5 files changed, 73 insertions(+), 62 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java index ef349f8b5..f8e28fc75 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java @@ -87,13 +87,13 @@ public ResponseEntity deleteOne(@PathVariable String resourceId) throws Forbi } @GetMapping("/EntityDescriptors") - @Transactional(readOnly = true) + @Transactional public ResponseEntity getAll() throws ForbiddenException { return ResponseEntity.ok(entityDescriptorService.getAllRepresentationsBasedOnUserAccess()); } @GetMapping("/EntityDescriptor/{resourceId}/Versions") - @Transactional(readOnly = true) + @Transactional public ResponseEntity getAllVersions(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException { // this "get by resource id" verifies that both the ED exists and the user has proper access, so needs to remain EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); @@ -101,21 +101,21 @@ public ResponseEntity getAllVersions(@PathVariable String resourceId) throws } @Secured("ROLE_ADMIN") - @Transactional(readOnly = true) + @Transactional @GetMapping(value = "/EntityDescriptor/disabledNonAdmin") public Iterable getDisabledAndNotOwnedByAdmin() throws ForbiddenException { return entityDescriptorService.getAllDisabledAndNotOwnedByAdmin(); } @GetMapping("/EntityDescriptor/{resourceId}") - @Transactional(readOnly = true) + @Transactional public ResponseEntity getOne(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException { return ResponseEntity.ok(entityDescriptorService .createRepresentationFromDescriptor(entityDescriptorService.getEntityDescriptorByResourceId(resourceId))); } @GetMapping(value = "/EntityDescriptor/{resourceId}", produces = "application/xml") - @Transactional(readOnly = true) + @Transactional public ResponseEntity getOneXml(@PathVariable String resourceId) throws MarshallingException, EntityNotFoundException, ForbiddenException { EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); final String xml = this.openSamlObjects.marshalToXmlString(ed); @@ -123,7 +123,7 @@ public ResponseEntity getOneXml(@PathVariable String resourceId) throws Marsh } @GetMapping("/EntityDescriptor/{resourceId}/Versions/{versionId}") - @Transactional(readOnly = true) + @Transactional public ResponseEntity getSpecificVersion(@PathVariable String resourceId, @PathVariable String versionId) throws EntityNotFoundException, ForbiddenException { // this "get by resource id" verifies that both the ED exists and the user has proper access, so needs to remain EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java index c505c727a..a6e35d470 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java @@ -1,13 +1,16 @@ package edu.internet2.tier.shibboleth.admin.ui.security.service; +import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor; import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException; import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; @@ -31,12 +34,28 @@ public GroupServiceImpl(GroupsRepository repo) { this.repo = repo; } + /** + * Ensure (mostly for migrations) that we have defined a default admin group + */ + @Override + @Transactional + public void afterPropertiesSet() { + Group g = repo.findByResourceId("admingroup"); + if (g == null) { + g = new Group(); + g.setName("ADMIN-GROUP"); + g.setResourceId("admingroup"); + g = repo.save(g); + } + Group.ADMIN_GROUP = g; + } + @Override public void clearAllForTesting() { repo.deleteAll(); afterPropertiesSet(); } - + @Override @Transactional public Group createGroup(Group group) throws GroupExistsConflictException { @@ -74,6 +93,21 @@ public List findAll() { return repo.findAll(); } + @Override + @Transactional + public void removeEntityFromGroup(final EntityDescriptor ed) { + Group g = repo.findByResourceId(ed.getGroup().getResourceId()); + Set eds = g.getEntityDescriptors(); + final HashSet updatedSet = new HashSet<>(); + eds.forEach(entityDesc -> { + if (!entityDesc.getEntityID().equals(ed.getEntityID())) { + updatedSet.add(entityDesc); + } + }); + g.setEntityDescriptors(updatedSet); + repo.save(g); + } + @Override public Group updateGroup(Group group) throws EntityNotFoundException { Group g = find(group.getResourceId()); @@ -83,20 +117,4 @@ public Group updateGroup(Group group) throws EntityNotFoundException { } return repo.save(group); } - - /** - * Ensure (mostly for migrations) that we have defined a default admin group - */ - @Override - @Transactional - public void afterPropertiesSet() { - Group g = repo.findByResourceId("admingroup"); - if (g == null) { - g = new Group(); - g.setName("ADMIN-GROUP"); - g.setResourceId("admingroup"); - g = repo.save(g); - } - Group.ADMIN_GROUP = g; - } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java index 1554558e9..8972aa09f 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java @@ -2,6 +2,7 @@ import java.util.List; +import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor; import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException; import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; @@ -19,6 +20,8 @@ public interface IGroupService { List findAll(); + void removeEntityFromGroup(EntityDescriptor ed); + Group updateGroup(Group g) throws EntityNotFoundException; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index fa4844601..6484ea4ad 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -1,37 +1,12 @@ package edu.internet2.tier.shibboleth.admin.ui.service; -import com.google.common.base.Strings; - -import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; -import edu.internet2.tier.shibboleth.admin.ui.domain.AssertionConsumerService; import edu.internet2.tier.shibboleth.admin.ui.domain.Attribute; -import edu.internet2.tier.shibboleth.admin.ui.domain.AttributeBuilder; -import edu.internet2.tier.shibboleth.admin.ui.domain.AttributeValue; -import edu.internet2.tier.shibboleth.admin.ui.domain.ContactPerson; -import edu.internet2.tier.shibboleth.admin.ui.domain.Description; -import edu.internet2.tier.shibboleth.admin.ui.domain.DisplayName; -import edu.internet2.tier.shibboleth.admin.ui.domain.EmailAddress; import edu.internet2.tier.shibboleth.admin.ui.domain.EntityAttributes; -import edu.internet2.tier.shibboleth.admin.ui.domain.EntityAttributesBuilder; import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor; -import edu.internet2.tier.shibboleth.admin.ui.domain.Extensions; -import edu.internet2.tier.shibboleth.admin.ui.domain.GivenName; -import edu.internet2.tier.shibboleth.admin.ui.domain.InformationURL; import edu.internet2.tier.shibboleth.admin.ui.domain.KeyDescriptor; -import edu.internet2.tier.shibboleth.admin.ui.domain.Logo; -import edu.internet2.tier.shibboleth.admin.ui.domain.NameIDFormat; -import edu.internet2.tier.shibboleth.admin.ui.domain.Organization; -import edu.internet2.tier.shibboleth.admin.ui.domain.OrganizationDisplayName; -import edu.internet2.tier.shibboleth.admin.ui.domain.OrganizationName; -import edu.internet2.tier.shibboleth.admin.ui.domain.OrganizationURL; -import edu.internet2.tier.shibboleth.admin.ui.domain.PrivacyStatementURL; import edu.internet2.tier.shibboleth.admin.ui.domain.IRelyingPartyOverrideProperty; -import edu.internet2.tier.shibboleth.admin.ui.domain.SPSSODescriptor; -import edu.internet2.tier.shibboleth.admin.ui.domain.SingleLogoutService; import edu.internet2.tier.shibboleth.admin.ui.domain.UIInfo; -import edu.internet2.tier.shibboleth.admin.ui.domain.XSAny; import edu.internet2.tier.shibboleth.admin.ui.domain.XSBoolean; - import edu.internet2.tier.shibboleth.admin.ui.domain.XSInteger; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.AssertionConsumerServiceRepresentation; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.ContactRepresentation; @@ -52,23 +27,13 @@ import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import edu.internet2.tier.shibboleth.admin.util.MDDCConstants; import edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions; -import groovy.util.logging.Slf4j; -import jline.internal.Log; - -import org.opensaml.core.xml.XMLObject; -import org.opensaml.core.xml.schema.XSBooleanValue; -import org.opensaml.xmlsec.signature.KeyInfo; -import org.opensaml.xmlsec.signature.X509Certificate; -import org.opensaml.xmlsec.signature.X509Data; +import lombok.extern.slf4j.Slf4j; + import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.ConcurrentModificationException; import java.util.HashMap; import java.util.List; @@ -76,7 +41,6 @@ import java.util.Optional; import java.util.stream.Collectors; -import javax.annotation.PostConstruct; import javax.transaction.Transactional; import static edu.internet2.tier.shibboleth.admin.util.EntityDescriptorConversionUtils.*; @@ -379,6 +343,8 @@ public void delete(String resourceId) throws ForbiddenException, EntityNotFoundE if (ed.isServiceEnabled()) { throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again."); } + groupService.removeEntityFromGroup(ed); + ed.setGroup(null); entityDescriptorRepository.delete(ed); } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy index 70affd608..e1fd8b225 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy @@ -166,7 +166,31 @@ class EntityDescriptorControllerTests extends Specification { EntityDescriptorConversionUtils.setOpenSamlObjects(openSamlObjects) EntityDescriptorConversionUtils.setEntityService(entityService) } - + + @Rollback + @WithMockUser(value = "admin", roles = ["ADMIN"]) + def 'DELETE as admin'() { + given: + authentication.getName() >> 'admin' + groupService.find("admingroup") >> Group.ADMIN_GROUP + def entityDescriptor = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1', serviceEnabled: false) + entityDescriptorRepository.save(entityDescriptor) + + when: 'pre-check' + entityManager.flush() + + then: + entityDescriptorRepository.findAll().size() == 1 + + when: + def result = mockMvc.perform(delete("/api/EntityDescriptor/uuid-1")) + + then: + result.andExpect(status().isNoContent()) + entityDescriptorRepository.findByResourceId("uuid-1") == null + entityDescriptorRepository.findAll().size() == 0 + } + @Rollback @WithMockUser(value = "admin", roles = ["ADMIN"]) def 'GET /EntityDescriptors with empty repository as admin'() {