Skip to content

Commit

Permalink
SHIBUI-2033
Browse files Browse the repository at this point in the history 
fix ownership issue
  • Loading branch information
@chasegawa
chasegawa committed Aug 20, 2021
1 parent f2b6975 commit 92d8529
Show file tree
Hide file tree
Showing 6 changed files with 280 additions and 116 deletions.
7 changes: 2 additions & 5 deletions ...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public UserAccess getCurrentUserAccess() {
if (user.getRole().equals("ROLE_ADMIN")) {
return ADMIN;
}
if (user.getRole().equals("ROLE_USER")) {
if (user.getRole().equals("ROLE_USER") || user.getRole().equals("ROLE_ENABLE")) {
return GROUP;
}
return NONE;
Expand All @@ -131,9 +131,7 @@ public Group getCurrentUserGroup() {
public Set<String> getUserRoles(String username) {
Optional<User> user = userRepository.findByUsername(username);
HashSet<String> result = new HashSet<>();
if (user.isPresent() ) {
user.get().getRoles().forEach(role -> result.add(role.getName()));
}
user.ifPresent(value -> value.getRoles().forEach(role -> result.add(role.getName())));
return result;
}

Expand Down Expand Up @@ -209,7 +207,6 @@ public User save(User user) {
* This currently exists because users should only ever have one role in the system at this time. However, user
* roles are persisted as a set of roles (for future-proofing). Once we start allowing a user to have multiple roles,
* this method and User.role can go away.
* @param user
*/
public void updateUserRole(User user) {
if (StringUtils.isNotBlank(user.getRole())) {
Expand Down
23 changes: 15 additions & 8 deletions ...n/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@
import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects;
import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository;
import edu.internet2.tier.shibboleth.admin.ui.security.model.Group;
import edu.internet2.tier.shibboleth.admin.ui.security.model.User;
import edu.internet2.tier.shibboleth.admin.ui.security.model.*;
import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository;
import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService;
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;
Expand All @@ -30,9 +28,6 @@ public class JPAEntityDescriptorServiceImpl implements EntityDescriptorService {
@Autowired
private EntityDescriptorRepository entityDescriptorRepository;

@Autowired
private IGroupService groupService;

@Autowired
private OpenSamlObjects openSamlObjects;

Expand Down Expand Up @@ -87,7 +82,12 @@ public EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation e

EntityDescriptor ed = (EntityDescriptor) createDescriptorFromRepresentation(edRep);
ed.setIdOfOwner(userService.getCurrentUserGroup().getOwnerId());
return createRepresentationFromDescriptor(entityDescriptorRepository.save(ed));
ed = entityDescriptorRepository.save(ed);

ownershipRepository.deleteEntriesForOwnedObject(ed);
ownershipRepository.save(new Ownership(userService.getCurrentUserGroup(), ed));

return createRepresentationFromDescriptor(ed);
}

@Override
Expand Down Expand Up @@ -374,10 +374,17 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe
throw new ConcurrentModificationException(String.format("A concurrent modification has occured on entity descriptor with entity id [%s]. Please refresh and try again", edRep.getId()));
}
updateDescriptorFromRepresentation(existingEd, edRep);
return createRepresentationFromDescriptor(entityDescriptorRepository.save(existingEd));
existingEd = entityDescriptorRepository.save(existingEd);
ownershipRepository.deleteEntriesForOwnedObject(existingEd);
ownershipRepository.save(new Ownership(new Owner() {
public String getOwnerId() { return edRep.getIdOfOwner(); }
public OwnerType getOwnerType() { return OwnerType.GROUP; }
}, existingEd));
return createRepresentationFromDescriptor(existingEd);
}

@Override
// This should be private, but we use it in a couple different test classes not sure we should keep...
public void updateDescriptorFromRepresentation(org.opensaml.saml.saml2.metadata.EntityDescriptor entityDescriptor, EntityDescriptorRepresentation representation) {
if (!(entityDescriptor instanceof EntityDescriptor)) {
throw new UnsupportedOperationException("not yet implemented");
Expand Down
88 changes: 27 additions & 61 deletions .../edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,18 @@ 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.security.service.GroupServiceForTesting
import edu.internet2.tier.shibboleth.admin.ui.security.service.GroupServiceImpl
import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService
import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService
import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService
import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorVersionService
import edu.internet2.tier.shibboleth.admin.ui.service.EntityService
import edu.internet2.tier.shibboleth.admin.ui.service.JPAEntityDescriptorServiceImpl
import edu.internet2.tier.shibboleth.admin.ui.service.JPAEntityServiceImpl
import edu.internet2.tier.shibboleth.admin.ui.util.RandomGenerator
import edu.internet2.tier.shibboleth.admin.ui.util.TestHelpers
import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator
import edu.internet2.tier.shibboleth.admin.util.EntityDescriptorConversionUtils
import groovy.json.JsonOutput
import groovy.json.JsonSlurper

import org.skyscreamer.jsonassert.Customization
import org.skyscreamer.jsonassert.JSONAssert
import org.skyscreamer.jsonassert.JSONCompareMode
import org.skyscreamer.jsonassert.ValueMatcher
import org.skyscreamer.jsonassert.comparator.CustomComparator
import org.skyscreamer.jsonassert.comparator.JSONCompareUtil
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.beans.factory.support.RootBeanDefinition
import org.springframework.boot.autoconfigure.domain.EntityScan
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.ComponentScan
import org.springframework.context.annotation.Profile
import org.springframework.context.support.StaticApplicationContext
import org.springframework.data.jpa.repository.config.EnableJpaRepositories
import org.springframework.security.core.Authentication
import org.springframework.security.core.context.SecurityContext
Expand All @@ -62,21 +45,14 @@ import org.springframework.test.context.ContextConfiguration
import org.springframework.test.web.servlet.setup.MockMvcBuilders
import org.springframework.transaction.annotation.Transactional
import org.springframework.web.client.RestTemplate
import org.springframework.web.servlet.config.annotation.EnableWebMvc
import org.springframework.web.servlet.config.annotation.WebMvcConfigurationSupport
import org.springframework.web.servlet.mvc.method.annotation.ExceptionHandlerExceptionResolver

import spock.lang.Ignore
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Subject

import java.time.LocalDateTime

import javax.persistence.EntityManager

import static org.hamcrest.CoreMatchers.containsString
import static org.springframework.http.MediaType.*
import static org.springframework.http.MediaType.APPLICATION_JSON
import static org.springframework.http.MediaType.APPLICATION_XML
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*

Expand Down Expand Up @@ -143,14 +119,13 @@ class EntityDescriptorControllerTests extends Specification {
mapper = new ObjectMapper()

service.userService = userService
service.groupService = groupService

controller = new EntityDescriptorController(versionService)
controller.openSamlObjects = openSamlObjects
controller.entityDescriptorService = service
controller.restTemplate = mockRestTemplate

mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
mockMvc = MockMvcBuilders.standaloneSetup(controller).build()

securityContext.getAuthentication() >> authentication
SecurityContextHolder.setContext(securityContext)
Expand Down Expand Up @@ -241,11 +216,11 @@ class EntityDescriptorControllerTests extends Specification {
def result = mockMvc.perform(get('/api/EntityDescriptors'))

then:
def mvcResult = result.andExpect(expectedHttpResponseStatus).andExpect(content().contentType(expectedResponseContentType))
.andExpect(jsonPath("\$.[0].id").value("uuid-1"))
.andExpect(jsonPath("\$.[0].entityId").value("eid1"))
.andExpect(jsonPath("\$.[0].serviceEnabled").value(true))
.andExpect(jsonPath("\$.[0].idOfOwner").value("admingroup"))
result.andExpect(expectedHttpResponseStatus).andExpect(content().contentType(expectedResponseContentType))
.andExpect(jsonPath("\$.[0].id").value("uuid-1"))
.andExpect(jsonPath("\$.[0].entityId").value("eid1"))
.andExpect(jsonPath("\$.[0].serviceEnabled").value(true))
.andExpect(jsonPath("\$.[0].idOfOwner").value("admingroup"))
}

@Rollback
Expand Down Expand Up @@ -351,10 +326,10 @@ class EntityDescriptorControllerTests extends Specification {

then:
try {
def exceptionExpected = mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody))
mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody))
}
catch (Exception e) {
e instanceof ForbiddenException == true
e instanceof ForbiddenException
}
}

Expand Down Expand Up @@ -394,10 +369,10 @@ class EntityDescriptorControllerTests extends Specification {

then:
try {
def exceptionExpected = mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody))
mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody))
}
catch (Exception e) {
e instanceof EntityIdExistsException == true
e instanceof EntityIdExistsException
}
}

Expand All @@ -409,10 +384,10 @@ class EntityDescriptorControllerTests extends Specification {

then:
try {
def exceptionExpected = mockMvc.perform(get("/api/EntityDescriptor/uuid-1"))
mockMvc.perform(get("/api/EntityDescriptor/uuid-1"))
}
catch (Exception e) {
e instanceof EntityNotFoundException == true
e instanceof EntityNotFoundException
}
}

Expand Down Expand Up @@ -482,10 +457,10 @@ class EntityDescriptorControllerTests extends Specification {

then:
try {
def exceptionExpected = mockMvc.perform(get("/api/EntityDescriptor/uuid-2"))
mockMvc.perform(get("/api/EntityDescriptor/uuid-2"))
}
catch (Exception e) {
e instanceof ForbiddenException == true
e instanceof ForbiddenException
}
}

Expand Down Expand Up @@ -553,10 +528,10 @@ class EntityDescriptorControllerTests extends Specification {
then:
try {
def exceptionExpected = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId").accept(APPLICATION_XML))
mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId").accept(APPLICATION_XML))
}
catch (Exception e) {
e instanceof ForbiddenException == true
e instanceof ForbiddenException
}
}
Expand Down Expand Up @@ -640,10 +615,10 @@ class EntityDescriptorControllerTests extends Specification {
then:
try {
def exceptionExpected = mockMvc.perform(post("/api/EntityDescriptor").contentType(APPLICATION_XML).content(postedBody).param("spName", spName))
mockMvc.perform(post("/api/EntityDescriptor").contentType(APPLICATION_XML).content(postedBody).param("spName", spName))
}
catch (Exception e) {
e instanceof EntityIdExistsException == true
e instanceof EntityIdExistsException
}
}
Expand Down Expand Up @@ -694,10 +669,10 @@ class EntityDescriptorControllerTests extends Specification {
then:
try {
def exceptionExpected = mockMvc.perform(put("/api/EntityDescriptor/uuid-1").contentType(APPLICATION_JSON).content(postedJsonBody))
mockMvc.perform(put("/api/EntityDescriptor/uuid-1").contentType(APPLICATION_JSON).content(postedJsonBody))
}
catch (Exception e) {
e instanceof ForbiddenException == true
e instanceof ForbiddenException
}
}
Expand All @@ -721,10 +696,10 @@ class EntityDescriptorControllerTests extends Specification {
then:
try {
def exceptionExpected = mockMvc.perform(put("/api/EntityDescriptor/uuid-1").contentType(APPLICATION_JSON).content(postedJsonBody))
mockMvc.perform(put("/api/EntityDescriptor/uuid-1").contentType(APPLICATION_JSON).content(postedJsonBody))
}
catch (Exception e) {
e instanceof ForbiddenException == true
e instanceof ForbiddenException
}
}
Expand All @@ -747,10 +722,10 @@ class EntityDescriptorControllerTests extends Specification {
then:
try {
def exception = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody))
mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody))
}
catch (Exception e) {
e instanceof ConcurrentModificationException == true
e instanceof ConcurrentModificationException
}
}
Expand All @@ -768,13 +743,4 @@ class EntityDescriptorControllerTests extends Specification {
return result
}
}
}
//when:
//def Set<Ownership> ownerships = ownershipRepository.findOwnableObjectOwners(ed)
//
//then:
//ownerships.size() == 1
//ownerships.each {
// it.ownerId == groupFromDb.resourceId
//}
}
Loading

0 comments on commit 92d8529

Please sign in to comment.